Passwords are stored in clear text

[johanfleury]

Hey @deluan, first of all, thank you for navidrome.

I noticed that passwords are stored in clear text (I figured out that as an admin you can see other users password and I confirmed by checking directly in the database). You might want to consider hashing them prior to store them in the database.

I don’t know what the 2020's best practices are regarding password hashing, but for the software I know:

• Django uses PBKDF2 with SHA256 by default and supports Argon2
• Apache supports bcrypt in basic auth password files (htpasswd)

[deluan]

Passwords should not be stored in the database, not even encrypted (not in a reversible form, anyway).

The reason we need to store the password in a reversible form in the DB is because how the new token based authentication works in the Subsonic API. It requires the server to know the password :(

Having said that, I'll change the way password are working right now, including storing them at least with some sort of encryption, and also allowing users to change their own password (#199).

[johanfleury]

Thank you for the quick answer.

So I’ve been looking a bit and it seems that Ampache just chose not to support token authentication at all since they support multiple authentication backends and they use sha256 for the MySQL backend.

[deluan]

Yeah, I know that some other Subsonic compatible servers don't support the token authentication to avoid storing the password. But this comes with the cost of not supporting all clients, as some assume that, if the server supports the API above version 1.13, it should support the token based authentication.

I'm not sure if I want to lose support from some clients... :/ Maybe I could implement an optional secondary password, just for the API. This one would be stored in reversible form in the DB, not the main one. This way, you could use the main password with the simple authentication method if you want (should be secure if you use HTTPS), or the secondary with all methods. What do you think?

[johanfleury]

Maybe I could implement an optional secondary password, just for the API.

I’ve just read most of the discussions in airsonic/airsonic#69, and that seems to be the way they’re going too.

some assume that, if the server supports the API above version 1.13, it should support the token based authentication.

In the Airsonic issue mentioned above, some people argue that Subsonic supports LDAP which prevent token authentication from working. Therefore, you could just reply with an error and client should fallback.

That being said, I’m only a DSub user and I don’t know what player requires token authentication.

[deluan]

Thanks for the research. I'm gonna work on this ASAP

[JaneX8]

In addition:

For the time being I suggest to have an empty password field and not show the clear text password as that would open new attack vectors to "grap and steal the password using XSS or something" for no reason, it doesn't add anything. Also maybe disable the "view password" option on that text input field if possible.

And I'm not sure if I can change my password here with a single field or I will get another "confirm" field. Also, changing password usually should require "current password".

[jvoisin]

Also maybe disable the "view password" option on that text input field if possible.

I disagree about this: it doesn't add anything security-wise, but significantly decreases usability (eg. dyslexic users).

[JaneX8]

Fair enough to keep it for accessibility but only when you're setting a new password. Not when creating a new one, then the old one shouldn't show (or be available in plain text) in the first place for security reasons.

[deluan]

@msafadieh said (in #575 (comment))

I think it would be great if you would consider implementing password hashing. I see issue #202 is open for it. It would be easier if you use something like bcrypt which offers hashpw and verifypw interfaces. I've tried implementing it myself but I've never programmed in Go before so I couldn't figure out how to stop sending the hashed password back to the front-end in user settings and to implement proper db migrations.

Essentially you shouldn't be able to see your password in the UI anymore after it has been hashed and saved. For migrations, I was thinking of creating a new column called hashed_password and dropping the old password column after hashing all the entries. I'm happy to open a PR with the work on my fork so far if you think it's a good starting point.

Ideally you would not see the password anywhere after it's been hashed (not even in the DB, neither plaintext nor encrypted). I've already implemented this in another (private) project, using the same tech as Navidrome (Go + ReacAdmin), and it works perfectly.

The problem is that the token authentication, used in most Subsonic clients, rely on the server having access to the original password, so I cannot simply drop the password once it is hashed. There's a lot of discussions about this topic, specially here: airsonic/airsonic#69, where you can see there's no consensus on how to "fix" this mess :/

My plan is after reaching 1.0, start working with other client and server developers and propose a way to move forward with enhancements to the Subsonic API, the first one being a better authentication mechanism

[msafadieh]

I see. Do you think it's reasonable then to at least remove the ability to retrieve the password from the front-end in user settings?

[deluan]

Yeah, I think that can be done. I was not spending much time on this because every solution, even though may improve a bit, does not address the main issue that the passwords are not totally secured. But I may spend some time removing it from the /user endpoint response, and also implementing the Change Password functionality (#199)

[dani]

IMHO this should be a top priority. I just found navidrome and was ready to install as I really like the look and feel. But then I found this issue, which clearly is a blocker (this, and the absence of LDAP / SSO auth)

Funkwhale solved this by leting user create a subsonic token in their profile, independant from their password. It's probably the way to go if you want to keep the token auth working.

[deluan]

Yeah @dani , I've been thinking about doing something similar to this. Just have to find a good way to let users that are coming from Airsonic/Subsonic to understand how this would work. Some people would not be able to use DSub out-of-box for example, and would just give up trying to make it work....

I wish clients implemented an auto-fallback from token-based to password-based authentication. It is possible with the current API specification, but I couldn't find a single client that implements it :(

[dharmaInit]

Any luck with this?

[deluan]

Next version (0.44.0) will provide two features to address this issue:
#1187 - Passwords will be encrypted in the DB, but not hashed, to keep compatibility with Subsonic API
#1152 - Reverse Proxy Authentication , allowing the use of SSO/OIDC solutions like Vouch or Authelia, or plugins for your current reverse proxy

After the release, I'll close this issue. Multiple authentication backends may be implemented in the future (#858), allowing more sophisticated auth scenarios.

[enigmaoftech]

OIDC would be nice but so would SAML with authentik

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.