Bug 496581 - Amarok does not get proper version from Ampache 5.6.4 and uses wrong password hash algorithm
Summary: Amarok does not get proper version from Ampache 5.6.4 and uses wrong password...
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Services/Ampache (other bugs)
Version First Reported In: 3.1.1
Platform: Debian testing Linux
: NOR normal
Target Milestone: kf5
Assignee: Amarok Bugs
URL:
Keywords:
: 451263 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-11-22 15:09 UTC by Ian Abbott
Modified: 2024-12-04 21:22 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Abbott 2024-11-22 15:09:56 UTC
SUMMARY
 
Amarok is failing to parse the correct version number from the Ampache URL base+`/server/xml.server.php?action=ping` when using Ampache 5.6.4 and so is using the wrong hash algorithm for the authentication handshake, leading to authentication failure.

The document returned from the Ampache server is as follows:

```
<?xml version="1.0" encoding="UTF-8"?>
<root>
  <server><![CDATA[5.6.4-release]]></server>
  <version><![CDATA[5.6.4]]></version>
  <compatible><![CDATA[350001]]></compatible>
</root>
```

The Amarok debug log output contains the following (sensitive information masked):

```
amarok: BEGIN: void AmpacheAccountLogin::authenticate(const QUrl&, const QByteArray&, const NetworkAccessManagerProxy::Error&)
amarok:   Version reply:  "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<root>\n  <server><![CDATA[5.6.4-release]]></server>\n  <version><![CDATA[5.6.4]]></version>\n  <compatible><![CDATA[350001]]></compatible>\n</root>\n"
amarok:   BEGIN: int AmpacheAccountLogin::getVersion(const QDomDocument&) const
amarok:     getVersion returned:  "5.6.4"
amarok:   END__: int AmpacheAccountLogin::getVersion(const QDomDocument&) const [Took: 0s]
amarok:   Version Older than 35001 Generated MD5 Auth  0
amarok:   Authenticating with string:  "https://xxx.xxx/server/xml.server.php?action=handshake&user=xxx&timestamp=xxxxxxxxxx&auth=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
amarok: END__: void AmpacheAccountLogin::authenticate(const QUrl&, const QByteArray&, const NetworkAccessManagerProxy::Error&) [Took: 0s]
amarok: BEGIN: void AmpacheAccountLogin::authenticationComplete(const QUrl&, const QByteArray&, const NetworkAccessManagerProxy::Error&)
amarok:   generalVerify error:  "handshakeaccountReceived Invalid Handshake - Incorrect username or password"
amarok: END__: void AmpacheAccountLogin::authenticationComplete(const QUrl&, con
```

The handshake failed because it used the wrong hash algorithm.

Amarok converted the version string "5.6.4" to integer value 0 and so reports "Version Older than 35001 Generated MD5 Auth  0". I think it ought to have used the "compatible" version number from this element of the XML: `<compatible><![CDATA[350001]]></compatible>` and got the version number 350001.

SOFTWARE/OS VERSIONS
Linux: Debian trixie/sid
KDE Frameworks Version: 5.115.0
Qt Version: 5.15.15

ADDITIONAL INFORMATION

`AmpacheAccountLogin::getVersion` should probably check for a non-empty `compatible` element before falling back to the `version` element.
Comment 1 Tuomas Nurmi 2024-11-29 22:12:42 UTC
Thank you for the report! This seems similar to the issue described in https://bugs.kde.org/show_bug.cgi?id=451263 but with more details, could this be the same issue in question?

If so, I'd imagine fixing this isn't a huge effort and I actually planned to try doing that, but I didn't get to setting up an Ampache server yet. I don't know when I will (would be awesome if someone with an existing Ampache would find out and submit the required changes)
Comment 2 Ian Abbott 2024-12-03 17:13:15 UTC
After a bit more digging in the Ampache sources on GitHub, the "<compatible>" element was introduced in 3.5.2 and its value 350001 has remained consistent ever since. The value of the "<version>" element was a single, monotonically increasing integer until version 5.0.0 when it changed to the dotted triplet format. (The monotonically increasing integer vaguely mapped onto the dotted triplet, but not in a consistent way.)

So if the code used the value of the "<compatible>" element in preference to the value of the "<version>" element, it will get an old number 350001 if the "<compatible>" element exists, but that will be sufficiently large to determine that the SHA256 hash method should be used.

@Tuomas I can set up a test account for you on my home Ampache server if you like. It's a couple of major versions behind the latest 7.x.x so only supports Ampache API versions 3, 4 and 5, but it should be good enough for testing authentication methods.
Comment 3 Bug Janitor Service 2024-12-04 17:51:10 UTC
A possibly relevant merge request was started @ https://invent.kde.org/multimedia/amarok/-/merge_requests/130
Comment 4 Tuomas Nurmi 2024-12-04 21:18:25 UTC
Git commit e5363c31ee9ecf1ecc7cb87b59e69f69e188d9b4 by Tuomas Nurmi, on behalf of Ian Abbott.
Committed on 04/12/2024 at 21:18.
Pushed by nurmi into branch 'master'.

Update Ampache server version check

The value of the `<value>` element in the response from a ping request
to the Ampache server changed from a six-digit integer to a dotted
triple in Ampache version 5.0.0.  This screws up the check for whether
to use the new style of authentication string based on a SHA256 hash, or
the old (now ancient) style of authentication string based on an MD5
hash.  It will try and login to Ampache server version 5.0.0 or later
using the old style of authentication string and so fail to login
successfully.

Use the value of the `<compatible>` element instead if it exists,
falling back to the `<version>` element if it doesn't.  The
`<compatible>` element was introduced in Ampache 3.5.2 and has the
integer value 350001.  This indicates a version older than the actual
version, but it is good enough to determine that the new style of
authentication string should be used.

Also correct the "Version Older than 35001" debug message that has a
missing digit (it should be 350001).

Signed-off-by: Ian Abbott <ian@abbott.org>

M  +17   -3    src/services/ampache/AmpacheAccountLogin.cpp

https://invent.kde.org/multimedia/amarok/-/commit/e5363c31ee9ecf1ecc7cb87b59e69f69e188d9b4
Comment 5 Tuomas Nurmi 2024-12-04 21:22:05 UTC
*** Bug 451263 has been marked as a duplicate of this bug. ***