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

Tweaks to the QR code login crypto #4129

Merged
merged 12 commits into from
Apr 16, 2024

Conversation

dkasak
Copy link
Member

@dkasak dkasak commented Apr 11, 2024

These are suggested changes to #4108:

  • Derive separate encryption keys and nonces for the two sides of the secure channel
  • Mandate that the new device commits to a particular identity key (by setting the device ID to the public part) and proves ownership of it (via a combination of ECDH and HMAC-SHA256) these have been moved to Mandate that the new device commits to a particular identity key #4130
  • Spell out HKDF parameters in text too
  • Misc style fixes

Copy link
Contributor

@poljar poljar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some nits.

proposals/4108-oidc-qr-login.md Outdated Show resolved Hide resolved
proposals/4108-oidc-qr-login.md Outdated Show resolved Hide resolved
proposals/4108-oidc-qr-login.md Outdated Show resolved Hide resolved
proposals/4108-oidc-qr-login.md Outdated Show resolved Hide resolved
@hughns hughns self-assigned this Apr 12, 2024
@hughns hughns self-requested a review April 12, 2024 11:33
@hughns
Copy link
Member

hughns commented Apr 15, 2024

FYI - I pulled most of the style fixes to the MSC to make the material changes in this PR easier to parse.

@hughns hughns merged commit aa37af9 into element-hq/oidc-qr-login Apr 16, 2024
1 check passed
@hughns hughns deleted the dkasak/oidc-qr-login-crypto-tweaks branch April 16, 2024 12:23
Copy link
Member

@hughns hughns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants