From 2e961c249c975806e857218bf75a6e1bbce7811a Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Tue, 26 Mar 2024 23:19:13 +0000 Subject: [PATCH 1/3] allow empty lists in SSL_CTX_set_ciphersuites --- ssl/ssl_cipher.cc | 8 +++++--- ssl/ssl_test.cc | 17 +++++++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc index b8bc9ee0b7..dcc99a6efa 100644 --- a/ssl/ssl_cipher.cc +++ b/ssl/ssl_cipher.cc @@ -1336,9 +1336,11 @@ bool ssl_create_cipher_list(UniquePtr *out_cipher_list, *out_cipher_list = std::move(pref_list); - // Configuring an empty cipher list is an error but still updates the - // output. - if (sk_SSL_CIPHER_num((*out_cipher_list)->ciphers.get()) == 0) { + // Configuring an empty cipher list is an error when |strict| is true, but + // still updates the output. When otherwise, OpenSSL explicitly allows an + // empty list. + if ((strict || (*rule_str != '\0')) && + sk_SSL_CIPHER_num((*out_cipher_list)->ciphers.get()) == 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH); return false; } diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 0e78d03578..b24a99d4d0 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -548,8 +548,6 @@ static const char *kBadRules[] = { "[+RSA]", // Unknown directive. "@BOGUS", - // Empty cipher lists error at SSL_CTX_set_cipher_list. - "", "BOGUS", // COMPLEMENTOFDEFAULT is empty. "COMPLEMENTOFDEFAULT", @@ -5757,12 +5755,19 @@ TEST(SSLTest, EmptyCipherList) { // Initially, the cipher list is not empty. EXPECT_NE(0u, sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get()))); - // Configuring the empty cipher list fails. - EXPECT_FALSE(SSL_CTX_set_cipher_list(ctx.get(), "")); + // Configuring the empty cipher list with |SSL_CTX_set_cipher_list| + // succeeds. + EXPECT_TRUE(SSL_CTX_set_cipher_list(ctx.get(), "")); + + // The cipher list is updated to empty. + EXPECT_EQ(0u, sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get()))); + + // Configuring the empty cipher list with |SSL_CTX_set_strict_cipher_list| + // fails. + EXPECT_FALSE(SSL_CTX_set_strict_cipher_list(ctx.get(), "")); ERR_clear_error(); - // Configuring the empty cipher list fails. - EXPECT_FALSE(SSL_CTX_set_ciphersuites(ctx.get(), "")); + EXPECT_FALSE(SSL_CTX_set_strict_cipher_list(ctx.get(), "")); ERR_clear_error(); // But the cipher list is still updated to empty. From a062de46e2490c6985607c7640c06892cda0e72b Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Wed, 27 Mar 2024 19:22:27 +0000 Subject: [PATCH 2/3] forgot about SSL_CTX_set_ciphersuites --- ssl/ssl_test.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index b24a99d4d0..94ef453494 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -5758,18 +5758,19 @@ TEST(SSLTest, EmptyCipherList) { // Configuring the empty cipher list with |SSL_CTX_set_cipher_list| // succeeds. EXPECT_TRUE(SSL_CTX_set_cipher_list(ctx.get(), "")); - // The cipher list is updated to empty. EXPECT_EQ(0u, sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get()))); + // Configuring the empty cipher list with |SSL_CTX_set_ciphersuites| + // also succeeds. + EXPECT_TRUE(SSL_CTX_set_ciphersuites(ctx.get(), "")); + EXPECT_EQ(0u, sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get()))); + // Configuring the empty cipher list with |SSL_CTX_set_strict_cipher_list| // fails. EXPECT_FALSE(SSL_CTX_set_strict_cipher_list(ctx.get(), "")); ERR_clear_error(); - EXPECT_FALSE(SSL_CTX_set_strict_cipher_list(ctx.get(), "")); - ERR_clear_error(); - // But the cipher list is still updated to empty. EXPECT_EQ(0u, sk_SSL_CIPHER_num(SSL_CTX_get_ciphers(ctx.get()))); } From fae78ed3be1b1e7666305f5c7a136678011cb76d Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Thu, 28 Mar 2024 18:54:43 +0000 Subject: [PATCH 3/3] add documentation to functions --- include/openssl/ssl.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index bbd02d1549..099f976fef 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1684,7 +1684,10 @@ OPENSSL_EXPORT int SSL_CTX_set_strict_cipher_list(SSL_CTX *ctx, // |str| as a cipher string. It returns one on success and zero on failure. // // Prefer to use |SSL_CTX_set_strict_cipher_list|. This function tolerates -// garbage inputs, unless an empty cipher list results. +// garbage inputs, unless an empty cipher list results. However, an empty +// string which also results in an empty cipher list, is allowed. This +// behavior is strongly advised against and only meant for OpenSSL +// compatibility. // // Note: this API only sets the TLSv1.2 and below ciphers. // Use |SSL_CTX_set_ciphersuites| to configure TLS 1.3 specific ciphers. @@ -1704,7 +1707,9 @@ OPENSSL_EXPORT int SSL_CTX_set_ciphersuites(SSL_CTX *ctx, const char *str); // a cipher string. It returns one on success and zero on failure. // // Prefer to use |SSL_set_strict_cipher_list|. This function tolerates garbage -// inputs, unless an empty cipher list results. +// inputs, unless an empty cipher list results. However, an empty string which +// also results in an empty cipher list, is allowed. This behavior is strongly +// advised against and only meant for OpenSSL compatibility. OPENSSL_EXPORT int SSL_set_cipher_list(SSL *ssl, const char *str); // SSL_CTX_get_ciphers returns the cipher list for |ctx|, in order of