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: QUIC 0-RTT APIs #60107

Closed
FiloSottile opened this issue May 10, 2023 · 22 comments
Closed

crypto/tls: QUIC 0-RTT APIs #60107

FiloSottile opened this issue May 10, 2023 · 22 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@FiloSottile
Copy link
Contributor

This proposal adds a couple features on top of #60105 and #44886 to let QUIC stacks implement 0-RTT. It does NOT add 0-RTT support for TLS connections. The hard part of supporting 0-RTT in TLS is figuring out a safe application API which doesn't break the net.Conn contract. A QUIC stack has a easier to adapt interface with its applications.

const QUICEncryptionLevelEarly QUICEncryptionLevel

const QUICRejectedEarlyData QUICEventKind

type SessionState struct {
    EarlyData bool
}

func (*QUICConn) SendSessionTicket(earlyData bool) ([]QUICEvent, error)

The flow on the server side is:

  1. After the handshake has completed, the server calls SendSessionTicket with earlyData set to true, and gets back a QUICWriteData event to send a fresh session ticket with the early_data extension. During this call, the Config.WrapSession callback is called and allows the QUIC stack to store parameters in SessionState.Extra.
  2. In a following handshake, the server receives a ticket which gets passed to Config.UnwrapSession. The state has SessionState.EarlyData set to true, if the server wants to reject early data it sets it to false before returning it.
  3. If crypto/tls likes the session for resumption and for early data, HandleData returns a QUICSetReadSecret with level QUICEncryptionLevelEarly before QUICEncryptionLevelHandshake. If the former is not returned before the latter, early data was rejected.

The flow on the client side is:

  1. The client receives a session ticket and ClientSessionCache.Put gets called. ClientSessionState.ResumptionSession returns a SessionState with EarlyData set to true if the server enabled early data support.
  2. ClientSessionCache.Get returns that session for a later connection during Start. If the client doesn't want to request early data, it can set SessionState.EarlyData to false.
  3. Start returns a QUICSetWriteSecret with QUICEncryptionLevelEarly if the session is good and early data was be offered. The client can start sending early data.
  4. If the server rejects early data in its Encrypted Extensions, HandleData returns a QUICRejectedEarlyData event before QUICEncryptionLevelApplication keys are returned. If the former is not returned before the latter, the early data was accepted.

This was designed with @marten-seemann and if I'm not mistaken along with #60105 and #44886 it should allow quic-go to drop its crypto/tls fork ✨

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

@FiloSottile FiloSottile added Proposal Proposal-Crypto Proposal related to crypto packages or other security issues labels May 10, 2023
@FiloSottile FiloSottile added this to the Proposal milestone May 10, 2023
@marten-seemann
Copy link
Contributor

Thank you @FiloSottile! You're right, this would allow quic-go to drop the crypto/tls fork.
Needless to say, I'm planning to do exactly that the minute this proposal is implemented.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals May 10, 2023
@ianlancetaylor
Copy link
Member

CC @neild

@neild
Copy link
Contributor

neild commented May 10, 2023

After the handshake has completed, the server calls SendSessionTicket with earlyData set to true, and gets back a QUICWriteData event to send a fresh session ticket with the early_data extension.

Currently, the server will include a session ticket (without the early_data extension set) in the flight of data which concludes the handshake. We presumably don't want to send both a session ticket without early_data and one with. How will that work? Do QUIC connections only send session tickets when SendSessionTicket is called? What happens if QUICConn.SendSessionTicket is called when Config.SessionTicketsDisabled is true?

The state has SessionState.EarlyData set to true, if the server wants to reject early data it sets it to false before returning it.

I think it'd be safer if the server needs to explicitly accept early data, but it doesn't matter all that much.

@FiloSottile
Copy link
Contributor Author

Currently, the server will include a session ticket (without the early_data extension set) in the flight of data which concludes the handshake. We presumably don't want to send both a session ticket without early_data and one with. How will that work? Do QUIC connections only send session tickets when SendSessionTicket is called?

Yep, that's the idea. The current behavior would be equivalent to calling HandleData and then SendSessionTicket immediately after. This also allows sending multiple tickets if desired.

What happens if QUICConn.SendSessionTicket is called when Config.SessionTicketsDisabled is true?

I'd say SendSessionTicket returns an error.

I think it'd be safer if the server needs to explicitly accept early data, but it doesn't matter all that much.

For what it's worth, the server needs to have called SendSessionTicket with earlyData true for this to happen, the client can't force it unilaterally.

@rsc
Copy link
Contributor

rsc commented May 10, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals May 10, 2023
@rsc
Copy link
Contributor

rsc commented May 17, 2023

This is a lot of new API, but my understanding is that @FiloSottile, @neild, and @marten-seemann are all in favor of it.

Have all remaining concerns been addressed?

@neild
Copy link
Contributor

neild commented May 17, 2023

LGTM. Minor change: Under the latest version of #44886, the SendSessionTicket signature will be:

func (*QUICConn) SendSessionTicket(earlyData bool) error

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/496995 mentions this issue: crypto/tls: add QUIC 0-RTT APIs

@marten-seemann
Copy link
Contributor

