From 033e5f47d1a95fb9972bc3e4bebe1986b724dc5f Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 13 Nov 2014 18:47:41 -0500 Subject: [PATCH] Remove CERT_PKEY::valid_flags. CERT_PKEY_SIGN isn't meaningful since, without strict mode, we always fall back to SHA-1 anyway. So the digest is never NULL when CERT_PKEY_SIGN is computed. The entire valid_flags is now back to it's pre-1.0.2 check of seeing if the certificate and key are configured. This finally removes the sensitivity between valid_flags and selecting the digest, so we can defer choosing the digest all we like. Change-Id: I9f9952498f512d7f0cc799497f7c5b52145a48af Reviewed-on: https://boringssl-review.googlesource.com/2288 Reviewed-by: Adam Langley --- ssl/s3_clnt.c | 3 +- ssl/s3_lib.c | 1 - ssl/ssl_cert.c | 3 -- ssl/ssl_lib.c | 23 +++++++-------- ssl/ssl_locl.h | 21 +++++--------- ssl/t1_lib.c | 77 +++++++++++++------------------------------------- 6 files changed, 39 insertions(+), 89 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index e6096e9b2..0bff2ef17 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1649,11 +1649,10 @@ int ssl3_get_certificate_request(SSL *s) OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR); goto err; } - /* Clear certificate digests and validity flags */ + /* Clear certificate digests. */ for (i = 0; i < SSL_PKEY_NUM; i++) { s->cert->pkeys[i].digest = NULL; - s->cert->pkeys[i].valid_flags = 0; } if (!tls1_process_sigalgs(s, &supported_signature_algorithms)) { diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 66d6354bc..812a3296c 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -1806,7 +1806,6 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, allow = srvr; } - tls1_set_cert_validity(s); ssl_get_compatible_server_ciphers(s, &mask_k, &mask_a); for (i=0; ivalid_flags = 0; } /* Set digests to defaults. NB: we don't copy existing values as they @@ -389,8 +388,6 @@ void ssl_cert_clear_certs(CERT *c) sk_X509_pop_free(cpk->chain, X509_free); cpk->chain = NULL; } - /* Clear all flags. */ - cpk->valid_flags = 0; } } diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index e6ef24d0e..2059032fb 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2104,11 +2104,16 @@ void SSL_set_cert_cb(SSL *s, int (*cb)(SSL *ssl, void *arg), void *arg) ssl_cert_set_cert_cb(s->cert, cb, arg); } +static int ssl_has_key(SSL *s, size_t idx) + { + CERT_PKEY *cpk = &s->cert->pkeys[idx]; + return cpk->x509 && cpk->privatekey; + } + void ssl_get_compatible_server_ciphers(SSL *s, unsigned long *out_mask_k, unsigned long *out_mask_a) { CERT *c = s->cert; - CERT_PKEY *cpk; int rsa_enc, rsa_sign, dh_tmp; unsigned long mask_k, mask_a; int have_ecc_cert, ecdsa_ok; @@ -2126,12 +2131,9 @@ void ssl_get_compatible_server_ciphers(SSL *s, unsigned long *out_mask_k, dh_tmp = (c->dh_tmp != NULL || c->dh_tmp_cb != NULL); have_ecdh_tmp = (c->ecdh_tmp || c->ecdh_tmp_cb || c->ecdh_tmp_auto); - cpk = &(c->pkeys[SSL_PKEY_RSA_ENC]); - rsa_enc = cpk->valid_flags & CERT_PKEY_VALID; - cpk = &(c->pkeys[SSL_PKEY_RSA_SIGN]); - rsa_sign = cpk->valid_flags & CERT_PKEY_SIGN; - cpk = &(c->pkeys[SSL_PKEY_ECC]); - have_ecc_cert = cpk->valid_flags & CERT_PKEY_VALID; + rsa_enc = ssl_has_key(s, SSL_PKEY_RSA_ENC); + rsa_sign = ssl_has_key(s, SSL_PKEY_RSA_SIGN); + have_ecc_cert = ssl_has_key(s, SSL_PKEY_ECC); mask_k = 0; mask_a = 0; @@ -2149,16 +2151,15 @@ void ssl_get_compatible_server_ciphers(SSL *s, unsigned long *out_mask_k, mask_a |= SSL_aNULL; /* An ECC certificate may be usable for ECDSA cipher suites depending on - * the key usage extension. */ + * the key usage extension and on the client's curve preferences. */ if (have_ecc_cert) { - cpk = &c->pkeys[SSL_PKEY_ECC]; - x = cpk->x509; + x = c->pkeys[SSL_PKEY_ECC].x509; /* This call populates extension flags (ex_flags) */ X509_check_purpose(x, -1, 0); ecdsa_ok = (x->ex_flags & EXFLAG_KUSAGE) ? (x->ex_kusage & X509v3_KU_DIGITAL_SIGNATURE) : 1; - if (!(cpk->valid_flags & CERT_PKEY_SIGN)) + if (!tls1_check_ec_cert(s, x)) ecdsa_ok = 0; if (ecdsa_ok) { diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index c2aeb61af..7f20b849c 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -421,13 +421,6 @@ #define SSL_GET_MESSAGE_DONT_HASH_MESSAGE 0 #define SSL_GET_MESSAGE_HASH_MESSAGE 1 -/* Flags for a CERT_PKEY. */ - -/* Certificate can be used with this session */ -#define CERT_PKEY_VALID 0x1 -/* Certificate can also be used for signing */ -#define CERT_PKEY_SIGN 0x2 - typedef struct cert_pkey_st { X509 *x509; @@ -436,11 +429,6 @@ typedef struct cert_pkey_st const EVP_MD *digest; /* Chain for this certificate */ STACK_OF(X509) *chain; - /* Set if CERT_PKEY can be used with current SSL session: e.g. - * appropriate curve, signature algorithms etc. If zero it can't be - * used at all. - */ - int valid_flags; } CERT_PKEY; typedef struct cert_st @@ -1046,6 +1034,13 @@ int tls1_get_shared_curve(SSL *s); int tls1_set_curves(uint16_t **out_curve_ids, size_t *out_curve_ids_len, const int *curves, size_t ncurves); +/* tls1_check_ec_cert returns one if |x| is an ECC certificate with curve and + * point format compatible with the client's preferences. Otherwise it returns + * zero. */ +int tls1_check_ec_cert(SSL *s, X509 *x); + +/* tls1_check_ec_tmp_key returns one if the EC temporary key is compatible with + * client extensions and zero otherwise. */ int tls1_check_ec_tmp_key(SSL *s); int tls1_shared_list(SSL *s, @@ -1073,8 +1068,6 @@ int tls1_record_handshake_hashes_for_channel_id(SSL *s); int tls1_set_sigalgs_list(CERT *c, const char *str, int client); int tls1_set_sigalgs(CERT *c, const int *salg, size_t salglen, int client); -void tls1_check_chain(SSL *s, size_t idx); -void tls1_set_cert_validity(SSL *s); /* ssl_ctx_log_rsa_client_key_exchange logs |premaster| to |ctx|, if logging is * enabled. It returns one on success and zero on failure. The entry is diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index a3d722836..d88fdea85 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -654,36 +654,30 @@ static void tls1_get_formatlist(SSL *s, const unsigned char **pformats, } } -/* Check cert parameters compatible with extensions: currently just checks - * EC certificates have compatible curves and compression. - */ -static int tls1_check_cert_param(SSL *s, X509 *x) +int tls1_check_ec_cert(SSL *s, X509 *x) { - uint8_t comp_id; + int ret = 0; + EVP_PKEY *pkey = X509_get_pubkey(x); uint16_t curve_id; - EVP_PKEY *pkey; - int rv; - pkey = X509_get_pubkey(x); - if (!pkey) - return 0; - /* If not EC nothing to do */ - if (pkey->type != EVP_PKEY_EC) + uint8_t comp_id; + + if (!pkey || + pkey->type != EVP_PKEY_EC || + !tls1_curve_params_from_ec_key(&curve_id, &comp_id, pkey->pkey.ec) || + !tls1_check_curve_id(s, curve_id) || + !tls1_check_point_format(s, comp_id)) { - EVP_PKEY_free(pkey); - return 1; + goto done; } - rv = tls1_curve_params_from_ec_key(&curve_id, &comp_id, pkey->pkey.ec); - EVP_PKEY_free(pkey); - if (!rv) - return 0; - /* Can't check curve_id for client certs as we don't have a - * supported curves extension. */ - if (s->server && !tls1_check_curve_id(s, curve_id)) - return 0; - return tls1_check_point_format(s, comp_id); + + ret = 1; + +done: + if (pkey) + EVP_PKEY_free(pkey); + return ret; } -/* Check EC temporary key is compatible with client extensions */ int tls1_check_ec_tmp_key(SSL *s) { uint16_t curve_id; @@ -1457,11 +1451,10 @@ static int ssl_scan_clienthello_tlsext(SSL *s, CBS *cbs, int *out_alert) OPENSSL_free(s->cert->shared_sigalgs); s->cert->shared_sigalgs = NULL; } - /* Clear certificate digests and validity flags */ + /* Clear certificate digests. */ for (i = 0; i < SSL_PKEY_NUM; i++) { s->cert->pkeys[i].digest = NULL; - s->cert->pkeys[i].valid_flags = 0; } /* Clear ECC extensions */ if (s->s3->tmp.peer_ecpointformatlist != 0) @@ -2915,35 +2908,3 @@ int tls1_set_sigalgs(CERT *c, const int *psig_nids, size_t salglen, int client) return 0; } -/* Check certificate chain is consistent with TLS extensions and is usable by - * server. This allows the server to check chains before attempting to use them. - */ - -void tls1_check_chain(SSL *s, size_t idx) - { - CERT_PKEY *cpk = &s->cert->pkeys[idx]; - - /* Clear the flags. */ - cpk->valid_flags = 0; - - /* If no cert or key, forget it. */ - if (!cpk->x509 || !cpk->privatekey) - return; - - /* Check cert parameters are consistent */ - if (!tls1_check_cert_param(s, cpk->x509)) - return; - - cpk->valid_flags = CERT_PKEY_VALID; - if (!SSL_USE_SIGALGS(s) || cpk->digest) - cpk->valid_flags |= CERT_PKEY_SIGN; - } - -/* Set validity of certificates in an SSL structure */ -void tls1_set_cert_validity(SSL *s) - { - tls1_check_chain(s, SSL_PKEY_RSA_ENC); - tls1_check_chain(s, SSL_PKEY_RSA_SIGN); - tls1_check_chain(s, SSL_PKEY_ECC); - } -