From 70dbf042b61c68294474923accd49f1971032fab Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 8 Aug 2017 18:51:37 -0400 Subject: [PATCH] Add SSL_CTX_cipher_in_group. This allows us to fix another consumer that directly accesses SSL_CTX. I've made ssl_test use it for test coverage, though we're okay with ssl_test depending on ssl/internal.h. Bug: 6 Change-Id: I464325e3faa9f0194bbd357fbb31a996afc0c2e1 Reviewed-on: https://boringssl-review.googlesource.com/18964 Commit-Queue: David Benjamin Commit-Queue: Adam Langley Reviewed-by: Adam Langley --- include/openssl/ssl.h | 5 +++++ ssl/ssl_lib.cc | 9 ++++++++- ssl/ssl_test.cc | 33 +++++++++++++++++---------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 0eb2cc58..c989dd61 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1414,6 +1414,11 @@ OPENSSL_EXPORT int SSL_set_cipher_list(SSL *ssl, const char *str); * preference. */ OPENSSL_EXPORT STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx); +/* SSL_CTX_cipher_in_group returns one if the |i|th cipher (see + * |SSL_CTX_get_ciphers|) is in the same equipreference group as the one + * following it and zero otherwise. */ +OPENSSL_EXPORT int SSL_CTX_cipher_in_group(const SSL_CTX *ctx, size_t i); + /* SSL_get_ciphers returns the cipher list for |ssl|, in order of preference. */ OPENSSL_EXPORT STACK_OF(SSL_CIPHER) *SSL_get_ciphers(const SSL *ssl); diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 950bbf84..10128d82 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc @@ -1658,10 +1658,17 @@ int SSL_set_tmp_dh(SSL *ssl, const DH *dh) { return 1; } -OPENSSL_EXPORT STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx) { +STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx) { return ctx->cipher_list->ciphers; } +int SSL_CTX_cipher_in_group(const SSL_CTX *ctx, size_t i) { + if (i >= sk_SSL_CIPHER_num(ctx->cipher_list->ciphers)) { + return 0; + } + return ctx->cipher_list->in_group_flags[i]; +} + STACK_OF(SSL_CIPHER) *SSL_get_ciphers(const SSL *ssl) { if (ssl == NULL) { return NULL; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 4556fb77..88c2ed24 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -361,12 +361,13 @@ static const char *kBadCurvesLists[] = { ":X25519:P-256", }; -static std::string CipherListToString(ssl_cipher_preference_list_st *list) { +static std::string CipherListToString(SSL_CTX *ctx) { bool in_group = false; std::string ret; - for (size_t i = 0; i < sk_SSL_CIPHER_num(list->ciphers); i++) { - const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(list->ciphers, i); - if (!in_group && list->in_group_flags[i]) { + const STACK_OF(SSL_CIPHER) *ciphers = SSL_CTX_get_ciphers(ctx); + for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { + const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i); + if (!in_group && SSL_CTX_cipher_in_group(ctx, i)) { ret += "\t[\n"; in_group = true; } @@ -376,7 +377,7 @@ static std::string CipherListToString(ssl_cipher_preference_list_st *list) { } ret += SSL_CIPHER_get_name(cipher); ret += "\n"; - if (in_group && !list->in_group_flags[i]) { + if (in_group && !SSL_CTX_cipher_in_group(ctx, i)) { ret += "\t]\n"; in_group = false; } @@ -384,16 +385,17 @@ static std::string CipherListToString(ssl_cipher_preference_list_st *list) { return ret; } -static bool CipherListsEqual(ssl_cipher_preference_list_st *list, +static bool CipherListsEqual(SSL_CTX *ctx, const std::vector &expected) { - if (sk_SSL_CIPHER_num(list->ciphers) != expected.size()) { + const STACK_OF(SSL_CIPHER) *ciphers = SSL_CTX_get_ciphers(ctx); + if (sk_SSL_CIPHER_num(ciphers) != expected.size()) { return false; } for (size_t i = 0; i < expected.size(); i++) { - const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(list->ciphers, i); + const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i); if (expected[i].id != SSL_CIPHER_get_id(cipher) || - expected[i].in_group_flag != list->in_group_flags[i]) { + expected[i].in_group_flag != !!SSL_CTX_cipher_in_group(ctx, i)) { return false; } } @@ -409,18 +411,18 @@ TEST(SSLTest, CipherRules) { // Test lax mode. ASSERT_TRUE(SSL_CTX_set_cipher_list(ctx.get(), t.rule)); - EXPECT_TRUE(CipherListsEqual(ctx->cipher_list, t.expected)) + EXPECT_TRUE(CipherListsEqual(ctx.get(), t.expected)) << "Cipher rule evaluated to:\n" - << CipherListToString(ctx->cipher_list); + << CipherListToString(ctx.get()); // Test strict mode. if (t.strict_fail) { EXPECT_FALSE(SSL_CTX_set_strict_cipher_list(ctx.get(), t.rule)); } else { ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(ctx.get(), t.rule)); - EXPECT_TRUE(CipherListsEqual(ctx->cipher_list, t.expected)) + EXPECT_TRUE(CipherListsEqual(ctx.get(), t.expected)) << "Cipher rule evaluated to:\n" - << CipherListToString(ctx->cipher_list); + << CipherListToString(ctx.get()); } } @@ -439,9 +441,8 @@ TEST(SSLTest, CipherRules) { ASSERT_TRUE(ctx); ASSERT_TRUE(SSL_CTX_set_strict_cipher_list(ctx.get(), rule)); - for (size_t i = 0; i < sk_SSL_CIPHER_num(ctx->cipher_list->ciphers); i++) { - EXPECT_FALSE(SSL_CIPHER_is_NULL( - sk_SSL_CIPHER_value(ctx->cipher_list->ciphers, i))); + for (const SSL_CIPHER *cipher : SSL_CTX_get_ciphers(ctx.get())) { + EXPECT_FALSE(SSL_CIPHER_is_NULL(cipher)); } } }