From e54e43a6f31368338de68eeea77a87ad2be5b85f Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Mon, 25 Mar 2019 17:21:18 +0000
Subject: Defer sigalgs selection until the certificate is known.

Previously the signature algorithm was selected when the TLS extension was
parsed (or the client received a certificate request), however the actual
certificate to be used is not known at this stage. This leads to various
problems, including the selection of a signature algorithm that cannot be
used with the certificate key size (as found by jeremy@ via ruby regress).

Instead, store the signature algorithms list and only select a signature
algorithm when we're ready to do signature generation.

Joint work with beck@.
---
 src/lib/libssl/ssl_clnt.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

(limited to 'src/lib/libssl/ssl_clnt.c')

diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 262e09fe5e..2174e3a83d 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.59 2019/03/25 16:35:48 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.60 2019/03/25 17:21:18 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1512,7 +1512,7 @@ ssl3_get_server_key_exchange(SSL *s)
 			if (!CBS_get_u16(&cbs, &sigalg_value))
 				goto truncated;
 			if ((sigalg = ssl_sigalg(sigalg_value, tls12_sigalgs,
-				    tls12_sigalgs_len)) == NULL) {
+			    tls12_sigalgs_len)) == NULL) {
 				SSLerror(s, SSL_R_UNKNOWN_DIGEST);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
@@ -1522,7 +1522,7 @@ ssl3_get_server_key_exchange(SSL *s)
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
 			}
-			if (!ssl_sigalg_pkey_ok(sigalg, pkey)) {
+			if (!ssl_sigalg_pkey_ok(sigalg, pkey, 0)) {
 				SSLerror(s, SSL_R_WRONG_SIGNATURE_TYPE);
 				al = SSL_AD_DECODE_ERROR;
 				goto f_err;
@@ -1671,21 +1671,19 @@ ssl3_get_certificate_request(SSL *s)
 			SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
 			goto err;
 		}
-
-		/* Check we have enough room for signature algorithms and
-		 * following length value.
-		 */
 		if (!CBS_get_u16_length_prefixed(&cert_request, &sigalgs)) {
 			ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
 			SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
 			goto err;
 		}
-		if (!tls1_process_sigalgs(s, &sigalgs, tls12_sigalgs,
-		    tls12_sigalgs_len)) {
+		if (CBS_len(&sigalgs) % 2 != 0 || CBS_len(&sigalgs) > 64) {
 			ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
 			SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
 			goto err;
 		}
+		if (!CBS_stow(&sigalgs, &S3I(s)->hs.sigalgs,
+		    &S3I(s)->hs.sigalgs_len))
+			goto err;
 	}
 
 	/* get the CA RDNs */
@@ -2372,6 +2370,7 @@ err:
 static int
 ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
 {
+	const struct ssl_sigalg *sigalg;
 	CBB cbb_signature;
 	EVP_PKEY_CTX *pctx = NULL;
 	EVP_PKEY *pkey;
@@ -2387,10 +2386,17 @@ ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
 	EVP_MD_CTX_init(&mctx);
 
 	pkey = s->cert->key->privatekey;
-	md = s->cert->key->sigalg->md();
+	if ((sigalg = ssl_sigalg_select(s, pkey)) == NULL) {
+		SSLerror(s, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
+		goto err;
+	}
+	if ((md = sigalg->md()) == NULL) {
+		SSLerror(s, SSL_R_UNKNOWN_DIGEST);
+		goto err;
+	}
 
 	if (!tls1_transcript_data(s, &hdata, &hdatalen) ||
-	    !CBB_add_u16(cert_verify, s->cert->key->sigalg->value)) {
+	    !CBB_add_u16(cert_verify, sigalg->value)) {
 		SSLerror(s, ERR_R_INTERNAL_ERROR);
 		goto err;
 	}
@@ -2398,7 +2404,7 @@ ssl3_send_client_verify_sigalgs(SSL *s, CBB *cert_verify)
 		SSLerror(s, ERR_R_EVP_LIB);
 		goto err;
 	}
-	if ((s->cert->key->sigalg->flags & SIGALG_FLAG_RSA_PSS) &&
+	if ((sigalg->flags & SIGALG_FLAG_RSA_PSS) &&
 	    (!EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING) ||
 	    !EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, -1))) {
 		SSLerror(s, ERR_R_EVP_LIB);
-- 
cgit v1.2.3-55-g6feb