Updating Key Schedule and KeyUpdate to draft 16.

This doesn't currently honor the required KeyUpdate response. That will
be done in a follow-up.

BUG=74

Change-Id: I750fc41278736cb24230303815e839c6f6967b6a
Reviewed-on: https://boringssl-review.googlesource.com/11412
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/internal.h

@@ -896,7 +896,8 @@ struct ssl_handshake_st {
   size_t hash_len;
   uint8_t resumption_hash[EVP_MAX_MD_SIZE];
   uint8_t secret[EVP_MAX_MD_SIZE];
-  uint8_t traffic_secret_0[EVP_MAX_MD_SIZE];
+  uint8_t client_traffic_secret_0[EVP_MAX_MD_SIZE];
+  uint8_t server_traffic_secret_0[EVP_MAX_MD_SIZE];
 
   union {
     /* sent is a bitset where the bits correspond to elements of kExtensions
@@ -1347,6 +1348,11 @@ extern const SSL3_ENC_METHOD SSLv3_enc_data;
 #define SSL_PSK_AUTH      0x0
 #define SSL_PSK_SIGN_AUTH 0x1
 
+/* From draft-ietf-tls-tls13-16, used in determining whether to respond with a
+ * KeyUpdate. */
+#define SSL_KEY_UPDATE_NOT_REQUESTED 0
+#define SSL_KEY_UPDATE_REQUESTED 1
+
 CERT *ssl_cert_new(void);
 CERT *ssl_cert_dup(CERT *cert);
 void ssl_cert_clear_certs(CERT *c);

ssl/s3_both.c

@@ -148,7 +148,10 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) {
   }
 
   OPENSSL_cleanse(hs->secret, sizeof(hs->secret));
-  OPENSSL_cleanse(hs->traffic_secret_0, sizeof(hs->traffic_secret_0));
+  OPENSSL_cleanse(hs->client_traffic_secret_0,
+                  sizeof(hs->client_traffic_secret_0));
+  OPENSSL_cleanse(hs->server_traffic_secret_0,
+                  sizeof(hs->server_traffic_secret_0));
   SSL_ECDH_CTX_cleanup(&hs->ecdh_ctx);
   OPENSSL_free(hs->peer_key);
   OPENSSL_free(hs->server_params);

ssl/test/runner/common.go

@@ -206,6 +206,12 @@ const (
 	pskSignAuthMode = 1
 )
 
+// KeyUpdateRequest values (see draft-ietf-tls-tls13-16, section 4.5.3)
+const (
+	keyUpdateNotRequested = 0
+	keyUpdateRequested    = 1
+)
+
 // ConnectionState records basic TLS details about the connection.
 type ConnectionState struct {
 	Version                    uint16                // TLS version used by the connection (e.g. VersionTLS12)
@@ -956,10 +962,6 @@ type ProtocolBugs struct {
 	// message. This only makes sense for a server.
 	SendHelloRequestBeforeEveryHandshakeMessage bool
 
-	// SendKeyUpdateBeforeEveryAppDataRecord, if true, causes a KeyUpdate
-	// handshake message to be sent before each application data record.
-	SendKeyUpdateBeforeEveryAppDataRecord bool
-
 	// RequireDHPublicValueLen causes a fatal error if the length (in
 	// bytes) of the server's Diffie-Hellman public value is not equal to
 	// this.

ssl/test/runner/conn.go

@@ -92,6 +92,8 @@ type Conn struct {
 	handMsgLen       int      // handshake message length, not including the header
 	pendingFragments [][]byte // pending outgoing handshake fragments.
 
+	keyUpdateRequested bool
+
 	tmp [16]byte
 }
 
@@ -159,8 +161,7 @@ type halfConn struct {
 	// used to save allocating a new buffer for each MAC.
 	inDigestBuf, outDigestBuf []byte
 
-	trafficSecret       []byte
-	keyUpdateGeneration int
+	trafficSecret []byte
 
 	config *Config
 }
@@ -223,7 +224,6 @@ func (hc *halfConn) doKeyUpdate(c *Conn, isOutgoing bool) {
 		side = clientWrite
 	}
 	hc.useTrafficSecret(hc.version, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), hc.trafficSecret), applicationPhase, side)
-	hc.keyUpdateGeneration++
 }
 
 // incSeq increments the sequence number.
