Convert ssl3_send_certificate_request() to CBB.

ok beck@ doug@

lib/libssl/s3_lib.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.155 2017/08/10 17:18:38 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.156 2017/08/11 17:54:41 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -2438,36 +2438,45 @@ ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
 }
 
 int
-ssl3_get_req_cert_type(SSL *s, unsigned char *p)
+ssl3_get_req_cert_types(SSL *s, CBB *cbb)
 {
-	int		ret = 0;
+	unsigned long alg_k;
-	unsigned long	alg_k;
 
 	alg_k = S3I(s)->hs.new_cipher->algorithm_mkey;
 
 #ifndef OPENSSL_NO_GOST
-	if ((alg_k & SSL_kGOST)) {
+	if ((alg_k & SSL_kGOST) != 0) {
-		p[ret++] = TLS_CT_GOST94_SIGN;
+		if (!CBB_add_u8(cbb, TLS_CT_GOST94_SIGN))
-		p[ret++] = TLS_CT_GOST01_SIGN;
+			return 0;
-		p[ret++] = TLS_CT_GOST12_256_SIGN;
+		if (!CBB_add_u8(cbb, TLS_CT_GOST01_SIGN))
-		p[ret++] = TLS_CT_GOST12_512_SIGN;
+			return 0;
+		if (!CBB_add_u8(cbb, TLS_CT_GOST12_256_SIGN))
+			return 0;
+		if (!CBB_add_u8(cbb, TLS_CT_GOST12_512_SIGN))
+			return 0;
 	}
 #endif
 
-	if (alg_k & SSL_kDHE) {
+	if ((alg_k & SSL_kDHE) != 0) {
-		p[ret++] = SSL3_CT_RSA_FIXED_DH;
+		if (!CBB_add_u8(cbb, SSL3_CT_RSA_FIXED_DH))
-		p[ret++] = SSL3_CT_DSS_FIXED_DH;
+			return 0;
+		if (!CBB_add_u8(cbb, SSL3_CT_DSS_FIXED_DH))
+			return 0;
 	}
-	p[ret++] = SSL3_CT_RSA_SIGN;
+
-	p[ret++] = SSL3_CT_DSS_SIGN;
+	if (!CBB_add_u8(cbb, SSL3_CT_RSA_SIGN))
+		return 0;
+	if (!CBB_add_u8(cbb, SSL3_CT_DSS_SIGN))
+		return 0;
 
 	/*
 	 * ECDSA certs can be used with RSA cipher suites as well
 	 * so we don't need to check for SSL_kECDH or SSL_kECDHE.
 	 */
-	p[ret++] = TLS_CT_ECDSA_SIGN;
+	if (!CBB_add_u8(cbb, TLS_CT_ECDSA_SIGN))
+		return 0;
 
-	return (ret);
+	return 1;
 }
 
 int

lib/libssl/ssl_locl.h

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.185 2017/08/11 05:06:34 doug Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.186 2017/08/11 17:54:41 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1138,7 +1138,7 @@ int ssl3_get_finished(SSL *s, int state_a, int state_b);
 int ssl3_send_change_cipher_spec(SSL *s, int state_a, int state_b);
 int ssl3_do_write(SSL *s, int type);
 int ssl3_send_alert(SSL *s, int level, int desc);
-int ssl3_get_req_cert_type(SSL *s, unsigned char *p);
+int ssl3_get_req_cert_types(SSL *s, CBB *cbb);
 long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok);
 int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen);
 int ssl3_num_ciphers(void);

