diff --git a/common.go b/common.go index d0a1fe7..b906f59 100644 --- a/common.go +++ b/common.go @@ -762,10 +762,6 @@ type Config struct { } const ( - // ticketKeyNameLen is the number of bytes of identifier that is prepended to - // an encrypted session ticket in order to identify the key used to encrypt it. - ticketKeyNameLen = 16 - // ticketKeyLifetime is how long a ticket key remains valid and can be used to // resume a client connection. ticketKeyLifetime = 7 * 24 * time.Hour // 7 days @@ -777,9 +773,6 @@ const ( // ticketKey is the internal representation of a session ticket key. type ticketKey struct { - // keyName is an opaque byte string that serves to identify the session - // ticket key. It's exposed as plaintext in every session ticket. - keyName [ticketKeyNameLen]byte aesKey [16]byte hmacKey [16]byte // created is the time at which this ticket key was created. See Config.ticketKeys. @@ -791,15 +784,18 @@ type ticketKey struct { // bytes and this function expands that into sufficient name and key material. func (c *Config) ticketKeyFromBytes(b [32]byte) (key ticketKey) { hashed := sha512.Sum512(b[:]) - copy(key.keyName[:], hashed[:ticketKeyNameLen]) - copy(key.aesKey[:], hashed[ticketKeyNameLen:ticketKeyNameLen+16]) - copy(key.hmacKey[:], hashed[ticketKeyNameLen+16:ticketKeyNameLen+32]) + // The first 16 bytes of the hash used to be exposed on the wire as a ticket + // prefix. They MUST NOT be used as a secret. In the future, it would make + // sense to use a proper KDF here, like HKDF with a fixed salt. + const legacyTicketKeyNameLen = 16 + copy(key.aesKey[:], hashed[legacyTicketKeyNameLen:]) + copy(key.hmacKey[:], hashed[legacyTicketKeyNameLen+len(key.aesKey):]) key.created = c.time() return key } // maxSessionTicketLifetime is the maximum allowed lifetime of a TLS 1.3 session -// ticket, and the lifetime we set for tickets we send. +// ticket, and the lifetime we set for all tickets we send. const maxSessionTicketLifetime = 7 * 24 * time.Hour // Clone returns a shallow clone of c or nil if c is nil. It is safe to clone a Config that is diff --git a/handshake_server.go b/handshake_server.go index 6e1021a..fd78e13 100644 --- a/handshake_server.go +++ b/handshake_server.go @@ -406,16 +406,19 @@ func (hs *serverHandshakeState) checkForResumption() bool { return false } - plaintext, usedOldKey := c.decryptTicket(hs.clientHello.sessionTicket) + plaintext := c.decryptTicket(hs.clientHello.sessionTicket) if plaintext == nil { return false } - hs.sessionState = &sessionState{usedOldKey: usedOldKey} + hs.sessionState = &sessionState{} ok := hs.sessionState.unmarshal(plaintext) if !ok { return false } + // TLS 1.2 tickets don't natively have a lifetime, but we want to avoid + // re-wrapping the same master secret in different tickets over and over for + // too long, weakening forward secrecy. createdAt := time.Unix(int64(hs.sessionState.createdAt), 0) if c.config.time().Sub(createdAt) > maxSessionTicketLifetime { return false @@ -465,7 +468,10 @@ func (hs *serverHandshakeState) doResumeHandshake() error { // We echo the client's session ID in the ServerHello to let it know // that we're doing a resumption. hs.hello.sessionId = hs.clientHello.sessionId - hs.hello.ticketSupported = hs.sessionState.usedOldKey + // We always send a new session ticket, even if it wraps the same master + // secret and it's potentially encrypted with the same key, to help the + // client avoid cross-connection tracking from a network observer. + hs.hello.ticketSupported = true hs.finishedHash = newFinishedHash(c.vers, hs.suite) hs.finishedHash.discardHandshakeBuffer() if err := transcriptMsg(hs.clientHello, &hs.finishedHash); err != nil { @@ -748,9 +754,6 @@ func (hs *serverHandshakeState) readFinished(out []byte) error { } func (hs *serverHandshakeState) sendSessionTicket() error { - // ticketSupported is set in a resumption handshake if the - // ticket from the client was encrypted with an old session - // ticket key and thus a refreshed ticket should be sent. if !hs.hello.ticketSupported { return nil } diff --git a/handshake_server_tls13.go b/handshake_server_tls13.go index b2ab371..9b9a7a9 100644 --- a/handshake_server_tls13.go +++ b/handshake_server_tls13.go @@ -327,7 +327,7 @@ func (hs *serverHandshakeStateTLS13) checkForResumption() error { break } - plaintext, _ := c.decryptTicket(identity.label) + plaintext := c.decryptTicket(identity.label) if plaintext == nil { continue } diff --git a/ticket.go b/ticket.go index c6f51b3..918e78a 100644 --- a/ticket.go +++ b/ticket.go @@ -5,7 +5,6 @@ package reality import ( - "bytes" "crypto/aes" "crypto/cipher" "crypto/hmac" @@ -26,10 +25,6 @@ type sessionState struct { masterSecret []byte // opaque master_secret<1..2^16-1>; // struct { opaque certificate<1..2^24-1> } Certificate; certificates [][]byte // Certificate certificate_list<0..2^24-1>; - - // usedOldKey is true if the ticket from which this session came from - // was encrypted with an older key and thus should be refreshed. - usedOldKey bool } func (m *sessionState) marshal() ([]byte, error) { @@ -51,7 +46,7 @@ func (m *sessionState) marshal() ([]byte, error) { } func (m *sessionState) unmarshal(data []byte) bool { - *m = sessionState{usedOldKey: m.usedOldKey} + *m = sessionState{} s := cryptobyte.String(data) if ok := s.ReadUint16(&m.vers) && s.ReadUint16(&m.cipherSuite) && @@ -121,65 +116,56 @@ func (c *Conn) encryptTicket(state []byte) ([]byte, error) { return nil, errors.New("tls: internal error: session ticket keys unavailable") } - encrypted := make([]byte, ticketKeyNameLen+aes.BlockSize+len(state)+sha256.Size) - keyName := encrypted[:ticketKeyNameLen] - iv := encrypted[ticketKeyNameLen : ticketKeyNameLen+aes.BlockSize] + encrypted := make([]byte, aes.BlockSize+len(state)+sha256.Size) + iv := encrypted[:aes.BlockSize] + ciphertext := encrypted[aes.BlockSize : len(encrypted)-sha256.Size] + authenticated := encrypted[:len(encrypted)-sha256.Size] macBytes := encrypted[len(encrypted)-sha256.Size:] if _, err := io.ReadFull(c.config.rand(), iv); err != nil { return nil, err } key := c.ticketKeys[0] - copy(keyName, key.keyName[:]) block, err := aes.NewCipher(key.aesKey[:]) if err != nil { return nil, errors.New("tls: failed to create cipher while encrypting ticket: " + err.Error()) } - cipher.NewCTR(block, iv).XORKeyStream(encrypted[ticketKeyNameLen+aes.BlockSize:], state) + cipher.NewCTR(block, iv).XORKeyStream(ciphertext, state) mac := hmac.New(sha256.New, key.hmacKey[:]) - mac.Write(encrypted[:len(encrypted)-sha256.Size]) + mac.Write(authenticated) mac.Sum(macBytes[:0]) return encrypted, nil } -func (c *Conn) decryptTicket(encrypted []byte) (plaintext []byte, usedOldKey bool) { - if len(encrypted) < ticketKeyNameLen+aes.BlockSize+sha256.Size { - return nil, false +func (c *Conn) decryptTicket(encrypted []byte) []byte { + if len(encrypted) < aes.BlockSize+sha256.Size { + return nil } - keyName := encrypted[:ticketKeyNameLen] - iv := encrypted[ticketKeyNameLen : ticketKeyNameLen+aes.BlockSize] + iv := encrypted[:aes.BlockSize] + ciphertext := encrypted[aes.BlockSize : len(encrypted)-sha256.Size] + authenticated := encrypted[:len(encrypted)-sha256.Size] macBytes := encrypted[len(encrypted)-sha256.Size:] - ciphertext := encrypted[ticketKeyNameLen+aes.BlockSize : len(encrypted)-sha256.Size] + for _, key := range c.ticketKeys { + mac := hmac.New(sha256.New, key.hmacKey[:]) + mac.Write(authenticated) + expected := mac.Sum(nil) - keyIndex := -1 - for i, candidateKey := range c.ticketKeys { - if bytes.Equal(keyName, candidateKey.keyName[:]) { - keyIndex = i - break + if subtle.ConstantTimeCompare(macBytes, expected) != 1 { + continue } - } - if keyIndex == -1 { - return nil, false - } - key := &c.ticketKeys[keyIndex] - mac := hmac.New(sha256.New, key.hmacKey[:]) - mac.Write(encrypted[:len(encrypted)-sha256.Size]) - expected := mac.Sum(nil) + block, err := aes.NewCipher(key.aesKey[:]) + if err != nil { + return nil + } + plaintext := make([]byte, len(ciphertext)) + cipher.NewCTR(block, iv).XORKeyStream(plaintext, ciphertext) - if subtle.ConstantTimeCompare(macBytes, expected) != 1 { - return nil, false + return plaintext } - block, err := aes.NewCipher(key.aesKey[:]) - if err != nil { - return nil, false - } - plaintext = make([]byte, len(ciphertext)) - cipher.NewCTR(block, iv).XORKeyStream(plaintext, ciphertext) - - return plaintext, keyIndex > 0 + return nil }