@@ -1328,11 +1328,11 @@ func (c *Conn) Write(b []byte) (int, error) {
 		return 0, alertInternalError
 	}
 
-	// Catch up with KeyUpdates from the peer.
-	for c.out.keyUpdateGeneration < c.in.keyUpdateGeneration {
-		if err := c.sendKeyUpdateLocked(); err != nil {
+	if c.keyUpdateRequested {
+		if err := c.sendKeyUpdateLocked(keyUpdateNotRequested); err != nil {
 			return 0, err
 		}
+		c.keyUpdateRequested = false
 	}
 
 	if c.config.Bugs.SendSpuriousAlert != 0 {
@@ -1344,12 +1344,6 @@ func (c *Conn) Write(b []byte) (int, error) {
 		c.flushHandshake()
 	}
 
-	if c.config.Bugs.SendKeyUpdateBeforeEveryAppDataRecord {
-		if err := c.sendKeyUpdateLocked(); err != nil {
-			return 0, err
-		}
-	}
-
 	// SSL 3.0 and TLS 1.0 are susceptible to a chosen-plaintext
 	// attack when using block mode ciphers due to predictable IVs.
 	// This can be prevented by splitting each Application Data
@@ -1441,8 +1435,11 @@ func (c *Conn) handlePostHandshakeMessage() error {
 		}
 	}
 
-	if _, ok := msg.(*keyUpdateMsg); ok {
+	if keyUpdate, ok := msg.(*keyUpdateMsg); ok {
 		c.in.doKeyUpdate(c, false)
+		if keyUpdate.keyUpdateRequest == keyUpdateRequested {
+			c.keyUpdateRequested = true
+		}
 		return nil
 	}
 
@@ -1751,18 +1748,20 @@ func (c *Conn) SendNewSessionTicket() error {
 	return err
 }
 
-func (c *Conn) SendKeyUpdate() error {
+func (c *Conn) SendKeyUpdate(keyUpdateRequest byte) error {
 	c.out.Lock()
 	defer c.out.Unlock()
-	return c.sendKeyUpdateLocked()
+	return c.sendKeyUpdateLocked(keyUpdateRequest)
 }
 
-func (c *Conn) sendKeyUpdateLocked() error {
+func (c *Conn) sendKeyUpdateLocked(keyUpdateRequest byte) error {
 	if c.vers < VersionTLS13 {
 		return errors.New("tls: attempted to send KeyUpdate before TLS 1.3")
 	}
 
-	m := new(keyUpdateMsg)
+	m := keyUpdateMsg{
+		keyUpdateRequest: keyUpdateRequest,
+	}
 	if _, err := c.writeRecord(recordTypeHandshake, m.marshal()); err != nil {
 		return err
 	}

ssl/test/runner/handshake_client.go

@@ -644,9 +644,10 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 	handshakeSecret := hs.finishedHash.extractKey(earlySecret, ecdheSecret)
 
 	// Switch to handshake traffic keys.
-	handshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, handshakeTrafficLabel)
-	c.out.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, clientWrite)
-	c.in.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, serverWrite)
+	clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, clientHandshakeTrafficLabel)
+	c.out.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, handshakePhase, clientWrite)
+	serverHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, serverHandshakeTrafficLabel)
+	c.in.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, handshakePhase, serverWrite)
 
 	msg, err := c.readHandshake()
 	if err != nil {
@@ -756,7 +757,7 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 		return unexpectedMessageError(serverFinished, msg)
 	}
 
