Use MD5 hash in mopidy.conf instead of plaintext passwords

mopidy / mopidy-scrobbler

Mopidy extension for scrobbling played tracks to Last.fm

https://mopidy.com/ext/scrobbler/

Apache License 2.0

66 stars 14 forks source link

Use MD5 hash in mopidy.conf instead of plaintext passwords #27

Closed evamvid closed 3 years ago

evamvid commented 5 years ago

This requires the user to put the MD5 of their password in the Scrobbler section of mopidy.conf instead of their plaintext password.

jjok commented 5 years ago

I think you'd definitely need something in the README explaining how users can create hash of their password.

jjok commented 5 years ago

Relates to https://github.com/mopidy/mopidy-spotify/pull/65

kingosticks commented 5 years ago

We should be really clear that the extra security this provides is minimal. If some has your md5 hash, they can likely get your password from that in a reasonable amount of time. Or they could just use the hash as-is to log in as you.

evamvid commented 5 years ago

Agreed that this isn't a huge leap in terms of added security.

I guess using OAuth would be better, but can that be done through pyLast? And how do we feel about requiring the user to do setup in the browser? Maybe we'd need to have some sort of installation wizard that users can run for authentication?

kingosticks commented 5 years ago

Yes, we could use their Web Application OAuth-style flow and have the user save the session key in their Mopidy config. It'd be very similar to what we already require users to do for Spotify and Soundcloud access where they must store the client_secret. pylast appears to support last.fm session keys.

The session key should remain secret and if you don't trust the config file's security (which seems to be the concern but not something I'm personally convinced about) then you are still exposed. However, I do agree it's better since you have not exposed your actual password and can revoke the session key using the last.fm website. Additionally, there isn't much you can actually do through last.fm's API.

However, it would take some work to do this. Proper Mopidy integration with the system keychain might be a better alternative since other extensions could also benefit from that.

jjok commented 5 years ago

Proper Mopidy integration with the system keychain might be a better alternative since other extensions could also benefit from that. I don't really know anything about keychains, but it would definitely be good to have one solution that works for all extensions. I think most extensions have a "Password stored in plain text" issue open against them.

kingosticks commented 5 years ago

Keyrings are kind of supported (see https://github.com/mopidy/mopidy/issues/116#issuecomment-18026012) but I don't know how this works, if at all, when running Mopidy as a service. I think we had talk of added mopidy config subcommands to make setting passwords more user-friendly.

Maybe the session key support is worth doing here after all...

jodal commented 3 years ago

Closing this as:

I'm not interested in a one-off solution for a single extension,
keeping passwords in plain text isn't that bad if you don't reuse passwords for multiple services,
hasing with MD5 adds close to zero extra security.

Sorry I didn't respond to this two years ago!