From c4aa727e739df266371fdf5b0c84aaa1d852f24d Mon Sep 17 00:00:00 2001 From: Steven Valdez Date: Mon, 3 Oct 2016 12:25:56 -0400 Subject: [PATCH] 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 Reviewed-by: Steven Valdez Reviewed-by: David Benjamin --- ssl/internal.h | 8 +- ssl/s3_both.c | 5 +- ssl/test/runner/common.go | 10 +- ssl/test/runner/conn.go | 33 +++---- ssl/test/runner/handshake_client.go | 18 ++-- ssl/test/runner/handshake_messages.go | 26 ++++- ssl/test/runner/handshake_server.go | 18 ++-- ssl/test/runner/prf.go | 24 ++--- ssl/test/runner/runner.go | 40 +++++--- ssl/tls13_both.c | 11 ++- ssl/tls13_client.c | 4 +- ssl/tls13_enc.c | 137 +++++++++++++------------- ssl/tls13_server.c | 4 +- 13 files changed, 193 insertions(+), 145 deletions(-) diff --git a/ssl/internal.h b/ssl/internal.h index 0243ac49..f615073e 100644 --- a/ssl/internal.h +++ b/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); diff --git a/ssl/s3_both.c b/ssl/s3_both.c index cb5afaba..2d5fc801 100644 --- a/ssl/s3_both.c +++ b/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); diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 8fe61a43..c03cedb8 100644 --- a/ssl/test/runner/common.go +++ b/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. diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 24f0d609..f5014d44 100644 --- a/ssl/test/runner/conn.go +++ b/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 } diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index c5be2b76..291a3b4b 100644 --- a/ssl/test/runner/handshake_client.go +++ b/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) diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 1e91ac3c..b92735e5 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/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 { diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 59b34faa..7b26cb6c 100644 --- a/ssl/test/runner/handshake_server.go +++ b/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) diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go index 5c7b3ab6..99ef64fc 100644 --- a/ssl/test/runner/prf.go +++ b/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. diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index fe2cf847..22563468 100644 --- a/ssl/test/runner/runner.go +++ b/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"}, }) } diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c index 3928ab79..e32464d4 100644 --- a/ssl/tls13_both.c +++ b/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); } diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index 72cf8b11..69d2a356 100644 --- a/ssl/tls13_client.c +++ b/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; } diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 88fe8f07..1fcde514 100644 --- a/ssl/tls13_enc.c +++ b/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]; diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index be485a1e..532b7087 100644 --- a/ssl/tls13_server.c +++ b/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; }