Clean up ssl_set_cert_masks.
It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.
So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.
The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.
Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 0c7307a..66d6354 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -1783,7 +1783,6 @@
size_t i;
int ok;
size_t cipher_index;
- CERT *cert;
unsigned long alg_k,alg_a,mask_k,mask_a;
/* in_group_flags will either be NULL, or will point to an array of
* bytes which indicate equal-preference groups in the |prio| stack.
@@ -1794,34 +1793,6 @@
* if no such value exists yet. */
int group_min = -1;
- /* Let's see which ciphers we can support */
- cert=s->cert;
-
-#if 0
- /* Do not set the compare functions, because this may lead to a
- * reordering by "id". We want to keep the original ordering.
- * We may pay a price in performance during sk_SSL_CIPHER_find(),
- * but would have to pay with the price of sk_SSL_CIPHER_dup().
- */
- sk_SSL_CIPHER_set_cmp_func(srvr, ssl_cipher_ptr_id_cmp);
- sk_SSL_CIPHER_set_cmp_func(clnt, ssl_cipher_ptr_id_cmp);
-#endif
-
-#ifdef CIPHER_DEBUG
- printf("Server has %d from %p:\n", sk_SSL_CIPHER_num(srvr), (void *)srvr);
- for(i=0 ; i < sk_SSL_CIPHER_num(srvr) ; ++i)
- {
- c=sk_SSL_CIPHER_value(srvr,i);
- printf("%p:%s\n",(void *)c,c->name);
- }
- printf("Client sent %d from %p:\n", sk_SSL_CIPHER_num(clnt), (void *)clnt);
- for(i=0 ; i < sk_SSL_CIPHER_num(clnt) ; ++i)
- {
- c=sk_SSL_CIPHER_value(clnt,i);
- printf("%p:%s\n",(void *)c,c->name);
- }
-#endif
-
if (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE)
{
prio = srvr;
@@ -1836,6 +1807,7 @@
}
tls1_set_cert_validity(s);
+ ssl_get_compatible_server_ciphers(s, &mask_k, &mask_a);
for (i=0; i<sk_SSL_CIPHER_num(prio); i++)
{
@@ -1848,27 +1820,10 @@
!SSL_USE_TLS1_2_CIPHERS(s))
ok = 0;
- ssl_set_cert_masks(cert,c);
- mask_k = cert->mask_k;
- mask_a = cert->mask_a;
-
alg_k=c->algorithm_mkey;
alg_a=c->algorithm_auth;
- /* with PSK there must be server callback set */
- if ((alg_a & SSL_aPSK) && s->psk_server_callback == NULL)
- ok = 0;
-
ok = ok && (alg_k & mask_k) && (alg_a & mask_a);
-#ifdef CIPHER_DEBUG
- printf("%d:[%08lX:%08lX:%08lX:%08lX]%p:%s\n",ok,alg_k,alg_a,mask_k,mask_a,(void *)c,
- c->name);
-#endif
-
- /* if we are considering an ECC cipher suite that uses
- * an ephemeral EC key check it */
- if (alg_k & SSL_kEECDH)
- ok = ok && tls1_check_ec_tmp_key(s);
if (ok && sk_SSL_CIPHER_find(allow, &cipher_index, c))
{
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 660f1d1..03f212e 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -199,7 +199,6 @@
/* or ret->key = ret->pkeys + (cert->key - cert->pkeys),
* if you find that more readable */
- ret->valid = cert->valid;
ret->mask_k = cert->mask_k;
ret->mask_a = cert->mask_a;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index eb55a79..e6ef24d 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2104,46 +2104,49 @@
ssl_cert_set_cert_cb(s->cert, cb, arg);
}
-void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher)
+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 rsa_enc, rsa_sign, dh_tmp;
+ unsigned long mask_k, mask_a;
int have_ecc_cert, ecdsa_ok;
int have_ecdh_tmp;
- X509 *x = NULL;
- if (c == NULL) return;
+ X509 *x;
- dh_tmp=(c->dh_tmp != NULL || c->dh_tmp_cb != NULL);
+ if (c == NULL)
+ {
+ /* TODO(davidben): Is this codepath possible? */
+ *out_mask_k = 0;
+ *out_mask_a = 0;
+ return;
+ }
- 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;
- mask_k=0;
- mask_a=0;
+ dh_tmp = (c->dh_tmp != NULL || c->dh_tmp_cb != NULL);
-#ifdef CIPHER_DEBUG
- printf("rt=%d rte=%d dht=%d ecdht=%d re=%d ree=%d rs=%d ds=%d dhr=%d dhd=%d\n",
- rsa_tmp,rsa_tmp_export,dh_tmp,have_ecdh_tmp,
- rsa_enc,rsa_enc_export,rsa_sign,dsa_sign,dh_rsa,dh_dsa);
-#endif
-
+ 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;
+ mask_k = 0;
+ mask_a = 0;
+
if (rsa_enc)
- mask_k|=SSL_kRSA;
+ mask_k |= SSL_kRSA;
if (dh_tmp)
- mask_k|=SSL_kEDH;
+ mask_k |= SSL_kEDH;
if (rsa_enc || rsa_sign)
{
- mask_a|=SSL_aRSA;
+ mask_a |= SSL_aRSA;
}
- mask_a|=SSL_aNULL;
+ mask_a |= SSL_aNULL;
/* An ECC certificate may be usable for ECDSA cipher suites depending on
* the key usage extension. */
@@ -2159,21 +2162,26 @@
ecdsa_ok = 0;
if (ecdsa_ok)
{
- mask_a|=SSL_aECDSA;
+ mask_a |= SSL_aECDSA;
}
}
- if (have_ecdh_tmp)
+ /* If we are considering an ECC cipher suite that uses an ephemeral EC
+ * key, check it. */
+ if (have_ecdh_tmp && tls1_check_ec_tmp_key(s))
{
- mask_k|=SSL_kEECDH;
+ mask_k |= SSL_kEECDH;
}
- mask_k |= SSL_kPSK;
- mask_a |= SSL_aPSK;
+ /* PSK requires a server callback. */
+ if (s->psk_server_callback != NULL)
+ {
+ mask_k |= SSL_kPSK;
+ mask_a |= SSL_aPSK;
+ }
- c->mask_k=mask_k;
- c->mask_a=mask_a;
- c->valid=1;
+ *out_mask_k = mask_k;
+ *out_mask_a = mask_a;
}
/* This handy macro borrowed from crypto/x509v3/v3_purp.c */
@@ -2223,20 +2231,14 @@
CERT_PKEY *ssl_get_server_send_pkey(const SSL *s)
{
- CERT *c;
- int i;
-
- c = s->cert;
- ssl_set_cert_masks(c, s->s3->tmp.new_cipher);
-
- i = ssl_get_server_cert_index(s);
+ int i = ssl_get_server_cert_index(s);
/* This may or may not be an error. */
if (i < 0)
return NULL;
/* May be NULL. */
- return &c->pkeys[i];
+ return &s->cert->pkeys[i];
}
EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *cipher, const EVP_MD **pmd)
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 3b5bff7..c2aeb61 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -450,16 +450,19 @@
* Probably it would make more sense to store
* an index, not a pointer. */
- /* For servers the following masks are for the key and auth
- * algorithms that are supported by the certs below.
- * For clients they are masks of *disabled* algorithms based
- * on the current session.
- */
- int valid;
+ /* For clients the following masks are of *disabled* key and auth
+ * algorithms based on the current session.
+ *
+ * TODO(davidben): Remove these. They get checked twice: when sending
+ * the ClientHello and when processing the ServerHello. However,
+ * mask_ssl is a different value both times. mask_k and mask_a are not,
+ * but is a round-about way of checking the server's cipher was one of
+ * the advertised ones. (Currently it checks the masks and then the list
+ * of ciphers prior to applying the masks in ClientHello.) */
unsigned long mask_k;
unsigned long mask_a;
- /* Client only */
unsigned long mask_ssl;
+
DH *dh_tmp;
DH *(*dh_tmp_cb)(SSL *ssl,int is_export,int keysize);
EC_KEY *ecdh_tmp;
@@ -817,7 +820,14 @@
CERT_PKEY *ssl_get_server_send_pkey(const SSL *s);
EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *c, const EVP_MD **pmd);
int ssl_cert_type(X509 *x,EVP_PKEY *pkey);
-void ssl_set_cert_masks(CERT *c, const SSL_CIPHER *cipher);
+
+/* ssl_get_compatible_server_ciphers determines the key exchange and
+ * authentication cipher suite masks compatible with the server configuration
+ * and current ClientHello parameters of |s|. It sets |*out_mask_k| to the key
+ * exchange mask and |*out_mask_a| to the authentication mask. */
+void ssl_get_compatible_server_ciphers(SSL *s, unsigned long *out_mask_k,
+ unsigned long *out_mask_a);
+
STACK_OF(SSL_CIPHER) *ssl_get_ciphers_by_id(SSL *s);
int ssl_verify_alarm_type(long type);
int ssl_fill_hello_random(SSL *s, int server, unsigned char *field, int len);
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index 30629d3..32469a2 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -214,7 +214,6 @@
c->pkeys[i].privatekey = EVP_PKEY_dup(pkey);
c->key = &(c->pkeys[i]);
- c->valid=0;
return(1);
}
@@ -431,7 +430,6 @@
c->pkeys[i].x509 = X509_up_ref(x);
c->key= &(c->pkeys[i]);
- c->valid=0;
return(1);
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index c31720c..a3d7228 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -877,7 +877,6 @@
c->mask_a |= SSL_aPSK;
c->mask_k |= SSL_kPSK;
}
- c->valid = 1;
}
/* header_len is the length of the ClientHello header written so far, used to
