Skip to content

Commit

Permalink
Modify SSL to inherit ciphersuites from SSL_CTX at initialization (#2198
Browse files Browse the repository at this point in the history
)

### Description of changes: 
This is a follow up to
154f998.
These changes initialize the SSL object with configured ciphersuites
from the parent SSL_CTX object at initialization. Now both SSL and
SSL_CTX objects will have their own copy of ciphersuites.

We inherited the behavior of defaulting to SSL_CTX ciphersuites when
none are set on SSL from BoringSSL. OpenSSL does things differently and
copies the ciphersuites to SSL at init. This behavioral difference is
aligned in this PR.

### Testing:
A test to ensure changes to SSL_CTX don't impact SSL configurations and
we correctly inherit ciphersuites. 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
smittals2 authored Feb 20, 2025
1 parent becf578 commit acdf53b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 0 deletions.
10 changes: 10 additions & 0 deletions ssl/ssl_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,15 @@ SSL *SSL_new(SSL_CTX *ctx) {
return nullptr;
}

if (ctx->cipher_list) {
ssl->config->cipher_list = MakeUnique<SSLCipherPreferenceList>();
ssl->config->cipher_list->Init(*ctx->cipher_list.get());
}
if (ctx->tls13_cipher_list) {
ssl->config->tls13_cipher_list = MakeUnique<SSLCipherPreferenceList>();
ssl->config->tls13_cipher_list->Init(*ctx->tls13_cipher_list.get());
}

if (ctx->psk_identity_hint) {
ssl->config->psk_identity_hint.reset(
OPENSSL_strdup(ctx->psk_identity_hint.get()));
Expand Down Expand Up @@ -2158,6 +2167,7 @@ STACK_OF(SSL_CIPHER) *SSL_get_ciphers(const SSL *ssl) {
if (ssl == NULL) {
return NULL;
}

if (ssl->config && ssl->config->cipher_list) {
return ssl->config->cipher_list->ciphers.get();
}
Expand Down
60 changes: 60 additions & 0 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2821,6 +2821,66 @@ TEST(SSLTest, FindingCipher) {
ASSERT_FALSE(cipher3);
}

TEST(SSLTest, CheckSSLCipherInheritance) {
// This configures SSL_CTX objects with default TLS 1.2 and 1.3 ciphersuites
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> server_ctx =
CreateContextWithTestCertificate(TLS_method());
ASSERT_TRUE(client_ctx);
ASSERT_TRUE(server_ctx);

ASSERT_TRUE(SSL_CTX_set_min_proto_version(client_ctx.get(), TLS1_2_VERSION));
ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), TLS1_3_VERSION));
ASSERT_TRUE(SSL_CTX_set_min_proto_version(server_ctx.get(), TLS1_2_VERSION));
ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), TLS1_3_VERSION));

ASSERT_TRUE(SSL_CTX_set_ciphersuites(client_ctx.get(), "TLS_AES_128_GCM_SHA256"));
ASSERT_TRUE(SSL_CTX_set_ciphersuites(server_ctx.get(), "TLS_AES_128_GCM_SHA256"));
ASSERT_TRUE(SSL_CTX_set_cipher_list(client_ctx.get(), "TLS_RSA_WITH_AES_128_CBC_SHA"));
ASSERT_TRUE(SSL_CTX_set_cipher_list(server_ctx.get(), "TLS_RSA_WITH_AES_128_CBC_SHA"));

bssl::UniquePtr<SSL> client, server;
ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), server_ctx.get()));

// Modify CTX ciphersuites
ASSERT_TRUE(SSL_CTX_set_ciphersuites(client_ctx.get(), "TLS_AES_256_GCM_SHA384"));
ASSERT_TRUE(SSL_CTX_set_ciphersuites(server_ctx.get(), "TLS_AES_256_GCM_SHA384"));
ASSERT_TRUE(SSL_CTX_set_cipher_list(client_ctx.get(), "TLS_RSA_WITH_AES_256_CBC_SHA"));
ASSERT_TRUE(SSL_CTX_set_cipher_list(server_ctx.get(), "TLS_RSA_WITH_AES_256_CBC_SHA"));

// Ensure SSL object inherits initial CTX cipher suites
STACK_OF(SSL_CIPHER) *client_ciphers = SSL_get_ciphers(client.get());
STACK_OF(SSL_CIPHER) *server_ciphers = SSL_get_ciphers(server.get());
ASSERT_TRUE(client_ciphers);
ASSERT_TRUE(server_ciphers);
ASSERT_EQ(sk_SSL_CIPHER_num(client_ciphers), 2u);
ASSERT_EQ(sk_SSL_CIPHER_num(server_ciphers), 2u);
const SSL_CIPHER *tls13_cipher = SSL_get_cipher_by_value(TLS1_3_CK_AES_128_GCM_SHA256 & 0xFFFF);
const SSL_CIPHER *tls12_cipher = SSL_get_cipher_by_value(TLS1_CK_RSA_WITH_AES_128_SHA & 0xFFFF);
ASSERT_TRUE(tls13_cipher);
ASSERT_TRUE(tls12_cipher);

ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(client_ciphers, NULL, tls12_cipher));
ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(client_ciphers, NULL, tls13_cipher));
ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(server_ciphers, NULL, tls12_cipher));
ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(server_ciphers, NULL, tls13_cipher));

SSL_set_shed_handshake_config(client.get(), 1);
SSL_set_shed_handshake_config(server.get(), 1);

ASSERT_TRUE(CompleteHandshakes(client.get(), server.get()));

// Ensure we fall back to ctx once config is shed
client_ciphers = SSL_get_ciphers(client.get());
server_ciphers = SSL_get_ciphers(server.get());

tls13_cipher = SSL_get_cipher_by_value(TLS1_3_CK_AES_256_GCM_SHA384 & 0xFFFF);
tls12_cipher = SSL_get_cipher_by_value(TLS1_CK_RSA_WITH_AES_256_SHA & 0xFFFF);

ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(client_ciphers, NULL, tls13_cipher));
ASSERT_TRUE(sk_SSL_CIPHER_find_awslc(server_ciphers, NULL, tls12_cipher));
}

TEST(SSLTest, SSLGetCiphersReturnsTLS13Default) {
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
bssl::UniquePtr<SSL_CTX> server_ctx =
Expand Down

0 comments on commit acdf53b

Please sign in to comment.