Use the same EVP_AEADs for TLS and DTLS The funny AEADs enforce that the outgoing sequence numbers monotonically increase, which is true for DTLS too. This is one less is_dtls boolean to plumb. When we first added this in https://boringssl-review.googlesource.com/c/boringssl/+/16625, it didn't work for DTLS because it checked the nonce started at zero, and our DTLS 1.2 nonce construction (matching what ultimately ended up in the DTLS 1.2 ChaCha20-Poly1305 ciphers) uses the combined epoch + seqnum. But then https://boringssl-review.googlesource.com/c/boringssl/+/25384 made the AEAD check for any monotonically increasing sequence, so it's perfectly DTLS-compatible. Change-Id: I8e2e21527db85c682f2f4bd590d174f9015fcd35 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71567 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h index 4bbbeb7..f956418 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -672,7 +672,7 @@ bool ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead, size_t *out_mac_secret_len, size_t *out_fixed_iv_len, const SSL_CIPHER *cipher, - uint16_t version, bool is_dtls); + uint16_t version); // ssl_get_handshake_digest returns the |EVP_MD| corresponding to |version| and // |cipher|. @@ -839,10 +839,9 @@ // Create creates an |SSLAEADContext| using the supplied key material. It // returns nullptr on error. Only one of |Open| or |Seal| may be used with the - // resulting object, depending on |direction|. |version| is the normalized - // protocol version, so DTLS 1.0 is represented as 0x0301, not 0xffef. + // resulting object, depending on |direction|. |version| is the wire version. static UniquePtr<SSLAEADContext> Create(enum evp_aead_direction_t direction, - uint16_t version, bool is_dtls, + uint16_t version, const SSL_CIPHER *cipher, Span<const uint8_t> enc_key, Span<const uint8_t> mac_key,
diff --git a/ssl/ssl_aead_ctx.cc b/ssl/ssl_aead_ctx.cc index 652a5e9..9a4da85 100644 --- a/ssl/ssl_aead_ctx.cc +++ b/ssl/ssl_aead_ctx.cc
@@ -51,7 +51,7 @@ } UniquePtr<SSLAEADContext> SSLAEADContext::Create( - enum evp_aead_direction_t direction, uint16_t version, bool is_dtls, + enum evp_aead_direction_t direction, uint16_t version, const SSL_CIPHER *cipher, Span<const uint8_t> enc_key, Span<const uint8_t> mac_key, Span<const uint8_t> fixed_iv) { const EVP_AEAD *aead; @@ -59,8 +59,8 @@ size_t expected_mac_key_len, expected_fixed_iv_len; if (!ssl_protocol_version_from_wire(&protocol_version, version) || !ssl_cipher_get_evp_aead(&aead, &expected_mac_key_len, - &expected_fixed_iv_len, cipher, protocol_version, - is_dtls) || + &expected_fixed_iv_len, cipher, + protocol_version) || // Ensure the caller returned correct key sizes. expected_fixed_iv_len != fixed_iv.size() || expected_mac_key_len != mac_key.size()) {
diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc index 29e32ce..97e69ff 100644 --- a/ssl/ssl_cipher.cc +++ b/ssl/ssl_cipher.cc
@@ -576,31 +576,24 @@ bool ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead, size_t *out_mac_secret_len, size_t *out_fixed_iv_len, const SSL_CIPHER *cipher, - uint16_t version, bool is_dtls) { + uint16_t version) { *out_aead = NULL; *out_mac_secret_len = 0; *out_fixed_iv_len = 0; - const bool is_tls12 = version == TLS1_2_VERSION && !is_dtls; - const bool is_tls13 = version == TLS1_3_VERSION && !is_dtls; - if (cipher->algorithm_mac == SSL_AEAD) { if (cipher->algorithm_enc == SSL_AES128GCM) { - if (is_tls12) { + if (version < TLS1_3_VERSION) { *out_aead = EVP_aead_aes_128_gcm_tls12(); - } else if (is_tls13) { - *out_aead = EVP_aead_aes_128_gcm_tls13(); } else { - *out_aead = EVP_aead_aes_128_gcm(); + *out_aead = EVP_aead_aes_128_gcm_tls13(); } *out_fixed_iv_len = 4; } else if (cipher->algorithm_enc == SSL_AES256GCM) { - if (is_tls12) { + if (version < TLS1_3_VERSION) { *out_aead = EVP_aead_aes_256_gcm_tls12(); - } else if (is_tls13) { - *out_aead = EVP_aead_aes_256_gcm_tls13(); } else { - *out_aead = EVP_aead_aes_256_gcm(); + *out_aead = EVP_aead_aes_256_gcm_tls13(); } *out_fixed_iv_len = 4; } else if (cipher->algorithm_enc == SSL_CHACHA20POLY1305) {
diff --git a/ssl/t1_enc.cc b/ssl/t1_enc.cc index 44075ab..1895bac 100644 --- a/ssl/t1_enc.cc +++ b/ssl/t1_enc.cc
@@ -169,7 +169,7 @@ const SSL_CIPHER *cipher) { const EVP_AEAD *aead = NULL; if (!ssl_cipher_get_evp_aead(&aead, out_mac_secret_len, out_iv_len, cipher, - ssl_protocol_version(ssl), SSL_is_dtls(ssl))) { + ssl_protocol_version(ssl))) { OPENSSL_PUT_ERROR(SSL, SSL_R_CIPHER_OR_HASH_UNAVAILABLE); return false; } @@ -243,9 +243,8 @@ iv = iv_override; } - UniquePtr<SSLAEADContext> aead_ctx = - SSLAEADContext::Create(direction, ssl->s3->version, SSL_is_dtls(ssl), - session->cipher, key, mac_secret, iv); + UniquePtr<SSLAEADContext> aead_ctx = SSLAEADContext::Create( + direction, ssl->s3->version, session->cipher, key, mac_secret, iv); if (!aead_ctx) { return false; }
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc index 19631f5..fad4117 100644 --- a/ssl/tls13_enc.cc +++ b/ssl/tls13_enc.cc
@@ -198,7 +198,7 @@ const EVP_AEAD *aead; size_t discard; if (!ssl_cipher_get_evp_aead(&aead, &discard, &discard, session->cipher, - version, is_dtls)) { + version)) { return false; } @@ -213,8 +213,8 @@ return false; } - traffic_aead = SSLAEADContext::Create( - direction, session->ssl_version, is_dtls, session->cipher, key, {}, iv); + traffic_aead = SSLAEADContext::Create(direction, session->ssl_version, + session->cipher, key, {}, iv); } if (!traffic_aead) {