Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

login process sends plain text passwords to the server #2544

Closed
MurzNN opened this issue May 15, 2020 · 25 comments
Closed

login process sends plain text passwords to the server #2544

MurzNN opened this issue May 15, 2020 · 25 comments
Labels
client-server Client-Server API improvement A suggestion for a relatively simple improvement to the protocol

Comments

@MurzNN
Copy link
Contributor

MurzNN commented May 15, 2020

Regarding to current spec - on login process user password of Matrix account is sent from client to server into json as plaintext. So any homeserver owner can easily get user password via simple dump json of login action!

This dramatically decrease security of user accounts! Especially after recently enforcing E2EE and cross-signing on Riot clients - now users need to create two passwords for Matrix account (one - for login, second - for SSS encryption), and most of them will use one password for both things, because generate (and remember!) two unique passwords is too hard task for regular modern users, that used to use WhatsApp, Telegram, Viber, which works even without using passwords, and have E2EE (for private chats only, but users don't read this nuance)!

So, if Matrix homeserver admin will dump login passwords of all users, most of them will be suitable to decrypt their Secure Secret Storage and admin get access to their e2ee keys!

For solve this security problem - my suggestion is implement new login type, when user password is hashed on client side, and sent to server as hashed string. For enhance protection, server (or even client) can generate salt, that will be used on client side to generate unique hash of password each time. For not broke behavior on old clients, we can keep old plaintext "m.login.password" login type, and provide new "m.login.hash" mode for more secured login type.

What do you think about described security problem and proposed solution? Maybe others can provide better solutions for this problem?

@turt2live turt2live added client-server Client-Server API improvement A suggestion for a relatively simple improvement to the protocol labels May 15, 2020
@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

Also, as I understand, we can use same hashing method on user register process, for not send plaintext password to server even on account registering process!

@pv
Copy link

pv commented May 15, 2020

Client-side key derivation ("expensive hash function") from the password at first sight seems safe. What to do with key derivation parameters & salt probably also needs consideration. Password changes would also need to change the SSSS password at the same time.

@richvdh
Copy link
Member

richvdh commented May 15, 2020

something I missed in the wall of text on first reading: the specific problem here is that the password is common to SSSS and login, so an HS admin can snoop submitted passwords and use them to decrypt SSSS.

@uhoreg (or someone else more familiar with SSSS): is this actually correct, or is there more to it?

@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

The main problem is that account password can be easily leaked via any homeserver admin, and provided way to make it more secure.

Second problem is that this password often match to SSSS password, so leaking account password helps with unlocking SSSS too for most of homeserver users, and leak all E2EE keys.

@richvdh
Copy link
Member

richvdh commented May 15, 2020

The main problem is that account password can be easily leaked via any homeserver admin

right, but this isn't a problem unless that password is used elsewhere. If the HS owner wants to act as a user, they have a myriad of ways to do so without having to capture the password.

@Cadair
Copy link
Contributor

Cadair commented May 15, 2020

I think @richvdh's summary matches my understanding with a slight tweak to the wording:

the specific problem here is that the password can be common to SSSS and login, so an HS admin can snoop submitted passwords and try to use them to decrypt SSSS.

@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

right, but this isn't a problem unless that password is used elsewhere. If the HS owner wants to act as a user, they have a myriad of ways to do so without having to capture the password.

But why we don't store plaintext passwords in database, but use hash and even salt? :) Because user password is secret, that must be not known to service owner, because most of users use same passwords in other services, and this homeserver admin can hack all other user accounts, if will know plaintext password. But if admin have only hash of password, he can't do this.

Yes, homeserver admin can force set new password for user, or got access using token (without touching password), but not read current user password in plaintext.

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

Ignoring the lack of backwards compatibility between clients here.

The biggest issue with this suggestion is that it would require an "escape hatch" where the server would need a method to signal to the client that it NEEDS the plaintext password to submit to a password provider such as LDAP.

This sort of mechanism could easily be exploited by a malicious server just the same as the plaintext password being sniffed in the first place.
Equally a server could throw you into a fake SSO flow which does the same thing.

@Cadair
Copy link
Contributor

Cadair commented May 15, 2020

I think it's probably worth separating the discussion relating to SSSS and the wish to not send plain text passwords to the HS. I think providing a way to ensure the HS can't decrypt SSSS is valuable.

@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

I suggest creating new login type, something like type: ["m.login.password", "m.login.hash", "m.login.token" ]

This will not break old clients, but new clients will can show for users achive "Your password is client-side encrypted" on login form, if server support this.

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

I think providing a way to ensure the HS can't decrypt SSSS is valuable.

We don't send the SSSS passphrase to the server if that's what you're thinking

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

This will not break old clients, but new clients will can show for users achive "Your password is client-side encrypted" on login form, if server support this.

It would have to be the inverse, and show a massive warning if the server claims it doesn't support it.

It still wouldn't work around the fake SSO to capture credentials.

@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

It still wouldn't work around the fake SSO to capture credentials.

Yes, but SSO not leak plain-text password to homeserver :)

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

