-
Notifications
You must be signed in to change notification settings - Fork 382
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
[WIP] MSC3262 aPAKE authentication #3262
base: old_master
Are you sure you want to change the base?
Conversation
the new password, and as such it doesn't protect against a mitm.* | ||
|
||
|
||
## Security considerations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if a malicious server starts reporting that it no longer supports SRP or that the client does not have SRP configured? It will take some really good UX design by clients to ensure that the client doesn't fall back to a regular login and leak the password to the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a big one, for quite a while we won't be able to assume the server has no access to the plaintext password. I would imagine that the long-term solution is to remove the password
authentication from the matrix spec, and declare all apps still supporting the old password login as incompatible.
I added some thoughts about this. Do you have any additions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is fine. I don't think there is way to enforce it yet. Maybe add a note that clients can consider indicating when SRP is/isn't used. Maybe that indication would even take a while to be enabled by default but it provides a nice path away from sending the password to the server.
- Opportunistic, the client uses SRP if advertised.
- Recommended. The client warns if SRP isn't going to be used. (Or shows a "lock" if it is)
- Enforced. The client refuses to send the password to the server. (At least without some very explicit disclaimer)
Of course we shouldn't mandate client behaviour, however I think it is often helpful to explain what it could look like.
Co-authored-by: Kevin Cox <kevincox@kevincox.ca>
Co-authored-by: Matthew Hodgson <matthew@arasphere.net>
Co-authored-by: Matthew Hodgson <matthew@arasphere.net>
Co-authored-by: Matthew Hodgson <matthew@arasphere.net>
|
||
``` | ||
{ | ||
"type": "m.login.srp6a.verify", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This raises an interesting question about how the Matrix user-interactive auth is supposed to work.
For a stage that requires multiple rounds of communication, should the type
be the same for every request? ie, here it would be m.login.srp6a
for both init and verify, and the details would go in the auth
object.
Or should each "round" of communication be its own UIA stage? In that case, it seems like the flows
returned by GET /_matrix/client/r0/login
should include both stages, ie ["m.login.srp6a.init", "m.login.srp6a.verify"]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flow only has one GET
request at the very start, so returning just m.login.srp6a
seems like it's sufficient to me, supporting just init
or just verify
makes little sense so I would argue that's implied.
If we compare against the webrtc spec (which also has multiple stages) I think using m.login.srp6a.init
and m.login.srp6a.verify
in POST to /login
makes sense. (webRTC in matrix use m.call.invite
, m.call.candidates
, m.call.answer
and m.call.hangup
). We defined an auth_id
in this MSC because we do need to keep track of the flows, you can't verify
before having done the init
.
Then again: srp can map fairly well on the UIA, but that's a seperate MSC and I kept that out of scope here because there's no reason either this msc or the UIA msc needs to depend one another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh, I think I was getting confused between the UIA vs normal login. I thought at some point they were going to all use the UIA? Or maybe that was just a proposal that I saw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so too, I may have missed that the proposal got merged?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course this MSC actually needs to support UIA, because authentication doesn't only happen at /login
so this is added now. It's more or less a duplicate of /login
except all SRP json stuff in put inside an auth
object.
For my $0.02, I think this proposal represents the best immediate path forward for a more secure login mechanism in Matrix. SCRAM has some unfortunate weaknesses, and while @uhoreg's MSC2957 looks great, it has not yet undergone the same level of scrutiny that SRP has. My proposal in MSC3265 is a short-term hack, and only really improves the protection of the SSSS password; MSC3265 was intended to be more informative than prescriptive. |
At @uhoreg's suggestion, I made a quick survey of available SRP libraries that Matrix implementations might use to implement this MSC.
|
The groups are referenced by name as: | ||
|
||
``` | ||
["2048","3072","4096","6144","8192","1536MODP","2048MODP","3072MODP","4096MODP","6144MODP","8192MODP"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these groups, the password hashes and the hash functions be put under the m.*
prefix. This way it is obvious to clients how to add more, and which ones are "official" as part of the spec.
Rendered
This MSC is more or less competing with MSC2957 and MSC3265.
Comments and thoughts very welcome.
I am specifically on the fence about advocating for SRP or OPAQUE.
Signed-off-by: Mathijs van Gorcum mvgorcum@gmail.com