lib/libssl/ssl_srvr.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.18 2017/08/10 17:18:38 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.19 2017/08/11 17:54:41 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1573,69 +1573,70 @@ ssl3_send_server_key_exchange(SSL *s)
 int
 ssl3_send_certificate_request(SSL *s)
 {
-	unsigned char *p, *d;
+	CBB cbb, cert_request, cert_types, sigalgs, cert_auth, dn;
-	int i, j, nl, off, n;
 	STACK_OF(X509_NAME) *sk = NULL;
 	X509_NAME *name;
-	BUF_MEM *buf;
+	int i;
+
+	/*
+	 * Certificate Request - RFC 5246 section 7.4.4.
+	 */
+
+	memset(&cbb, 0, sizeof(cbb));
 
 	if (S3I(s)->hs.state == SSL3_ST_SW_CERT_REQ_A) {
-		buf = s->internal->init_buf;
+		if (!ssl3_handshake_msg_start_cbb(s, &cbb, &cert_request,
+		    SSL3_MT_CERTIFICATE_REQUEST))
+			goto err;
 
-		d = p = ssl3_handshake_msg_start(s,
+		if (!CBB_add_u8_length_prefixed(&cert_request, &cert_types))
-		    SSL3_MT_CERTIFICATE_REQUEST);
+			goto err;
-
+		if (!ssl3_get_req_cert_types(s, &cert_types))
-		/* get the list of acceptable cert types */
+			goto err;
-		p++;
-		n = ssl3_get_req_cert_type(s, p);
-		d[0] = n;
-		p += n;
-		n++;
 
 		if (SSL_USE_SIGALGS(s)) {
-			nl = tls12_get_req_sig_algs(s, p + 2);
+			unsigned char *sigalgs_data;
-			s2n(nl, p);
+			size_t sigalgs_len;
-			p += nl + 2;
+
-			n += nl + 2;
+			sigalgs_len = tls12_get_req_sig_algs(s, NULL);
+			if (!CBB_add_u16_length_prefixed(&cert_request, &sigalgs))
+				goto err;
+			if (!CBB_add_space(&sigalgs, &sigalgs_data, sigalgs_len))
+				goto err;
+			tls12_get_req_sig_algs(s, sigalgs_data);
 		}
 
-		off = n;
+		if (!CBB_add_u16_length_prefixed(&cert_request, &cert_auth))
-		p += 2;
+			goto err;
-		n += 2;
 
 		sk = SSL_get_client_CA_list(s);
-		nl = 0;
+		for (i = 0; i < sk_X509_NAME_num(sk); i++) {
-		if (sk != NULL) {
+			unsigned char *name_data;
-			for (i = 0; i < sk_X509_NAME_num(sk); i++) {
+			size_t name_len;
-				name = sk_X509_NAME_value(sk, i);
-				j = i2d_X509_NAME(name, NULL);
-				if (!BUF_MEM_grow_clean(buf,
-				    ssl3_handshake_msg_hdr_len(s) + n + j
-				    + 2)) {
-					SSLerror(s, ERR_R_BUF_LIB);
-					goto err;
-				}
-				p = ssl3_handshake_msg_start(s,
-				    SSL3_MT_CERTIFICATE_REQUEST) + n;
-				s2n(j, p);
-				i2d_X509_NAME(name, &p);
-				n += 2 + j;
-				nl += 2 + j;
-			}
-		}
-		/* else no CA names */
-		p = ssl3_handshake_msg_start(s,
-		    SSL3_MT_CERTIFICATE_REQUEST) + off;
-		s2n(nl, p);
 
-		ssl3_handshake_msg_finish(s, n);
+			name = sk_X509_NAME_value(sk, i);
+			name_len = i2d_X509_NAME(name, NULL);
+
+			if (!CBB_add_u16_length_prefixed(&cert_auth, &dn))
+				goto err;
+			if (!CBB_add_space(&dn, &name_data, name_len))
+				goto err;
+			if (i2d_X509_NAME(name, &name_data) != name_len)
+				goto err;
+		}
+
+		if (!ssl3_handshake_msg_finish_cbb(s, &cbb))
+			goto err;
 
 		S3I(s)->hs.state = SSL3_ST_SW_CERT_REQ_B;
 	}
 
 	/* SSL3_ST_SW_CERT_REQ_B */
 	return (ssl3_handshake_write(s));
-err:
+
+ err:
+	CBB_cleanup(&cbb);
+
 	return (-1);
 }