It would if it is a Fake (phishing style) SSO which the hs hosts itself and takes credentials from.

@richvdh
Copy link
Member

richvdh commented May 15, 2020

@MurzNN:

But why we don't store plaintext passwords in database, but use hash and even salt? :)

it guards against an attacker who gets access to a copy of the database somehow. That's a very different bar to an admin who has the ability to capture live traffic and write to the database. "Encryption at rest", etc.

I really feel like you're obsessing about the wrong problem here, but if you really wanted to solve the problem you are obsessing about: clearly a simple hash is ineffective (otherwise the hash just becomes a password); you'd need a challenge/response system. No doubt there is prior art here, but I don't know what it is.

As @Cadair says: maybe we should separate "user password can be used to decrypt SSSS" from "plain text passwords are sent to the HS". The former (if true) is a valid concern to me; the latter is a very, very minor concern compared with the difficulty of addressing it.

@richvdh
Copy link
Member

richvdh commented May 15, 2020

I think providing a way to ensure the HS can't decrypt SSSS is valuable.

We don't send the SSSS passphrase to the server if that's what you're thinking

But isn't the SSSS passphrase the same as the login password, which we do send to the server?

@MurzNN
Copy link
Contributor Author

MurzNN commented May 15, 2020

My current issue is only about "plain text passwords are sent to the HS", and mentioned SSSS password is only example of potential problem with this issue. Same problem if homeserver admin got user plaintext password and try to use it with his email account, github account, etc.

And, if we solve this problem, we will can reuse account password as SSSS password, because it will be typed only on client side, and not leaked to homeserver admins or any other.

@richvdh richvdh changed the title Hashing user password on client side and send only hashed password to server on user login login process sends plain text passwords to the server May 15, 2020
@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

But isn't the SSSS passphrase the same as the login password, which we do send to the server?

No, SSSS passphrase is the "Recovery Passphrase" riot asks for, it can be set to the same but it is explicitly stated it should not be in the prompts.

Your access token is used to control the ability to fetch the SSSS blob, then the Recovery Passphrase (or Recovery Key) is used locally to decrypt it.

@richvdh
Copy link
Member

richvdh commented May 15, 2020

@t3chguy right. in which case, this whole issue seems like a huge storm in a tiny teacup.

@Cadair
Copy link
Contributor

Cadair commented May 15, 2020

No, SSSS passphrase is the "Recovery Passphrase" riot asks for, it can be set to the same but it is explicitly stated it should not be in the prompts.

My thoughts here is can we make it so it can't be set to the same as doing so is baad. 😁

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

in which case, this whole issue seems like a huge storm in a tiny teacup.

Well this issue would in theory, best case, allow the Recovery Passphrase to go away completely.

Take one user input, $input;

User submits password=hash($input)
User uses $input as the SSSS Recovery passphrase

My thoughts here is can we make it so it can't be set to the same as doing so is baad.

Not without sending the recovery passphrase to the server to test if it is the same which defeats the point of it not being the same and the server not knowing it.

@Cadair
Copy link
Contributor

Cadair commented May 15, 2020

Not without sending the recovery passphrase to the server to test if it is the same which defeats the point of it not being the same and the server not knowing it.

I was thinking the server could attempt to use the account password to decrypt it and if it succeeds it would fail?

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

But the server doesn't know the account password unless you submit it again,
plus even then the server could lie to have you upload your real data and then decrypt it.

@t3chguy
Copy link
Member

t3chguy commented May 15, 2020

The only way it'd be possible is if clients stored hash2($password) [other hash func]
and compare it to that. Though it would require all existing sessions to re-auth :(

@uhoreg
Copy link
Member

uhoreg commented May 16, 2020

This issue is largely a dup of https://github.com/matrix-org/matrix-doc/issues/1699 We may not use the method suggested in the doc that is linked in the issue, but I think that issue sufficiently captures the intent of allowing users to only use one password without the server finding out the password. I'll put a comment in that issue that outlines some considerations.

@uhoreg uhoreg closed this as completed May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API improvement A suggestion for a relatively simple improvement to the protocol
Projects
None yet
Development

No branches or pull requests

7 participants