From 1dc53d2840bb7f6551afa4364e3a2f71816e810e Mon Sep 17 00:00:00 2001 From: Steven Valdez Date: Tue, 26 Jul 2016 12:27:38 -0400 Subject: [PATCH] Adding handling for KeyUpdate post-handshake message. BUG=74 Change-Id: I72d52c1fbc3413e940dddbc0b20c7f22459da693 Reviewed-on: https://boringssl-review.googlesource.com/8981 Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/internal.h | 4 ++++ ssl/test/runner/common.go | 4 ++++ ssl/test/runner/conn.go | 8 ++++++-- ssl/test/runner/runner.go | 10 ++++++++++ ssl/tls13_both.c | 16 +++++++++++++++- ssl/tls13_enc.c | 31 +++++++++++++++++++++++++++---- 6 files changed, 66 insertions(+), 7 deletions(-) diff --git a/ssl/internal.h b/ssl/internal.h index 14265f94..fe8bbf5a 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -823,6 +823,10 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, * zero on error. */ int tls13_set_handshake_traffic(SSL *ssl); +/* tls13_rotate_traffic_key derives the next read or write traffic secret. It + * returns one on success and zero on error. */ +int tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction); + /* tls13_derive_traffic_secret_0 derives the initial application data traffic * secret based on the handshake transcripts and |master_secret|. It returns one * on success and zero on error. */ diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 141f160d..dfd5b30d 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -885,6 +885,10 @@ 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 d01643c9..703908ae 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -1325,6 +1325,10 @@ func (c *Conn) Write(b []byte) (int, error) { c.flushHandshake() } + if c.config.Bugs.SendKeyUpdateBeforeEveryAppDataRecord { + c.sendKeyUpdateLocked() + } + // 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 @@ -1394,7 +1398,7 @@ func (c *Conn) handlePostHandshakeMessage() error { } if _, ok := msg.(*keyUpdateMsg); ok { - c.in.doKeyUpdate(c, true) + c.in.doKeyUpdate(c, false) return nil } @@ -1704,6 +1708,6 @@ func (c *Conn) sendKeyUpdateLocked() error { if err := c.flushHandshake(); err != nil { return err } - c.out.doKeyUpdate(c, false) + c.out.doKeyUpdate(c, true) return nil } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 48d87037..ccd25d4d 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -2166,6 +2166,16 @@ func addBasicTests() { shouldFail: true, expectedError: ":WRONG_VERSION_NUMBER:", }, + { + testType: clientTest, + name: "KeyUpdate", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendKeyUpdateBeforeEveryAppDataRecord: true, + }, + }, + }, } testCases = append(testCases, basicTests...) } diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c index 1f604539..15f9a29f 100644 --- a/ssl/tls13_both.c +++ b/ssl/tls13_both.c @@ -453,7 +453,22 @@ int tls13_prepare_finished(SSL *ssl) { return 1; } +static int tls13_receive_key_update(SSL *ssl) { + if (ssl->init_num != 0) { + 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. + return tls13_rotate_traffic_key(ssl, evp_aead_open); +} + int tls13_post_handshake(SSL *ssl) { + if (ssl->s3->tmp.message_type == SSL3_MT_KEY_UPDATE) { + return tls13_receive_key_update(ssl); + } + if (ssl->s3->tmp.message_type == SSL3_MT_NEW_SESSION_TICKET && !ssl->server) { // TODO(svaldez): Handle NewSessionTicket. @@ -461,7 +476,6 @@ int tls13_post_handshake(SSL *ssl) { } // TODO(svaldez): Handle post-handshake authentication. - // TODO(svaldez): Handle KeyUpdate. ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE); OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE); diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 1f4fe211..70b041a8 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -176,7 +176,7 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, const EVP_MD *digest = ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)); size_t mac_secret_len, fixed_iv_len; if (!ssl_cipher_get_evp_aead(&aead, &mac_secret_len, &fixed_iv_len, - ssl->s3->new_session->cipher, + SSL_get_session(ssl)->cipher, ssl3_protocol_version(ssl))) { return 0; } @@ -207,7 +207,7 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, } SSL_AEAD_CTX *traffic_aead = SSL_AEAD_CTX_new( - direction, ssl3_protocol_version(ssl), ssl->s3->new_session->cipher, key, + direction, ssl3_protocol_version(ssl), SSL_get_session(ssl)->cipher, key, key_len, NULL, 0, iv, iv_len); if (traffic_aead == NULL) { return 0; @@ -225,10 +225,10 @@ int tls13_set_traffic_key(SSL *ssl, enum tls_record_type_t type, /* Save the traffic secret. */ if (direction == evp_aead_open) { - memcpy(ssl->s3->read_traffic_secret, traffic_secret, traffic_secret_len); + memmove(ssl->s3->read_traffic_secret, traffic_secret, traffic_secret_len); ssl->s3->read_traffic_secret_len = traffic_secret_len; } else { - memcpy(ssl->s3->write_traffic_secret, traffic_secret, traffic_secret_len); + memmove(ssl->s3->write_traffic_secret, traffic_secret, traffic_secret_len); ssl->s3->write_traffic_secret_len = traffic_secret_len; } @@ -267,6 +267,29 @@ int tls13_derive_traffic_secret_0(SSL *ssl) { hs->hash_len); } +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)); + + uint8_t *secret; + size_t secret_len; + if (direction == evp_aead_open) { + secret = ssl->s3->read_traffic_secret; + secret_len = ssl->s3->read_traffic_secret_len; + } else { + secret = ssl->s3->write_traffic_secret; + secret_len = ssl->s3->write_traffic_secret_len; + } + + if (!hkdf_expand_label(secret, digest, secret, secret_len, + (const uint8_t *)kTLS13LabelApplicationTraffic, + strlen(kTLS13LabelApplicationTraffic), NULL, 0, + secret_len)) { + return 0; + } + + return tls13_set_traffic_key(ssl, type_data, direction, secret, secret_len); +} + static const char kTLS13LabelExporter[] = "exporter master secret"; static const char kTLS13LabelResumption[] = "resumption master secret";