golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.97k stars 17.67k forks source link

crypto/tls: advanced sessions APIs #60105

Open FiloSottile opened 1 year ago

FiloSottile commented 1 year ago

Currently, we support TLS 1.2 and TLS 1.3 session resumption with very little flexibility: applications can only store sessions in memory on the client side, and can only provide session ticket encryption keys on the server side.

This has a few limitations: we don't support external PSKs, we don't support session IDs on the server side, we don't support storing sessions offline on the client side. Moreover, complex applications like a QUIC stack need extra control over session storage to implement features like 0-RTT.

We propose an advanced sessions API that addresses all of the above.

// A SessionState is a resumable session.
type SessionState struct {
    // Secret is the secret necessary for resumption.
    Secret []byte

    // Version is the TLS version with which this session can be used.
    Version uint16

    // CipherSuite needs to be compatible with the cipher suite used
    // to resume this session, meaning its hash has to match.
    CipherSuite uint16

    // 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 map[string][]byte

    // Unexported fields: certificates, timestamps, and other crypto/tls private
    // information that is not necessary for external PSKs.
}

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

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

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.
    //
    // TODO: how does the application specify that the PSK must be used, and
    // what do we do if it's incompatible?
    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)

    ClientSessionCache ClientSessionCache // unchanged
}

// WrapSession encodes and encrypts a session ticket with the default or
// configured session ticket encryption key.
func (*Config) func WrapSession(*ConnectionState, *SessionState) ([]byte, error)

// UnwrapSession decrypts a ticket encrypted by [Config.WrapSession].
func (*Config) func UnwrapSession(idenity []byte, cs *ConnectionState) (*SessionState, error)

type ClientSessionCache interface { // unchanged
    // TODO: should the sessionKey include the ALPN? What about the suite?
    Get(sessionKey string) (session *ClientSessionState, ok bool)
    Put(sessionKey string, cs *ClientSessionState)
}

type ClientSessionState struct { // unchanged
    // No unexported fields.
}

// ResumptionSession returns the session ticket sent by the server and the state
// necessary to resume this session. It does not return any sessions added with
// AddSession. It's meant to be called by [ClientSessionCache.Put] to serialize
// (with [SessionState.Bytes]) and store the session.
func (*ClientSessionState) ResumptionSession() (identity []byte, state *SessionState, err error)

// AddSession adds a resumable session. This can be a session previously
// retrieved via [ClientSessionState.ResumptionSession], or an external PSK.
//
// TODO: again, how does the application express a requirement for the PSK to be
// used, for example because it provides authentication?
func (*ClientSessionState) AddSession(identity []byte, state *SessionState) error

Although we don't implement the spec itself, this API enables an implementation of RFC 9258 PSK importers either externally or in the future in the standard library. On the client side ClientSessionState.AddSession is called for each ipskx (one for each KDF supported by the supported cipher suites), and on the server side the identity is parsed in Config.UnwrapSession and the correct ipskx is derived and returned. We'll need to add a Imported bool to SessionState to request the use of the imp binder derivation path. (Should we add that already even if we don't add RFC 9258 helpers yet? Should we add RFC 9258 helpers?)

This was designed with @marten-seemann, although note that I changed ClientSessionState: instead of exposing a single session as fields, I added methods so that multiple PSKs can be provisioned for use with different hashes.

/cc @golang/security @golang/proposal-review @davidben (if he has opinions)

neild commented 1 year ago

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?

FiloSottile commented 1 year ago

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).

rsc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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?

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

FiloSottile commented 1 year ago

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?

rolandshoemaker commented 1 year ago

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

FiloSottile commented 1 year ago

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)
marten-seemann commented 1 year ago

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

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.

FiloSottile commented 1 year ago

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 (https://github.com/golang/go/issues/60105#issuecomment-1557686871 without ConnectionState.Session) for Go 1.21, which is sufficient for quic-go, and leave the rest for later.

rsc commented 1 year ago

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

marten-seemann commented 1 year ago

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.

rsc commented 1 year ago

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

rsc commented 1 year ago

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

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

hkishn commented 4 months ago

@FiloSottile can we disable this in golang 1.21 with some flag ? This causes an issue with large threads https://github.com/golang/go/issues/68302