From 51695b98f128f8e091256c601266b1dd4fb731bd Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Thu, 9 Oct 2014 20:37:27 +0100 Subject: [PATCH] Process signature algorithms in ClientHello late. Reviewed-by: Tim Hudson (cherry picked from commit c800c27a8c47c8e63254ec594682452c296f1e8e) Conflicts: ssl/ssl.h ssl/ssl_err.c ssl/ssl_locl.h --- ssl/s3_clnt.c | 8 ++++- ssl/ssl.h | 1 + ssl/ssl_err.c | 1 + ssl/ssl_locl.h | 3 +- ssl/t1_lib.c | 84 ++++++++++++++++++++++++++++++-------------------- 5 files changed, 62 insertions(+), 35 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 3e89e5204d..7d526ddac7 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -2168,12 +2168,18 @@ int ssl3_get_certificate_request(SSL *s) s->cert->pkeys[i].digest = NULL; s->cert->pkeys[i].valid_flags = 0; } - if ((llen & 1) || !tls1_process_sigalgs(s, p, llen)) + if ((llen & 1) || !tls1_save_sigalgs(s, p, llen)) { ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR); SSLerr(SSL_F_SSL3_GET_CERTIFICATE_REQUEST,SSL_R_SIGNATURE_ALGORITHMS_ERROR); goto err; } + if (!tls1_process_sigalgs(s)) + { + ssl3_send_alert(s,SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + SSLerr(SSL_F_SSL3_GET_CERTIFICATE_REQUEST, ERR_R_MALLOC_FAILURE); + goto err; + } p += llen; } diff --git a/ssl/ssl.h b/ssl/ssl.h index 9e3ca9d3f2..bbf3180276 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -2582,6 +2582,7 @@ void ERR_load_SSL_strings(void); #define SSL_F_SSL_CERT_INST 222 #define SSL_F_SSL_CERT_INSTANTIATE 214 #define SSL_F_SSL_CERT_NEW 162 +#define SSL_F_SSL_CHECK_CLIENTHELLO_TLSEXT_LATE 335 #define SSL_F_SSL_CHECK_PRIVATE_KEY 163 #define SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT 280 #define SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG 279 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 63b5c35bea..bcd124ed6b 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -199,6 +199,7 @@ static ERR_STRING_DATA SSL_str_functs[]= {ERR_FUNC(SSL_F_SSL_CERT_INST), "ssl_cert_inst"}, {ERR_FUNC(SSL_F_SSL_CERT_INSTANTIATE), "SSL_CERT_INSTANTIATE"}, {ERR_FUNC(SSL_F_SSL_CERT_NEW), "ssl_cert_new"}, +{ERR_FUNC(SSL_F_SSL_CHECK_CLIENTHELLO_TLSEXT_LATE), "ssl_check_clienthello_tlsext_late"}, {ERR_FUNC(SSL_F_SSL_CHECK_PRIVATE_KEY), "SSL_check_private_key"}, {ERR_FUNC(SSL_F_SSL_CHECK_SERVERHELLO_TLSEXT), "SSL_CHECK_SERVERHELLO_TLSEXT"}, {ERR_FUNC(SSL_F_SSL_CHECK_SRVR_ECC_CERT_AND_ALG), "ssl_check_srvr_ecc_cert_and_alg"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index ce5856aa8d..6d8047c91c 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1353,7 +1353,8 @@ int ssl_add_clienthello_renegotiate_ext(SSL *s, unsigned char *p, int *len, int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len, int *al); long ssl_get_algorithm2(SSL *s); -int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize); +int tls1_save_sigalgs(SSL *s, const unsigned char *data, int dsize); +int tls1_process_sigalgs(SSL *s); size_t tls12_get_psigalgs(SSL *s, const unsigned char **psigs); int tls12_check_peer_sigalg(const EVP_MD **pmd, SSL *s, const unsigned char *sig, EVP_PKEY *pkey); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index fb0b736d1c..23aee0794b 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1908,7 +1908,6 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char unsigned short len; unsigned char *data = *p; int renegotiate_seen = 0; - size_t i; s->servername_done = 0; s->tlsext_status_type = -1; @@ -1938,18 +1937,6 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char OPENSSL_free(s->cert->peer_sigalgs); s->cert->peer_sigalgs = NULL; } - /* Clear any shared sigtnature algorithms */ - if (s->cert->shared_sigalgs) - { - OPENSSL_free(s->cert->shared_sigalgs); - s->cert->shared_sigalgs = NULL; - } - /* Clear certificate digests and validity flags */ - for (i = 0; i < SSL_PKEY_NUM; i++) - { - s->cert->pkeys[i].digest = NULL; - s->cert->pkeys[i].valid_flags = 0; - } if (data >= (d+n-2)) goto ri_check; @@ -2236,21 +2223,11 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *al = SSL_AD_DECODE_ERROR; return 0; } - if (!tls1_process_sigalgs(s, data, dsize)) + if (!tls1_save_sigalgs(s, data, dsize)) { *al = SSL_AD_DECODE_ERROR; return 0; } - /* If sigalgs received and no shared algorithms fatal - * error. - */ - if (s->cert->peer_sigalgs && !s->cert->shared_sigalgs) - { - SSLerr(SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT, - SSL_R_NO_SHARED_SIGATURE_ALGORITHMS); - *al = SSL_AD_ILLEGAL_PARAMETER; - return 0; - } } else if (type == TLSEXT_TYPE_status_request) { @@ -2442,9 +2419,6 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char 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; } @@ -3000,6 +2974,7 @@ int ssl_check_clienthello_tlsext_late(SSL *s) { int ret = SSL_TLSEXT_ERR_OK; int al; + size_t i; /* If status request then ask callback what to do. * Note: this must be called after servername callbacks in case @@ -3045,6 +3020,43 @@ int ssl_check_clienthello_tlsext_late(SSL *s) else s->tlsext_status_expected = 0; + /* Clear any shared sigtnature algorithms */ + if (s->cert->shared_sigalgs) + { + OPENSSL_free(s->cert->shared_sigalgs); + s->cert->shared_sigalgs = NULL; + } + /* Clear certificate digests and validity flags */ + for (i = 0; i < SSL_PKEY_NUM; i++) + { + s->cert->pkeys[i].digest = NULL; + s->cert->pkeys[i].valid_flags = 0; + } + + /* If sigalgs received process it. */ + if (s->cert->peer_sigalgs) + { + if (!tls1_process_sigalgs(s)) + { + SSLerr(SSL_F_SSL_CHECK_CLIENTHELLO_TLSEXT_LATE, + ERR_R_MALLOC_FAILURE); + ret = SSL_TLSEXT_ERR_ALERT_FATAL; + al = SSL_AD_INTERNAL_ERROR; + goto err; + } + /* Fatal error is no shared signature algorithms */ + if (!s->cert->shared_sigalgs) + { + SSLerr(SSL_F_SSL_CHECK_CLIENTHELLO_TLSEXT_LATE, + SSL_R_NO_SHARED_SIGATURE_ALGORITHMS); + ret = SSL_TLSEXT_ERR_ALERT_FATAL; + al = SSL_AD_ILLEGAL_PARAMETER; + goto err; + } + } + else + ssl_cert_set_default_md(s->cert); + err: switch (ret) { @@ -3677,13 +3689,9 @@ static int tls1_set_shared_sigalgs(SSL *s) /* Set preferred digest for each key type */ -int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize) +int tls1_save_sigalgs(SSL *s, const unsigned char *data, int dsize) { - 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)) return 1; @@ -3698,8 +3706,18 @@ int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize) return 0; c->peer_sigalgslen = dsize; memcpy(c->peer_sigalgs, data, dsize); + return 1; + } - tls1_set_shared_sigalgs(s); +int tls1_process_sigalgs(SSL *s) + { + int idx; + size_t i; + const EVP_MD *md; + CERT *c = s->cert; + TLS_SIGALGS *sigptr; + if (!tls1_set_shared_sigalgs(s)) + return 0; #ifdef OPENSSL_SSL_DEBUG_BROKEN_PROTOCOL if (s->cert->cert_flags & SSL_CERT_FLAG_BROKEN_PROTOCOL)