Refactor TLS 1.3 cipher selection and fix SSL_get_ciphers

include/openssl/ssl.h

@@ -1690,23 +1690,27 @@ OPENSSL_EXPORT size_t SSL_get_all_standard_cipher_names(const char **out,
 // Once an equal-preference group is used, future directives must be
 // opcode-less. Inside an equal-preference group, spaces are not allowed.
 //
-// TLS 1.3 ciphers do not participate in this mechanism and instead have a
-// built-in preference order. Functions to set cipher lists do not affect TLS
-// 1.3, and functions to query the cipher list do not include TLS 1.3
-// ciphers.
+// Note: TLS 1.3 ciphersuites are only configurable via
+// |SSL_CTX_set_ciphersuites| or |SSL_set_ciphersuites|. Other setter functions have
+// no impact on TLS 1.3 ciphersuites.
 
 // SSL_DEFAULT_CIPHER_LIST is the default cipher suite configuration. It is
 // substituted when a cipher string starts with 'DEFAULT'.
 #define SSL_DEFAULT_CIPHER_LIST "ALL"
 
+
 // SSL_CTX_set_strict_cipher_list configures the cipher list for |ctx|,
 // evaluating |str| as a cipher string and returning error if |str| contains
-// anything meaningless. It returns one on success and zero on failure.
+// anything meaningless. It updates |ctx->cipher_list| with any values in
+// |ctx->tls13_cipher_list|.
+//
+// It returns one on success and zero on failure.
 OPENSSL_EXPORT int SSL_CTX_set_strict_cipher_list(SSL_CTX *ctx,
                                                   const char *str);
 
 // SSL_CTX_set_cipher_list configures the cipher list for |ctx|, evaluating
-// |str| as a cipher string. It returns one on success and zero on failure.
+// |str| as a cipher string. It updates |ctx->cipher_list| with any values in
+// |ctx->tls13_cipher_list|. 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. However, an empty
@@ -1720,24 +1724,34 @@ OPENSSL_EXPORT int SSL_CTX_set_cipher_list(SSL_CTX *ctx, const char *str);
 
 // SSL_set_strict_cipher_list configures the cipher list for |ssl|, evaluating
 // |str| as a cipher string and returning error if |str| contains anything
-// meaningless. It returns one on success and zero on failure.
+// meaningless.
+// It updates the cipher list |ssl->config->cipher_list| with any configured
+// TLS 1.3 cipher suites by first checking |ssl->config->tls13_cipher_list| and
+// otherwise falling back to |ssl->ctx->tls13_cipher_list|.
+//
+// It returns one on success and zero on failure.
 OPENSSL_EXPORT int SSL_set_strict_cipher_list(SSL *ssl, const char *str);
 
-// SSL_CTX_set_ciphersuites configure the available TLSv1.3 ciphersuites for
-// |ctx|, evaluating |str| as a cipher string. It returns one on success and
+// SSL_CTX_set_ciphersuites configures the available TLSv1.3 ciphersuites on
+// |ctx|, evaluating |str| as a cipher string. It updates |ctx->cipher_list|
+// with any values in |ctx->tls13_cipher_list|. It returns one on success and
 // zero on failure.
 OPENSSL_EXPORT int SSL_CTX_set_ciphersuites(SSL_CTX *ctx, const char *str);
 
-// SSL_set_ciphersuites sets the available TLSv1.3 ciphersuites on an |ssl|,
-// returning one on success and zero on failure. In OpenSSL, the only
-// difference between |SSL_CTX_set_ciphersuites| and |SSL_set_ciphersuites| is
-// that the latter copies the |SSL|'s |cipher_list| to its associated
-// |SSL_CONNECTION|. In AWS-LC, we track everything on the |ssl|'s |config| so
-// duplication is not necessary.
+// SSL_set_ciphersuites configures the available TLSv1.3 ciphersuites on
+// |ssl|, evaluating |str| as a cipher string. It updates
+// |ssl->config->cipher_list| with any values in
+// |ssl->config->tls13_cipher_list|. It returns one on success and zero on
+// failure.
 OPENSSL_EXPORT int SSL_set_ciphersuites(SSL *ssl, const char *str);
 
 // SSL_set_cipher_list configures the cipher list for |ssl|, evaluating |str| as
-// a cipher string. It returns one on success and zero on failure.
+// a cipher string. It updates the cipher list |ssl->config->cipher_list| with
+// any configured TLS 1.3 cipher suites by first checking
+// |ssl->config->tls13_cipher_list| and otherwise falling back to
+// |ssl->ctx->tls13_cipher_list|.
+//
+// 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. However, an empty string which
@@ -1746,7 +1760,7 @@ OPENSSL_EXPORT int SSL_set_ciphersuites(SSL *ssl, const char *str);
 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
-// preference.
+// preference. This includes TLS 1.3 and 1.2 and below cipher suites.
 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
@@ -1755,6 +1769,7 @@ OPENSSL_EXPORT STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx);
 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.
