What's the motivation for allowing WrapSession to do its own encryption of the session ticket? Users may need to add additional data into the session ticket, but when would they need to encrypt it? This seems like it'd be safer if users could set SessionState.Extra but the server always handled encoding and encrypting the state into a ticket.

Extra map[string][]byte

A map is convenient for the user here, but is an allocation-heavy way of providing extra data to include in the ticket. I'm also a bit dubious about serialization order of map entries; I can't think of any concrete issues at the moment that might be caused by the serialization of this section being unstable, but it doesn't feel right. String keys are also an inefficient way of encoding data on the wire.

What about making this a plain []byte? It's slightly less convenient for the user if they want to encode a more complex structure, but avoids all these issues. If the user doesn't care about perfect efficiency, they can gob-encode a map.

func (*ClientSessionState) AddSession(identity []byte, state *SessionState) error

Is the idea that persistent storage of a session will retrieve a session with something like this?

sess, err := tls.ParseSessionState(storedBytes)
css := &tls.ClientSessionState{}
err = css.AddSession(storedIdentity, sess)

If so, "AddSession" is a bit of an odd name; "add" sounds like something I can call multiple times. Also, I wonder if this should be a constructor returning a *ClientSessionState:

func NewResumedClientSessionState(identity []byte, state *SessionState) (*ClientSessionState, error)

ticket/identity

What's an identity? Why isn't it a field of SessionState?

What's the motivation for allowing WrapSession to do its own encryption of the session ticket?

That way the application can implement "session IDs" which are just a legacy name for "I'm not encrypting this, I am putting it in a database and giving you back its lookup ID". They are especially useful when doing 0-RTT because you can prevent replays by removing the entry from the db the first time it's retrieved.

What about making this a plain []byte?

Oh I should have documented the intention here. The idea was that a wrapper implementation would put its data under its identifier, so that you could nest wrappers without conflicting. But maybe that's overthinking this, and it would be just fine to say "if Extra is not nil, you need a way to preserve it across a round-trip". Let's do that.

If so, "AddSession" is a bit of an odd name; "add" sounds like something I can call multiple times.

You can call it multiple times if doing PSK because each PSK only works with a certain hash and protocol version. Also, you can just try multiple PSKs if for some reason you have multiple, maybe if you're rotating them.

The ordinary case for resumption will have only one though, so maybe the constructor is a useful helper?

What's an identity? Why isn't it a field of SessionState?

The identity is the opaque bytes the client sends to the server to make it understand what session/PSK it's resuming/using. It can be an encrypted ticket, a session ID, or just a name. It's logically not part of the session state, it's the "name" of the session state, and only the client stores it, while we use SessionState on the server too. Importantly, it's not serialized by SessionState.Bytes (actually it might be the encryption of that output).

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

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?

There are some TODOs in the proposed API which need to be resolved.

I'm guessing that if UnwrapSession returns a non-nil error, the ticket will be discarded but the handshake will proceed. Is that correct?

When resuming a QUIC session, we want to verify that the client's remembered transport parameters match the server's current transport parameters. The server would do that by putting the parameters (or a hash of the parameters) in SessionState.Extra in WrapSession, and then verifying that the parameters still match in UnwrapSession. And if they don't match, UnwrapSession returns an error and we proceed without using the previous session. Correct?

Is there a way to determine which SessionState was used to resume a connection? Say a client has two tickets from two previous sessions with different transport parameters. Can it determine which ticket was used when resuming?

Change https://go.dev/cl/496820 mentions this issue: crypto/tls: add ClientSessionState.ResumptionState and NewResumptionState

Change https://go.dev/cl/496817 mentions this issue: crypto/tls: reduce session ticket linkability

Change https://go.dev/cl/496819 mentions this issue: crypto/tls: use SessionState on the client side

Change https://go.dev/cl/496822 mentions this issue: crypto/tls: add SessionState.Extra and ConnectionState.Session

Change https://go.dev/cl/496821 mentions this issue: crypto/tls: add WrapSession and UnwrapSession

Change https://go.dev/cl/496818 mentions this issue: crypto/tls: add SessionState and use it on the server side

