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) {
