Skip to content

Commit

Permalink
Test SSL_get_ivs across all versions and TLS/DTLS
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Sep 20, 2024
1 parent 0d9bb20 commit 718900a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
1 change: 1 addition & 0 deletions crypto/cipher_extra/e_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ static int aead_tls_get_iv(const EVP_AEAD_CTX *ctx, const uint8_t **out_iv,
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;
}

Expand Down
1 change: 1 addition & 0 deletions crypto/fipsmodule/cipher/aead.c.inc
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ const EVP_AEAD *EVP_AEAD_CTX_aead(const EVP_AEAD_CTX *ctx) { return ctx->aead; }
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;
}

Expand Down
44 changes: 44 additions & 0 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9693,5 +9693,49 @@ TEST_P(SSLVersionTest, KeyLog) {
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

0 comments on commit 718900a

Please sign in to comment.