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

crypto/tls: implement and enable X25519Kyber768Draft00 #67061

Closed
FiloSottile opened this issue Apr 26, 2024 · 5 comments
Closed

crypto/tls: implement and enable X25519Kyber768Draft00 #67061

FiloSottile opened this issue Apr 26, 2024 · 5 comments
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Apr 26, 2024

X25519Kyber768Draft00 is a hybrid post-quantum key exchange based on Kyber768 (the predecessor of ML-KEM-768) and X25519 (our current default preferred key exchange). It is defined in draft-tls-westerbaan-xyber768d00-03 and implemented by BoringSSL and rustls (rustls/rustls#1785).

I propose that we implement support for it in crypto/tls (on top of the recently merged crypto/internal/mlkem768 package) and enable it by default, with a GODEBUG opt-out.

We will send both an X25519 and an X25519Kyber768Draft00 key share in our Client Hello messages. This will make the CH significantly bigger, which initially caused some compatibility issues in the ecosystem, now hopefully resolved. The performance overhead will be by default that of an mlkem768.GenerateKey (as the X25519 share will be reused as allowed by draft-ietf-tls-hybrid-design-10).

That's allowed by the current docs of CurvePreferences.

// CurvePreferences contains the elliptic curves that will be used in
// an ECDHE handshake, in preference order. If empty, the default will
// be used. The client will use the first preference as the type for
// its key share in TLS 1.3. This may change in the future.
CurvePreferences []CurveID

Note that CurvePreferences and CurveID are now hopelessly misnomers, due to how TLS 1.3 reused them for key exchange algorithms.

Since this is an experimental algorithm that we plan to remove in the future, we will NOT define a CurveID constant for it. CurveID.String() will support it, though, so e.g. logs are correctly printed.

If applications set Config.CurvePreferences, their behavior will not change and X25519Kyber768Draft00 will not be enabled.

/cc @golang/security @golang/proposal-review

@FiloSottile FiloSottile added Proposal Proposal-Crypto Proposal related to crypto packages or other security issues labels Apr 26, 2024
@FiloSottile FiloSottile added this to the Proposal milestone Apr 26, 2024
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Apr 26, 2024
@rsc
Copy link
Contributor

rsc commented May 8, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to enable X25519Kyber768Draft00 by default (when CurvePreferences = nil). If clients set CurvePreferences, it will be disabled, with no way to force-enable it. A future Go version may make that possible, once the key exchange is no longer a draft.

A new GODEBUG setting will control the behavior. GODEBUG=tlskyber=1 (the new default for go 1.23+ main modules) means enable kyber by default; GODEBUG=tlskyber=0 means don’t.

@rsc rsc moved this from Incoming to Likely Accept in Proposals May 8, 2024
@rsc
Copy link
Contributor

rsc commented May 15, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to enable X25519Kyber768Draft00 by default (when CurvePreferences = nil). If clients set CurvePreferences, it will be disabled, with no way to force-enable it. A future Go version may make that possible, once the key exchange is no longer a draft.

A new GODEBUG setting will control the behavior. GODEBUG=tlskyber=1 (the new default for go 1.23+ main modules) means enable kyber by default; GODEBUG=tlskyber=0 means don’t.

@rsc rsc changed the title proposal: crypto/tls: implement and enable X25519Kyber768Draft00 crypto/tls: implement and enable X25519Kyber768Draft00 May 15, 2024
@rsc rsc moved this from Likely Accept to Accepted in Proposals May 15, 2024
@rsc rsc modified the milestones: Proposal, Backlog May 15, 2024
@dmitshur dmitshur modified the milestones: Backlog, Go1.23 May 18, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/586656 mentions this issue: crypto/tls: implement X25519Kyber768Draft00

@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label May 18, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/592196 mentions this issue: doc/next: improve wording for proposal 67061

gopherbot pushed a commit that referenced this issue Jun 18, 2024
For #67061.
For #65614.

Change-Id: I0e324b02537e0d40c2092139d31941a1a85a0e69
Reviewed-on: https://go-review.googlesource.com/c/go/+/592196
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
@hub-adda
Copy link

hub-adda commented Jun 30, 2024

Hi, I hope that my comment is ok.
It's great to have the curveID parameter as a tls.Conn member. I find it very useful to validate the context in testing, and configuration. Will it be exposed as part of Go1.23?
Moreover, The GODEBUG=tlskyber=1 could enable the new curve. Looking in the code, any other value or a nonexistent 'tlskyber' value will allow it to. Right? if yes, maybe the documentation should describe this method.
Does the function 'IsPQEnabled' makes sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Accepted
Development

No branches or pull requests

5 participants