Account for EVP_PKEY capabilities in selecting hash functions.
tls1_process_sigalgs now only determines the intersection between the peer
algorithms and those configured locally. That list is queried later to
determine the hash algorithm to use when signing CertificateVerify or
ServerKeyExchange.
This is needed to support client auth on Windows where smartcards or CAPI may
not support all hash functions.
As a bonus, this does away with more connection-global state. This avoids the
current situation where digests are chosen before keys are known (for
CertificateVerify) or for slots that don't exist.
Change-Id: Iec3619a103d691291d8ebe08ef77d574f2faf0e8
Reviewed-on: https://boringssl-review.googlesource.com/2280
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 0bff2ef..47ae767 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1565,7 +1565,6 @@
{
int ok,ret=0;
unsigned long n;
- unsigned int i;
X509_NAME *xn=NULL;
STACK_OF(X509_NAME) *ca_sk=NULL;
CBS cbs;
@@ -1649,11 +1648,6 @@
OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR);
goto err;
}
- /* Clear certificate digests. */
- for (i = 0; i < SSL_PKEY_NUM; i++)
- {
- s->cert->pkeys[i].digest = NULL;
- }
if (!tls1_process_sigalgs(s, &supported_signature_algorithms))
{
ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
@@ -2278,7 +2272,7 @@
/* Write out the digest type if needbe. */
if (SSL_USE_SIGALGS(s))
{
- md = s->cert->key->digest;
+ md = tls1_choose_signing_digest(s, pkey);
if (!tls12_get_sigandhash(p, pkey, md))
{
OPENSSL_PUT_ERROR(SSL, ssl3_send_cert_verify, ERR_R_INTERNAL_ERROR);
@@ -2337,17 +2331,11 @@
return(-1);
}
-/* Check a certificate can be used for client authentication. Currently
- * check the cert exists and if we have a suitable digest for TLS 1.2.
- */
-static int ssl3_check_client_certificate(SSL *s)
+/* ssl3_has_client_certificate returns true if a client certificate is
+ * configured. */
+static int ssl3_has_client_certificate(SSL *s)
{
- if (!s->cert || !s->cert->key->x509 || !s->cert->key->privatekey)
- return 0;
- /* If no suitable signature algorithm can't use certificate */
- if (SSL_USE_SIGALGS(s) && !s->cert->key->digest)
- return 0;
- return 1;
+ return s->cert && s->cert->key->x509 && s->cert->key->privatekey;
}
int ssl3_send_client_certificate(SSL *s)
@@ -2374,7 +2362,7 @@
}
s->rwstate=SSL_NOTHING;
}
- if (ssl3_check_client_certificate(s))
+ if (ssl3_has_client_certificate(s))
s->state=SSL3_ST_CW_CERT_C;
else
s->state=SSL3_ST_CW_CERT_B;
@@ -2408,7 +2396,7 @@
if (x509 != NULL) X509_free(x509);
if (pkey != NULL) EVP_PKEY_free(pkey);
- if (i && !ssl3_check_client_certificate(s))
+ if (i && !ssl3_has_client_certificate(s))
i = 0;
if (i == 0)
{
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index d591e19..2496783 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1288,7 +1288,6 @@
const char* psk_identity_hint = NULL;
size_t psk_identity_hint_len = 0;
EVP_PKEY *pkey;
- const EVP_MD *md = NULL;
unsigned char *p,*d;
int al,i;
unsigned long alg_k;
@@ -1510,7 +1509,7 @@
if (ssl_cipher_has_server_public_key(s->s3->tmp.new_cipher))
{
- if ((pkey=ssl_get_sign_pkey(s,s->s3->tmp.new_cipher,&md))
+ if ((pkey=ssl_get_sign_pkey(s,s->s3->tmp.new_cipher))
== NULL)
{
al=SSL_AD_DECODE_ERROR;
@@ -1607,13 +1606,15 @@
n+=u+2;
}
else
- if (md)
{
+ const EVP_MD *md;
size_t sig_len = EVP_PKEY_size(pkey);
+
/* send signature algorithm */
if (SSL_USE_SIGALGS(s))
{
+ md = tls1_choose_signing_digest(s, pkey);
if (!tls12_get_sigandhash(p, pkey, md))
{
/* Should never happen */
@@ -1623,6 +1624,10 @@
}
p+=2;
}
+ else
+ {
+ md = EVP_sha1();
+ }
if (!EVP_DigestSignInit(&md_ctx, NULL, md, NULL, pkey) ||
!EVP_DigestSignUpdate(&md_ctx, s->s3->client_random, SSL3_RANDOM_SIZE) ||
!EVP_DigestSignUpdate(&md_ctx, s->s3->server_random, SSL3_RANDOM_SIZE) ||
@@ -1637,13 +1642,6 @@
if (SSL_USE_SIGALGS(s))
n += 2;
}
- else
- {
- /* Is this error check actually needed? */
- al=SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, SSL_R_UNKNOWN_PKEY_TYPE);
- goto f_err;
- }
}
ssl_set_handshake_header(s, SSL3_MT_SERVER_KEY_EXCHANGE, n);
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 761192f..dddfb9b 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -156,14 +156,6 @@
return ssl_x509_store_ctx_idx;
}
-void ssl_cert_set_default_md(CERT *cert)
- {
- /* Set digest values to defaults */
- cert->pkeys[SSL_PKEY_RSA_SIGN].digest = EVP_sha1();
- cert->pkeys[SSL_PKEY_RSA_ENC].digest = EVP_sha1();
- cert->pkeys[SSL_PKEY_ECC].digest = EVP_sha1();
- }
-
CERT *ssl_cert_new(void)
{
CERT *ret;
@@ -177,7 +169,6 @@
memset(ret,0,sizeof(CERT));
ret->key= &(ret->pkeys[SSL_PKEY_RSA_ENC]);
- ssl_cert_set_default_md(ret);
return(ret);
}
@@ -292,10 +283,6 @@
}
}
- /* Set digests to defaults. NB: we don't copy existing values as they
- * will be set during handshake.
- */
- ssl_cert_set_default_md(ret);
/* Peer sigalgs set to NULL as we get these from handshake too */
ret->peer_sigalgs = NULL;
ret->peer_sigalgslen = 0;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 2059032..94383e5 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2242,7 +2242,7 @@
return &s->cert->pkeys[i];
}
-EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *cipher, const EVP_MD **pmd)
+EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *cipher)
{
unsigned long alg_a;
CERT *c;
@@ -2266,8 +2266,6 @@
OPENSSL_PUT_ERROR(SSL, ssl_get_sign_pkey, ERR_R_INTERNAL_ERROR);
return(NULL);
}
- if (pmd)
- *pmd = c->pkeys[idx].digest;
return c->pkeys[idx].privatekey;
}
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 7f20b84..083e2f4 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -425,8 +425,6 @@
{
X509 *x509;
EVP_PKEY *privatekey;
- /* Digest to use when signing */
- const EVP_MD *digest;
/* Chain for this certificate */
STACK_OF(X509) *chain;
} CERT_PKEY;
@@ -759,7 +757,6 @@
int ssl_clear_bad_session(SSL *s);
CERT *ssl_cert_new(void);
CERT *ssl_cert_dup(CERT *cert);
-void ssl_cert_set_default_md(CERT *cert);
int ssl_cert_inst(CERT **o);
void ssl_cert_clear_certs(CERT *c);
void ssl_cert_free(CERT *c);
@@ -806,7 +803,7 @@
int ssl_undefined_void_function(void);
int ssl_undefined_const_function(const SSL *s);
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);
+EVP_PKEY *ssl_get_sign_pkey(SSL *s,const SSL_CIPHER *c);
int ssl_cert_type(X509 *x,EVP_PKEY *pkey);
/* ssl_get_compatible_server_ciphers determines the key exchange and
@@ -1095,6 +1092,11 @@
int ssl_parse_clienthello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert);
long ssl_get_algorithm2(SSL *s);
int tls1_process_sigalgs(SSL *s, const CBS *sigalgs);
+
+/* tls1_choose_signing_digest returns a digest for use with |pkey| based on the
+ * peer's preferences recorded for |s| and the digests supported by |pkey|. */
+const EVP_MD *tls1_choose_signing_digest(SSL *s, EVP_PKEY *pkey);
+
size_t tls12_get_psigalgs(SSL *s, const unsigned char **psigs);
int tls12_check_peer_sigalg(const EVP_MD **out_md, int *out_alert, SSL *s,
CBS *cbs, EVP_PKEY *pkey);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index d88fdea..ffd80d8 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1426,7 +1426,6 @@
{
int renegotiate_seen = 0;
CBS extensions;
- size_t i;
s->should_ack_sni = 0;
s->s3->next_proto_neg_seen = 0;
@@ -1451,11 +1450,6 @@
OPENSSL_free(s->cert->shared_sigalgs);
s->cert->shared_sigalgs = NULL;
}
- /* Clear certificate digests. */
- for (i = 0; i < SSL_PKEY_NUM; i++)
- {
- s->cert->pkeys[i].digest = NULL;
- }
/* Clear ECC extensions */
if (s->s3->tmp.peer_ecpointformatlist != 0)
{
@@ -1815,9 +1809,6 @@
OPENSSL_PUT_ERROR(SSL, ssl_scan_clienthello_tlsext, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
return 0;
}
- /* If no signature algorithms extension set default values */
- if (!s->cert->peer_sigalgs)
- ssl_cert_set_default_md(s->cert);
return 1;
}
@@ -2566,14 +2557,16 @@
}
}
-static int tls12_get_pkey_idx(unsigned char sig_alg)
+/* tls12_get_pkey_type returns the EVP_PKEY type corresponding to TLS signature
+ * algorithm |sig_alg|. It returns -1 if the type is unknown. */
+static int tls12_get_pkey_type(uint8_t sig_alg)
{
switch(sig_alg)
{
case TLSEXT_signature_rsa:
- return SSL_PKEY_RSA_SIGN;
+ return EVP_PKEY_RSA;
case TLSEXT_signature_ecdsa:
- return SSL_PKEY_ECC;
+ return EVP_PKEY_EC;
}
return -1;
}
@@ -2620,7 +2613,7 @@
/* Skip disabled hashes or signature algorithms */
if (tls12_get_hash(ptmp[0]) == NULL)
continue;
- if (tls12_get_pkey_idx(ptmp[1]) == -1)
+ if (tls12_get_pkey_type(ptmp[1]) == -1)
continue;
for (j = 0, atmp = allow; j < allowlen; j+=2, atmp+=2)
{
@@ -2701,11 +2694,7 @@
int tls1_process_sigalgs(SSL *s, const CBS *sigalgs)
{
- int idx;
- size_t i;
- const EVP_MD *md;
CERT *c = s->cert;
- TLS_SIGALGS *sigptr;
/* Extension ignored for inappropriate versions */
if (!SSL_USE_SIGALGS(s))
@@ -2721,36 +2710,29 @@
return 0;
tls1_set_shared_sigalgs(s);
-
- for (i = 0, sigptr = c->shared_sigalgs;
- i < c->shared_sigalgslen; i++, sigptr++)
- {
- idx = tls12_get_pkey_idx(sigptr->rsign);
- if (idx > 0 && c->pkeys[idx].digest == NULL)
- {
- md = tls12_get_hash(sigptr->rhash);
- c->pkeys[idx].digest = md;
- if (idx == SSL_PKEY_RSA_SIGN)
- {
- c->pkeys[SSL_PKEY_RSA_ENC].digest = md;
- }
- }
-
- }
-
- /* Set any remaining keys to default values. NOTE: if alg is
- * not supported it stays as NULL.
- */
- if (!c->pkeys[SSL_PKEY_RSA_SIGN].digest)
- {
- c->pkeys[SSL_PKEY_RSA_SIGN].digest = EVP_sha1();
- c->pkeys[SSL_PKEY_RSA_ENC].digest = EVP_sha1();
- }
- if (!c->pkeys[SSL_PKEY_ECC].digest)
- c->pkeys[SSL_PKEY_ECC].digest = EVP_sha1();
return 1;
}
+const EVP_MD *tls1_choose_signing_digest(SSL *s, EVP_PKEY *pkey)
+ {
+ CERT *c = s->cert;
+ int type = EVP_PKEY_id(pkey);
+ size_t i;
+
+ /* Select the first shared digest supported by our key. */
+ for (i = 0; i < c->shared_sigalgslen; i++)
+ {
+ const EVP_MD *md = tls12_get_hash(c->shared_sigalgs[i].rhash);
+ if (md == NULL || tls12_get_pkey_type(c->shared_sigalgs[i].rsign) != type)
+ continue;
+ if (!EVP_PKEY_supports_digest(pkey, md))
+ continue;
+ return md;
+ }
+
+ /* If no suitable digest may be found, default to SHA-1. */
+ return EVP_sha1();
+ }
int SSL_get_sigalgs(SSL *s, int idx,
int *psign, int *phash, int *psignhash,