-	verify := hs.finishedHash.serverSum(handshakeTrafficSecret)
+	verify := hs.finishedHash.serverSum(serverHandshakeTrafficSecret)
 	if len(verify) != len(serverFinished.verifyData) ||
 		subtle.ConstantTimeCompare(verify, serverFinished.verifyData) != 1 {
 		c.sendAlert(alertHandshakeFailure)
@@ -768,7 +769,8 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 	// The various secrets do not incorporate the client's final leg, so
 	// derive them now before updating the handshake context.
 	masterSecret := hs.finishedHash.extractKey(handshakeSecret, zeroSecret)
-	trafficSecret := hs.finishedHash.deriveSecret(masterSecret, applicationTrafficLabel)
+	clientTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, clientApplicationTrafficLabel)
+	serverTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, serverApplicationTrafficLabel)
 
 	if certReq != nil && !c.config.Bugs.SkipClientCertificate {
 		certMsg := &certificateMsg{
@@ -813,7 +815,7 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 
 	// Send a client Finished message.
 	finished := new(finishedMsg)
-	finished.verifyData = hs.finishedHash.clientSum(handshakeTrafficSecret)
+	finished.verifyData = hs.finishedHash.clientSum(clientHandshakeTrafficSecret)
 	if c.config.Bugs.BadFinished {
 		finished.verifyData[0]++
 	}
@@ -830,8 +832,8 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 	c.flushHandshake()
 
 	// Switch to application data keys.
-	c.out.useTrafficSecret(c.vers, hs.suite, trafficSecret, applicationPhase, clientWrite)
-	c.in.useTrafficSecret(c.vers, hs.suite, trafficSecret, applicationPhase, serverWrite)
+	c.out.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, applicationPhase, clientWrite)
+	c.in.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, applicationPhase, serverWrite)
 
 	c.exporterSecret = hs.finishedHash.deriveSecret(masterSecret, exporterLabel)
 	c.resumptionSecret = hs.finishedHash.deriveSecret(masterSecret, resumptionLabel)

ssl/test/runner/handshake_messages.go

@@ -2094,14 +2094,32 @@ func (*helloRequestMsg) unmarshal(data []byte) bool {
 }
 
 type keyUpdateMsg struct {
+	raw              []byte
+	keyUpdateRequest byte
 }
 
-func (*keyUpdateMsg) marshal() []byte {
-	return []byte{typeKeyUpdate, 0, 0, 0}
+func (m *keyUpdateMsg) marshal() []byte {
+	if m.raw != nil {
+		return m.raw
+	}
+
+	return []byte{typeKeyUpdate, 0, 0, 1, m.keyUpdateRequest}
 }
 
-func (*keyUpdateMsg) unmarshal(data []byte) bool {
-	return len(data) == 4
+func (m *keyUpdateMsg) unmarshal(data []byte) bool {
+	m.raw = data
+
+	if len(data) != 5 {
+		return false
+	}
+
+	length := int(data[1])<<16 | int(data[2])<<8 | int(data[3])
+	if len(data)-4 != length {
+		return false
+	}
+
+	m.keyUpdateRequest = data[4]
+	return m.keyUpdateRequest == keyUpdateNotRequested || m.keyUpdateRequest == keyUpdateRequested
 }
 
 func eqUint16s(x, y []uint16) bool {

ssl/test/runner/handshake_server.go

@@ -693,9 +693,10 @@ Curves:
 	handshakeSecret := hs.finishedHash.extractKey(earlySecret, ecdheSecret)
 
 	// Switch to handshake traffic keys.
-	handshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, handshakeTrafficLabel)
-	c.out.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, serverWrite)
-	c.in.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, clientWrite)
+	serverHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, serverHandshakeTrafficLabel)
+	c.out.useTrafficSecret(c.vers, hs.suite, serverHandshakeTrafficSecret, handshakePhase, serverWrite)
+	clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(handshakeSecret, clientHandshakeTrafficLabel)
+	c.in.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, handshakePhase, clientWrite)
 
 	if hs.hello.useCertAuth {
 		if hs.clientHello.ocspStapling {
@@ -793,7 +794,7 @@ Curves:
 	}
 
 	finished := new(finishedMsg)
-	finished.verifyData = hs.finishedHash.serverSum(handshakeTrafficSecret)
+	finished.verifyData = hs.finishedHash.serverSum(serverHandshakeTrafficSecret)
 	if config.Bugs.BadFinished {
 		finished.verifyData[0]++
 	}
@@ -807,11 +808,12 @@ Curves:
 	// The various secrets do not incorporate the client's final leg, so
 	// derive them now before updating the handshake context.
 	masterSecret := hs.finishedHash.extractKey(handshakeSecret, hs.finishedHash.zeroSecret())
-	trafficSecret := hs.finishedHash.deriveSecret(masterSecret, applicationTrafficLabel)
+	clientTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, clientApplicationTrafficLabel)
+	serverTrafficSecret := hs.finishedHash.deriveSecret(masterSecret, serverApplicationTrafficLabel)
 
 	// Switch to application data keys on write. In particular, any alerts
 	// from the client certificate are sent over these keys.