I updated my branch in quic-go to make use of the new API exposed in https://go.dev/cl/496995.
My 0-RTT test suite passes. It works!

@rsc
Copy link
Contributor

rsc commented May 24, 2023

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

@rsc rsc moved this from Active to Likely Accept in Proposals May 24, 2023
@dmitshur dmitshur modified the milestones: Proposal, Go1.21 May 25, 2023
@dmitshur
Copy link
Contributor

This got closed but the proposal is still in progress. Reopening so it can get to a resolution (and if it doesn't make it to accepted state, to revert the CL).

@dmitshur dmitshur reopened this May 25, 2023
@rsc
Copy link
Contributor

rsc commented May 31, 2023

(It's all good @dmitshur, as long as it's in the right column of the Proposal board.)

@rsc
Copy link
Contributor

rsc commented May 31, 2023

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

@rsc rsc moved this from Likely Accept to Accepted in Proposals May 31, 2023
@rsc rsc changed the title proposal: crypto/tls: QUIC 0-RTT APIs crypto/tls: QUIC 0-RTT APIs May 31, 2023
@neild
Copy link
Contributor

neild commented Jul 29, 2023

I think we got the SendSessionTicket API a bit wrong. Currently:

func (q *QUICConn) SendSessionTicket(earlyData bool) error

I believe we're likely to want to allow configuring the session ticket in other ways than setting the early_data extension. To allow for future expansion, SendSessionTicket should take a struct instead:

type QUICSessionTicket struct {
  EarlyData bool
}

func (q *QUICConn) SendSessionTicket(ticket QUICSessionTicket) error

I've got ideas about what additional fields we might want to put in there, but that's a separate proposal and doesn't need to happen for Go 1.21. For now, I'd just like to keep our options open.

@marten-seemann
Copy link
Contributor

I agree. Using a config struct seems like an easy way to make this API more future proof.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514997 mentions this issue: crypto/tls: change SendSessionTicket to take an options struct

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/514999 mentions this issue: quic: don't send session tickets

@dr2chase
Copy link
Contributor

dr2chase commented Aug 2, 2023

Do these two CLs solve (close) the issue? If this is truly a release-blocker for 1.21, it needs to be taken care of soon. I can help with the process parts of kicking it along (release rotation this week) but might not be qualified for a technical review.

@neild
Copy link
Contributor

neild commented Aug 2, 2023

https://go.dev/cl/514997 is the proposed API change. If we accept it, then we'll also need a backport to 1.21.

I'm not sure what the right path is for tweaks to accepted API proposals like this; I think the change is uncontroversial, but perhaps the proposal committee should give it a thumbs up.

@rsc
Copy link
Contributor

rsc commented Aug 2, 2023

Given that @neild, @marten-seemann, and @FiloSottile all seem to be in favor of this tiny API change and are also the complete set of people who could possibly be affected, this sounds good. Tiny change approved.

gopherbot pushed a commit that referenced this issue Aug 2, 2023
To allow for future evolution of the API, make
QUICConn.SendSessionTicket take a QUICSessionTicketOptions
rather than a single bool.

For #60107

Change-Id: I798fd0feec5c7581e3c3574e2de99611c81df47f
Reviewed-on: https://go-review.googlesource.com/c/go/+/514997
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/515335 mentions this issue: [release-branch.go1.21] crypto/tls: change SendSessionTicket to take an options struct

gopherbot pushed a commit to golang/net that referenced this issue Aug 2, 2023
The crypto/tls QUIC session ticket API may change prior to the
go1.21 release (see golang/go#60107). Drop session tickets
entirely for now. We can revisit this when adding 0-RTT support
later, which will also need to interact with session tickets.

For golang/go#58547

Change-Id: Ib24c456508e39ed11fa284ca3832ba61dc5121f3
Reviewed-on: https://go-review.googlesource.com/c/net/+/514999
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
@neild neild closed this as completed Aug 2, 2023
gopherbot pushed a commit that referenced this issue Aug 2, 2023
…an options struct

To allow for future evolution of the API, make
QUICConn.SendSessionTicket take a QUICSessionTicketOptions
rather than a single bool.

For #60107

Change-Id: I798fd0feec5c7581e3c3574e2de99611c81df47f
Reviewed-on: https://go-review.googlesource.com/c/go/+/514997
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
(cherry picked from commit a915b99)
Reviewed-on: https://go-review.googlesource.com/c/go/+/515335
Auto-Submit: Damien Neil <dneil@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/549196 mentions this issue: doc/go1.22: remove reference to #60107

gopherbot pushed a commit that referenced this issue Dec 12, 2023
This was implemented in Go 1.21.

Change-Id: Ic434670938589f10f367b1f893c4427e6f0b991c
Reviewed-on: https://go-review.googlesource.com/c/go/+/549196
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
This was implemented in Go 1.21.

Change-Id: Ic434670938589f10f367b1f893c4427e6f0b991c
Reviewed-on: https://go-review.googlesource.com/c/go/+/549196
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
@rsc rsc removed this from Proposals Aug 7, 2024
@golang golang locked and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

No branches or pull requests

8 participants