commit 91222b8d3835844802b7564ac12e92055e8554eb
author David Benjamin <davidben@google.com> Thu Mar 09 20:10:56 2017 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Mar 10 01:20:25 2017 +0000
tree b9d99114a12be11e79cafcaea7e89826435ce8ce
parent 6ad20dc91231d5855cdd44c49008b6718d55e5a7

Fix configuring the empty cipher list.

Although it returns failure, the cipher list should still be updated.
Conscrypt relies on this behavior to support a Java API edge case.

Change-Id: If58efafc6a4a81e85a0e2ee2c38873a7a4938123
Reviewed-on: https://boringssl-review.googlesource.com/14165
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/ssl_cipher.c

diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c
index dc9cc2a..4ee3c12 100644
--- a/ssl/ssl_cipher.c
+++ b/ssl/ssl_cipher.c
@@ -1377,11 +1377,6 @@
   OPENSSL_free(co_list); /* Not needed any longer */
   co_list = NULL;
 
-  if (sk_SSL_CIPHER_num(cipherstack) == 0) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
-    goto err;
-  }
-
   pref_list = OPENSSL_malloc(sizeof(struct ssl_cipher_preference_list_st));
   if (!pref_list) {
     goto err;
@@ -1400,6 +1395,13 @@
   *out_cipher_list = pref_list;
   pref_list = NULL;
 
+  /* Configuring an empty cipher list is an error but still updates the
+   * output. */
+  if (sk_SSL_CIPHER_num((*out_cipher_list)->ciphers) == 0) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
+    return 0;
+  }
+
   return 1;
 
 err:

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 0dc240a..5f89b81 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -3229,6 +3229,23 @@
                                      nullptr /* no session */));
 }
 
+// Configuring the empty cipher list, though an error, should still modify the
+// configuration.
+TEST(SSLTest, EmptyCipherList) {
+  bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(ctx);
+
+  // 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(), ""));
+  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())));
+}
+
 // TODO(davidben): Convert this file to GTest properly.
 TEST(SSLTest, AllTests) {
   if (!TestCipherRules() ||