Remove mask_a and mask_k from CERT.
This resolves a TODO, trims per-connection memory, and makes more sense.
These masks have nothing to do with certificate configuration.
Change-Id: I783e6158e51f58cce88e3e68dfa0ed965bdc894c
Reviewed-on: https://boringssl-review.googlesource.com/13368
Reviewed-by: Steven Valdez <svaldez@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>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index c3d3572..d53cb01 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -554,11 +554,59 @@
return ret;
}
+/* ssl_get_client_disabled sets |*out_mask_a| and |*out_mask_k| to masks of
+ * disabled algorithms. */
+static void ssl_get_client_disabled(SSL *ssl, uint32_t *out_mask_a,
+ uint32_t *out_mask_k) {
+ int have_rsa = 0, have_ecdsa = 0;
+ *out_mask_a = 0;
+ *out_mask_k = 0;
+
+ /* Now go through all signature algorithms seeing if we support any for RSA or
+ * ECDSA. Do this for all versions not just TLS 1.2. */
+ const uint16_t *sigalgs;
+ size_t num_sigalgs = tls12_get_verify_sigalgs(ssl, &sigalgs);
+ for (size_t i = 0; i < num_sigalgs; i++) {
+ switch (sigalgs[i]) {
+ case SSL_SIGN_RSA_PSS_SHA512:
+ case SSL_SIGN_RSA_PSS_SHA384:
+ case SSL_SIGN_RSA_PSS_SHA256:
+ case SSL_SIGN_RSA_PKCS1_SHA512:
+ case SSL_SIGN_RSA_PKCS1_SHA384:
+ case SSL_SIGN_RSA_PKCS1_SHA256:
+ case SSL_SIGN_RSA_PKCS1_SHA1:
+ have_rsa = 1;
+ break;
+
+ case SSL_SIGN_ECDSA_SECP521R1_SHA512:
+ case SSL_SIGN_ECDSA_SECP384R1_SHA384:
+ case SSL_SIGN_ECDSA_SECP256R1_SHA256:
+ case SSL_SIGN_ECDSA_SHA1:
+ have_ecdsa = 1;
+ break;
+ }
+ }
+
+ /* Disable auth if we don't include any appropriate signature algorithms. */
+ if (!have_rsa) {
+ *out_mask_a |= SSL_aRSA;
+ }
+ if (!have_ecdsa) {
+ *out_mask_a |= SSL_aECDSA;
+ }
+
+ /* PSK requires a client callback. */
+ if (ssl->psk_client_callback == NULL) {
+ *out_mask_a |= SSL_aPSK;
+ *out_mask_k |= SSL_kPSK;
+ }
+}
+
static int ssl_write_client_cipher_list(SSL *ssl, CBB *out,
uint16_t min_version,
uint16_t max_version) {
- /* Prepare disabled cipher masks. */
- ssl_set_client_disabled(ssl);
+ uint32_t mask_a, mask_k;
+ ssl_get_client_disabled(ssl, &mask_a, &mask_k);
CBB child;
if (!CBB_add_u16_length_prefixed(out, &child)) {
@@ -594,8 +642,8 @@
for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i);
/* Skip disabled ciphers */
- if ((cipher->algorithm_mkey & ssl->cert->mask_k) ||
- (cipher->algorithm_auth & ssl->cert->mask_a)) {
+ if ((cipher->algorithm_mkey & mask_k) ||
+ (cipher->algorithm_auth & mask_a)) {
continue;
}
if (SSL_CIPHER_get_min_version(cipher) > max_version ||
@@ -799,7 +847,6 @@
static int ssl3_get_server_hello(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- CERT *ct = ssl->cert;
int al = SSL_AD_INTERNAL_ERROR;
CBS server_hello, server_random, session_id;
uint16_t server_wire_version, cipher_suite;
@@ -916,7 +963,9 @@
}
/* The cipher must be allowed in the selected version and enabled. */
- if ((c->algorithm_mkey & ct->mask_k) || (c->algorithm_auth & ct->mask_a) ||
+ uint32_t mask_a, mask_k;
+ ssl_get_client_disabled(ssl, &mask_a, &mask_k);
+ if ((c->algorithm_mkey & mask_k) || (c->algorithm_auth & mask_a) ||
SSL_CIPHER_get_min_version(c) > ssl3_protocol_version(ssl) ||
SSL_CIPHER_get_max_version(c) < ssl3_protocol_version(ssl) ||
!sk_SSL_CIPHER_find(SSL_get_ciphers(ssl), NULL, c)) {
diff --git a/ssl/internal.h b/ssl/internal.h
index 13e3605..ed3f62c 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1242,14 +1242,6 @@
* operations. */
const SSL_PRIVATE_KEY_METHOD *key_method;
- /* For clients the following masks are of *disabled* key and auth algorithms
- * based on the current configuration.
- *
- * TODO(davidben): Remove these. They get checked twice: when sending the
- * ClientHello and when processing the ServerHello. */
- uint32_t mask_k;
- uint32_t mask_a;
-
DH *dh_tmp;
DH *(*dh_tmp_cb)(SSL *ssl, int is_export, int keysize);
@@ -1948,8 +1940,6 @@
uint32_t ssl_get_algorithm_prf(const SSL *ssl);
-void ssl_set_client_disabled(SSL *ssl);
-
void ssl_get_current_time(const SSL *ssl, struct timeval *out_clock);
/* ssl_reset_error_state resets state for |SSL_get_error|. */
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 66edf37..048bf95 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -180,9 +180,6 @@
ret->key_method = cert->key_method;
- ret->mask_k = cert->mask_k;
- ret->mask_a = cert->mask_a;
-
if (cert->dh_tmp != NULL) {
ret->dh_tmp = DHparams_dup(cert->dh_tmp);
if (ret->dh_tmp == NULL) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index ec5dce0..6fdef45 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -527,56 +527,6 @@
return 0;
}
-/* Get a mask of disabled algorithms: an algorithm is disabled if it isn't
- * supported or doesn't appear in supported signature algorithms. Unlike
- * ssl_cipher_get_disabled this applies to a specific session and not global
- * settings. */
-void ssl_set_client_disabled(SSL *ssl) {
- CERT *c = ssl->cert;
- int have_rsa = 0, have_ecdsa = 0;
- c->mask_a = 0;
- c->mask_k = 0;
-
- /* Now go through all signature algorithms seeing if we support any for RSA or
- * ECDSA. Do this for all versions not just TLS 1.2. */
- const uint16_t *sigalgs;
- size_t num_sigalgs = tls12_get_verify_sigalgs(ssl, &sigalgs);
- for (size_t i = 0; i < num_sigalgs; i++) {
- switch (sigalgs[i]) {
- case SSL_SIGN_RSA_PSS_SHA512:
- case SSL_SIGN_RSA_PSS_SHA384:
- case SSL_SIGN_RSA_PSS_SHA256:
- case SSL_SIGN_RSA_PKCS1_SHA512:
- case SSL_SIGN_RSA_PKCS1_SHA384:
- case SSL_SIGN_RSA_PKCS1_SHA256:
- case SSL_SIGN_RSA_PKCS1_SHA1:
- have_rsa = 1;
- break;
-
- case SSL_SIGN_ECDSA_SECP521R1_SHA512:
- case SSL_SIGN_ECDSA_SECP384R1_SHA384:
- case SSL_SIGN_ECDSA_SECP256R1_SHA256:
- case SSL_SIGN_ECDSA_SHA1:
- have_ecdsa = 1;
- break;
- }
- }
-
- /* Disable auth if we don't include any appropriate signature algorithms. */
- if (!have_rsa) {
- c->mask_a |= SSL_aRSA;
- }
- if (!have_ecdsa) {
- c->mask_a |= SSL_aECDSA;
- }
-
- /* with PSK there must be client callback set */
- if (!ssl->psk_client_callback) {
- c->mask_a |= SSL_aPSK;
- c->mask_k |= SSL_kPSK;
- }
-}
-
/* tls_extension represents a TLS extension that is handled internally. The
* |init| function is called for each handshake, before any other functions of
* the extension. Then the add and parse callbacks are called as needed.
