Clean up TLSv1.2 certificate request handshake data.

Currently cert_req is used by clients and cert_request is used by servers.
Replace this by a single cert_request used by either client or server.
Remove the certificate types as they are currently unused. This also fixes
a bug whereby if the number of certificate types exceeds SSL3_CT_NUMBER
the number of bytes read in is insufficient, which will break decoding.

ok inoguchi@ tb@

5 files changed, 27 insertions, 43 deletions

diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index 6563de5be27..9dd6343b84e 100644
--- a/lib/libssl/s3_lib.c
+++ b/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.207 2021/04/19 16:47:25 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.208 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1570,7 +1570,7 @@ ssl3_free(SSL *s)
 	freezero(S3I(s)->hs.tls13.cookie, S3I(s)->hs.tls13.cookie_len);
 	tls13_clienthello_hash_clear(&S3I(s)->hs.tls13);
 
-	sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
+	sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
 
 	tls1_transcript_free(s);
 	tls1_transcript_hash_free(s);
@@ -1591,7 +1591,7 @@ ssl3_clear(SSL *s)
 	size_t		 rlen, wlen;
 
 	tls1_cleanup_key_block(s);
-	sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
+	sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
 
 	DH_free(S3I(s)->tmp.dh);
 	S3I(s)->tmp.dh = NULL;
diff --git a/lib/libssl/ssl_cert.c b/lib/libssl/ssl_cert.c
index 03ef8565ac5..d122c80f2cd 100644
--- a/lib/libssl/ssl_cert.c
+++ b/lib/libssl/ssl_cert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_cert.c,v 1.81 2021/03/27 17:56:28 tb Exp $ */
+/* $OpenBSD: ssl_cert.c,v 1.82 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -508,7 +508,7 @@ SSL_get_client_CA_list(const SSL *s)
 	if (!s->server) {
 		/* We are in the client. */
 		if ((s->version >> 8) == SSL3_VERSION_MAJOR)
