Give ssl_cipher_preference_list_st a destructor.
Change-Id: I578a284c6a8cae773a97d3d30ad8a5cd13f56164
Reviewed-on: https://boringssl-review.googlesource.com/27491
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 84004de..6404cc9 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -332,14 +332,14 @@
static const SSL_CIPHER *ssl3_choose_cipher(
SSL_HANDSHAKE *hs, const SSL_CLIENT_HELLO *client_hello,
- const struct ssl_cipher_preference_list_st *server_pref) {
+ const SSLCipherPreferenceList *server_pref) {
SSL *const ssl = hs->ssl;
const STACK_OF(SSL_CIPHER) *prio, *allow;
// in_group_flags will either be NULL, or will point to an array of bytes
// which indicate equal-preference groups in the |prio| stack. See the
- // comment about |in_group_flags| in the |ssl_cipher_preference_list_st|
+ // comment about |in_group_flags| in the |SSLCipherPreferenceList|
// struct.
- const uint8_t *in_group_flags;
+ const bool *in_group_flags;
// group_min contains the minimal index so far found in a group, or -1 if no
// such value exists yet.
int group_min = -1;
@@ -351,13 +351,13 @@
}
if (ssl->options & SSL_OP_CIPHER_SERVER_PREFERENCE) {
- prio = server_pref->ciphers;
+ prio = server_pref->ciphers.get();
in_group_flags = server_pref->in_group_flags;
allow = client_pref.get();
} else {
prio = client_pref.get();
in_group_flags = NULL;
- allow = server_pref->ciphers;
+ allow = server_pref->ciphers.get();
}
uint32_t mask_k, mask_a;
@@ -375,7 +375,7 @@
(c->algorithm_auth & mask_a) &&
// Check the cipher is in the |allow| list.
sk_SSL_CIPHER_find(allow, &cipher_index, c)) {
- if (in_group_flags != NULL && in_group_flags[i] == 1) {
+ if (in_group_flags != NULL && in_group_flags[i]) {
// This element of |prio| is in a group. Update the minimum index found
// so far and continue looking.
if (group_min == -1 || (size_t)group_min > cipher_index) {
@@ -389,7 +389,7 @@
}
}
- if (in_group_flags != NULL && in_group_flags[i] == 0 && group_min != -1) {
+ if (in_group_flags != NULL && !in_group_flags[i] && group_min != -1) {
// We are about to leave a group, but we found a match in it, so that's
// our answer.
return sk_SSL_CIPHER_value(allow, group_min);
diff --git a/ssl/internal.h b/ssl/internal.h
index f1fc63f..b258589 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -457,9 +457,48 @@
#define SSL_HANDSHAKE_MAC_SHA256 0x2
#define SSL_HANDSHAKE_MAC_SHA384 0x4
-// SSL_MAX_DIGEST is the number of digest types which exist. When adding a new
-// one, update the table in ssl_cipher.c.
-#define SSL_MAX_DIGEST 4
+// An SSLCipherPreferenceList contains a list of SSL_CIPHERs with equal-
+// preference groups. For TLS clients, the groups are moot because the server
+// picks the cipher and groups cannot be expressed on the wire. However, for
+// servers, the equal-preference groups allow the client's preferences to be
+// partially respected. (This only has an effect with
+// SSL_OP_CIPHER_SERVER_PREFERENCE).
+//
+// The equal-preference groups are expressed by grouping SSL_CIPHERs together.
+// All elements of a group have the same priority: no ordering is expressed
+// within a group.
+//
+// The values in |ciphers| are in one-to-one correspondence with
+// |in_group_flags|. (That is, sk_SSL_CIPHER_num(ciphers) is the number of
+// bytes in |in_group_flags|.) The bytes in |in_group_flags| are either 1, to
+// indicate that the corresponding SSL_CIPHER is not the last element of a
+// group, or 0 to indicate that it is.
+//
+// For example, if |in_group_flags| contains all zeros then that indicates a
+// traditional, fully-ordered preference. Every SSL_CIPHER is the last element
+// of the group (i.e. they are all in a one-element group).
+//
+// For a more complex example, consider:
+// ciphers: A B C D E F
+// in_group_flags: 1 1 0 0 1 0
+//
+// That would express the following, order:
+//
+// A E
+// B -> D -> F
+// C
+struct SSLCipherPreferenceList {
+ static constexpr bool kAllowUniquePtr = true;
+
+ SSLCipherPreferenceList() = default;
+ ~SSLCipherPreferenceList();
+
+ bool Init(UniquePtr<STACK_OF(SSL_CIPHER)> ciphers,
+ Span<const bool> in_group_flags);
+
+ UniquePtr<STACK_OF(SSL_CIPHER)> ciphers;
+ bool *in_group_flags = nullptr;
+};
// ssl_cipher_get_evp_aead sets |*out_aead| to point to the correct EVP_AEAD
// object for |cipher| protocol version |version|. It sets |*out_mac_secret_len|
@@ -477,13 +516,12 @@
const SSL_CIPHER *cipher);
// ssl_create_cipher_list evaluates |rule_str|. It sets |*out_cipher_list| to a
-// newly-allocated |ssl_cipher_preference_list_st| containing the result. It
-// returns true on success and false on failure. If |strict| is true, nonsense
-// will be rejected. If false, nonsense will be silently ignored. An empty
-// result is considered an error regardless of |strict|.
-bool ssl_create_cipher_list(
- struct ssl_cipher_preference_list_st **out_cipher_list,
- const char *rule_str, bool strict);
+// newly-allocated |SSLCipherPreferenceList| containing the result. It returns
+// true on success and false on failure. If |strict| is true, nonsense will be
+// rejected. If false, nonsense will be silently ignored. An empty result is
+// considered an error regardless of |strict|.
+bool ssl_create_cipher_list(SSLCipherPreferenceList **out_cipher_list,
+ const char *rule_str, bool strict);
// ssl_cipher_get_value returns the cipher suite id of |cipher|.
uint16_t ssl_cipher_get_value(const SSL_CIPHER *cipher);
@@ -1921,41 +1959,6 @@
// crypto/x509.
extern const SSL_X509_METHOD ssl_noop_x509_method;
-// ssl_cipher_preference_list_st contains a list of SSL_CIPHERs with
-// equal-preference groups. For TLS clients, the groups are moot because the
-// server picks the cipher and groups cannot be expressed on the wire. However,
-// for servers, the equal-preference groups allow the client's preferences to
-// be partially respected. (This only has an effect with
-// SSL_OP_CIPHER_SERVER_PREFERENCE).
-//
-// The equal-preference groups are expressed by grouping SSL_CIPHERs together.
-// All elements of a group have the same priority: no ordering is expressed
-// within a group.
-//
-// The values in |ciphers| are in one-to-one correspondence with
-// |in_group_flags|. (That is, sk_SSL_CIPHER_num(ciphers) is the number of
-// bytes in |in_group_flags|.) The bytes in |in_group_flags| are either 1, to
-// indicate that the corresponding SSL_CIPHER is not the last element of a
-// group, or 0 to indicate that it is.
-//
-// For example, if |in_group_flags| contains all zeros then that indicates a
-// traditional, fully-ordered preference. Every SSL_CIPHER is the last element
-// of the group (i.e. they are all in a one-element group).
-//
-// For a more complex example, consider:
-// ciphers: A B C D E F
-// in_group_flags: 1 1 0 0 1 0
-//
-// That would express the following, order:
-//
-// A E
-// B -> D -> F
-// C
-struct ssl_cipher_preference_list_st {
- STACK_OF(SSL_CIPHER) *ciphers;
- uint8_t *in_group_flags;
-};
-
struct tlsext_ticket_key {
static constexpr bool kAllowUniquePtr = true;
@@ -1998,7 +2001,7 @@
// configuration.
enum tls13_variant_t tls13_variant;
- struct ssl_cipher_preference_list_st *cipher_list;
+ SSLCipherPreferenceList *cipher_list;
X509_STORE *cert_store;
LHASH_OF(SSL_SESSION) *sessions;
@@ -2621,7 +2624,7 @@
X509_VERIFY_PARAM *param;
// crypto
- struct ssl_cipher_preference_list_st *cipher_list;
+ SSLCipherPreferenceList *cipher_list;
// session info
@@ -2853,13 +2856,9 @@
void ssl_session_renew_timeout(SSL *ssl, SSL_SESSION *session,
uint32_t timeout);
-void ssl_cipher_preference_list_free(
- struct ssl_cipher_preference_list_st *cipher_list);
-
// ssl_get_cipher_preferences returns the cipher preference list for TLS 1.2 and
// below.
-const struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(
- const SSL *ssl);
+const SSLCipherPreferenceList *ssl_get_cipher_preferences(const SSL *ssl);
void ssl_update_cache(SSL_HANDSHAKE *hs, int mode);
diff --git a/ssl/s3_lib.cc b/ssl/s3_lib.cc
index baa5a17..b1fc5fb 100644
--- a/ssl/s3_lib.cc
+++ b/ssl/s3_lib.cc
@@ -215,8 +215,7 @@
ssl->s3 = NULL;
}
-const struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(
- const SSL *ssl) {
+const SSLCipherPreferenceList *ssl_get_cipher_preferences(const SSL *ssl) {
if (ssl->cipher_list != NULL) {
return ssl->cipher_list;
}
diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc
index 32e6c2c..f02fa8a 100644
--- a/ssl/ssl_cipher.cc
+++ b/ssl/ssl_cipher.cc
@@ -811,9 +811,14 @@
*head = curr;
}
-static void ssl_cipher_collect_ciphers(CIPHER_ORDER *co_list,
- CIPHER_ORDER **head_p,
- CIPHER_ORDER **tail_p) {
+static bool ssl_cipher_collect_ciphers(Array<CIPHER_ORDER> *out_co_list,
+ CIPHER_ORDER **out_head,
+ CIPHER_ORDER **out_tail) {
+ Array<CIPHER_ORDER> co_list;
+ if (!co_list.Init(kCiphersLen)) {
+ return false;
+ }
+
size_t co_list_num = 0;
for (const SSL_CIPHER &cipher : kCiphers) {
// TLS 1.3 ciphers do not participate in this mechanism.
@@ -844,9 +849,35 @@
co_list[co_list_num - 1].next = NULL;
- *head_p = &co_list[0];
- *tail_p = &co_list[co_list_num - 1];
+ *out_head = &co_list[0];
+ *out_tail = &co_list[co_list_num - 1];
+ } else {
+ *out_head = nullptr;
+ *out_tail = nullptr;
}
+ *out_co_list = std::move(co_list);
+ return true;
+}
+
+SSLCipherPreferenceList::~SSLCipherPreferenceList() {
+ OPENSSL_free(in_group_flags);
+}
+
+bool SSLCipherPreferenceList::Init(UniquePtr<STACK_OF(SSL_CIPHER)> ciphers_arg,
+ Span<const bool> in_group_flags_arg) {
+ if (sk_SSL_CIPHER_num(ciphers_arg.get()) != in_group_flags_arg.size()) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+
+ Array<bool> copy;
+ if (!copy.CopyFrom(in_group_flags_arg)) {
+ return false;
+ }
+ ciphers = std::move(ciphers_arg);
+ size_t unused_len;
+ copy.Release(&in_group_flags, &unused_len);
+ return true;
}
// ssl_cipher_apply_rule applies the rule type |rule| to ciphers matching its
@@ -1201,15 +1232,8 @@
return true;
}
-bool ssl_create_cipher_list(
- struct ssl_cipher_preference_list_st **out_cipher_list,
- const char *rule_str, bool strict) {
- STACK_OF(SSL_CIPHER) *cipherstack = NULL;
- CIPHER_ORDER *co_list = NULL, *head = NULL, *tail = NULL, *curr;
- uint8_t *in_group_flags = NULL;
- unsigned int num_in_group_flags = 0;
- struct ssl_cipher_preference_list_st *pref_list = NULL;
-
+bool ssl_create_cipher_list(SSLCipherPreferenceList **out_cipher_list,
+ const char *rule_str, bool strict) {
// Return with error if nothing to do.
if (rule_str == NULL || out_cipher_list == NULL) {
return false;
@@ -1218,14 +1242,12 @@
// Now we have to collect the available ciphers from the compiled in ciphers.
// We cannot get more than the number compiled in, so it is used for
// allocation.
- co_list = (CIPHER_ORDER *)OPENSSL_malloc(sizeof(CIPHER_ORDER) * kCiphersLen);
- if (co_list == NULL) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ Array<CIPHER_ORDER> co_list;
+ CIPHER_ORDER *head = nullptr, *tail = nullptr;
+ if (!ssl_cipher_collect_ciphers(&co_list, &head, &tail)) {
return false;
}
- ssl_cipher_collect_ciphers(co_list, &head, &tail);
-
// Now arrange all ciphers by preference:
// TODO(davidben): Compute this order once and copy it.
@@ -1285,7 +1307,7 @@
if (strncmp(rule_str, "DEFAULT", 7) == 0) {
if (!ssl_cipher_process_rulestr(SSL_DEFAULT_CIPHER_LIST, &head, &tail,
strict)) {
- goto err;
+ return false;
}
rule_p += 7;
if (*rule_p == ':') {
@@ -1295,75 +1317,52 @@
if (*rule_p != '\0' &&
!ssl_cipher_process_rulestr(rule_p, &head, &tail, strict)) {
- goto err;
+ return false;
}
// Allocate new "cipherstack" for the result, return with error
// if we cannot get one.
- cipherstack = sk_SSL_CIPHER_new_null();
- if (cipherstack == NULL) {
- goto err;
- }
-
- in_group_flags = (uint8_t *)OPENSSL_malloc(kCiphersLen);
- if (!in_group_flags) {
- goto err;
+ UniquePtr<STACK_OF(SSL_CIPHER)> cipherstack(sk_SSL_CIPHER_new_null());
+ Array<bool> in_group_flags;
+ if (cipherstack == nullptr ||
+ !in_group_flags.Init(kCiphersLen)) {
+ return false;
}
// The cipher selection for the list is done. The ciphers are added
// to the resulting precedence to the STACK_OF(SSL_CIPHER).
- for (curr = head; curr != NULL; curr = curr->next) {
+ size_t num_in_group_flags = 0;
+ for (CIPHER_ORDER *curr = head; curr != NULL; curr = curr->next) {
if (curr->active) {
- if (!sk_SSL_CIPHER_push(cipherstack, curr->cipher)) {
- goto err;
+ if (!sk_SSL_CIPHER_push(cipherstack.get(), curr->cipher)) {
+ return false;
}
in_group_flags[num_in_group_flags++] = curr->in_group;
}
}
- OPENSSL_free(co_list); // Not needed any longer
- co_list = NULL;
- pref_list = (ssl_cipher_preference_list_st *)OPENSSL_malloc(
- sizeof(struct ssl_cipher_preference_list_st));
- if (!pref_list) {
- goto err;
+ UniquePtr<SSLCipherPreferenceList> pref_list =
+ MakeUnique<SSLCipherPreferenceList>();
+ if (!pref_list ||
+ !pref_list->Init(
+ std::move(cipherstack),
+ MakeConstSpan(in_group_flags).subspan(0, num_in_group_flags))) {
+ return false;
}
- pref_list->ciphers = cipherstack;
- pref_list->in_group_flags = NULL;
- if (num_in_group_flags) {
- pref_list->in_group_flags = (uint8_t *)OPENSSL_malloc(num_in_group_flags);
- if (!pref_list->in_group_flags) {
- goto err;
- }
- OPENSSL_memcpy(pref_list->in_group_flags, in_group_flags,
- num_in_group_flags);
+
+ if (*out_cipher_list) {
+ Delete(*out_cipher_list);
}
- OPENSSL_free(in_group_flags);
- in_group_flags = NULL;
- if (*out_cipher_list != NULL) {
- ssl_cipher_preference_list_free(*out_cipher_list);
- }
- *out_cipher_list = pref_list;
- pref_list = NULL;
+ *out_cipher_list = pref_list.release();
// Configuring an empty cipher list is an error but still updates the
// output.
- if (sk_SSL_CIPHER_num((*out_cipher_list)->ciphers) == 0) {
+ if (sk_SSL_CIPHER_num((*out_cipher_list)->ciphers.get()) == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CIPHER_MATCH);
return false;
}
return true;
-
-err:
- OPENSSL_free(co_list);
- OPENSSL_free(in_group_flags);
- sk_SSL_CIPHER_free(cipherstack);
- if (pref_list) {
- OPENSSL_free(pref_list->in_group_flags);
- }
- OPENSSL_free(pref_list);
- return false;
}
uint16_t ssl_cipher_get_value(const SSL_CIPHER *cipher) {
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 8f1de02..78a1860 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -272,16 +272,6 @@
return ret;
}
-void ssl_cipher_preference_list_free(
- struct ssl_cipher_preference_list_st *cipher_list) {
- if (cipher_list == NULL) {
- return;
- }
- sk_SSL_CIPHER_free(cipher_list->ciphers);
- OPENSSL_free(cipher_list->in_group_flags);
- OPENSSL_free(cipher_list);
-}
-
void ssl_update_cache(SSL_HANDSHAKE *hs, int mode) {
SSL *const ssl = hs->ssl;
SSL_CTX *ctx = ssl->session_ctx;
@@ -622,7 +612,7 @@
CRYPTO_MUTEX_cleanup(&ctx->lock);
lh_SSL_SESSION_free(ctx->sessions);
- ssl_cipher_preference_list_free(ctx->cipher_list);
+ Delete(ctx->cipher_list);
Delete(ctx->cert);
sk_SSL_CUSTOM_EXTENSION_pop_free(ctx->client_custom_extensions,
SSL_CUSTOM_EXTENSION_free);
@@ -765,7 +755,7 @@
BIO_free_all(ssl->wbio);
// add extra stuff
- ssl_cipher_preference_list_free(ssl->cipher_list);
+ Delete(ssl->cipher_list);
SSL_SESSION_free(ssl->session);
@@ -1820,11 +1810,11 @@
}
STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx) {
- return ctx->cipher_list->ciphers;
+ return ctx->cipher_list->ciphers.get();
}
int SSL_CTX_cipher_in_group(const SSL_CTX *ctx, size_t i) {
- if (i >= sk_SSL_CIPHER_num(ctx->cipher_list->ciphers)) {
+ if (i >= sk_SSL_CIPHER_num(ctx->cipher_list->ciphers.get())) {
return 0;
}
return ctx->cipher_list->in_group_flags[i];
@@ -1835,13 +1825,8 @@
return NULL;
}
- const struct ssl_cipher_preference_list_st *prefs =
- ssl_get_cipher_preferences(ssl);
- if (prefs == NULL) {
- return NULL;
- }
-
- return prefs->ciphers;
+ const SSLCipherPreferenceList *prefs = ssl_get_cipher_preferences(ssl);
+ return prefs == nullptr ? nullptr : prefs->ciphers.get();
}
const char *SSL_get_cipher_list(const SSL *ssl, int n) {
