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.