-			return (S3I(s)->tmp.ca_names);
+			return (S3I(s)->hs.tls12.ca_names);
 		else
 			return (NULL);
 	} else {
diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c
index 6b43b565b98..7f69b8ba980 100644
--- a/lib/libssl/ssl_clnt.c
+++ b/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.91 2021/04/19 16:51:56 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.92 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -391,7 +391,7 @@ ssl3_connect(SSL *s)
 				goto end;
 			if (SSL_is_dtls(s))
 				dtls1_stop_timer(s);
-			if (S3I(s)->tmp.cert_req)
+			if (S3I(s)->hs.tls12.cert_request)
 				S3I(s)->hs.state = SSL3_ST_CW_CERT_A;
 			else
 				S3I(s)->hs.state = SSL3_ST_CW_KEY_EXCH_A;
@@ -435,7 +435,7 @@ ssl3_connect(SSL *s)
 			 * message when client's ECDH public key is sent
 			 * inside the client certificate.
 			 */
-			if (S3I(s)->tmp.cert_req == 1) {
+			if (S3I(s)->hs.tls12.cert_request == 1) {
 				S3I(s)->hs.state = SSL3_ST_CW_CERT_VRFY_A;
 			} else {
 				S3I(s)->hs.state = SSL3_ST_CW_CHANGE_A;
@@ -1650,8 +1650,7 @@ ssl3_get_certificate_request(SSL *s)
 {
 	int			 ok, ret = 0;
 	long			 n;
-	uint8_t			 ctype_num;
-	CBS			 cert_request, ctypes, rdn_list;
+	CBS			 cert_request, cert_types, rdn_list;
 	X509_NAME		*xn = NULL;
 	const unsigned char	*q;
 	STACK_OF(X509_NAME)	*ca_sk = NULL;
@@ -1661,7 +1660,7 @@ ssl3_get_certificate_request(SSL *s)
 	if (!ok)
 		return ((int)n);
 
-	S3I(s)->tmp.cert_req = 0;
+	S3I(s)->hs.tls12.cert_request = 0;
 
 	if (S3I(s)->hs.tls12.message_type == SSL3_MT_SERVER_DONE) {
 		S3I(s)->hs.tls12.reuse_message = 1;
@@ -1695,19 +1694,9 @@ ssl3_get_certificate_request(SSL *s)
 		goto err;
 	}
 
-	/* get the certificate types */
-	if (!CBS_get_u8(&cert_request, &ctype_num))
+	if (!CBS_get_u8_length_prefixed(&cert_request, &cert_types))
 		goto decode_err;
 
-	if (ctype_num > SSL3_CT_NUMBER)
-		ctype_num = SSL3_CT_NUMBER;
-	if (!CBS_get_bytes(&cert_request, &ctypes, ctype_num) ||
-	    !CBS_write_bytes(&ctypes, (uint8_t *)S3I(s)->tmp.ctype,
-	    sizeof(S3I(s)->tmp.ctype), NULL)) {
-		SSLerror(s, SSL_R_DATA_LENGTH_TOO_LONG);
-		goto err;
-	}
-
 	if (SSL_USE_SIGALGS(s)) {
 		CBS sigalgs;
 
@@ -1778,10 +1767,9 @@ ssl3_get_certificate_request(SSL *s)
 	}
 
 	/* we should setup a certificate to return.... */
-	S3I(s)->tmp.cert_req = 1;
-	S3I(s)->tmp.ctype_num = ctype_num;
-	sk_X509_NAME_pop_free(S3I(s)->tmp.ca_names, X509_NAME_free);
-	S3I(s)->tmp.ca_names = ca_sk;
+	S3I(s)->hs.tls12.cert_request = 1;
+	sk_X509_NAME_pop_free(S3I(s)->hs.tls12.ca_names, X509_NAME_free);
+	S3I(s)->hs.tls12.ca_names = ca_sk;
 	ca_sk = NULL;
 
 	ret = 1;
@@ -2228,7 +2216,7 @@ ssl3_send_client_kex_gost(SSL *s, SESS_CERT *sess_cert, CBB *cbb)
 	/*
 	 * If we have client certificate, use its secret as peer key.
 	 */
-	if (S3I(s)->tmp.cert_req && s->cert->key->privatekey) {
+	if (S3I(s)->hs.tls12.cert_request && s->cert->key->privatekey) {
 		if (EVP_PKEY_derive_set_peer(pkey_ctx,
 		    s->cert->key->privatekey) <=0) {
 			/*
@@ -2681,7 +2669,7 @@ ssl3_send_client_certificate(SSL *s)
 		X509_free(x509);
 		EVP_PKEY_free(pkey);
 		if (i == 0) {
-			S3I(s)->tmp.cert_req = 2;
+			S3I(s)->hs.tls12.cert_request = 2;
 
 			/* There is no client certificate to verify. */
 			tls1_transcript_free(s);
@@ -2696,7 +2684,7 @@ ssl3_send_client_certificate(SSL *s)
 		    SSL3_MT_CERTIFICATE))
 			goto err;
 		if (!ssl3_output_cert_chain(s, &client_cert,
-		    (S3I(s)->tmp.cert_req == 2) ? NULL : s->cert->key))
+		    (S3I(s)->hs.tls12.cert_request == 2) ? NULL : s->cert->key))
 			goto err;
 		if (!ssl3_handshake_msg_finish(s, &cbb))
 			goto err;
diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 86d1b6e10b2..27397308efa 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.336 2021/04/19 17:26:39 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.337 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -427,6 +427,10 @@ typedef struct ssl_handshake_tls12_st {
 	/* Reuse current handshake message. */
 	int reuse_message;
 
+	/* Client certificate requests. */
+	int cert_request;
+	STACK_OF(X509_NAME) *ca_names;
+
 	/* Size of the MAC secret. */
 	int mac_secret_size;
 
@@ -946,14 +950,6 @@ typedef struct ssl3_state_internal_st {
 		int ecdh_nid;
 
 		uint8_t *x25519;
-
-		/* used for certificate requests */
-		int cert_req;
-		int ctype_num;
-		char ctype[SSL3_CT_NUMBER];
-		STACK_OF(X509_NAME) *ca_names;
-
-		int cert_request;
 	} tmp;
 
 	/* Connection binding to prevent renegotiation attacks */
diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 8241a59ac07..c85a25158f8 100644
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.102 2021/04/19 16:51:56 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.103 2021/04/21 19:27:56 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -464,13 +464,13 @@ ssl3_accept(SSL *s)
 			     SSL_VERIFY_FAIL_IF_NO_PEER_CERT))) {
 				/* No cert request. */
 				skip = 1;
-				S3I(s)->tmp.cert_request = 0;
+				S3I(s)->hs.tls12.cert_request = 0;
 				S3I(s)->hs.state = SSL3_ST_SW_SRVR_DONE_A;
 
 				if (!SSL_is_dtls(s))
 					tls1_transcript_free(s);
 			} else {
-				S3I(s)->tmp.cert_request = 1;
+				S3I(s)->hs.tls12.cert_request = 1;
 				if (SSL_is_dtls(s))
 					dtls1_start_timer(s);
 				ret = ssl3_send_certificate_request(s);
@@ -522,7 +522,7 @@ ssl3_accept(SSL *s)
 
 		case SSL3_ST_SR_CERT_A:
 		case SSL3_ST_SR_CERT_B:
-			if (S3I(s)->tmp.cert_request) {
+			if (S3I(s)->hs.tls12.cert_request) {
 				ret = ssl3_get_client_certificate(s);
 				if (ret <= 0)
 					goto end;
@@ -2379,7 +2379,7 @@ ssl3_get_client_certificate(SSL *s)
 		 * If tls asked for a client cert,
 		 * the client must return a 0 list.
 		 */
-		if (S3I(s)->tmp.cert_request) {
+		if (S3I(s)->hs.tls12.cert_request) {
 			SSLerror(s, SSL_R_TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST
 			    );
 			al = SSL_AD_UNEXPECTED_MESSAGE;