-
Notifications
You must be signed in to change notification settings - Fork 31
Documentation for PSQv2 & client-authentication proposal #1248
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
base: main
Are you sure you want to change the base?
Conversation
|
Looks like the macOS ASAN nightly has gone a bit stale. Attempting to unpin that with #1258. |
franziskuskiefer
left a comment
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 didn't go too deep yet. But this looks good in general.
There are a bunch of public items without docs. But also no warnings. So they may not be exposed.
| } | ||
|
|
||
| /// Provide a principal's long-term ML-DSA signing key. | ||
| pub fn longterm_mldsa_signing_key(mut self, signing_key: &'a MLDSA65SigningKey) -> Self { |
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.
We have ecdh, mlkem, mldsa, ed25519. The ecdh is the only generic one. Is that on purpose?
| pq_shared_secret, | ||
| }; | ||
|
|
||
| // XXX: This makes clippy unhappy, but is a lifetime error for feature `classic-mceliece` if we return directlyr |
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.
Disable the lint here then.
| // XXX: This makes clippy unhappy, but is a lifetime error for feature `classic-mceliece` if we return directlyr | |
| // XXX: This makes clippy unhappy, but is a lifetime error for feature `classic-mceliece` if we return directly |
| pub fn deserialize( | ||
| bytes: &[u8], | ||
| initiator_ecdh_pk: &DHPublicKey, | ||
| initiator_authenticator: &Authenticator, |
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.
Into directly here doesn't work. But the into on the outside is a little ugly. You could add an authenticator function to the DHKeyPair that does it.
| )) | ||
| } | ||
| } | ||
| #[cfg(not(feature = "classic-mceliece"))] |
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.
Do you need to duplicate all of these matches? Couldn't you just add the additional bits with the feature flag?
| | CiphersuiteName::X25519_MLKEM768_X25519_CHACHA20POLY1305_HKDFSHA256 | ||
| | CiphersuiteName::X25519_NONE_X25519_AESGCM128_HKDFSHA256 | ||
| | CiphersuiteName::X25519_MLKEM768_X25519_AESGCM128_HKDFSHA256 => { | ||
| Authenticator::Dh(self.initiator_ecdh_keys.pk.clone()) |
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 could just be that authenticator function on the dh key pair.
| let pq_encapsulation_deserialized = | ||
| working_ciphersuite.deserialize_encapsulation(pq_encapsulation.as_ref())?; | ||
|
|
||
| let tx1 = tx1_dh( |
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.
Why not move the match into the tx1 and derive_k1 functions? Then everything else appears to be the same.
This PR adds
tls_codecto ML-DSAResponderonce the first initiator message was processed for out-of-band verification. I've added a functionResponder::abort_handshaketo reset the responder if this verification fails.README.mdSession. This will export a secret derived asHKDF-SHA256(K_session, context || "PSQ secret export")wherecontextcan be provided by the application.TODO: