The Wayback Machine - https://web.archive.org/web/20200714071738/https://github.com/vector-im/riot-web/issues/13286
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

Two cross-signing keys uploads (of which the first is empty) during registration #13286

Closed
bwindels opened this issue Apr 20, 2020 · 7 comments
Closed

Comments

@bwindels
Copy link
Member

@bwindels bwindels commented Apr 20, 2020 •

/_matrix/client/unstable/keys/device_signing/upload with request body {} which returns a 401. Then later, it does the same request with the cross-signing keys it generated.

@bwindels bwindels changed the title we're uploading an empty device signing upon registration Riot uploads an empty device signing during registration Apr 20, 2020
@bwindels bwindels changed the title Riot uploads an empty device signing during registration Two cross-signing keys uploads (of which the first is empty) during registration Apr 20, 2020
@bwindels
Copy link
Member Author

@bwindels bwindels commented Apr 20, 2020

Are we ok with it coming with a cost of a longer (15s under current circumstances) login / registration flow?

@jryans
Copy link
Member

@jryans jryans commented Apr 20, 2020

Currently we do this to skip a second account password prompt if that's all we would need to upload keys, so I suppose the choices look like:

  • Check auth flows (potentially adding 15s of network delay) to possibly avoid a prompt
  • Show a second password prompt
  • Use some other mechanism (perhaps previous knowledge of auth flows) to avoid the request and preserve behaviour
@dbkr
Copy link
Member

@dbkr dbkr commented Apr 20, 2020

Another option would be for the synapse side to move this request to a work that passes the request to the master if it actually gets past auth, so at least the first request that intentionally fails auth is fast?

@bwindels
Copy link
Member Author

@bwindels bwindels commented Apr 21, 2020

Another option would be for the synapse side to move this request to a work that passes the request to the master if it actually gets past auth, so at least the first request that intentionally fails auth is fast?

Apparently this would not be straightforward to do on synapse, so maybe not for now.

@bwindels
Copy link
Member Author

@bwindels bwindels commented Apr 22, 2020

One thing I don't understand here is why we can't assume that the server will always require interactive auth? The MSC seems to imply so at least. Are there any conditions that the homeserver would not enforce this? E.g. if you have interactively authenticated recently already?

@jryans
Copy link
Member

@jryans jryans commented Apr 22, 2020

During the standup, we were it may be possible to skip the first request that tests auth and instead pass along as password if we have one. In the current world, it's highly likely for a server to require the same auth for login and key upload, so it's reasonable to pass along if we have it. No information is leaked, since we sending the password to the homeserver which already has it.

@jryans jryans self-assigned this Apr 22, 2020
@jryans jryans added this to In Progress in Workflow via automation Apr 22, 2020
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Apr 22, 2020
If we already have an account password to use during secret storage setup, then
it's highly likely that the homeserver accepts passwords for device signing key
upload as well. This change then assumes password auth will work without
checking to avoid a request when the server is under high load.

Fixes vector-im/riot-web#13286
jryans added a commit to matrix-org/matrix-react-sdk that referenced this issue Apr 22, 2020
If we already have an account password to use during secret storage setup, then
it's highly likely that the homeserver accepts passwords for device signing key
upload as well. This change then assumes password auth will work without
checking to avoid a request when the server is under high load.

Fixes vector-im/riot-web#13286
@jryans jryans moved this from In Progress to In Review in Workflow Apr 22, 2020
Workflow automation moved this from In Review to In Test Apr 22, 2020
@jryans jryans moved this from In Test to In Review in Workflow Apr 22, 2020
@jryans jryans moved this from In Review to In RC in Workflow Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.