Mailed the CL stack, including the changes suggested above. (The last could two are lacking tests, but I'll add them today or tomorrow.)

In the interest of limiting the complexity and API surface added late in the cycle, I didn't implement external PSKs and TLS 1.2 Session IDs. We know the API can be extended to support them, and there's no rush in doing so.

I had to change the WrapSession and UnwrapSession Config method names to EncryptTicket and DecryptTicket since they would clash with the Config fields.

There are some TODOs in the proposed API which need to be resolved.

The TODO about forcing PSK use is only relevant for external PSKs, since for resumption you can always fallback to a full handshake, but I expect the answer will be "VerifyConnection aborts if DidResume is false".

The TODO about the ClientSessionCache key I think doesn't actually matter: simple clients don't do earlyData, and QUIC stacks will know the cache key better than crypto/tls and probably ignore it and make ClientSessionCache a couple of closures.

I'm guessing that if UnwrapSession returns a non-nil error, the ticket will be discarded but the handshake will proceed. Is that correct?

No, I think we should do lice GetConfigForClient: non-nil error aborts, (nil, nil) discards and continues. I am not a fan of errors getting dropped on the floor.

When resuming a QUIC session, we want to verify that the client's remembered transport parameters match the server's current transport parameters. The server would do that by putting the parameters (or a hash of the parameters) in SessionState.Extra in WrapSession, and then verifying that the parameters still match in UnwrapSession. And if they don't match, UnwrapSession returns an error and we proceed without using the previous session. Correct?

Correct!

Is there a way to determine which SessionState was used to resume a connection? Say a client has two tickets from two previous sessions with different transport parameters. Can it determine which ticket was used when resuming?

Good question. I exposed SessionState.Extra as ConnectionState.Session. Does that work?

Extremely minor nit, but for completeness sake you should probably include NewResumptionState in the API diff above.

Here's the updated API diff. Note that I used ResumptionState instead of ResumptionSession, to rhyme with ClientSessionState and SessionState.

// A SessionState is a resumable session.
type SessionState struct {
    // Extra is ignored by crypto/tls, but is encoded by [SessionState.Bytes]
    // and parsed by [ParseSessionState].
    //
    // This allows [Config.UnwrapSession]/[Config.WrapSession] and
    // [ClientSessionCache] wrappers to store and retrieve additional data.
    Extra []byte
}

// Bytes encodes the session, including any private fields, so that it can be
// parsed by [ParseSessionState]. The encoding contains secret values critical
// to the security of future and possibly past sessions.
//
// The specific encoding should be considered opaque and may change incompatibly
// between Go versions.
func (s *SessionState) Bytes() ([]byte, error)

// ParseSessionState parses a [SessionState] encoded by [SessionState.Bytes].
func ParseSessionState([]byte) (*SessionState, error)

type ConnectionState struct {
    // Session is the [SessionState.Extra] field from a resumed session, if any.
    Session []byte
}

type Config struct {
    // UnwrapSession is called on the server to turn a ticket/identity into a
    // usable session. If used with [Config.WrapSession], it involves decrypting
    // the ticket or using it as a handle to recover a serialized
    // [SessionState], and calling [ParseSessionState]. Alternatively, cs can
    // represent an external PSK.
    UnwrapSession func(identity []byte, cs ConnectionState) (*SessionState, error)

    // WrapSession is called on the server to produce a session ticket. It
    // usually involves calling SessionState.Bytes and encrypting its return
    // value, or storing it and returning a handle for it. Alternatively, the
    // application may choose to store additional data in [SessionState.Extra]
    // and call [Config.WrapSession] to use the default encryption method.
    //
    // Warning: this is a dangerous API. The application is in charge of
    // encrypting tickets and rotating keys, and failing to do so correctly can
    // compromise current, previous, and future connections depending on the
    // protocol version.
    WrapSession func(ConnectionState, *SessionState) ([]byte, error)
}

// EncryptTicket encrypts a ticket with the Config's configured (or default)
// session ticket keys. It can be used as a [Config.WrapSession] implementation.
func (c *Config) EncryptTicket(cs ConnectionState, ss *SessionState) ([]byte, error)

// DecryptTicket decrypts a ticket encrypted by [Config.EncryptTicket]. It can
// be used as a [Config.UnwrapSession] implementation.
//
// If the ticket can't be decrypted or parsed, DecryptTicket returns (nil, nil).
func (c *Config) DecryptTicket(identity []byte, cs ConnectionState) (*SessionState, error)

// ResumptionState returns the session ticket sent by the server (also known as
// the session's identity) and the state necessary to resume this session.
//
// It can be called by [ClientSessionCache.Put] to serialize (with
// [SessionState.Bytes]) and store the session.
func (cs *ClientSessionState) ResumptionState() (ticket []byte, state *SessionState, err error)

// NewResumptionState returns a state value that can be returned by
// [ClientSessionCache.Get] to resume a previous session.
//
// state needs to be returned by [ParseSessionState], and the ticket and session
// state must have been returned by [ClientSessionState.ResumptionState].
func NewResumptionState(ticket []byte, state *SessionState) (*ClientSessionState, error)

I can confirm that this API and the implementation work for the QUIC use case:

• on the server side: adding QUIC transport parameters to the session ticket before sending it to the client
• on the client side: storing QUIC transport parameters alongside the ticket before putting it into the session cache, and restoring them on session resumption

My quic-go branch also contains tests to make sure this works recursively: The application using quic-go can use the WrapSession and UnwrapSession callbacks to store its own data, without colliding with the data stored by the QUIC stack.

We talked with @marten-seemann (see also here), and the late ConnectionState.Session addition is not working as well as intended: a middleware like quic-go can't strip its additional data from it, clashing with applications that expect to use it like in crypto/tls.

The good news is that we don't need it for Go 1.21. It's only needed for #46718 and as @neild pointed out to disambiguate between multiple tickets, but currently crypto/tls only supports sending one ticket per connection (and even on the server where it has to accept multiple, only the first can be used for 0-RTT by spec).

I suggest we go ahead with this downscoped API (#60105 (comment) without ConnectionState.Session) for Go 1.21, which is sufficient for quic-go, and leave the rest for later.

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

After playing around with this API in quic-go for the last week, and writing some tests where the application stores random data und one that uses session IDs, here's what I've found:

First of all, it works: The QUIC stack can store the QUIC transport parameters (and other connection parameters, such as the RTT) in the session ticket. That's great news!

I'd like to propose a small change to the API though: SessionState currently has an Extra []byte field, which the application can use to store application-level information with the session ticket. This is exactly what the QUIC stack does for the transport parameters.

However, the QUIC stack might not be the only one that's interested in using that slice to store additional information. For example, the HTTP/3 layer also needs to store certain values in the session ticket (see Section 7.2.4.2 of RFC 9114). Other implementations running on top of QUIC will probably want to do the same, and there could be multiple layers that want to do that.

This is pretty inconvenient, since at the time the WrapSession callback is called for the application, the QUIC layer will already have filled the Extra slice. Now the application will have to take care of adding its data into the same slice, in a way that allows it to extract that information in the UnwrapSession callback later, and then restore the data before passing it on to the QUIC layer's UnwrapSession callback.
Most likely, this means coming up with some kind of serialization format. It's possible, but every layer that wants to store information in the session ticket will have to invent its own way of doing that. Doing that in a forwards-compatible way most likely also means adding a versioning scheme...

What if we made it more explicit that this is a stack that every layer in the application can push to (on Wrap) and pop from (on Unwrap):

type SessionState struct {
  // not exported
  // PushExtra would append to this slice.
  // PopExtra would pop the last element of this slice.
  extra [][]byte 
}

// PushExtra adds some extra information to the session state.
// This information is ignored by crypto/tls, but is encoded by [SessionState.Bytes]
// and parsed by [ParseSessionState].
//
// This allows [Config.UnwrapSession]/[Config.WrapSession] and
// [ClientSessionCache] wrappers to store and retrieve additional data.
func (SessionState) PushExtra([]byte)

// PopExtra removes extra information from the session state.
func (SessionState) PopExtra() (_ []byte, ok bool)

Any layer of the application (incl. the QUIC stack) would call PushExtra in their WrapSession callback, and retrieve that data saved using PopExtra in UnwrapSession. As today, serializing this field would be handled by crypto/tls, it's just that now we're serialzing a [][]byte instead of a []byte.

Going to move this to accepted, and filed a separate issue for @marten-seemann's latest comment, #60539.

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

Change https://go.dev/cl/501303 mentions this issue: go1.21: document crypto/tls additions

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