Port Certificate Request parsing to crypto/bytestring

Along the way, clean up the certificate types code to not have the
hard-coded fixed-size array.

Change-Id: If3e5978f7c5099478a3dfa37a0a7059072f5454a
Reviewed-on: https://boringssl-review.googlesource.com/1103
Reviewed-by: Adam Langley <agl@google.com>

8 files changed, 43 insertions, 78 deletions

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index dfcc568f..7b140470 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1835,12 +1835,14 @@ static int ca_dn_cmp(const X509_NAME **a, const X509_NAME **b)
 int ssl3_get_certificate_request(SSL *s)
 	{
 	int ok,ret=0;
-	unsigned long n,nc,l;
-	unsigned int llen, ctype_num,i;
+	unsigned long n;
+	unsigned int i;
 	X509_NAME *xn=NULL;
-	const unsigned char *p,*q;
-	unsigned char *d;
 	STACK_OF(X509_NAME) *ca_sk=NULL;
+	CBS cbs;
+	CBS certificate_types;
+	CBS certificate_authorities;
+	const uint8_t *data;
 
 	n=s->method->ssl_get_message(s,
 		SSL3_ST_CR_CERT_REQ_A,
@@ -1885,7 +1887,7 @@ int ssl3_get_certificate_request(SSL *s)
 			}
 		}
 
-	p=d=(unsigned char *)s->init_msg;
+	CBS_init(&cbs, (uint8_t *)s->init_msg, n);
 
 	ca_sk = sk_X509_NAME_new(ca_dn_cmp);
 	if (ca_sk == NULL)
@@ -1895,33 +1897,24 @@ int ssl3_get_certificate_request(SSL *s)
 		}
 
 	/* get the certificate types */
-	ctype_num= *(p++);
-	if (s->cert->ctypes)
+	if (!CBS_get_u8_length_prefixed(&cbs, &certificate_types))
 		{
-		OPENSSL_free(s->cert->ctypes);
-		s->cert->ctypes = NULL;
+		ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+		OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR);
+		goto err;
 		}
-	if (ctype_num > SSL3_CT_NUMBER)
+	if (!CBS_stow(&certificate_types, &s->cert->ctypes, &s->cert->ctype_num))
 		{
-		/* If we exceed static buffer copy all to cert structure */
-		s->cert->ctypes = OPENSSL_malloc(ctype_num);
-		memcpy(s->cert->ctypes, p, ctype_num);
-		s->cert->ctype_num = (size_t)ctype_num;
-		ctype_num=SSL3_CT_NUMBER;
+		ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+		goto err;
 		}
-	for (i=0; i<ctype_num; i++)
-		s->s3->tmp.ctype[i]= p[i];
-	p+=p[-1];
 	if (SSL_USE_SIGALGS(s))
 		{
-		n2s(p, llen);
-		/* Check we have enough room for signature algorithms and
-		 * following length value.
-		 */
-		if ((unsigned long)(p - d + llen + 2) > n)
+		CBS supported_signature_algorithms;
+		if (!CBS_get_u16_length_prefixed(&cbs, &supported_signature_algorithms))
 			{
 			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
-			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DATA_LENGTH_TOO_LONG);
+			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_DECODE_ERROR);
 			goto err;
 			}
 		/* Clear certificate digests and validity flags */
@@ -1930,53 +1923,49 @@ 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 (!tls1_process_sigalgs(s,
+				CBS_data(&supported_signature_algorithms),
+				CBS_len(&supported_signature_algorithms)))
 			{
 			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
 			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_SIGNATURE_ALGORITHMS_ERROR);
 			goto err;
 			}
-		p += llen;
 		}
 
 	/* get the CA RDNs */
-	n2s(p,llen);
-#if 0
-{
-FILE *out;
-out=fopen("/tmp/vsign.der","w");
-fwrite(p,1,llen,out);
-fclose(out);
-}
-#endif
-
-	if ((unsigned long)(p - d + llen) != n)
+	if (!CBS_get_u16_length_prefixed(&cbs, &certificate_authorities))
 		{
 		ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
 		OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_LENGTH_MISMATCH);
 		goto err;
 		}
 
-	for (nc=0; nc<llen; )
+	while (CBS_len(&certificate_authorities) > 0)
 		{
-		n2s(p,l);
-		if ((l+nc+2) > llen)
+		CBS distinguished_name;
+		if (!CBS_get_u16_length_prefixed(&certificate_authorities, &distinguished_name))
 			{
 			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
 			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_CA_DN_TOO_LONG);
 			goto err;
 			}
 
-		q=p;
-
-		if ((xn=d2i_X509_NAME(NULL,&q,l)) == NULL)
+		data = CBS_data(&distinguished_name);
+		if ((xn=d2i_X509_NAME(NULL, &data, CBS_len(&distinguished_name))) == NULL)
 			{
 			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
 			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, ERR_R_ASN1_LIB);
 			goto err;
 			}
 
-		if (q != (p+l))
+		if (CBS_skip(&distinguished_name, data - CBS_data(&distinguished_name)))
+			{
+			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
+			OPENSSL_PUT_ERROR(SSL, ssl3_get_server_certificate, ERR_R_INTERNAL_ERROR);
+			goto err;
+			}
+		if (CBS_len(&distinguished_name) != 0)
 			{
 			ssl3_send_alert(s,SSL3_AL_FATAL,SSL_AD_DECODE_ERROR);
 			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, SSL_R_CA_DN_LENGTH_MISMATCH);
@@ -1987,14 +1976,10 @@ fclose(out);
 			OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, ERR_R_MALLOC_FAILURE);
 			goto err;
 			}