+// This includes TLS 1.3 and 1.2 and below cipher suites.
 OPENSSL_EXPORT STACK_OF(SSL_CIPHER) *SSL_get_ciphers(const SSL *ssl);
 
 

ssl/handshake_client.cc

@@ -224,6 +224,7 @@ static bool collect_cipher_protocol_ids(STACK_OF(SSL_CIPHER) *ciphers,
   CBB *cbb, uint32_t mask_k, uint32_t mask_a, uint16_t max_version,
   uint16_t min_version, bool *any_enabled) {
   *any_enabled = false;
+
   for (const SSL_CIPHER *cipher : ciphers) {
     // Skip disabled ciphers
     if ((cipher->algorithm_mkey & mask_k) ||
@@ -259,49 +260,33 @@ static bool ssl_write_client_cipher_list(const SSL_HANDSHAKE *hs, CBB *out,
     return false;
   }
 
-  // Add TLS 1.3 ciphers.
-  if (hs->max_version >= TLS1_3_VERSION && ssl->ctx->tls13_cipher_list) {
-    // Use the configured TLSv1.3 ciphers list.
-    STACK_OF(SSL_CIPHER) *ciphers = ssl->ctx->tls13_cipher_list->ciphers.get();
+  // Add all ciphers unless TLS 1.3 only connection
+  if (hs->min_version < TLS1_3_VERSION && type != ssl_client_hello_inner) {
     bool any_enabled = false;
-    if (!collect_cipher_protocol_ids(ciphers, &child, mask_k,
+    if (!collect_cipher_protocol_ids(SSL_get_ciphers(ssl), &child, mask_k,
       mask_a, hs->max_version, hs->min_version, &any_enabled)) {
       return false;
     }
+
     // If all ciphers were disabled, return the error to the caller.
     if (!any_enabled) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE);
       return false;
     }
   } else if (hs->max_version >= TLS1_3_VERSION) {
-    // Use the built in TLSv1.3 ciphers. Order ChaCha20-Poly1305 relative to
-    // AES-GCM based on hardware support.
-    const bool has_aes_hw = ssl->config->aes_hw_override
-                                ? ssl->config->aes_hw_override_value
-                                : EVP_has_aes_hardware();
-
-    if (!has_aes_hw &&
-        !CBB_add_u16(&child, TLS1_CK_CHACHA20_POLY1305_SHA256 & 0xffff)) {
-      return false;
-    }
-    if (!CBB_add_u16(&child, TLS1_3_CK_AES_128_GCM_SHA256 & 0xffff) ||
-        !CBB_add_u16(&child, TLS1_3_CK_AES_256_GCM_SHA384 & 0xffff)) {
-      return false;
-    }
-    if (has_aes_hw &&
-        !CBB_add_u16(&child, TLS1_3_CK_CHACHA20_POLY1305_SHA256 & 0xffff)) {
-      return false;
-    }
-  }
+    // Only TLS 1.3 ciphers
+    STACK_OF(SSL_CIPHER) *ciphers = (ssl->config && ssl->config->tls13_cipher_list) ?
+      ssl->config->tls13_cipher_list->ciphers.get() : ssl->ctx->tls13_cipher_list->ciphers.get();
 
-  if (hs->min_version < TLS1_3_VERSION && type != ssl_client_hello_inner) {
     bool any_enabled = false;
-    if (!collect_cipher_protocol_ids(SSL_get_ciphers(ssl), &child, mask_k,
+
+    if (!collect_cipher_protocol_ids(ciphers, &child, mask_k,
       mask_a, hs->max_version, hs->min_version, &any_enabled)) {
       return false;
     }
+
     // If all ciphers were disabled, return the error to the caller.
-    if (!any_enabled && hs->max_version < TLS1_3_VERSION) {
+    if (!any_enabled) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHERS_AVAILABLE);
       return false;
     }

ssl/internal.h

@@ -722,6 +722,20 @@ bool ssl_create_cipher_list(UniquePtr<SSLCipherPreferenceList> *out_cipher_list,
                             const bool has_aes_hw, const char *rule_str,
                             bool strict, bool config_tls13);
 
+// update_cipher_list creates a new |SSLCipherPreferenceList| containing ciphers
+// from both |ciphers| and |tls13_ciphers| and assigns it to |dst|. The function:
+//
+// 1. Creates a copy of |ciphers|
+// 2. Removes any stale TLS 1.3 ciphersuites from the copy
+// 3. Adds any configured TLS 1.3 ciphersuites from |tls13_ciphers| to the
+// front of the list.
+// 3. Combines |in_group_flags| from both input lists into |dst->in_group_flags|
+//
+// Returns one on success, zero on error.
+int update_cipher_list(UniquePtr<SSLCipherPreferenceList> &dst,
+                       UniquePtr<SSLCipherPreferenceList> &ciphers,
+                       UniquePtr<SSLCipherPreferenceList> &tls13_ciphers);
+
 // ssl_get_certificate_slot_index returns the |SSL_PKEY_*| certificate slot
 // index corresponding to the private key type of |pkey|. It returns -1 if not
 // supported. This was |ssl_cert_type| in OpenSSL 1.0.2.
@@ -3244,9 +3258,14 @@ struct SSL_CONFIG {
 
   X509_VERIFY_PARAM *param = nullptr;
 
-  // crypto
+  // cipher_list holds all available cipher suites for tls 1.3,
+  // and 1.2 and below
   UniquePtr<SSLCipherPreferenceList> cipher_list;
 
+  // tls13_cipher_list holds the default or configured tls1.3 and above
+  // cipher suites.
+  UniquePtr<SSLCipherPreferenceList> tls13_cipher_list;
+
   // This is used to hold the local certificate used (i.e. the server
   // certificate for a server or the client certificate for a client).
   UniquePtr<CERT> cert;
@@ -3721,6 +3740,18 @@ struct ssl_method_st {
   const bssl::SSL_X509_METHOD *x509_method;
 };
 
+// TLS13_DEFAULT_CIPHER_LIST_AES_HW is the default TLS 1.3 cipher suite
+// configuration when AES hardware acceleration is enabled.
+#define TLS13_DEFAULT_CIPHER_LIST_AES_HW "TLS_AES_128_GCM_SHA256:" \
+                                         "TLS_AES_256_GCM_SHA384:" \
+                                         "TLS_CHACHA20_POLY1305_SHA256"
+
+// TLS13_DEFAULT_CIPHER_LIST_NO_AES_HW is the default TLS 1.3 cipher suite
+// configuration when no AES hardware acceleration is enabled.
+#define TLS13_DEFAULT_CIPHER_LIST_NO_AES_HW "TLS_CHACHA20_POLY1305_SHA256:" \
+                                            "TLS_AES_128_GCM_SHA256:" \
+                                            "TLS_AES_256_GCM_SHA384"
+
 #define MIN_SAFE_FRAGMENT_SIZE 512
 struct ssl_ctx_st : public bssl::RefCounted<ssl_ctx_st> {
   explicit ssl_ctx_st(const SSL_METHOD *ssl_method);
@@ -3754,12 +3785,12 @@ struct ssl_ctx_st : public bssl::RefCounted<ssl_ctx_st> {
   // quic_method is the method table corresponding to the QUIC hooks.
   const SSL_QUIC_METHOD *quic_method = nullptr;
 
-  // Currently, cipher_list holds the tls1.2 and below ciphersuites.
-  // TODO: move |tls13_cipher_list| to |cipher_list| during cipher
-  // configuration.
+  // cipher_list holds all available cipher suites for tls 1.3,
+  // and 1.2 and below
   bssl::UniquePtr<bssl::SSLCipherPreferenceList> cipher_list;
 
-  // tls13_cipher_list holds the tls1.3 and above ciphersuites.
+  // tls13_cipher_list holds the default or configured tls1.3 and above
+  // cipher suites.
   bssl::UniquePtr<bssl::SSLCipherPreferenceList> tls13_cipher_list;
 
   X509_STORE *cert_store = nullptr;

ssl/ssl_cipher.cc

@@ -1234,6 +1234,84 @@ static bool is_known_default_alias_keyword_filter_rule(const char *rule,
   return false;
 }
 
+int update_cipher_list(UniquePtr<SSLCipherPreferenceList> &dst,
+                       UniquePtr<SSLCipherPreferenceList> &ciphers,
+                       UniquePtr<SSLCipherPreferenceList> &tls13_ciphers) {
+  bssl::UniquePtr<STACK_OF(SSL_CIPHER)> tmp_cipher_list;
+  int num_removed_tls13_ciphers = 0, num_added_tls13_ciphers = 0;
+  Array<bool> updated_in_group_flags;
+
+  if (ciphers && ciphers->ciphers) {
+    tmp_cipher_list.reset(sk_SSL_CIPHER_dup(ciphers->ciphers.get()));
+  } else {
+    tmp_cipher_list.reset(sk_SSL_CIPHER_new_null());
+  }
+
+  if (tmp_cipher_list == nullptr) {
+    return 0;
+  }
+
+  // Delete any existing TLSv1.3 ciphersuites. These will be first in the list
+  while (sk_SSL_CIPHER_num(tmp_cipher_list.get()) > 0 &&
+         SSL_CIPHER_get_min_version(sk_SSL_CIPHER_value(tmp_cipher_list.get(), 0))
+         == TLS1_3_VERSION) {
+    sk_SSL_CIPHER_delete(tmp_cipher_list.get(), 0);
+    num_removed_tls13_ciphers++;
+  }
+
+  int num_updated_tls12_ciphers = sk_SSL_CIPHER_num(tmp_cipher_list.get());
+
+  // Add any configure tls 1.3 ciphersuites
+  if (tls13_ciphers && tls13_ciphers->ciphers) {
+    STACK_OF(SSL_CIPHER) *tls13_cipher_stack = tls13_ciphers->ciphers.get();
+    num_added_tls13_ciphers = sk_SSL_CIPHER_num(tls13_cipher_stack);
+    for (int i = sk_SSL_CIPHER_num(tls13_cipher_stack) - 1; i >= 0; i--) {
+      const SSL_CIPHER *tls13_cipher = sk_SSL_CIPHER_value(tls13_cipher_stack, i);
+      if (!sk_SSL_CIPHER_unshift(tmp_cipher_list.get(), tls13_cipher)) {
+        return 0;
+      }
+    }
+  }
+
+
+  if (!updated_in_group_flags.Init(num_added_tls13_ciphers +
+                                   num_updated_tls12_ciphers)) {
+    return 0;
+  }
+  std::fill(updated_in_group_flags.begin(), updated_in_group_flags.end(),
+           false);
+
+  // Copy in_group_flags from |ctx->tls13_cipher_list|
+  if (tls13_ciphers && tls13_ciphers->in_group_flags) {
+    const auto& tls13_flags = tls13_ciphers->in_group_flags;
+    // Ensure value of last element in |in_group_flags| is 0. The last cipher
+    // in a list must be the end of any group in that list.
+    if (tls13_flags[num_added_tls13_ciphers - 1] != 0) {
+      tls13_flags[num_added_tls13_ciphers - 1] = false;
+    }
+    for (int i = 0; i < num_added_tls13_ciphers; i++) {
+      updated_in_group_flags[i] = tls13_flags[i];
+    }
+  }
+
+  // Copy remaining in_group_flags from |ctx->cipher_list|
+  if (ciphers && ciphers->in_group_flags) {
+    for (int i = 0; i < num_updated_tls12_ciphers; i++) {
+      updated_in_group_flags[i + num_added_tls13_ciphers] =
+        ciphers->in_group_flags[i + num_removed_tls13_ciphers];
+    }
+  }
+
+  Span<const bool> flags_span(updated_in_group_flags.data(), updated_in_group_flags.size());
+  UniquePtr<SSLCipherPreferenceList> new_list = MakeUnique<SSLCipherPreferenceList>();
+  if (!new_list || !new_list->Init(std::move(tmp_cipher_list), flags_span)) {
+    return 0;
+  }
+
+  dst = std::move(new_list);
+  return 1;
+}
+
 bool ssl_create_cipher_list(UniquePtr<SSLCipherPreferenceList> *out_cipher_list,
                             const bool has_aes_hw, const char *rule_str,
                             bool strict, bool config_tls13) {