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

Spec SAS verification and the common key verification framework #2072

Merged
merged 5 commits into from Jun 7, 2019

Conversation

Projects
None yet
4 participants
@turt2live
Copy link
Member

commented Jun 4, 2019

Reference implementations:

Proposals:

No alterations to either proposal have been made intentionally here.

Note: this is largely plagiarized from @uhoreg's extremely well written proposals. The implementation, as far as I can tell, matches perfectly with the proposals.

@@ -0,0 +1,66 @@
[

This comment has been minimized.

Copy link
@turt2live

turt2live Jun 4, 2019

Author Member

rationale behind providing this as JSON: if people are expected to use these 64 emoji, they should not have to manually copy/paste it. Unicode is provided as a fallback if for some reason the emoji field can't be used as-is.

This comment has been minimized.

Copy link
@ara4n

ara4n Jun 12, 2019

Member

this is a thing of beauty

@dbkr
Copy link
Member

left a comment

Looks vaguely plausible although I'm not really familiar enough with the protocol-level to OK that part.

@turt2live turt2live requested a review from matrix-org/spec-core-team Jun 5, 2019

@richvdh

richvdh approved these changes Jun 7, 2019

Copy link
Member

left a comment

this looks generally excellent. I have a few niggles and questions, but am happy for you to address the ones you think are appropriate and merge.

Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated
as well as more uniform data to work with.

For verification of each party's device keys, HKDF is as defined in RFC 5869 and
uses SHA-256 as the hash function. The shared secret is supplied as the input keying

This comment has been minimized.

Copy link
@richvdh

richvdh Jun 7, 2019

Member

it seems a bit odd that SHA-256 is hardcoded here but is negotiated for the key agreement phase.

This comment has been minimized.

Copy link
@turt2live

turt2live Jun 7, 2019

Author Member

This is an awkward function of the spec: the agreement phase says that one must agree on a hash function, but then the rest of the spec (including the swagger) says SHA-256

I think it's fine for now because it at least makes it clear that there's only one option for agreement.

Show resolved Hide resolved specification/modules/end_to_end_encryption.rst Outdated

turt2live and others added some commits Jun 7, 2019

Apply suggestions from code review
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

@turt2live turt2live requested a review from richvdh Jun 7, 2019

@richvdh

This comment has been minimized.

Copy link
Member

commented on specification/modules/end_to_end_encryption.rst in 3877896 Jun 7, 2019

contacting

@richvdh

richvdh approved these changes Jun 7, 2019

ing

@turt2live turt2live merged commit 4b6e2cc into master Jun 7, 2019

8 checks passed

buildkite/matrix-doc Build #267 passed (56 seconds)
Details
ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

@turt2live turt2live deleted the travis/1.0/msc1717-msc1267-sas-verification branch Jun 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.