commit 718900aeb84c601523e71abbd18fd70c9e2ad884
author David Benjamin <davidben@google.com> Fri Sep 20 13:49:42 2024 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 20 22:24:58 2024 +0000
tree b20bd202749dc7f6444252e9bc8fce001eebc2ea
parent 0d9bb204ab04fd1e3eee9b3926c7449505ec6159

Test SSL_get_ivs across all versions and TLS/DTLS

I briefly thought we would need to look at this for DTLS, but DTLS
doesn't have the TLS 1.0 implicit IV bug. Add some basic tests to ensure
it doesn't crash as we rework the record layer.

Also add a missing error code in the implementation.

Bug: 42290594
Change-Id: I16e4dfb85aa1b84c38fd2433e01499347b8bf8d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71487
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/cipher_extra/e_tls.c

diff --git a/crypto/cipher_extra/e_tls.c b/crypto/cipher_extra/e_tls.c
index c462a2b..d796541 100644
--- a/crypto/cipher_extra/e_tls.c
+++ b/crypto/cipher_extra/e_tls.c
@@ -442,6 +442,7 @@
   const AEAD_TLS_CTX *tls_ctx = (AEAD_TLS_CTX *)&ctx->state;
   const size_t iv_len = EVP_CIPHER_CTX_iv_length(&tls_ctx->cipher_ctx);
   if (iv_len <= 1) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
     return 0;
   }
 

crypto/fipsmodule/cipher/aead.c.inc

diff --git a/crypto/fipsmodule/cipher/aead.c.inc b/crypto/fipsmodule/cipher/aead.c.inc
index 97f0b0d..4a578d3 100644
--- a/crypto/fipsmodule/cipher/aead.c.inc
+++ b/crypto/fipsmodule/cipher/aead.c.inc
@@ -262,6 +262,7 @@
 int EVP_AEAD_CTX_get_iv(const EVP_AEAD_CTX *ctx, const uint8_t **out_iv,
                         size_t *out_len) {
   if (ctx->aead->get_iv == NULL) {
+    OPENSSL_PUT_ERROR(CIPHER, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
     return 0;
   }
 

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 393b48e..4f4f978 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -9693,5 +9693,49 @@
   EXPECT_EQ(client_log, server_log);
 }
 
+TEST_P(SSLVersionTest, GetIVs) {
+  std::vector<const char *> ciphers;
+  if (version() == TLS1_2_VERSION || version() == DTLS1_2_VERSION) {
+    // Try both CBC and AEAD ciphers.
+    ciphers = {"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+               "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"};
+  } else {
+    // The defaults are fine to test. In 1.0 and 1.1, all remaining supported
+    // ciphers are CBC. In 1.3, all ciphers are AEADs.
+    ciphers = {"ALL"};
+  }
+
+  for (const char *cipher : ciphers) {
+    SCOPED_TRACE(cipher);
+
+    ASSERT_NO_FATAL_FAILURE(ResetContexts());
+    ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(client_ctx_.get(), cipher));
+    ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(server_ctx_.get(), cipher));
+    ASSERT_TRUE(Connect());
+
+    const uint8_t *client_read_iv, *client_write_iv, *server_read_iv,
+        *server_write_iv;
+    size_t client_iv_len, server_iv_len;
+    bool client_ivs_ok = SSL_get_ivs(client_.get(), &client_read_iv,
+                                     &client_write_iv, &client_iv_len);
+    bool server_ivs_ok = SSL_get_ivs(server_.get(), &server_read_iv,
+                                     &server_write_iv, &server_iv_len);
+
+    // Only TLS 1.0 should support |SSL_get_ivs|. Other cases should cleanly
+    // fail this operation.
+    if (version() == TLS1_VERSION) {
+      ASSERT_TRUE(client_ivs_ok);
+      ASSERT_TRUE(server_ivs_ok);
+      EXPECT_EQ(Bytes(client_write_iv, client_iv_len),
+                Bytes(server_read_iv, server_iv_len));
+      EXPECT_EQ(Bytes(client_read_iv, client_iv_len),
+                Bytes(server_write_iv, server_iv_len));
+    } else {
+      EXPECT_FALSE(client_ivs_ok);
+      EXPECT_FALSE(server_ivs_ok);
+    }
+  }
+}
+
 }  // namespace
 BSSL_NAMESPACE_END