Process signature algorithms in ClientHello late.

Reviewed-by: Tim Hudson <tjh@openssl.org>

5 files changed, 62 insertions, 35 deletions

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index e8e374a0ba..d3836bfd77 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -2191,12 +2191,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 f45264ce59..343247ccf4 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2682,6 +2682,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_F_SSL_CERT_INSTANTIATE			 214
 #define SSL_F_SSL_CERT_NEW				 162
 #define SSL_F_SSL_CERT_SET0_CHAIN			 340
+#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 fabdc884b9..3c83fc8ae5 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -201,6 +201,7 @@ static ERR_STRING_DATA SSL_str_functs[]=
 {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_CERT_SET0_CHAIN),	"ssl_cert_set0_chain"},
+{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 3f87da7d53..1fd6bb1e06 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1382,7 +1382,8 @@ int ssl_parse_clienthello_renegotiate_ext(SSL *s, unsigned char *d, int len,
 long ssl_get_algorithm2(SSL *s);
 size_t tls12_copy_sigalgs(SSL *s, unsigned char *out,
 				const unsigned char *psig, size_t psiglen);
-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 77fe232526..d0e0b0f2a4 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1920,7 +1920,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;
@@ -1950,18 +1949,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;
-		}
 
 #ifdef TLSEXT_TYPE_encrypt_then_mac
 	s->s3->flags &= ~TLS1_FLAGS_ENCRYPT_THEN_MAC;
@@ -2252,21 +2239,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)
 			{
@@ -2476,9 +2453,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;
 	}
@@ -2998,6 +2972,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
@@ -3043,6 +3018,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)
 		{
@@ -3771,13 +3783,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;
@@ -3792,8 +3800,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)