Implement TLS 1.3's downgrade signal. For now, skip the 1.2 -> 1.1 signal since that will affect shipping code. We may as well enable it too, but wait until things have settled down. This implements the version in draft-14 since draft-13's isn't backwards-compatible. Change-Id: I46be43e6f4c5203eb4ae006d1c6a2fe7d7a949ec Reviewed-on: https://boringssl-review.googlesource.com/8724 Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index 2314344..f211a24 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata
@@ -41,6 +41,7 @@ SSL,140,DH_PUBLIC_VALUE_LENGTH_IS_WRONG SSL,141,DH_P_TOO_LONG SSL,142,DIGEST_CHECK_FAILED +SSL,254,DOWNGRADE_DETECTED SSL,143,DTLS_MESSAGE_TOO_BIG SSL,144,ECC_CERT_NOT_FOR_SIGNING SSL,145,EMS_STATE_INCONSISTENT
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 7c27266..8747d58 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -4661,6 +4661,7 @@ #define SSL_R_INVALID_OUTER_RECORD_TYPE 251 #define SSL_R_UNSUPPORTED_PROTOCOL_FOR_CUSTOM_KEY 252 #define SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS 253 +#define SSL_R_DOWNGRADE_DETECTED 254 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 774754a..0f9a26e 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c
@@ -635,8 +635,7 @@ /* If resending the ClientHello in DTLS after a HelloVerifyRequest, don't * renegerate the client_random. The random must be reused. */ if ((!SSL_IS_DTLS(ssl) || !ssl->d1->send_cookie) && - !ssl_fill_hello_random(ssl->s3->client_random, - sizeof(ssl->s3->client_random), 0 /* client */)) { + !RAND_bytes(ssl->s3->client_random, sizeof(ssl->s3->client_random))) { goto err; } @@ -763,15 +762,16 @@ server_version = ssl->method->version_from_wire(server_wire_version); + uint16_t min_version, max_version; + if (!ssl_get_version_range(ssl, &min_version, &max_version) || + server_version < min_version || server_version > max_version) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL); + al = SSL_AD_PROTOCOL_VERSION; + goto f_err; + } + assert(ssl->s3->have_version == ssl->s3->initial_handshake_complete); if (!ssl->s3->have_version) { - uint16_t min_version, max_version; - if (!ssl_get_version_range(ssl, &min_version, &max_version) || - server_version < min_version || server_version > max_version) { - OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL); - al = SSL_AD_PROTOCOL_VERSION; - goto f_err; - } ssl->version = server_wire_version; ssl->s3->enc_method = ssl3_get_enc_method(server_version); assert(ssl->s3->enc_method != NULL); @@ -787,6 +787,21 @@ /* Copy over the server random. */ memcpy(ssl->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE); + /* Check for a TLS 1.3 downgrade signal. See draft-ietf-tls-tls13-14. + * + * TODO(davidben): Also implement the TLS 1.1 sentinel when things have + * settled down. */ + static const uint8_t kDowngradeTLS12[8] = {0x44, 0x4f, 0x57, 0x4e, + 0x47, 0x52, 0x44, 0x01}; + if (max_version >= TLS1_3_VERSION && + ssl3_protocol_version(ssl) <= TLS1_2_VERSION && + memcmp(ssl->s3->server_random + SSL3_RANDOM_SIZE - 8, kDowngradeTLS12, + 8) == 0) { + al = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, SSL_R_DOWNGRADE_DETECTED); + goto f_err; + } + assert(ssl->session == NULL || ssl->session->session_id_length > 0); if (!ssl->s3->initial_handshake_complete && ssl->session != NULL && CBS_mem_equal(&session_id, ssl->session->session_id,
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index 836eadf..8bc490e 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c
@@ -873,12 +873,30 @@ ssl->s3->tlsext_channel_id_valid = 0; } - if (!ssl_fill_hello_random(ssl->s3->server_random, SSL3_RANDOM_SIZE, - 1 /* server */)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + const uint32_t current_time = time(NULL); + ssl->s3->server_random[0] = current_time >> 24; + ssl->s3->server_random[1] = current_time >> 16; + ssl->s3->server_random[2] = current_time >> 8; + ssl->s3->server_random[3] = current_time; + if (!RAND_bytes(ssl->s3->server_random + 4, SSL3_RANDOM_SIZE - 4)) { return -1; } + /* Fill in the TLS 1.2 downgrade signal. See draft-ietf-tls-tls13-14. + * + * TODO(davidben): Also implement the TLS 1.1 sentinel when things have + * settled down. */ + uint16_t min_version, max_version; + if (!ssl_get_version_range(ssl, &min_version, &max_version)) { + return -1; + } + if (max_version >= TLS1_3_VERSION && + ssl3_protocol_version(ssl) <= TLS1_2_VERSION) { + static const uint8_t kDowngradeTLS12[8] = {0x44, 0x4f, 0x57, 0x4e, + 0x47, 0x52, 0x44, 0x01}; + memcpy(ssl->s3->server_random + SSL3_RANDOM_SIZE - 8, kDowngradeTLS12, 8); + } + CBB cbb, body, session_id; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO) || !CBB_add_u16(&body, ssl->version) ||
diff --git a/ssl/internal.h b/ssl/internal.h index c971bab..4e1c458 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1027,10 +1027,6 @@ STACK_OF(SSL_CIPHER) *ssl_get_ciphers_by_id(SSL *ssl); int ssl_verify_alarm_type(long type); -/* ssl_fill_hello_random fills a client_random or server_random field of length - * |len|. It returns one on success and zero on failure. */ -int ssl_fill_hello_random(uint8_t *out, size_t len, int is_server); - int ssl3_get_finished(SSL *ssl); int ssl3_send_change_cipher_spec(SSL *ssl); void ssl3_cleanup_key_block(SSL *ssl);
diff --git a/ssl/s3_both.c b/ssl/s3_both.c index b5c2ed5..9d4a55e 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c
@@ -672,21 +672,3 @@ return al; } - -int ssl_fill_hello_random(uint8_t *out, size_t len, int is_server) { - if (is_server) { - const uint32_t current_time = time(NULL); - uint8_t *p = out; - - if (len < 4) { - return 0; - } - p[0] = current_time >> 24; - p[1] = current_time >> 16; - p[2] = current_time >> 8; - p[3] = current_time; - return RAND_bytes(p + 4, len - 4); - } else { - return RAND_bytes(out, len); - } -}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 0fda4b5..25dab0c 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -564,6 +564,11 @@ // TLS version in the ClientHello than the maximum supported version. SendClientVersion uint16 + // NegotiateVersion, if non-zero, causes the server to negotiate the + // specifed TLS version rather than the version supported by either + // peer. + NegotiateVersion uint16 + // ExpectFalseStart causes the server to, on full handshakes, // expect the peer to False Start; the server Finished message // isn't sent until we receive an application data record
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index d820001..a67b242 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -302,15 +302,15 @@ c.haveVers = true // Check for downgrade signals in the server random, per - // draft-ietf-tls-tls13-13, section 6.3.1.2. + // draft-ietf-tls-tls13-14, section 6.3.1.2. if c.vers <= VersionTLS12 && c.config.maxVersion(c.isDTLS) >= VersionTLS13 { - if bytes.Equal(serverHello.random[:8], downgradeTLS13) { + if bytes.Equal(serverHello.random[len(serverHello.random)-8:], downgradeTLS13) { c.sendAlert(alertProtocolVersion) return errors.New("tls: downgrade from TLS 1.3 detected") } } if c.vers <= VersionTLS11 && c.config.maxVersion(c.isDTLS) >= VersionTLS12 { - if bytes.Equal(serverHello.random[:8], downgradeTLS12) { + if bytes.Equal(serverHello.random[len(serverHello.random)-8:], downgradeTLS12) { c.sendAlert(alertProtocolVersion) return errors.New("tls: downgrade from TLS 1.2 detected") }
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index eecb4fc..c8ddf91 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -211,10 +211,14 @@ } } - c.vers, ok = config.mutualVersion(hs.clientHello.vers, c.isDTLS) - if !ok { - c.sendAlert(alertProtocolVersion) - return fmt.Errorf("tls: client offered an unsupported, maximum protocol version of %x", hs.clientHello.vers) + if config.Bugs.NegotiateVersion != 0 { + c.vers = config.Bugs.NegotiateVersion + } else { + c.vers, ok = config.mutualVersion(hs.clientHello.vers, c.isDTLS) + if !ok { + c.sendAlert(alertProtocolVersion) + return fmt.Errorf("tls: client offered an unsupported, maximum protocol version of %x", hs.clientHello.vers) + } } c.haveVers = true @@ -263,12 +267,13 @@ c.sendAlert(alertInternalError) return false, err } - // Signal downgrades in the server random, per draft-ietf-tls-tls13-13, section 6.3.1.2. + // Signal downgrades in the server random, per draft-ietf-tls-tls13-14, + // section 6.3.1.2. if c.vers <= VersionTLS12 && config.maxVersion(c.isDTLS) >= VersionTLS13 { - copy(hs.hello.random[:8], downgradeTLS13) + copy(hs.hello.random[len(hs.hello.random)-8:], downgradeTLS13) } if c.vers <= VersionTLS11 && config.maxVersion(c.isDTLS) == VersionTLS12 { - copy(hs.hello.random[:8], downgradeTLS12) + copy(hs.hello.random[len(hs.hello.random)-8:], downgradeTLS12) } foundCompression := false
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index a54c357..3531909 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -3614,6 +3614,29 @@ shouldFail: true, expectedError: ":UNSUPPORTED_PROTOCOL:", }) + + // Test TLS 1.3's downgrade signal. + testCases = append(testCases, testCase{ + name: "Downgrade-TLS12-Client", + config: Config{ + Bugs: ProtocolBugs{ + NegotiateVersion: VersionTLS12, + }, + }, + shouldFail: true, + expectedError: ":DOWNGRADE_DETECTED:", + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Downgrade-TLS12-Server", + config: Config{ + Bugs: ProtocolBugs{ + SendClientVersion: VersionTLS12, + }, + }, + shouldFail: true, + expectedLocalError: "tls: downgrade from TLS 1.3 detected", + }) } func addMinimumVersionTests() {