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

Change how suites are identified and finalize test vectors #371

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

chris-wood
Copy link
Collaborator

@chris-wood chris-wood commented Dec 19, 2022

This is a breaking update that changes how ciphersuites are identified. Previously, we used two-byte identifiers under the assumption that these identifiers would be needed for agreement on ciphersuites in different applications. That's really not necessary, especially when applications can just require certain configurations as desired. For example, Privacy Pass only uses one ciphersuite (P384-SHA384), and we can just say "use the (P384-SHA384) version" instead of "use the version identified by the two-byte value 0x0004." Plus, the two-byte value would require establishment of a registry to manage the space. Too much complexity!

This change follows in the footsteps of FROST. It identifies each ciphersuite by a unique string and then uses that string in place of the two-byte identifier. No registry is needed to manage these things. Protocols that need negotiation for different ciphersuites can establish identifiers (and an appropriate registry) as needed for their use case.

While updating this part, I also "finalized" the test vectors by removing the draft version number from the context string.

cc @kevinlewi, @bytemare, @aldenml, @rolfeschmidt for awareness and to help confirm the new (and final) test vectors

@bytemare
Copy link
Contributor

I strongly agree with this change. I think this is more convenient, even from a developer perspective. A one-byte identifier should be largely enough to cover a large scale of ciphersuites.

@chris-wood
Copy link
Collaborator Author

A one-byte identifier should be largely enough to cover a large scale of ciphersuites.

To clarify (just in case there was confusion from the PR's description), this change replaces the two-byte identifier with an ASCII string identifier, not a one-byte identifier.

Copy link
Collaborator

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

remove this file: poc/opaque_drbg.sage ?

@chris-wood
Copy link
Collaborator Author

Good catch -- thanks. @armfazh, could you please verify the test vectors against the CIRCL implementation?

poc/oprf.sage Show resolved Hide resolved
@kevinlewi
Copy link
Contributor

cc: @daxpedda as the change of identifier from u16 to string will mean we need to change the type of the "ID" that was defined in VoprfParameters here: RustCrypto/traits#878

@armfazh
Copy link
Collaborator

armfazh commented Dec 20, 2022

could you please verify the test vectors against the CIRCL implementation?

confirmed test vectors at: cloudflare/circl#388

@bytemare
Copy link
Contributor

@chris-wood my bad, I misread. Why use an ASCII identifier?
I understand you want to lift identifier handling to higher-level applications

@chris-wood
Copy link
Collaborator Author

@chris-wood my bad, I misread. Why use an ASCII identifier? I understand you want to lift identifier handling to higher-level applications

We use the identifiers for domain separation across the different suites.

@bytemare
Copy link
Contributor

Vectors look good for bytemare/voprf and bytemare/opaque 👍

@bytemare
Copy link
Contributor

Should #347 be merged into this ?

@chris-wood
Copy link
Collaborator Author

Should #347 be merged into this ?

I'm going to leave that change out for now. It's aesthetic in nature and something we can address separately without changing the test vectors.

@chris-wood
Copy link
Collaborator Author

@kevinlewi do you think you'd be able to verify test vectors?

@kevinlewi
Copy link
Contributor

@chris-wood Unfortunately I cannot really do so until the type change is registered in the upstream RustCrypto/traits library. But it seems like we want to wait until making that change until this one is confirmed. I think it's ok to land this without me verifying the test vectors.

@chris-wood
Copy link
Collaborator Author

Sounds good @kevinlewi. I think I'm comfortable merging this and revving the document based on confirmation from @armfazh and @bytemare. I'll do so early next week.

@chris-wood chris-wood merged commit 5d7d94c into main Jan 9, 2023
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.

4 participants