Update crypto negotation to draft 15. BUG=77 Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7 Reviewed-on: https://boringssl-review.googlesource.com/10840 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index c5b3b1f..31394c0 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c
@@ -626,18 +626,6 @@ if (!CBB_add_u16(&child, ssl_cipher_get_value(cipher))) { return 0; } - /* Add PSK ciphers for TLS 1.3 resumption. */ - uint16_t session_version; - if (ssl->session != NULL && - ssl->method->version_from_wire(&session_version, - ssl->session->ssl_version) && - session_version >= TLS1_3_VERSION) { - uint16_t resumption_cipher; - if (ssl_cipher_get_ecdhe_psk_cipher(cipher, &resumption_cipher) && - !CBB_add_u16(&child, resumption_cipher)) { - return 0; - } - } } /* If all ciphers were disabled, return the error to the caller. */ @@ -952,16 +940,16 @@ } if (ssl->session != NULL) { - if (ssl->session->cipher != c) { - al = SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); - goto f_err; - } if (ssl->session->ssl_version != ssl->version) { al = SSL_AD_ILLEGAL_PARAMETER; OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_VERSION_NOT_RETURNED); goto f_err; } + if (ssl->session->cipher != c) { + al = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); + goto f_err; + } if (!ssl_session_is_context_valid(ssl, ssl->session)) { /* This is actually a client application bug. */ al = SSL_AD_ILLEGAL_PARAMETER;
diff --git a/ssl/internal.h b/ssl/internal.h index 4719acb..3e7c053 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -172,12 +172,14 @@ /* SSL_kPSK is only set for plain PSK, not ECDHE_PSK. */ #define SSL_kPSK 0x00000008L #define SSL_kCECPQ1 0x00000010L +#define SSL_kGENERIC 0x00000020L /* Bits for |algorithm_auth| (server authentication). */ #define SSL_aRSA 0x00000001L #define SSL_aECDSA 0x00000002L /* SSL_aPSK is set for both PSK and ECDHE_PSK. */ #define SSL_aPSK 0x00000004L +#define SSL_aGENERIC 0x00000008L #define SSL_aCERT (SSL_aRSA | SSL_aECDSA) @@ -241,11 +243,6 @@ /* ssl_cipher_get_value returns the cipher suite id of |cipher|. */ uint16_t ssl_cipher_get_value(const SSL_CIPHER *cipher); -/* ssl_cipher_get_resumption_cipher returns the cipher suite id of the cipher - * matching |cipher| with PSK enabled. */ -int ssl_cipher_get_ecdhe_psk_cipher(const SSL_CIPHER *cipher, - uint16_t *out_cipher); - /* ssl_cipher_get_key_type returns the |EVP_PKEY_*| value corresponding to the * server key used in |cipher| or |EVP_PKEY_NONE| if there is none. */ int ssl_cipher_get_key_type(const SSL_CIPHER *cipher);
diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c index 4831e9b..08a4e65 100644 --- a/ssl/ssl_cipher.c +++ b/ssl/ssl_cipher.c
@@ -343,6 +343,41 @@ SSL_HANDSHAKE_MAC_SHA384, }, + /* TLS 1.3 suites. */ + + /* Cipher 1301 */ + { + TLS1_TXT_AES_128_GCM_SHA256, + TLS1_CK_AES_128_GCM_SHA256, + SSL_kGENERIC, + SSL_aGENERIC, + SSL_AES128GCM, + SSL_AEAD, + SSL_HANDSHAKE_MAC_SHA256, + }, + + /* Cipher 1302 */ + { + TLS1_TXT_AES_256_GCM_SHA384, + TLS1_CK_AES_256_GCM_SHA384, + SSL_kGENERIC, + SSL_aGENERIC, + SSL_AES256GCM, + SSL_AEAD, + SSL_HANDSHAKE_MAC_SHA384, + }, + + /* Cipher 1303 */ + { + TLS1_TXT_CHACHA20_POLY1305_SHA256, + TLS1_CK_CHACHA20_POLY1305_SHA256, + SSL_kGENERIC, + SSL_aGENERIC, + SSL_CHACHA20POLY1305, + SSL_AEAD, + SSL_HANDSHAKE_MAC_SHA256, + }, + /* CECPQ1 (combined elliptic curve + post-quantum) suites. */ /* Cipher 16B7 */ @@ -608,28 +643,6 @@ SSL_HANDSHAKE_MAC_SHA256, }, - /* Cipher D001 */ - { - TLS1_TXT_ECDHE_PSK_WITH_AES_128_GCM_SHA256, - TLS1_CK_ECDHE_PSK_WITH_AES_128_GCM_SHA256, - SSL_kECDHE, - SSL_aPSK, - SSL_AES128GCM, - SSL_AEAD, - SSL_HANDSHAKE_MAC_SHA256, - }, - - /* Cipher D002 */ - { - TLS1_TXT_ECDHE_PSK_WITH_AES_256_GCM_SHA384, - TLS1_CK_ECDHE_PSK_WITH_AES_256_GCM_SHA384, - SSL_kECDHE, - SSL_aPSK, - SSL_AES256GCM, - SSL_AEAD, - SSL_HANDSHAKE_MAC_SHA384, - }, - }; static const size_t kCiphersLen = OPENSSL_ARRAY_SIZE(kCiphers); @@ -1063,14 +1076,6 @@ (min_version != 0 && SSL_CIPHER_get_min_version(cp) != min_version)) { continue; } - - /* The following ciphers are internal implementation details of TLS 1.3 - * resumption but are not yet finalized. Disable them by default until - * then. */ - if (cp->id == TLS1_CK_ECDHE_PSK_WITH_AES_128_GCM_SHA256 || - cp->id == TLS1_CK_ECDHE_PSK_WITH_AES_256_GCM_SHA384) { - continue; - } } /* add the cipher if it has not been added yet. */ @@ -1410,15 +1415,17 @@ /* Now arrange all ciphers by preference: * TODO(davidben): Compute this order once and copy it. */ - /* Everything else being equal, prefer ECDHE_ECDSA then ECDHE_RSA over other - * key exchange mechanisms */ + /* Everything else being equal, prefer TLS 1.3 ciphers then ECDHE_ECDSA then + * ECDHE_RSA over other key exchange mechanisms */ + ssl_cipher_apply_rule(0, SSL_kGENERIC, SSL_aGENERIC, ~0u, ~0u, 0, CIPHER_ADD, + -1, 0, &head, &tail); ssl_cipher_apply_rule(0, SSL_kECDHE, SSL_aECDSA, ~0u, ~0u, 0, CIPHER_ADD, -1, 0, &head, &tail); ssl_cipher_apply_rule(0, SSL_kECDHE, ~0u, ~0u, ~0u, 0, CIPHER_ADD, -1, 0, &head, &tail); - ssl_cipher_apply_rule(0, SSL_kECDHE, ~0u, ~0u, ~0u, 0, CIPHER_DEL, -1, 0, - &head, &tail); + ssl_cipher_apply_rule(0, ~0u, ~0u, ~0u, ~0u, 0, CIPHER_DEL, -1, 0, &head, + &tail); /* Order the bulk ciphers. First the preferred AEAD ciphers. We prefer * CHACHA20 unless there is hardware support for fast and constant-time @@ -1458,7 +1465,7 @@ &tail); /* Move ciphers without forward secrecy to the end. */ - ssl_cipher_apply_rule(0, ~(SSL_kDHE | SSL_kECDHE), ~0u, ~0u, ~0u, 0, + ssl_cipher_apply_rule(0, (SSL_kRSA | SSL_kPSK), ~0u, ~0u, ~0u, 0, CIPHER_ORD, -1, 0, &head, &tail); /* Now disable everything (maintaining the ordering!) */ @@ -1569,30 +1576,6 @@ return id & 0xffff; } -int ssl_cipher_get_ecdhe_psk_cipher(const SSL_CIPHER *cipher, - uint16_t *out_cipher) { - switch (cipher->id) { - case TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256: - case TLS1_CK_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256: - case TLS1_CK_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256: - *out_cipher = TLS1_CK_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256 & 0xffff; - return 1; - - case TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256: - case TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: - case TLS1_CK_ECDHE_PSK_WITH_AES_128_GCM_SHA256: - *out_cipher = TLS1_CK_ECDHE_PSK_WITH_AES_128_GCM_SHA256 & 0xffff; - return 1; - - case TLS1_CK_ECDHE_RSA_WITH_AES_256_GCM_SHA384: - case TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384: - case TLS1_CK_ECDHE_PSK_WITH_AES_256_GCM_SHA384: - *out_cipher = TLS1_CK_ECDHE_PSK_WITH_AES_256_GCM_SHA384 & 0xffff; - return 1; - } - return 0; -} - int SSL_CIPHER_is_AES(const SSL_CIPHER *cipher) { return (cipher->algorithm_enc & SSL_AES) != 0; } @@ -1656,6 +1639,11 @@ } uint16_t SSL_CIPHER_get_min_version(const SSL_CIPHER *cipher) { + if (cipher->algorithm_mkey == SSL_kGENERIC || + cipher->algorithm_auth == SSL_aGENERIC) { + return TLS1_3_VERSION; + } + if (cipher->algorithm_prf != SSL_HANDSHAKE_MAC_DEFAULT) { /* Cipher suites before TLS 1.2 use the default PRF, while all those added * afterwards specify a particular hash. */ @@ -1665,11 +1653,8 @@ } uint16_t SSL_CIPHER_get_max_version(const SSL_CIPHER *cipher) { - if (cipher->algorithm_mac == SSL_AEAD && - (cipher->algorithm_enc & SSL_CHACHA20POLY1305_OLD) == 0 && - (cipher->algorithm_mkey & SSL_kECDHE) != 0 && - /* TODO(davidben,svaldez): Support PSK-based ciphers in TLS 1.3. */ - (cipher->algorithm_auth & SSL_aCERT) != 0) { + if (cipher->algorithm_mkey == SSL_kGENERIC || + cipher->algorithm_auth == SSL_aGENERIC) { return TLS1_3_VERSION; } return TLS1_2_VERSION; @@ -1730,6 +1715,10 @@ assert(cipher->algorithm_auth == SSL_aPSK); return "PSK"; + case SSL_kGENERIC: + assert(cipher->algorithm_auth == SSL_aGENERIC); + return "GENERIC"; + default: assert(0); return "UNKNOWN"; @@ -1788,16 +1777,23 @@ const char *enc_name = ssl_cipher_get_enc_name(cipher); const char *prf_name = ssl_cipher_get_prf_name(cipher); - /* The final name is TLS_{kx_name}_WITH_{enc_name}_{prf_name}. */ - size_t len = 4 + strlen(kx_name) + 6 + strlen(enc_name) + 1 + - strlen(prf_name) + 1; + /* The final name is TLS_{kx_name}_WITH_{enc_name}_{prf_name} or + * TLS_{enc_name}_{prf_name} depending on whether the cipher is AEAD-only. */ + size_t len = 4 + strlen(enc_name) + 1 + strlen(prf_name) + 1; + + if (cipher->algorithm_mkey != SSL_kGENERIC) { + len += strlen(kx_name) + 6; + } + char *ret = OPENSSL_malloc(len); if (ret == NULL) { return NULL; } + if (BUF_strlcpy(ret, "TLS_", len) >= len || - BUF_strlcat(ret, kx_name, len) >= len || - BUF_strlcat(ret, "_WITH_", len) >= len || + (cipher->algorithm_mkey != SSL_kGENERIC && + (BUF_strlcat(ret, kx_name, len) >= len || + BUF_strlcat(ret, "_WITH_", len) >= len)) || BUF_strlcat(ret, enc_name, len) >= len || BUF_strlcat(ret, "_", len) >= len || BUF_strlcat(ret, prf_name, len) >= len) { @@ -1805,6 +1801,7 @@ OPENSSL_free(ret); return NULL; } + assert(strlen(ret) + 1 == len); return ret; } @@ -1885,6 +1882,10 @@ kx = "PSK"; break; + case SSL_kGENERIC: + kx = "GENERIC"; + break; + default: kx = "unknown"; } @@ -1902,6 +1903,10 @@ au = "PSK"; break; + case SSL_aGENERIC: + au = "GENERIC"; + break; + default: au = "unknown"; break;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index b580d95..f17dc0a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c
@@ -1515,7 +1515,8 @@ SSL_SESSION *session = SSL_get_session(ssl); if (session == NULL || session->cipher == NULL || - !SSL_CIPHER_is_ECDHE(session->cipher)) { + (ssl3_protocol_version(ssl) < TLS1_3_VERSION && + !SSL_CIPHER_is_ECDHE(session->cipher))) { return 0; } @@ -2030,6 +2031,12 @@ void ssl_get_compatible_server_ciphers(SSL *ssl, uint32_t *out_mask_k, uint32_t *out_mask_a) { + if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) { + *out_mask_k = SSL_kGENERIC; + *out_mask_a = SSL_aGENERIC; + return; + } + uint32_t mask_k = 0; uint32_t mask_a = 0;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index be6443c..f6d1732 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -814,30 +814,34 @@ } CIPHER_RFC_NAME_TEST; static const CIPHER_RFC_NAME_TEST kCipherRFCNameTests[] = { - { SSL3_CK_RSA_DES_192_CBC3_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA" }, - { TLS1_CK_RSA_WITH_AES_128_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA" }, - { TLS1_CK_DHE_RSA_WITH_AES_256_SHA, "TLS_DHE_RSA_WITH_AES_256_CBC_SHA" }, - { TLS1_CK_DHE_RSA_WITH_AES_256_SHA256, - "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256" }, - { TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256, - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256" }, - { TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384, - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384" }, - { TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" }, - { TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" }, - { TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" }, - { TLS1_CK_ECDHE_PSK_WITH_AES_128_CBC_SHA, - "TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA" }, - { TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256" }, - // These names are non-standard: - { TLS1_CK_ECDHE_RSA_CHACHA20_POLY1305_OLD, - "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256" }, - { TLS1_CK_ECDHE_ECDSA_CHACHA20_POLY1305_OLD, - "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256" }, + {SSL3_CK_RSA_DES_192_CBC3_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA"}, + {TLS1_CK_RSA_WITH_AES_128_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA"}, + {TLS1_CK_DHE_RSA_WITH_AES_256_SHA, "TLS_DHE_RSA_WITH_AES_256_CBC_SHA"}, + {TLS1_CK_DHE_RSA_WITH_AES_256_SHA256, + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256"}, + {TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256, + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256"}, + {TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384, + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384"}, + {TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"}, + {TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"}, + {TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"}, + {TLS1_CK_ECDHE_PSK_WITH_AES_128_CBC_SHA, + "TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA"}, + {TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"}, + {TLS1_CK_AES_256_GCM_SHA384, "TLS_AES_256_GCM_SHA384"}, + {TLS1_CK_AES_128_GCM_SHA256, "TLS_AES_128_GCM_SHA256"}, + {TLS1_CK_CHACHA20_POLY1305_SHA256, "TLS_CHACHA20_POLY1305_SHA256"}, + + // These names are non-standard: + {TLS1_CK_ECDHE_RSA_CHACHA20_POLY1305_OLD, + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"}, + {TLS1_CK_ECDHE_ECDSA_CHACHA20_POLY1305_OLD, + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"}, }; static bool TestCipherGetRFCName(void) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 0da212b..b9d359e 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -1240,13 +1240,10 @@ return 1; } - /* OCSP stapling is forbidden on a non-certificate cipher. */ - if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { - return 0; - } - if (ssl3_protocol_version(ssl) < TLS1_3_VERSION) { - if (CBS_len(contents) != 0) { + /* OCSP stapling is forbidden on non-certificate ciphers. */ + if (CBS_len(contents) != 0 || + !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { return 0; } @@ -1258,6 +1255,11 @@ return 1; } + /* In TLS 1.3, OCSP stapling is forbidden on resumption. */ + if (ssl->s3->session_reused) { + return 0; + } + uint8_t status_type; CBS ocsp_response; if (!CBS_get_u8(contents, &status_type) || @@ -1298,7 +1300,9 @@ static int ext_ocsp_add_serverhello(SSL *ssl, CBB *out) { if (!ssl->s3->tmp.ocsp_stapling_requested || ssl->ctx->ocsp_response_length == 0 || - !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { + ssl->s3->session_reused || + (ssl3_protocol_version(ssl) < TLS1_3_VERSION && + !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher))) { return 1; } @@ -2231,8 +2235,13 @@ uint8_t *out_alert, CBS *contents) { uint16_t group_id; CBS key_shares; - if (!tls1_get_shared_group(ssl, &group_id) || - !CBS_get_u16_length_prefixed(contents, &key_shares) || + if (!tls1_get_shared_group(ssl, &group_id)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_GROUP); + *out_alert = SSL_AD_HANDSHAKE_FAILURE; + return 0; + } + + if (!CBS_get_u16_length_prefixed(contents, &key_shares) || CBS_len(contents) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); return 0; @@ -2286,6 +2295,7 @@ OPENSSL_free(secret); SSL_ECDH_CTX_cleanup(&group); CBB_cleanup(&public_key); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; return 0; } @@ -2298,10 +2308,6 @@ } int ssl_ext_key_share_add_serverhello(SSL *ssl, CBB *out) { - if (ssl->s3->tmp.new_cipher->algorithm_mkey != SSL_kECDHE) { - return 1; - } - uint16_t group_id; CBB kse_bytes, public_key; if (!tls1_get_shared_group(ssl, &group_id) ||
diff --git a/ssl/test/runner/cipher_suites.go b/ssl/test/runner/cipher_suites.go index 4ce4629..656a3d0 100644 --- a/ssl/test/runner/cipher_suites.go +++ b/ssl/test/runner/cipher_suites.go
@@ -101,38 +101,20 @@ return crypto.SHA256 } -// TODO(nharper): Remove this function when TLS 1.3 cipher suites get -// refactored to break out the AEAD/PRF from everything else. Once that's -// done, this won't be necessary anymore. -func ecdhePSKSuite(id uint16) uint16 { - switch id { - case TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, - TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256: - return TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256 - case TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256: - return TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256 - case TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384: - return TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384 - } - return 0 -} - var cipherSuites = []*cipherSuite{ // Ciphersuite order is chosen so that ECDHE comes before plain RSA // and RC4 comes before AES (because of the Lucky13 attack). - {TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12 | suiteTLS13, nil, nil, aeadCHACHA20POLY1305}, - {TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdheRSAKA, suiteECDHE | suiteTLS12 | suiteTLS13, nil, nil, aeadCHACHA20POLY1305}, + {TLS_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, nil, suiteTLS13, nil, nil, aeadCHACHA20POLY1305}, + {TLS_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, nil, suiteTLS13, nil, nil, aeadAESGCM}, + {TLS_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, nil, suiteTLS13 | suiteSHA384, nil, nil, aeadAESGCM}, + {TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12, nil, nil, aeadCHACHA20POLY1305}, + {TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdheRSAKA, suiteECDHE | suiteTLS12, nil, nil, aeadCHACHA20POLY1305}, {TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256_OLD, 32, 0, noIV, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12, nil, nil, aeadCHACHA20POLY1305Old}, {TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256_OLD, 32, 0, noIV, ecdheRSAKA, suiteECDHE | suiteTLS12, nil, nil, aeadCHACHA20POLY1305Old}, - {TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, ecdheRSAKA, suiteECDHE | suiteTLS12 | suiteTLS13, nil, nil, aeadAESGCM}, - {TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12 | suiteTLS13, nil, nil, aeadAESGCM}, - {TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, ecdheRSAKA, suiteECDHE | suiteTLS12 | suiteTLS13 | suiteSHA384, nil, nil, aeadAESGCM}, - {TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12 | suiteTLS13 | suiteSHA384, nil, nil, aeadAESGCM}, + {TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, ecdheRSAKA, suiteECDHE | suiteTLS12, nil, nil, aeadAESGCM}, + {TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12, nil, nil, aeadAESGCM}, + {TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, ecdheRSAKA, suiteECDHE | suiteTLS12 | suiteSHA384, nil, nil, aeadAESGCM}, + {TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteTLS12 | suiteSHA384, nil, nil, aeadAESGCM}, {TLS_ECDHE_RSA_WITH_RC4_128_SHA, 16, 20, noIV, ecdheRSAKA, suiteECDHE | suiteNoDTLS, cipherRC4, macSHA1, nil}, {TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, 16, 20, noIV, ecdheECDSAKA, suiteECDHE | suiteECDSA | suiteNoDTLS, cipherRC4, macSHA1, nil}, {TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, 16, 32, ivLenAES, ecdheRSAKA, suiteECDHE | suiteTLS12, cipherAES, macSHA256, nil}, @@ -164,9 +146,7 @@ {TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, 24, 20, ivLen3DES, ecdheRSAKA, suiteECDHE, cipher3DES, macSHA1, nil}, {TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA, 24, 20, ivLen3DES, dheRSAKA, 0, cipher3DES, macSHA1, nil}, {TLS_RSA_WITH_3DES_EDE_CBC_SHA, 24, 20, ivLen3DES, rsaKA, 0, cipher3DES, macSHA1, nil}, - {TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdhePSKKA, suiteECDHE | suitePSK | suiteTLS12 | suiteTLS13, nil, nil, aeadCHACHA20POLY1305}, - {TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256, 16, 0, ivLenAESGCM, ecdhePSKKA, suiteECDHE | suitePSK | suiteTLS12 | suiteTLS13, nil, nil, aeadAESGCM}, - {TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384, 32, 0, ivLenAESGCM, ecdhePSKKA, suiteECDHE | suitePSK | suiteTLS12 | suiteTLS13 | suiteSHA384, nil, nil, aeadAESGCM}, + {TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256, 32, 0, ivLenChaCha20Poly1305, ecdhePSKKA, suiteECDHE | suitePSK | suiteTLS12, nil, nil, aeadCHACHA20POLY1305}, {TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA, 16, 20, ivLenAES, ecdhePSKKA, suiteECDHE | suitePSK, cipherAES, macSHA1, nil}, {TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA, 32, 20, ivLenAES, ecdhePSKKA, suiteECDHE | suitePSK, cipherAES, macSHA1, nil}, {TLS_PSK_WITH_RC4_128_SHA, 16, 20, noIV, pskKA, suiteNoDTLS | suitePSK, cipherRC4, macSHA1, nil}, @@ -562,8 +542,9 @@ const ( TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256_OLD uint16 = 0xcc13 TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256_OLD uint16 = 0xcc14 - TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256 uint16 = 0xd001 - TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384 uint16 = 0xd002 + TLS_AES_128_GCM_SHA256 uint16 = 0x1301 + TLS_AES_256_GCM_SHA384 uint16 = 0x1302 + TLS_CHACHA20_POLY1305_SHA256 uint16 = 0x1303 TLS_CECPQ1_RSA_WITH_CHACHA20_POLY1305_SHA256 uint16 = 0x16b7 TLS_CECPQ1_ECDSA_WITH_CHACHA20_POLY1305_SHA256 uint16 = 0x16b8 TLS_CECPQ1_RSA_WITH_AES_256_GCM_SHA384 uint16 = 0x16b9
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 599ce02..ae46505 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -27,7 +27,7 @@ ) // A draft version of TLS 1.3 that is sent over the wire for the current draft. -const tls13DraftVersion = 0x7f0e +const tls13DraftVersion = 0x7f0f const ( maxPlaintext = 16384 // maximum plaintext payload length @@ -1026,6 +1026,18 @@ // advertised in server extensions AdvertiseTicketExtension bool + // NegotiatePSKResumption, if true, causes the server to attempt pure PSK + // resumption. + NegotiatePSKResumption bool + + // OmitServerHelloSignatureAlgorithms, if true, causes the server to omit the + // signature_algorithms extension in the ServerHello. + OmitServerHelloSignatureAlgorithms bool + + // IncludeServerHelloSignatureAlgorithms, if true, causes the server to + // include the signature_algorithms extension in all ServerHellos. + IncludeServerHelloSignatureAlgorithms bool + // MissingKeyShare, if true, causes the TLS 1.3 implementation to skip // sending a key_share extension and use the zero ECDHE secret // instead.
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index f03e169..c5be2b7 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -217,24 +217,10 @@ // Check that the ciphersuite/version used for the // previous session are still valid. cipherSuiteOk := false - if candidateSession.vers >= VersionTLS13 { - // Account for ciphers changing on resumption. - // - // TODO(davidben): This will be gone with the - // new cipher negotiation scheme. - resumeCipher := ecdhePSKSuite(candidateSession.cipherSuite) - for _, id := range hello.cipherSuites { - if ecdhePSKSuite(id) == resumeCipher { - cipherSuiteOk = true - break - } - } - } else { - for _, id := range hello.cipherSuites { - if id == candidateSession.cipherSuite { - cipherSuiteOk = true - break - } + for _, id := range hello.cipherSuites { + if id == candidateSession.cipherSuite { + cipherSuiteOk = true + break } } @@ -274,7 +260,6 @@ } hello.pskIdentities = []pskIdentity{psk} - hello.cipherSuites = append(hello.cipherSuites, ecdhePSKSuite(session.cipherSuite)) } if session.vers < VersionTLS13 || c.config.Bugs.SendBothTickets { @@ -606,10 +591,10 @@ // TODO(davidben): This will need to be handled slightly earlier once // 0-RTT is implemented. var psk []byte - if hs.suite.flags&suitePSK != 0 { - if !hs.serverHello.hasPSKIdentity { - c.sendAlert(alertMissingExtension) - return errors.New("tls: server omitted the PSK identity extension") + if hs.serverHello.hasPSKIdentity { + if hs.serverHello.useCertAuth || !hs.serverHello.hasKeyShare { + c.sendAlert(alertUnsupportedExtension) + return errors.New("tls: server omitted KeyShare or included SignatureAlgorithms on resumption.") } // We send at most one PSK identity. @@ -617,17 +602,17 @@ c.sendAlert(alertUnknownPSKIdentity) return errors.New("tls: server sent unknown PSK identity") } - if ecdhePSKSuite(hs.session.cipherSuite) != hs.suite.id { + if hs.session.cipherSuite != hs.suite.id { c.sendAlert(alertHandshakeFailure) - return errors.New("tls: server sent invalid cipher suite for PSK") + return errors.New("tls: server sent invalid cipher suite") } psk = deriveResumptionPSK(hs.suite, hs.session.masterSecret) hs.finishedHash.setResumptionContext(deriveResumptionContext(hs.suite, hs.session.masterSecret)) c.didResume = true } else { - if hs.serverHello.hasPSKIdentity { + if !hs.serverHello.useCertAuth || !hs.serverHello.hasKeyShare { c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server sent unexpected PSK identity") + return errors.New("tls: server omitted KeyShare and SignatureAlgorithms on non-resumption.") } psk = zeroSecret @@ -638,12 +623,7 @@ // Resolve ECDHE and compute the handshake secret. var ecdheSecret []byte - if hs.suite.flags&suiteECDHE != 0 && !c.config.Bugs.MissingKeyShare && !c.config.Bugs.SecondClientHelloMissingKeyShare { - if !hs.serverHello.hasKeyShare { - c.sendAlert(alertMissingExtension) - return errors.New("tls: server omitted the key share extension") - } - + if !c.config.Bugs.MissingKeyShare && !c.config.Bugs.SecondClientHelloMissingKeyShare { curve, ok := hs.keyShares[hs.serverHello.keyShare.group] if !ok { c.sendAlert(alertHandshakeFailure) @@ -657,11 +637,6 @@ return err } } else { - if hs.serverHello.hasKeyShare { - c.sendAlert(alertUnsupportedExtension) - return errors.New("tls: server sent unexpected key share extension") - } - ecdheSecret = zeroSecret } @@ -692,7 +667,7 @@ var chainToSend *Certificate var certReq *certificateRequestMsg - if hs.suite.flags&suitePSK != 0 { + if !hs.serverHello.useCertAuth { if encryptedExtensions.extensions.ocspResponse != nil { c.sendAlert(alertUnsupportedExtension) return errors.New("tls: server sent OCSP response without a certificate")
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index e7a1235..19578e8 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -779,6 +779,7 @@ keyShare keyShareEntry hasPSKIdentity bool pskIdentity uint16 + useCertAuth bool earlyDataIndication bool compressionMethod uint8 extensions serverExtensions @@ -831,6 +832,10 @@ extensions.addU16(2) // Length extensions.addU16(m.pskIdentity) } + if m.useCertAuth { + extensions.addU16(extensionSignatureAlgorithms) + extensions.addU16(0) // Length + } if m.earlyDataIndication { extensions.addU16(extensionEarlyData) extensions.addU16(0) // Length @@ -928,6 +933,11 @@ } m.pskIdentity = uint16(d[0])<<8 | uint16(d[1]) m.hasPSKIdentity = true + case extensionSignatureAlgorithms: + if len(d) != 0 { + return false + } + m.useCertAuth = true case extensionEarlyData: if len(d) != 0 { return false
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 934e052..c2fae55 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -396,7 +396,10 @@ } } - _, ecdsaOk := hs.cert.PrivateKey.(*ecdsa.PrivateKey) + if !supportedCurve { + c.sendAlert(alertHandshakeFailure) + return errors.New("tls: no curve supported by both client and server") + } pskIdentities := hs.clientHello.pskIdentities if len(pskIdentities) == 0 && len(hs.clientHello.sessionTicket) > 0 && c.config.Bugs.AcceptAnySession { @@ -431,46 +434,43 @@ if !ok { continue } - if !config.Bugs.AcceptAnySession { + if config.Bugs.AcceptAnySession { + // Replace the cipher suite with one known to work, to + // test cross-version resumption attempts. + sessionState.cipherSuite = TLS_AES_128_GCM_SHA256 + } else { if sessionState.vers != c.vers && c.config.Bugs.AcceptAnySession { continue } if sessionState.ticketExpiration.Before(c.config.time()) { continue } - } - suiteId := ecdhePSKSuite(sessionState.cipherSuite) - - // Check the client offered the cipher. - clientCipherSuites := hs.clientHello.cipherSuites - if config.Bugs.AcceptAnySession { - clientCipherSuites = []uint16{suiteId} - } - suite := mutualCipherSuite(clientCipherSuites, suiteId) - - // Check the cipher is enabled by the server or is a resumption - // suite of one enabled by the server. Account for the cipher - // change on resume. - // - // TODO(davidben): The ecdhePSKSuite mess will be gone with the - // new cipher negotiation scheme. - var found bool - for _, id := range config.cipherSuites() { - if ecdhePSKSuite(id) == suiteId { - found = true - break + cipherSuiteOk := false + // Check that the client is still offering the ciphersuite in the session. + for _, id := range hs.clientHello.cipherSuites { + if id == sessionState.cipherSuite { + cipherSuiteOk = true + break + } + } + if !cipherSuiteOk { + continue } } - if suite != nil && found { - hs.sessionState = sessionState - hs.suite = suite - hs.hello.hasPSKIdentity = true - hs.hello.pskIdentity = uint16(i) - c.didResume = true - break + // Check that we also support the ciphersuite from the session. + suite := c.tryCipherSuite(sessionState.cipherSuite, c.config.cipherSuites(), c.vers, true, true) + if suite == nil { + continue } + + hs.sessionState = sessionState + hs.suite = suite + hs.hello.hasPSKIdentity = true + hs.hello.pskIdentity = uint16(i) + c.didResume = true + break } // If not resuming, select the cipher suite. @@ -485,7 +485,7 @@ } for _, id := range preferenceList { - if hs.suite = c.tryCipherSuite(id, supportedList, c.vers, supportedCurve, ecdsaOk, false); hs.suite != nil { + if hs.suite = c.tryCipherSuite(id, supportedList, c.vers, true, true); hs.suite != nil { break } } @@ -505,9 +505,11 @@ hs.finishedHash.discardHandshakeBuffer() hs.writeClientHash(hs.clientHello.marshal()) + hs.hello.useCertAuth = hs.sessionState == nil + // Resolve PSK and compute the early secret. var psk []byte - if hs.suite.flags&suitePSK != 0 { + if hs.sessionState != nil { psk = deriveResumptionPSK(hs.suite, hs.sessionState.masterSecret) hs.finishedHash.setResumptionContext(deriveResumptionContext(hs.suite, hs.sessionState.masterSecret)) } else { @@ -517,9 +519,23 @@ earlySecret := hs.finishedHash.extractKey(hs.finishedHash.zeroSecret(), psk) + if config.Bugs.OmitServerHelloSignatureAlgorithms { + hs.hello.useCertAuth = false + } else if config.Bugs.IncludeServerHelloSignatureAlgorithms { + hs.hello.useCertAuth = true + } + + hs.hello.hasKeyShare = true + if hs.sessionState != nil && config.Bugs.NegotiatePSKResumption { + hs.hello.hasKeyShare = false + } + if config.Bugs.MissingKeyShare { + hs.hello.hasKeyShare = false + } + // Resolve ECDHE and compute the handshake secret. var ecdheSecret []byte - if hs.suite.flags&suiteECDHE != 0 && !config.Bugs.MissingKeyShare { + if hs.hello.hasKeyShare { // Look for the key share corresponding to our selected curve. var selectedKeyShare *keyShareEntry for i := range hs.clientHello.keyShares { @@ -671,7 +687,7 @@ c.out.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, serverWrite) c.in.useTrafficSecret(c.vers, hs.suite, handshakeTrafficSecret, handshakePhase, clientWrite) - if hs.suite.flags&suitePSK == 0 { + if hs.hello.useCertAuth { if hs.clientHello.ocspStapling { encryptedExtensions.extensions.ocspResponse = hs.cert.OCSPStaple } @@ -696,7 +712,7 @@ c.writeRecord(recordTypeHandshake, encryptedExtensions.marshal()) } - if hs.suite.flags&suitePSK == 0 { + if hs.hello.useCertAuth { if config.ClientAuth >= RequestClientCert { // Request a client certificate certReq := &certificateRequestMsg{ @@ -757,7 +773,7 @@ hs.writeServerHash(certVerify.marshal()) c.writeRecord(recordTypeHandshake, certVerify.marshal()) - } else { + } else if hs.sessionState != nil { // Pick up certificates from the session instead. if len(hs.sessionState.certificates) > 0 { if _, err := hs.processCertsFromClient(hs.sessionState.certificates); err != nil { @@ -968,7 +984,7 @@ } for _, id := range preferenceList { - if hs.suite = c.tryCipherSuite(id, supportedList, c.vers, hs.ellipticOk, hs.ecdsaOk, true); hs.suite != nil { + if hs.suite = c.tryCipherSuite(id, supportedList, c.vers, hs.ellipticOk, hs.ecdsaOk); hs.suite != nil { break } } @@ -1134,7 +1150,11 @@ } } - if !c.config.Bugs.AcceptAnySession { + if c.config.Bugs.AcceptAnySession { + // Replace the cipher suite with one known to work, to test + // cross-version resumption attempts. + hs.sessionState.cipherSuite = TLS_RSA_WITH_AES_128_CBC_SHA + } else { // Never resume a session for a different SSL version. if c.vers != hs.sessionState.vers { return false @@ -1154,7 +1174,8 @@ } // Check that we also support the ciphersuite from the session. - hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), hs.sessionState.vers, hs.ellipticOk, hs.ecdsaOk, true) + hs.suite = c.tryCipherSuite(hs.sessionState.cipherSuite, c.config.cipherSuites(), c.vers, hs.ellipticOk, hs.ecdsaOk) + if hs.suite == nil { return false } @@ -1726,7 +1747,7 @@ // tryCipherSuite returns a cipherSuite with the given id if that cipher suite // is acceptable to use. -func (c *Conn) tryCipherSuite(id uint16, supportedCipherSuites []uint16, version uint16, ellipticOk, ecdsaOk, pskOk bool) *cipherSuite { +func (c *Conn) tryCipherSuite(id uint16, supportedCipherSuites []uint16, version uint16, ellipticOk, ecdsaOk bool) *cipherSuite { for _, supported := range supportedCipherSuites { if id == supported { var candidate *cipherSuite @@ -1740,10 +1761,14 @@ if candidate == nil { continue } + // Don't select a ciphersuite which we can't // support for this client. - if (candidate.flags&suitePSK != 0) && !pskOk { - continue + if version >= VersionTLS13 || candidate.flags&suiteTLS13 != 0 { + if version < VersionTLS13 || candidate.flags&suiteTLS13 == 0 { + continue + } + return candidate } if (candidate.flags&suiteECDHE != 0) && !ellipticOk { continue @@ -1754,9 +1779,6 @@ if version < VersionTLS12 && candidate.flags&suiteTLS12 != 0 { continue } - if version >= VersionTLS13 && candidate.flags&suiteTLS13 == 0 { - continue - } if c.isDTLS && candidate.flags&suiteNoDTLS != 0 { continue }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index f536ceb..e164843 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -789,6 +789,14 @@ } func runTest(test *testCase, shimPath string, mallocNumToFail int64) error { + // Help debugging panics on the Go side. + defer func() { + if r := recover(); r != nil { + fmt.Fprintf(os.Stderr, "Test '%s' panicked.\n", test.name) + panic(r) + } + }() + if !test.shouldFail && (len(test.expectedError) > 0 || len(test.expectedLocalError) > 0) { panic("Error expected without shouldFail in " + test.name) } @@ -1059,8 +1067,9 @@ {"ECDHE-PSK-AES128-CBC-SHA", TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA}, {"ECDHE-PSK-AES256-CBC-SHA", TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA}, {"ECDHE-PSK-CHACHA20-POLY1305", TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256}, - {"ECDHE-PSK-AES128-GCM-SHA256", TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256}, - {"ECDHE-PSK-AES256-GCM-SHA384", TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384}, + {"AEAD-CHACHA20-POLY1305", TLS_CHACHA20_POLY1305_SHA256}, + {"AEAD-AES128-GCM-SHA256", TLS_AES_128_GCM_SHA256}, + {"AEAD-AES256-GCM-SHA384", TLS_AES_256_GCM_SHA384}, {"NULL-SHA", TLS_RSA_WITH_NULL_SHA}, } @@ -1076,24 +1085,7 @@ } func isTLS13Suite(suiteName string) bool { - // Only AEADs. - if !hasComponent(suiteName, "GCM") && !hasComponent(suiteName, "POLY1305") { - return false - } - // No old CHACHA20_POLY1305. - if hasComponent(suiteName, "CHACHA20-POLY1305-OLD") { - return false - } - // Must have ECDHE. - // TODO(davidben,svaldez): Add pure PSK support. - if !hasComponent(suiteName, "ECDHE") { - return false - } - // TODO(davidben,svaldez): Add PSK support. - if hasComponent(suiteName, "PSK") { - return false - } - return true + return strings.HasPrefix(suiteName, "AEAD-") } func isDTLSCipher(suiteName string) bool { @@ -1418,19 +1410,6 @@ expectedError: ":WRONG_CERTIFICATE_TYPE:", }, { - name: "CertMismatchRSA-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, - Certificates: []Certificate{ecdsaP256Certificate}, - Bugs: ProtocolBugs{ - SendCipherSuite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - }, - }, - shouldFail: true, - expectedError: ":WRONG_CERTIFICATE_TYPE:", - }, - { name: "CertMismatchECDSA", config: Config{ MaxVersion: VersionTLS12, @@ -1444,23 +1423,9 @@ expectedError: ":WRONG_CERTIFICATE_TYPE:", }, { - name: "CertMismatchECDSA-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, - Certificates: []Certificate{rsaCertificate}, - Bugs: ProtocolBugs{ - SendCipherSuite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - }, - }, - shouldFail: true, - expectedError: ":WRONG_CERTIFICATE_TYPE:", - }, - { name: "EmptyCertificateList", config: Config{ - MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ EmptyCertificateList: true, }, @@ -1471,8 +1436,7 @@ { name: "EmptyCertificateList-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ EmptyCertificateList: true, }, @@ -2427,6 +2391,10 @@ shouldClientFail = true shouldServerFail = true } + if isTLS13Suite(suite.name) && ver.version < VersionTLS13 { + shouldClientFail = true + shouldServerFail = true + } if !isDTLSCipher(suite.name) && protocol == dtls { shouldClientFail = true shouldServerFail = true @@ -2622,18 +2590,21 @@ testType: serverTest, name: "UnknownCipher", config: Config{ + MaxVersion: VersionTLS12, CipherSuites: []uint16{bogusCipher, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, Bugs: ProtocolBugs{ AdvertiseAllConfiguredCiphers: true, }, }, }) + + // The server must be tolerant to bogus ciphers. testCases = append(testCases, testCase{ testType: serverTest, name: "UnknownCipher-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{bogusCipher, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + CipherSuites: []uint16{bogusCipher, TLS_AES_128_GCM_SHA256}, Bugs: ProtocolBugs{ AdvertiseAllConfiguredCiphers: true, }, @@ -2761,6 +2732,7 @@ testCases = append(testCases, testCase{ name: fmt.Sprintf("BadECDSA-%d-%d", badR, badS), config: Config{ + MaxVersion: VersionTLS12, CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, Certificates: []Certificate{ecdsaP256Certificate}, Bugs: ProtocolBugs{ @@ -2771,6 +2743,19 @@ shouldFail: true, expectedError: ":BAD_SIGNATURE:", }) + testCases = append(testCases, testCase{ + name: fmt.Sprintf("BadECDSA-%d-%d-TLS13", badR, badS), + config: Config{ + MaxVersion: VersionTLS13, + Certificates: []Certificate{ecdsaP256Certificate}, + Bugs: ProtocolBugs{ + BadECDSAR: badR, + BadECDSAS: badS, + }, + }, + shouldFail: true, + expectedError: ":BAD_SIGNATURE:", + }) } } } @@ -5271,15 +5256,12 @@ func addResumptionVersionTests() { for _, sessionVers := range tlsVersions { for _, resumeVers := range tlsVersions { - cipher := TLS_RSA_WITH_AES_128_CBC_SHA - if sessionVers.version >= VersionTLS13 || resumeVers.version >= VersionTLS13 { - // TLS 1.3 only shares ciphers with TLS 1.2, so - // we skip certain combinations and use a - // different cipher to test with. - cipher = TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 - if sessionVers.version < VersionTLS12 || resumeVers.version < VersionTLS12 { - continue - } + // SSL 3.0 does not have tickets and TLS 1.3 does not + // have session IDs, so skip their cross-resumption + // tests. + if (sessionVers.version >= VersionTLS13 && resumeVers.version == VersionSSL30) || + (resumeVers.version >= VersionTLS13 && sessionVers.version == VersionSSL30) { + continue } protocols := []protocol{tls} @@ -5298,8 +5280,7 @@ name: "Resume-Client" + suffix, resumeSession: true, config: Config{ - MaxVersion: sessionVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: sessionVers.version, Bugs: ProtocolBugs{ ExpectNoTLS12Session: sessionVers.version >= VersionTLS13, ExpectNoTLS13PSK: sessionVers.version < VersionTLS13, @@ -5324,13 +5305,11 @@ name: "Resume-Client-Mismatch" + suffix, resumeSession: true, config: Config{ - MaxVersion: sessionVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: sessionVers.version, }, expectedVersion: sessionVers.version, resumeConfig: &Config{ - MaxVersion: resumeVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: resumeVers.version, Bugs: ProtocolBugs{ AcceptAnySession: true, }, @@ -5346,13 +5325,11 @@ name: "Resume-Client-NoResume" + suffix, resumeSession: true, config: Config{ - MaxVersion: sessionVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: sessionVers.version, }, expectedVersion: sessionVers.version, resumeConfig: &Config{ - MaxVersion: resumeVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: resumeVers.version, }, newSessionsOnResume: true, expectResumeRejected: true, @@ -5365,14 +5342,12 @@ name: "Resume-Server" + suffix, resumeSession: true, config: Config{ - MaxVersion: sessionVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: sessionVers.version, }, expectedVersion: sessionVers.version, expectResumeRejected: sessionVers.version != resumeVers.version, resumeConfig: &Config{ - MaxVersion: resumeVers.version, - CipherSuites: []uint16{cipher}, + MaxVersion: resumeVers.version, Bugs: ProtocolBugs{ SendBothTickets: true, }, @@ -5406,13 +5381,13 @@ resumeSession: true, config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + CipherSuites: []uint16{TLS_AES_128_GCM_SHA256}, }, resumeConfig: &Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + CipherSuites: []uint16{TLS_AES_128_GCM_SHA256}, Bugs: ProtocolBugs{ - SendCipherSuite: TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA, + SendCipherSuite: TLS_AES_256_GCM_SHA384, }, }, shouldFail: true, @@ -5841,6 +5816,7 @@ // Not all ciphers involve a signature. Advertise a list which gives all // versions a signing cipher. signingCiphers := []uint16{ + TLS_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, @@ -6113,8 +6089,7 @@ testType: serverTest, name: "ServerAuth-SignatureType-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, VerifySignatureAlgorithms: []signatureAlgorithm{ signatureECDSAWithP521AndSHA512, signatureRSAPKCS1WithSHA384, @@ -6185,8 +6160,7 @@ testCases = append(testCases, testCase{ name: "Verify-ServerAuth-SignatureType-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, SignSignatureAlgorithms: []signatureAlgorithm{ signatureRSAPSSWithSHA256, }, @@ -6299,8 +6273,7 @@ testType: serverTest, name: "ServerAuth-NoFallback-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, VerifySignatureAlgorithms: []signatureAlgorithm{ signatureRSAPKCS1WithSHA1, }, @@ -6369,8 +6342,7 @@ testCases = append(testCases, testCase{ name: "ServerAuth-Enforced-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, SignSignatureAlgorithms: []signatureAlgorithm{ signatureRSAPKCS1WithMD5, }, @@ -6532,7 +6504,6 @@ name: "CheckLeafCurve-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, Certificates: []Certificate{ecdsaP256Certificate}, }, flags: []string{"-p384-only"}, @@ -6556,7 +6527,6 @@ name: "ECDSACurveMismatch-Verify-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, Certificates: []Certificate{ecdsaP256Certificate}, SignSignatureAlgorithms: []signatureAlgorithm{ signatureECDSAWithP384AndSHA384, @@ -6575,8 +6545,7 @@ testType: serverTest, name: "ECDSACurveMismatch-Sign-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, VerifySignatureAlgorithms: []signatureAlgorithm{ signatureECDSAWithP384AndSHA384, signatureECDSAWithP256AndSHA256, @@ -7207,7 +7176,6 @@ name: "CurveTest-Client-" + curve.name + "-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, CurvePreferences: []CurveID{curve.id}, }, flags: []string{ @@ -7235,7 +7203,6 @@ name: "CurveTest-Server-" + curve.name + "-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, CurvePreferences: []CurveID{curve.id}, }, flags: []string{ @@ -7251,11 +7218,22 @@ testType: serverTest, name: "UnknownCurve", config: Config{ + MaxVersion: VersionTLS12, CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, CurvePreferences: []CurveID{bogusCurve, CurveP256}, }, }) + // The server must be tolerant to bogus curves. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "UnknownCurve-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + CurvePreferences: []CurveID{bogusCurve, CurveP256}, + }, + }) + // The server must not consider ECDHE ciphers when there are no // supported curves. testCases = append(testCases, testCase{ @@ -7275,14 +7253,13 @@ testType: serverTest, name: "NoSupportedCurves-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ NoSupportedCurves: true, }, }, shouldFail: true, - expectedError: ":NO_SHARED_CIPHER:", + expectedError: ":NO_SHARED_GROUP:", }) // The server must fall back to another cipher when there are no @@ -7317,8 +7294,7 @@ testCases = append(testCases, testCase{ name: "BadECDHECurve-TLS13", config: Config{ - MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ SendCurve: bogusCurve, }, @@ -7347,8 +7323,7 @@ // HelloRetryRequest requests an unsupported curve. name: "UnsupportedCurve-ServerHello-TLS13", config: Config{ - MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, + MaxVersion: VersionTLS13, CurvePreferences: []CurveID{CurveP384}, Bugs: ProtocolBugs{ SendCurve: CurveP256, @@ -7377,7 +7352,6 @@ name: "InvalidECDHPoint-Client-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, CurvePreferences: []CurveID{CurveP256}, Bugs: ProtocolBugs{ InvalidECDHPoint: true, @@ -7405,7 +7379,6 @@ name: "InvalidECDHPoint-Server-TLS13", config: Config{ MaxVersion: VersionTLS13, - CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, CurvePreferences: []CurveID{CurveP256}, Bugs: ProtocolBugs{ InvalidECDHPoint: true, @@ -8300,6 +8273,47 @@ func addTLS13HandshakeTests() { testCases = append(testCases, testCase{ testType: clientTest, + name: "NegotiatePSKResumption-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + NegotiatePSKResumption: true, + }, + }, + resumeSession: true, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + testCases = append(testCases, testCase{ + testType: clientTest, + name: "OmitServerHelloSignatureAlgorithms", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + OmitServerHelloSignatureAlgorithms: true, + }, + }, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + testCases = append(testCases, testCase{ + testType: clientTest, + name: "IncludeServerHelloSignatureAlgorithms", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + IncludeServerHelloSignatureAlgorithms: true, + }, + }, + resumeSession: true, + shouldFail: true, + expectedError: ":UNEXPECTED_EXTENSION:", + }) + + testCases = append(testCases, testCase{ + testType: clientTest, name: "MissingKeyShare-Client", config: Config{ MaxVersion: VersionTLS13, @@ -8308,7 +8322,7 @@ }, }, shouldFail: true, - expectedError: ":MISSING_KEY_SHARE:", + expectedError: ":UNEXPECTED_EXTENSION:", }) testCases = append(testCases, testCase{
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index aa80b69..8f1b516 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c
@@ -145,9 +145,30 @@ return ssl_hs_error; } + assert(ssl->s3->have_version); + memcpy(ssl->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE); + + const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite); + if (cipher == NULL) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CIPHER_RETURNED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return ssl_hs_error; + } + + /* Check if the cipher is disabled. */ + if ((cipher->algorithm_mkey & ssl->cert->mask_k) || + (cipher->algorithm_auth & ssl->cert->mask_a) || + SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) || + SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl) || + !sk_SSL_CIPHER_find(ssl_get_ciphers_by_id(ssl), NULL, cipher)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return ssl_hs_error; + } + /* Parse out the extensions. */ - int have_key_share = 0, have_pre_shared_key = 0; - CBS key_share, pre_shared_key; + int have_key_share = 0, have_pre_shared_key = 0, have_sigalgs = 0; + CBS key_share, pre_shared_key, sigalgs; while (CBS_len(&extensions) != 0) { uint16_t type; CBS extension; @@ -177,6 +198,15 @@ pre_shared_key = extension; have_pre_shared_key = 1; break; + case TLSEXT_TYPE_signature_algorithms: + if (have_sigalgs) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + return ssl_hs_error; + } + sigalgs = extension; + have_sigalgs = 1; + break; default: OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); @@ -184,8 +214,12 @@ } } - assert(ssl->s3->have_version); - memcpy(ssl->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE); + /* We only support PSK_AUTH and PSK_DHE_KE. */ + if (!have_key_share || have_sigalgs == have_pre_shared_key) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return ssl_hs_error; + } uint8_t alert = SSL_AD_DECODE_ERROR; if (have_pre_shared_key) { @@ -207,6 +241,12 @@ return ssl_hs_error; } + if (ssl->session->cipher != cipher) { + OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return ssl_hs_error; + } + if (!ssl_session_is_context_valid(ssl, ssl->session)) { /* This is actually a client application bug. */ OPENSSL_PUT_ERROR(SSL, @@ -224,109 +264,56 @@ return ssl_hs_error; } ssl_set_session(ssl, NULL); - } else { - if (!ssl_get_new_session(ssl, 0)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } - } - - const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite); - if (cipher == NULL) { - OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CIPHER_RETURNED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + } else if (!ssl_get_new_session(ssl, 0)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; } - if (!ssl->s3->session_reused) { - /* Check if the cipher is disabled. */ - if ((cipher->algorithm_mkey & ssl->cert->mask_k) || - (cipher->algorithm_auth & ssl->cert->mask_a) || - SSL_CIPHER_get_min_version(cipher) > ssl3_protocol_version(ssl) || - SSL_CIPHER_get_max_version(cipher) < ssl3_protocol_version(ssl) || - !sk_SSL_CIPHER_find(ssl_get_ciphers_by_id(ssl), NULL, cipher)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return ssl_hs_error; - } - } else { - uint16_t resumption_cipher; - if (!ssl_cipher_get_ecdhe_psk_cipher(ssl->s3->new_session->cipher, - &resumption_cipher) || - resumption_cipher != ssl_cipher_get_value(cipher)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return ssl_hs_error; - } - } - ssl->s3->new_session->cipher = cipher; ssl->s3->tmp.new_cipher = cipher; /* The PRF hash is now known. Set up the key schedule. */ - static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0}; - size_t resumption_ctx_len = + size_t hash_len = EVP_MD_size(ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl))); + + /* Derive resumption material. */ + uint8_t resumption_ctx[EVP_MAX_MD_SIZE] = {0}; + uint8_t psk_secret[EVP_MAX_MD_SIZE] = {0}; if (ssl->s3->session_reused) { - uint8_t resumption_ctx[EVP_MAX_MD_SIZE]; - if (!tls13_resumption_context(ssl, resumption_ctx, resumption_ctx_len, + if (!tls13_resumption_context(ssl, resumption_ctx, hash_len, ssl->s3->new_session) || - !tls13_init_key_schedule(ssl, resumption_ctx, resumption_ctx_len)) { + !tls13_resumption_psk(ssl, psk_secret, hash_len, + ssl->s3->new_session)) { return ssl_hs_error; } - } else if (!tls13_init_key_schedule(ssl, kZeroes, resumption_ctx_len)) { - return ssl_hs_error; } - /* Resolve PSK and incorporate it into the secret. */ - if (cipher->algorithm_auth == SSL_aPSK) { - if (!ssl->s3->session_reused) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return ssl_hs_error; - } - - uint8_t resumption_psk[EVP_MAX_MD_SIZE]; - if (!tls13_resumption_psk(ssl, resumption_psk, hs->hash_len, - ssl->s3->new_session) || - !tls13_advance_key_schedule(ssl, resumption_psk, hs->hash_len)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); - return ssl_hs_error; - } - } else if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len)) { + /* Set up the key schedule, hash in the ClientHello, and incorporate the PSK + * into the running secret. */ + if (!tls13_init_key_schedule(ssl, resumption_ctx, hash_len) || + !tls13_advance_key_schedule(ssl, psk_secret, hash_len)) { return ssl_hs_error; } /* Resolve ECDHE and incorporate it into the secret. */ - if (cipher->algorithm_mkey == SSL_kECDHE) { - if (!have_key_share) { - OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_KEY_SHARE); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION); - return ssl_hs_error; - } + uint8_t *dhe_secret; + size_t dhe_secret_len; + if (!ssl_ext_key_share_parse_serverhello(ssl, &dhe_secret, &dhe_secret_len, + &alert, &key_share)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); + return ssl_hs_error; + } - uint8_t *dhe_secret; - size_t dhe_secret_len; - if (!ssl_ext_key_share_parse_serverhello(ssl, &dhe_secret, &dhe_secret_len, - &alert, &key_share)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); - return ssl_hs_error; - } - - int ok = tls13_advance_key_schedule(ssl, dhe_secret, dhe_secret_len); + if (!tls13_advance_key_schedule(ssl, dhe_secret, dhe_secret_len)) { OPENSSL_free(dhe_secret); - if (!ok) { - return ssl_hs_error; - } - } else { - if (have_key_share) { - OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION); - return ssl_hs_error; - } - if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len)) { - return ssl_hs_error; - } + return ssl_hs_error; + } + OPENSSL_free(dhe_secret); + + if (have_sigalgs && + CBS_len(&sigalgs) != 0) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + return ssl_hs_error; } /* If there was no HelloRetryRequest, the version negotiation logic has @@ -374,8 +361,8 @@ SSL_HANDSHAKE *hs) { ssl->s3->tmp.cert_request = 0; - /* CertificateRequest may only be sent in certificate-based ciphers. */ - if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { + /* CertificateRequest may only be sent in non-resumption handshakes. */ + if (ssl->s3->session_reused) { hs->state = state_process_server_finished; return ssl_hs_ok; } @@ -436,15 +423,6 @@ return ssl_hs_error; } - /* Check the certificate matches the cipher suite. - * - * TODO(davidben): Remove this check when switching to the new TLS 1.3 cipher - * suite negotiation. */ - if (!ssl_check_leaf_certificate(ssl, ssl->s3->new_session->peer)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return ssl_hs_error; - } - hs->state = state_process_server_certificate_verify; return ssl_hs_read_message; } @@ -464,7 +442,6 @@ static enum ssl_hs_wait_t do_process_server_finished(SSL *ssl, SSL_HANDSHAKE *hs) { static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0}; - if (!tls13_check_message_type(ssl, SSL3_MT_FINISHED) || !tls13_process_finished(ssl) || !ssl->method->hash_current_message(ssl) ||
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index 9e146e6..984cc5c 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c
@@ -51,32 +51,11 @@ static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0}; -static int resolve_psk_secret(SSL *ssl) { - SSL_HANDSHAKE *hs = ssl->s3->hs; - - if (ssl->s3->tmp.new_cipher->algorithm_auth != SSL_aPSK) { - return tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len); - } - - uint8_t resumption_psk[EVP_MAX_MD_SIZE]; - if (!tls13_resumption_psk(ssl, resumption_psk, hs->hash_len, - ssl->s3->new_session) || - !tls13_advance_key_schedule(ssl, resumption_psk, hs->hash_len)) { - return 0; - } - - return 1; -} - static int resolve_ecdhe_secret(SSL *ssl, int *out_need_retry, struct ssl_early_callback_ctx *early_ctx) { *out_need_retry = 0; - SSL_HANDSHAKE *hs = ssl->s3->hs; - if (ssl->s3->tmp.new_cipher->algorithm_mkey != SSL_kECDHE) { - return tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len); - } - + /* We only support connections that include an ECDHE key exchange. */ CBS key_share; if (!ssl_early_callback_get_extension(early_ctx, &key_share, TLSEXT_TYPE_key_share)) { @@ -139,13 +118,11 @@ return 0; } - uint16_t resumption_cipher; if (session != NULL && /* Only resume if the session's version matches. */ (session->ssl_version != ssl->version || - !ssl_cipher_get_ecdhe_psk_cipher(session->cipher, &resumption_cipher) || - !ssl_client_cipher_list_contains_cipher(&client_hello, - resumption_cipher))) { + !ssl_client_cipher_list_contains_cipher( + &client_hello, (uint16_t)SSL_CIPHER_get_id(session->cipher)))) { SSL_SESSION_free(session); session = NULL; } @@ -227,39 +204,32 @@ } ssl->s3->new_session->cipher = cipher; - ssl->s3->tmp.new_cipher = cipher; - } else { - uint16_t resumption_cipher; - if (!ssl_cipher_get_ecdhe_psk_cipher(ssl->s3->new_session->cipher, - &resumption_cipher)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); - return ssl_hs_error; - } - ssl->s3->tmp.new_cipher = SSL_get_cipher_by_value(resumption_cipher); } + ssl->s3->tmp.new_cipher = ssl->s3->new_session->cipher; ssl->method->received_flight(ssl); - /* The PRF hash is now known. Set up the key schedule and hash the - * ClientHello. */ - size_t resumption_ctx_len = + + /* The PRF hash is now known. */ + size_t hash_len = EVP_MD_size(ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl))); + + /* Derive resumption material. */ + uint8_t resumption_ctx[EVP_MAX_MD_SIZE] = {0}; + uint8_t psk_secret[EVP_MAX_MD_SIZE] = {0}; if (ssl->s3->session_reused) { - uint8_t resumption_ctx[EVP_MAX_MD_SIZE]; - if (!tls13_resumption_context(ssl, resumption_ctx, resumption_ctx_len, + if (!tls13_resumption_context(ssl, resumption_ctx, hash_len, ssl->s3->new_session) || - !tls13_init_key_schedule(ssl, resumption_ctx, resumption_ctx_len)) { - return ssl_hs_error; - } - } else { - if (!tls13_init_key_schedule(ssl, kZeroes, resumption_ctx_len)) { + !tls13_resumption_psk(ssl, psk_secret, hash_len, + ssl->s3->new_session)) { return ssl_hs_error; } } - /* Resolve PSK and incorporate it into the secret. */ - if (!resolve_psk_secret(ssl)) { + /* Set up the key schedule, hash in the ClientHello, and incorporate the PSK + * into the running secret. */ + if (!tls13_init_key_schedule(ssl, resumption_ctx, hash_len) || + !tls13_advance_key_schedule(ssl, psk_secret, hash_len)) { return ssl_hs_error; } @@ -345,14 +315,27 @@ !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) || !CBB_add_u16_length_prefixed(&body, &extensions) || !ssl_ext_pre_shared_key_add_serverhello(ssl, &extensions) || - !ssl_ext_key_share_add_serverhello(ssl, &extensions) || - !ssl->method->finish_message(ssl, &cbb)) { - CBB_cleanup(&cbb); - return ssl_hs_error; + !ssl_ext_key_share_add_serverhello(ssl, &extensions)) { + goto err; + } + + if (!ssl->s3->session_reused) { + if (!CBB_add_u16(&extensions, TLSEXT_TYPE_signature_algorithms) || + !CBB_add_u16(&extensions, 0)) { + goto err; + } + } + + if (!ssl->method->finish_message(ssl, &cbb)) { + goto err; } hs->state = state_send_encrypted_extensions; return ssl_hs_write_message; + +err: + CBB_cleanup(&cbb); + return ssl_hs_error; } static enum ssl_hs_wait_t do_send_encrypted_extensions(SSL *ssl, @@ -378,8 +361,8 @@ SSL_HANDSHAKE *hs) { /* Determine whether to request a client certificate. */ ssl->s3->tmp.cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER); - /* CertificateRequest may only be sent in certificate-based ciphers. */ - if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { + /* CertificateRequest may only be sent in non-resumption handshakes. */ + if (ssl->s3->session_reused) { ssl->s3->tmp.cert_request = 0; } @@ -424,7 +407,7 @@ static enum ssl_hs_wait_t do_send_server_certificate(SSL *ssl, SSL_HANDSHAKE *hs) { - if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) { + if (ssl->s3->session_reused) { hs->state = state_send_server_finished; return ssl_hs_ok; }