-	c.out.useTrafficSecret(c.vers, hs.suite, trafficSecret, applicationPhase, serverWrite)
+	c.out.useTrafficSecret(c.vers, hs.suite, serverTrafficSecret, applicationPhase, serverWrite)
 
 	// If we requested a client certificate, then the client must send a
 	// certificate message, even if it's empty.
@@ -875,7 +877,7 @@ Curves:
 		return unexpectedMessageError(clientFinished, msg)
 	}
 
-	verify := hs.finishedHash.clientSum(handshakeTrafficSecret)
+	verify := hs.finishedHash.clientSum(clientHandshakeTrafficSecret)
 	if len(verify) != len(clientFinished.verifyData) ||
 		subtle.ConstantTimeCompare(verify, clientFinished.verifyData) != 1 {
 		c.sendAlert(alertHandshakeFailure)
@@ -884,7 +886,7 @@ Curves:
 	hs.writeClientHash(clientFinished.marshal())
 
 	// Switch to application data keys on read.
-	c.in.useTrafficSecret(c.vers, hs.suite, trafficSecret, applicationPhase, clientWrite)
+	c.in.useTrafficSecret(c.vers, hs.suite, clientTrafficSecret, applicationPhase, clientWrite)
 
 	c.cipherSuite = hs.suite
 	c.exporterSecret = hs.finishedHash.deriveSecret(masterSecret, exporterLabel)

ssl/test/runner/prf.go

@@ -119,6 +119,7 @@ var extendedMasterSecretLabel = []byte("extended master secret")
 var keyExpansionLabel = []byte("key expansion")
 var clientFinishedLabel = []byte("client finished")
 var serverFinishedLabel = []byte("server finished")
+var finishedLabel = []byte("finished")
 var channelIDLabel = []byte("TLS Channel ID signature\x00")
 var channelIDResumeLabel = []byte("Resumption\x00")
 
@@ -311,7 +312,7 @@ func (h finishedHash) clientSum(baseKey []byte) []byte {
 		return out
 	}
 
-	clientFinishedKey := hkdfExpandLabel(h.hash, baseKey, clientFinishedLabel, nil, h.hash.Size())
+	clientFinishedKey := hkdfExpandLabel(h.hash, baseKey, finishedLabel, nil, h.hash.Size())
 	finishedHMAC := hmac.New(h.hash.New, clientFinishedKey)
 	finishedHMAC.Write(h.appendContextHashes(nil))
 	return finishedHMAC.Sum(nil)
@@ -330,7 +331,7 @@ func (h finishedHash) serverSum(baseKey []byte) []byte {
 		return out
 	}
 
-	serverFinishedKey := hkdfExpandLabel(h.hash, baseKey, serverFinishedLabel, nil, h.hash.Size())
+	serverFinishedKey := hkdfExpandLabel(h.hash, baseKey, finishedLabel, nil, h.hash.Size())
 	finishedHMAC := hmac.New(h.hash.New, serverFinishedKey)
 	finishedHMAC.Write(h.appendContextHashes(nil))
 	return finishedHMAC.Sum(nil)
@@ -417,11 +418,14 @@ func (h *finishedHash) appendContextHashes(b []byte) []byte {
 
 // The following are labels for traffic secret derivation in TLS 1.3.
 var (
-	earlyTrafficLabel       = []byte("early traffic secret")
-	handshakeTrafficLabel   = []byte("handshake traffic secret")
-	applicationTrafficLabel = []byte("application traffic secret")
-	exporterLabel           = []byte("exporter master secret")
-	resumptionLabel         = []byte("resumption master secret")
+	earlyTrafficLabel             = []byte("client early traffic secret")
+	clientHandshakeTrafficLabel   = []byte("client handshake traffic secret")
+	serverHandshakeTrafficLabel   = []byte("server handshake traffic secret")
+	clientApplicationTrafficLabel = []byte("client application traffic secret")
+	serverApplicationTrafficLabel = []byte("server application traffic secret")
+	applicationTrafficLabel       = []byte("application traffic secret")
+	exporterLabel                 = []byte("exporter master secret")
+	resumptionLabel               = []byte("resumption master secret")
 )
 
 // deriveSecret implements TLS 1.3's Derive-Secret function, as defined in
@@ -474,11 +478,7 @@ const (
 func deriveTrafficAEAD(version uint16, suite *cipherSuite, secret, phase []byte, side trafficDirection) interface{} {
 	label := make([]byte, 0, len(phase)+2+16)
 	label = append(label, phase...)
-	if side == clientWrite {
-		label = append(label, []byte(", client write key")...)
-	} else {
-		label = append(label, []byte(", server write key")...)
-	}
+	label = append(label, []byte(", key")...)
 	key := hkdfExpandLabel(suite.hash(), secret, label, nil, suite.keyLen)
 
 	label = label[:len(label)-3] // Remove "key" from the end.

ssl/test/runner/runner.go

@@ -359,6 +359,8 @@ type testCase struct {
 	// sendKeyUpdates is the number of consecutive key updates to send
 	// before and after the test message.
 	sendKeyUpdates int
+	// keyUpdateRequest is the KeyUpdateRequest value to send in KeyUpdate messages.
+	keyUpdateRequest byte
 	// expectMessageDropped, if true, means the test message is expected to
 	// be dropped by the client rather than echoed back.
 	expectMessageDropped bool
@@ -616,7 +618,7 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, nu
 	}
 
 	for i := 0; i < test.sendKeyUpdates; i++ {
-		if err := tlsConn.SendKeyUpdate(); err != nil {
+		if err := tlsConn.SendKeyUpdate(test.keyUpdateRequest); err != nil {
 			return err
 		}
 	}
@@ -678,7 +680,7 @@ func doExchange(test *testCase, config *Config, conn net.Conn, isResume bool, nu
 		tlsConn.Write(testMessage)
 
 		for i := 0; i < test.sendKeyUpdates; i++ {
-			tlsConn.SendKeyUpdate()
+			tlsConn.SendKeyUpdate(test.keyUpdateRequest)
 		}
 
 		for i := 0; i < test.sendEmptyRecords; i++ {
@@ -1981,13 +1983,14 @@ func addBasicTests() {
 			expectedError:     ":TOO_MANY_WARNING_ALERTS:",
 		},
 		{
-			name: "SendKeyUpdates",
+			name: "TooManyKeyUpdates",
 			config: Config{
 				MaxVersion: VersionTLS13,
 			},
-			sendKeyUpdates: 33,
-			shouldFail:     true,
-			expectedError:  ":TOO_MANY_KEY_UPDATES:",
+			sendKeyUpdates:   33,
+			keyUpdateRequest: keyUpdateNotRequested,
+			shouldFail:       true,
+			expectedError:    ":TOO_MANY_KEY_UPDATES:",
 		},
 		{
 			name: "EmptySessionID",
@@ -2195,14 +2198,22 @@ func addBasicTests() {
 			expectedError: ":WRONG_VERSION_NUMBER:",
 		},
 		{
-			testType: clientTest,
-			name:     "KeyUpdate",
+			name: "KeyUpdate",
 			config: Config{
 				MaxVersion: VersionTLS13,
-				Bugs: ProtocolBugs{
-					SendKeyUpdateBeforeEveryAppDataRecord: true,
-				},
 			},
+			sendKeyUpdates:   1,
+			keyUpdateRequest: keyUpdateNotRequested,
+		},
+		{
+			name: "KeyUpdate-InvalidRequestMode",
+			config: Config{
+				MaxVersion: VersionTLS13,
+			},
+			sendKeyUpdates:   1,
+			keyUpdateRequest: 42,
+			shouldFail:       true,
+			expectedError:    ":DECODE_ERROR:",
 		},
 		{
 			name: "SendSNIWarningAlert",
@@ -8723,11 +8734,10 @@ func addPeekTests() {
 		name: "Peek-KeyUpdate",
 		config: Config{
 			MaxVersion: VersionTLS13,
-			Bugs: ProtocolBugs{
-				SendKeyUpdateBeforeEveryAppDataRecord: true,
-			},
 		},
-		flags: []string{"-peek-then-read"},
+		sendKeyUpdates:   1,
+		keyUpdateRequest: keyUpdateNotRequested,
+		flags:            []string{"-peek-then-read"},
 	})
 }
 

ssl/tls13_both.c

@@ -403,13 +403,20 @@ int tls13_prepare_finished(SSL *ssl) {
 }
 
 static int tls13_receive_key_update(SSL *ssl) {
-  if (ssl->init_num != 0) {
+  CBS cbs;
+  uint8_t key_update_request;
+  CBS_init(&cbs, ssl->init_msg, ssl->init_num);
+  if (!CBS_get_u8(&cbs, &key_update_request) ||
+      CBS_len(&cbs) != 0 ||
+      (key_update_request != SSL_KEY_UPDATE_NOT_REQUESTED &&
+       key_update_request != SSL_KEY_UPDATE_REQUESTED)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
     return 0;
   }
 
-  // TODO(svaldez): Send KeyUpdate.
+  /* TODO(svaldez): Send KeyUpdate if |key_update_request| is
+   * |SSL_KEY_UPDATE_REQUESTED|. */
   return tls13_rotate_traffic_key(ssl, evp_aead_open);
 }
 

ssl/tls13_client.c

@@ -536,9 +536,9 @@ static enum ssl_hs_wait_t do_send_client_finished(SSL *ssl, SSL_HANDSHAKE *hs) {
 
 static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) {
   if (!tls13_set_traffic_key(ssl, type_data, evp_aead_open,
-                             hs->traffic_secret_0, hs->hash_len) ||
+                             hs->server_traffic_secret_0, hs->hash_len) ||
       !tls13_set_traffic_key(ssl, type_data, evp_aead_seal,
-                             hs->traffic_secret_0, hs->hash_len) ||
+                             hs->client_traffic_secret_0, hs->hash_len) ||
       !tls13_finalize_keys(ssl)) {
     return ssl_hs_error;
   }

ssl/tls13_enc.c

@@ -134,42 +134,27 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type,
     return 0;
   }
 
-  const char *phase;
+  const char *key_label, *iv_label;
   switch (type) {
     case type_early_handshake:
-      phase = "early handshake key expansion, ";
+      key_label = "early handshake key expansion, key";
+      iv_label = "early handshake key expansion, iv";
       break;
     case type_early_data:
-      phase = "early application data key expansion, ";
+      key_label = "early application data key expansion, key";
+      iv_label = "early application data key expansion, iv";
       break;
     case type_handshake:
-      phase = "handshake key expansion, ";
+      key_label = "handshake key expansion, key";
+      iv_label = "handshake key expansion, iv";
       break;
     case type_data:
-      phase = "application data key expansion, ";
+      key_label = "application data key expansion, key";
+      iv_label = "application data key expansion, iv";
       break;
     default:
       return 0;
   }
-  size_t phase_len = strlen(phase);
-
-  const char *purpose = "client write key";
-  if ((ssl->server && direction == evp_aead_seal) ||
-      (!ssl->server && direction == evp_aead_open)) {
-    purpose = "server write key";
-  }
-  size_t purpose_len = strlen(purpose);
-
-  /* The longest label has length 38 (type_early_data) + 16 (either purpose
-   * value). */
-  uint8_t label[38 + 16];
-  size_t label_len = phase_len + purpose_len;
-  if (label_len > sizeof(label)) {
-    assert(0);
-    return 0;
-  }
-  memcpy(label, phase, phase_len);
-  memcpy(label + phase_len, purpose, purpose_len);
 
   /* Look up cipher suite properties. */
   const EVP_AEAD *aead;
@@ -184,25 +169,18 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type,
   /* Derive the key. */
   size_t key_len = EVP_AEAD_key_length(aead);
   uint8_t key[EVP_AEAD_MAX_KEY_LENGTH];
-  if (!hkdf_expand_label(key, digest, traffic_secret, traffic_secret_len, label,
-                         label_len, NULL, 0, key_len)) {
+  if (!hkdf_expand_label(key, digest, traffic_secret, traffic_secret_len,
+                         (const uint8_t *)key_label, strlen(key_label), NULL, 0,
+                         key_len)) {
     return 0;
   }
 
-  /* The IV's label ends in "iv" instead of "key". */
-  if (label_len < 3) {
-    assert(0);
-    return 0;
-  }
-  label_len--;
-  label[label_len - 2] = 'i';
-  label[label_len - 1] = 'v';
-
   /* Derive the IV. */
   size_t iv_len = EVP_AEAD_nonce_length(aead);
   uint8_t iv[EVP_AEAD_MAX_NONCE_LENGTH];
-  if (!hkdf_expand_label(iv, digest, traffic_secret, traffic_secret_len, label,
-                         label_len, NULL, 0, iv_len)) {
+  if (!hkdf_expand_label(iv, digest, traffic_secret, traffic_secret_len,
+                         (const uint8_t *)iv_label, strlen(iv_label), NULL, 0,
+                         iv_len)) {
     return 0;
   }
 
@@ -235,38 +213,69 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type,
   return 1;
 }
 
-static const char kTLS13LabelHandshakeTraffic[] = "handshake traffic secret";
-static const char kTLS13LabelApplicationTraffic[] =
-    "application traffic secret";
+static const char kTLS13LabelClientHandshakeTraffic[] =
+    "client handshake traffic secret";
+static const char kTLS13LabelServerHandshakeTraffic[] =
+    "server handshake traffic secret";
+static const char kTLS13LabelClientApplicationTraffic[] =
+    "client application traffic secret";
+static const char kTLS13LabelServerApplicationTraffic[] =
+    "server application traffic secret";
 
 int tls13_set_handshake_traffic(SSL *ssl) {
   SSL_HANDSHAKE *hs = ssl->s3->hs;
 
-  uint8_t traffic_secret[EVP_MAX_MD_SIZE];
-  if (!derive_secret(ssl, traffic_secret, hs->hash_len,
-                     (const uint8_t *)kTLS13LabelHandshakeTraffic,
-                     strlen(kTLS13LabelHandshakeTraffic)) ||
-      !ssl_log_secret(ssl, "HANDSHAKE_TRAFFIC_SECRET", traffic_secret,
-                      hs->hash_len) ||
-      !tls13_set_traffic_key(ssl, type_handshake, evp_aead_open, traffic_secret,
-                             hs->hash_len) ||
-      !tls13_set_traffic_key(ssl, type_handshake, evp_aead_seal, traffic_secret,
-                             hs->hash_len)) {
+  uint8_t client_traffic_secret[EVP_MAX_MD_SIZE];
+  uint8_t server_traffic_secret[EVP_MAX_MD_SIZE];
+  if (!derive_secret(ssl, client_traffic_secret, hs->hash_len,
+                     (const uint8_t *)kTLS13LabelClientHandshakeTraffic,
+                     strlen(kTLS13LabelClientHandshakeTraffic)) ||
+      !ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET",
+                      client_traffic_secret, hs->hash_len) ||
+      !derive_secret(ssl, server_traffic_secret, hs->hash_len,
+                     (const uint8_t *)kTLS13LabelServerHandshakeTraffic,
+                     strlen(kTLS13LabelServerHandshakeTraffic)) ||
+      !ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET",
+                      server_traffic_secret, hs->hash_len)) {
     return 0;
   }
+
+  if (ssl->server) {
+    if (!tls13_set_traffic_key(ssl, type_handshake, evp_aead_open,
+                               client_traffic_secret, hs->hash_len) ||
+        !tls13_set_traffic_key(ssl, type_handshake, evp_aead_seal,
+                               server_traffic_secret, hs->hash_len)) {
+      return 0;
+    }
+  } else {
+    if (!tls13_set_traffic_key(ssl, type_handshake, evp_aead_open,
+                               server_traffic_secret, hs->hash_len) ||
+        !tls13_set_traffic_key(ssl, type_handshake, evp_aead_seal,
+                               client_traffic_secret, hs->hash_len)) {
+      return 0;
+    }
+  }
   return 1;
 }
 
 int tls13_derive_traffic_secret_0(SSL *ssl) {
   SSL_HANDSHAKE *hs = ssl->s3->hs;
 
-  return derive_secret(ssl, hs->traffic_secret_0, hs->hash_len,
-                       (const uint8_t *)kTLS13LabelApplicationTraffic,
-                       strlen(kTLS13LabelApplicationTraffic)) &&
-         ssl_log_secret(ssl, "TRAFFIC_SECRET_0", hs->traffic_secret_0,
-                        hs->hash_len);
+  return derive_secret(ssl, hs->client_traffic_secret_0, hs->hash_len,
+                       (const uint8_t *)kTLS13LabelClientApplicationTraffic,
+                       strlen(kTLS13LabelClientApplicationTraffic)) &&
+         ssl_log_secret(ssl, "CLIENT_TRAFFIC_SECRET_0",
+                        hs->client_traffic_secret_0, hs->hash_len) &&
+         derive_secret(ssl, hs->server_traffic_secret_0, hs->hash_len,
+                       (const uint8_t *)kTLS13LabelServerApplicationTraffic,
+                       strlen(kTLS13LabelServerApplicationTraffic)) &&
+         ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0",
+                        hs->server_traffic_secret_0, hs->hash_len);
 }
 
+static const char kTLS13LabelApplicationTraffic[] =
+    "application traffic secret";
+
 int tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction) {
   const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl));
 
@@ -319,21 +328,11 @@ int tls13_finished_mac(SSL *ssl, uint8_t *out, size_t *out_len, int is_server) {
   size_t key_len = EVP_MD_size(digest);
 
   const uint8_t *traffic_secret;
-  const char *label;
-  if (is_server) {
-    label = "server finished";
-    if (ssl->server) {
-      traffic_secret = ssl->s3->write_traffic_secret;
-    } else {
-      traffic_secret = ssl->s3->read_traffic_secret;
-    }
+  const char *label = "finished";
+  if (is_server == ssl->server) {
+    traffic_secret = ssl->s3->write_traffic_secret;
   } else {
-    label = "client finished";
-    if (!ssl->server) {
-      traffic_secret = ssl->s3->write_traffic_secret;
-    } else {
-      traffic_secret = ssl->s3->read_traffic_secret;
-    }
+    traffic_secret = ssl->s3->read_traffic_secret;
   }
 
   uint8_t context_hashes[2 * EVP_MAX_MD_SIZE];

ssl/tls13_server.c

@@ -459,7 +459,7 @@ static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) {
   if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len) ||
       !tls13_derive_traffic_secret_0(ssl) ||
       !tls13_set_traffic_key(ssl, type_data, evp_aead_seal,
-                             hs->traffic_secret_0, hs->hash_len)) {
+                             hs->server_traffic_secret_0, hs->hash_len)) {
     return ssl_hs_error;
   }
 
@@ -523,7 +523,7 @@ static enum ssl_hs_wait_t do_process_client_finished(SSL *ssl,
       !ssl->method->hash_current_message(ssl) ||
       /* evp_aead_seal keys have already been switched. */
       !tls13_set_traffic_key(ssl, type_data, evp_aead_open,
-                             hs->traffic_secret_0, hs->hash_len) ||
+                             hs->client_traffic_secret_0, hs->hash_len) ||
       !tls13_finalize_keys(ssl)) {
     return ssl_hs_error;
   }