crypto/tls: reduce session ticket linkability

Ever since session ticket key rotation was introduced in CL 9072, we've
been including a prefix in every ticket to identify what key it's
encrypted with. It's a small privacy gain, but the cost of trial
decryptions is also small, especially since the first key is probably
the most frequently used.

Also reissue tickets on every resumption so that the next connection
can't be linked to all the previous ones. Again the privacy gain is
small but the performance cost is small and it comes with a reduction in
complexity.

For #60105

Change-Id: I852f297162d2b79a3d9bf61f6171e8ce94b2537a
Reviewed-on: https://go-review.googlesource.com/c/go/+/496817
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

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

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
 	}

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
 		}

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]
-
-	keyIndex := -1
-	for i, candidateKey := range c.ticketKeys {
-		if bytes.Equal(keyName, candidateKey.keyName[:]) {
-			keyIndex = i
-			break
-		}
-	}
-	if keyIndex == -1 {
-		return nil, false
-	}
-	key := &c.ticketKeys[keyIndex]
-
+	for _, key := range c.ticketKeys {
 		mac := hmac.New(sha256.New, key.hmacKey[:])
-	mac.Write(encrypted[:len(encrypted)-sha256.Size])
+		mac.Write(authenticated)
 		expected := mac.Sum(nil)
 
 		if subtle.ConstantTimeCompare(macBytes, expected) != 1 {
-		return nil, false
+			continue
 		}
 
 		block, err := aes.NewCipher(key.aesKey[:])
 		if err != nil {
-		return nil, false
+			return nil
 		}
-	plaintext = make([]byte, len(ciphertext))
+		plaintext := make([]byte, len(ciphertext))
 		cipher.NewCTR(block, iv).XORKeyStream(plaintext, ciphertext)
 
-	return plaintext, keyIndex > 0
+		return plaintext
+	}
+
+	return nil
 }