-
-		p+=l;
-		nc+=l+2;
 		}
 
 	/* we should setup a certificate to return.... */
 	s->s3->tmp.cert_req=1;
-	s->s3->tmp.ctype_num=ctype_num;
 	if (s->s3->tmp.ca_names != NULL)
 		sk_X509_NAME_pop_free(s->s3->tmp.ca_names,X509_NAME_free);
 	s->s3->tmp.ca_names=ca_sk;
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index ec777a79..8908a6a2 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3097,15 +3097,9 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
 		const unsigned char **pctype = parg;
 		if (s->server || !s->s3->tmp.cert_req)
 			return 0;
-		if (s->cert->ctypes)
-			{
-			if (pctype)
-				*pctype = s->cert->ctypes;
-			return (int)s->cert->ctype_num;
-			}
 		if (pctype)
-			*pctype = s->s3->tmp.ctype;
-		return (int)s->s3->tmp.ctype_num;
+			*pctype = s->cert->ctypes;
+		return (int)s->cert->ctype_num;
 		}
 
 	case SSL_CTRL_SET_CLIENT_CERT_TYPES:
diff --git a/ssl/ssl.h b/ssl/ssl.h
index 80165dca..ecc4e743 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2938,5 +2938,6 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_INAPPROPRIATE_FALLBACK 436
 #define SSL_R_CLIENTHELLO_PARSE_FAILED 437
 #define SSL_R_CONNECTION_REJECTED 438
+#define SSL_R_DECODE_ERROR 439
 
 #endif
diff --git a/ssl/ssl3.h b/ssl/ssl3.h
index 165a1cc4..5e600319 100644
--- a/ssl/ssl3.h
+++ b/ssl/ssl3.h
@@ -386,11 +386,6 @@ typedef struct ssl3_buffer_st
 #define SSL3_CT_RSA_EPHEMERAL_DH		5
 #define SSL3_CT_DSS_EPHEMERAL_DH		6
 #define SSL3_CT_FORTEZZA_DMS			20
-/* SSL3_CT_NUMBER is used to size arrays and it must be large
- * enough to contain all of the cert types defined either for
- * SSLv3 and TLSv1.
- */
-#define SSL3_CT_NUMBER			9
 
 
 #define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS	0x0001
@@ -511,8 +506,6 @@ typedef struct ssl3_state_st
 
 		/* used for certificate requests */
 		int cert_req;
-		size_t ctype_num;
-		unsigned char ctype[SSL3_CT_NUMBER];
 		STACK_OF(X509_NAME) *ca_names;
 
 		int use_rsa_tmp;
diff --git a/ssl/ssl_error.c b/ssl/ssl_error.c
index 9d2c4bd7..ac61bcfc 100644
--- a/ssl/ssl_error.c
+++ b/ssl/ssl_error.c
@@ -272,6 +272,7 @@ const ERR_STRING_DATA SSL_error_string_data[] = {
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_D2I_ECDSA_SIG), "D2I_ECDSA_SIG"},
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DATA_BETWEEN_CCS_AND_FINISHED), "DATA_BETWEEN_CCS_AND_FINISHED"},
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DATA_LENGTH_TOO_LONG), "DATA_LENGTH_TOO_LONG"},
+  {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECODE_ERROR), "DECODE_ERROR"},
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECRYPTION_FAILED), "DECRYPTION_FAILED"},
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC), "DECRYPTION_FAILED_OR_BAD_RECORD_MAC"},
   {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_DH_PUBLIC_VALUE_LENGTH_IS_WRONG), "DH_PUBLIC_VALUE_LENGTH_IS_WRONG"},
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index dfa7df46..714e3200 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -565,8 +565,7 @@ typedef struct cert_st
 	CERT_PKEY pkeys[SSL_PKEY_NUM];
 
 	/* Certificate types (received or sent) in certificate request
-	 * message. On receive this is only set if number of certificate
-	 * types exceeds SSL3_CT_NUMBER.
+	 * message.
 	 */
 	unsigned char *ctypes;
 	size_t ctype_num;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 6f3089da..7bcdf90d 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3354,6 +3354,9 @@ int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize)
 	/* Extension ignored for inappropriate versions */
 	if (!SSL_USE_SIGALGS(s))
 		return 1;
+	/* Length must be even */
+	if (dsize % 2 != 0)
+		return 0;
 	/* Should never happen */
 	if (!c)
 		return 0;
@@ -3915,16 +3918,8 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
 			{
 			const unsigned char *ctypes;
 			int ctypelen;
-			if (c->ctypes)
-				{
-				ctypes = c->ctypes;
-				ctypelen = (int)c->ctype_num;
-				}
-			else
-				{
-				ctypes = (unsigned char *)s->s3->tmp.ctype;
-				ctypelen = s->s3->tmp.ctype_num;
-				}
+			ctypes = c->ctypes;
+			ctypelen = (int)c->ctype_num;
 			for (i = 0; i < ctypelen; i++)
 				{
 				if (ctypes[i] == check_type)
diff --git a/ssl/tls1.h b/ssl/tls1.h
index 81daa73a..e6ebd7e7 100644
--- a/ssl/tls1.h
+++ b/ssl/tls1.h
@@ -708,9 +708,6 @@ SSL_CTX_callback_ctrl(ssl,SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB,(void (*)(void))cb)
 #define TLS_CT_ECDSA_FIXED_ECDH 	66
 #define TLS_CT_GOST94_SIGN		21
 #define TLS_CT_GOST01_SIGN		22
-/* when correcting this number, correct also SSL3_CT_NUMBER in ssl3.h (see
- * comment there) */
-#define TLS_CT_NUMBER			9
 
 #define TLS1_FINISH_MAC_LENGTH		12