cc @neild and @FiloSottile

It would be great if we could come to a conclusion here before the 1.21 release and we've committed to the Extra []byte API.

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

Config.WrapSession and Config.UnwrapSession aren't a very good API for injecting additional data into the session ticket. UnwrapSession has no way of knowing whether the ticket it unwraps is going to be used or not. It's possible for UnwrapSession to be called multiple times; if a ticket is used, it will be the one from the last UnwrapSession call, but this isn't documented.

WrapSession and UnwrapSession need to close over some form of per-connection state that holds the additional per-session data. This is inconvenient.

A primary purpose of WrapSession and UnwrapSession is to provide the ability to use a handle to a stored session as the ticket. (For example, store the session in a database and return a handle to the database entry.) The composition of
a function that stores the session with functions that modify it is confusing: For example, if we have a QUIC implementation, an HTTP/3 implementation, and a user program that stores sessions in a database, which layer should call which other layer?

This can all be made to work, mostly, but I there's a missing API here for adding data to the session ticket. (We discussed this a bit in https://go.dev/cl/496822, which resulted in ConnectionState.Session, which formed part of that API, being deferred for 1.22.)

I think the general shape of that API will be a means of adding data to the ticket for a connection, and a means of getting the data from the ticket (if any) which was used in a resumed session. Perhaps something along the lines of:

type Config struct { // contains existing fields
  // SessionData contains data which will be included in session tickets.
  SessionData []byte
}

type ConnectionState struct { // contains existing fields.
  // SessionData contains data from the resumed session, if any.
  SessionData []byte
}

This separates the concern of ticket storage (WrapSession, UnwrapSession) from additional session data. (Perhaps a method on *Conn and *QUICConn would be a better way to provide the additional data--conn.SetSessionData(b). I don't have a firm opinion here.)

This doesn't address the concern of composing stored ticket data from different layers, such as QUIC and HTTP/3, however.

The proposal here is to use a [][]byte, allowing for data from multiple sources. The problem I see with this is that the ordering of items in that slice isn't very obvious. In the case of WrapSession/UnwrapSession, we might treat this as a stack, but I don't think WrapSession/UnwrapSession are the right API for this going forward.

I think the right approach is to address this at a layer above crypto/tls. The QUIC layer can provide the TLS layer with additional data to store in the ticket. The HTTP/3 layer can provide the QUIC layer with it's own additional data, which the QUIC layer will then pass on to TLS. This requires an API in the QUIC layer to accept and return additional per-connection data, but that seems much cleaner than having HTTP/3 reach through the QUIC layer to insert its own ticket data.

With that approach, it might be slightly simpler for the QUIC layer if crypto/tls accepted a [][]byte rather than a []byte for the ticket extra data. Ordering of data in the slice isn't a concern if the QUIC layer constructs the slice itself. On the other hand, marshaling the necessary data into a []byte doesn't seem difficult and might allow for improved efficiency over asking crypto/tls to do it.

So my inclination is to say that we should leave things as-is, and add a better API for storing data in tickets in 1.22.

A primary purpose of WrapSession and UnwrapSession is to provide the ability to use a handle to a stored session as the ticket. (For example, store the session in a database and return a handle to the database entry.) The composition of
a function that stores the session with functions that modify it is confusing: For example, if we have a QUIC implementation, an HTTP/3 implementation, and a user program that stores sessions in a database, which layer should call which other layer?

This is actually pretty straightforward. I implemented a test in quic-go to test this:

• On WrapSession, the QUIC layer sets its data in Extra, then calls the WrapSession callback that the HTTP/3 layer set.
• The WrapSession callback set by the HTTP/3 layer adds its data (this is where Extra being a byte slice is really inconvenient, since it now needs to come up with a way to serialize it, such that it can later tell its own data apart from the data that the QUIC layer saved). It then calls the WrapSession callback set by the application.
• The WrapSession callback set by the application now takes the SessionState, and calls the Bytes method. This marshals the session state (including the Extra field). It saves those bytes into its database, and returns the row ID for the database entry.

In UnwrapSession, these calls will happen in the reverse order.

This can all be made to work, mostly, but I there's a missing API here for adding data to the session ticket. (We discussed this a bit in https://go.dev/cl/496822, which resulted in ConnectionState.Session, which formed part of that API, being deferred for 1.22.)

Config.WrapSession and Config.UnwrapSession aren't a very good API for injecting additional data into the session ticket. UnwrapSession has no way of knowing whether the ticket it unwraps is going to be used or not. It's possible for UnwrapSession to be called multiple times; if a ticket is used, it will be the one from the last UnwrapSession call, but this isn't documented.

The fact that the server can receive a ClientHello containing multiple session tickets, and the fact that crypto/tls could (in principle) include multiple session tickets when establishing a new connection, makes it necessary to expose some kind of API that allows the application to learn which session ticket was actually chosen. https://go.dev/cl/496822 tried to solve this by exposing the Extra byte slice from the SessionState, but we realized that this was a bad fit, as this slice is modified by every layer it passes through.

I suggested that this could be easily be solved by instead exposing an opaque identifier on the SessionState (func (SessionState) ID() uint64 // or string or []byte), and put that identifier on the ConnectionState. Client and server (applications) can then save the results of the calls to UnwrapSession for every ID until the handshake completes, and then discard all but the one that was actually used.

As crypto/tls currently only uses a single session ticket on the client side, and on the server side, only the first session ticket offered by the client can be used for 0-RTT, this API is not needed to make 0-RTT support work in Go 1.21.

I think the right approach is to address this at a layer above crypto/tls. The QUIC layer can provide the TLS layer with additional data to store in the ticket. The HTTP/3 layer can provide the QUIC layer with it's own additional data, which the QUIC layer will then pass on to TLS. This requires an API in the QUIC layer to accept and return additional per-connection data, but that seems much cleaner than having HTTP/3 reach through the QUIC layer to insert its own ticket data.

First of all, the QUIC layer needs to be able handle tls.Configs that have WrapSession and UnwrapSession set. This is now part of the official API of tls.Config, and applications will use it. As a QUIC stack, you'll have to deal with that.

It seems like you're suggesting to add a new API. I believe this API would be very unclean for multiple reasons:

1. On the server side, we'd need a GetDataForSessionTicket() []byte callback that the QUIC stack calls when issuing a new session ticket, offering the application to add data to that session ticket.
2. Also on the server side, we'd need a ProcessDataFromSessionTicket([]byte) (allow0RTT bool) callback. This callback would be called when a new session ticket is processed in the UnwrapSession callback. The application will need to be able to reject 0-RTT based on the data restored from that ticket. Crucially, this callback would also need to handle being called multiple times, as the client can offer multiple session tickets, and we'd need a way to then learn which ticket was actually used.
3. On the client side, we'd need a AddDataToReceivedSessionTicket(*SessionState) []byte, allowing the client to store data with a newly received session ticket. This callback would be called just before the ClientSessionCache.Put call.
4. On the client side, we'd also need a RestoreDataFromSessionTicket(*SessionState, []byte) (allow0RTT bool) callback, that's called when the TLS stack gets a session ticket to use for a new connection. Again, this could in principle be called multiple times for a connection.

That's a huge additional API surface to expose, just to solve exactly the same problem that we've just solved with the two new WrapSession / UnwrapSession callbacks on the Config and one extra field on the SessionState.

With that approach, it might be slightly simpler for the QUIC layer if crypto/tls accepted a [][]byte rather than a []byte for the ticket extra data. Ordering of data in the slice isn't a concern if the QUIC layer constructs the slice itself. On the other hand, marshaling the necessary data into a []byte doesn't seem difficult and might allow for improved efficiency over asking crypto/tls to do it.

Having a [][]byte instead of a []byte would already be a huge win in terms of UX here. Every layer would then just append its []byte in WrapSession and pop it in UnwrapSession. Effectively, this is what the PushExtra / PopExtra methods I suggested would do, making it less easy to misuse the API.

Having crypto/tls define how the [][]byte is serialized actually simplifies things quite a bit. Realistically, in addition to coming up with a way to serialize the data on every layer, you'd also need to add a version number if they want to be able to ever change that serialization format. In crypto/tls, there already exists a version number in SessionState.version. This will be more efficient.

So my inclination is to say that we should leave things as-is, and add a better API for storing data in tickets in 1.22.

As I've described above, I don't think that the new API we have now is terrible. It has been in the making for years (for example, the issue to add session ID support is 5 years old). Quite a lot of thought has gone into it, and it solves a wide variety of use cases. fwiw, wrapping callbacks in the tls.Config already is a pretty common thing to do, and this is just one more instance of that.

Making a higher level API for storing session data is complex because there isn't even a unique definition of what a session is. Does the data propagate just one hop or through a chain of resumptions? Is the data available at UnwrapSession time (so you can choose whether to reject the ticket or not, which is necessary for 0-RTT) or only if the session is selected?

It also doesn't solve the original issue here of letting various layers (application, HTTP/3, QUIC) store each their own data. As I understand it, @neild suggests having each layer give the data to the one immediately below it, but @marten-seemann wants an application that works over crypto/tls to also work over QUIC without using two different APIs.

At this point in the process, I think we have space for very limited changes. Either we back something out, or we make minor alterations like turning Extra []byte into Extra [][]byte.

What's the consensus on Extra [][]byte being an improvement? I lean in favor of it: applications can be instructed to only append to it, and to only drop entries from the end. It's trivial to encode, and will cost just three extra bytes (for a uint24 prefix).

I'm okay with Extra [][]byte. In the worst case the cost is small, and it's potentially a small convenience even if not necessary. Maybe limit the number of entries to 255 and use a 1-byte prefix? I have a hard time imagining a use case that needs more than a few entries.

Another problem with setting the session data in WrapSession is that it requires each layer to copy the *tls.Config to add its data. Perhaps I'm off on how the *tls.Config is intended to be used, but needing to make multiple new copies of the config for each new connection doesn't seem right.

Another problem with setting the session data in WrapSession is that it requires each layer to copy the *tls.Config to add its data. Perhaps I'm off on how the *tls.Config is intended to be used, but needing to make multiple new copies of the config for each new connection doesn't seem right.

I agree that it's not ideal, but it seems like we haven't been able to come up with a better API.

What's the consensus on Extra [][]byte being an improvement? I lean in favor of it: applications can be instructed to only append to it, and to only drop entries from the end. It's trivial to encode, and will cost just three extra bytes (for a uint24 prefix).

That would be a significant improvement over []byte. I agree that 256 entries should be more than enough though.

Draft implementation in https://go.dev/cl/501306.

It turns out the RFC 8446 presentation language only allows for length prefixes measured in bytes, so this adds 3 bytes for the prefix rather than the 1 I suggested.

Change https://go.dev/cl/501306 mentions this issue: crypto/tls: change SessionState.Extra to [][]byte

Sounds like Extra [][]byte is the answer. Thanks.

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

It turns out the RFC 8446 presentation language only allows for length prefixes measured in bytes, so this adds 3 bytes for the prefix rather than the 1 I suggested.

Is that so? https://www.rfc-editor.org/rfc/rfc8446#section-3.4 has some examples for 2-byte length fields, and some of the slices in handshake messages are encoded using uint16s (e.g. certificateRequestMsgTLS13.certificateAuthorities).

Change https://go.dev/cl/501675 mentions this issue: crypto/tls: make SessionState.Extra a slice of byte slices

RFC 8446 presentation language encodes the bytes length, not the items length, as the prefix. (This is so unknown fields can be skipped without introspecting them.) If you want a field to be potentially more than 16kB, you need a uint24 for the size.

@golang/release flagging for freeze exception to fix a new Go 1.21 API.

@FiloSottile I'm unsure this needs one, and we're already tracking this since it has a release-blocker label in the Go1.21 milestone (but thanks for flagging anyway).

Fixing a new Go 1.21 API sounds like work that's permitted by the freeze policy. That is, if there's a problem with an API new to this release and it warrants being a release-blocker, then it needs to be resolved (one way or another) for the release to be issued. If after re-reading https://go.dev/s/release#freeze-exceptions you still think an exception is needed, can you please file it as a new issue and provide a rationale? I'll revert the title for now. Thanks.

RFC 8446 presentation language encodes the bytes length, not the items length, as the prefix. (This is so unknown fields can be skipped without introspecting them.) If you want a field to be potentially more than 16kB, you need a uint24 for the size.

Thanks for confirming, then my understanding of the representation language was correct. I don't think we want to have session tickets larger than 64k. This kind of defeats the purpose of session resumption speeding up the handshake. I don't think it's even possible, since RFC 8446, Section 4.2.11 uses a uint16 for the label prefix.

Change https://go.dev/cl/501775 mentions this issue: crypto/tls: unexport SessionState.Extra, add PushExtraData / PopExtraData

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