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>
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index dfcc568..7b14047 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1835,12 +1835,14 @@
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 @@
}
}
- 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 @@
}
/* 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 @@
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 @@
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 ec777a7..8908a6a 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3097,15 +3097,9 @@
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 80165dc..ecc4e74 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2938,5 +2938,6 @@
#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 165a1cc..5e60031 100644
--- a/ssl/ssl3.h
+++ b/ssl/ssl3.h
@@ -386,11 +386,6 @@
#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 @@
/* 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 9d2c4bd..ac61bcf 100644
--- a/ssl/ssl_error.c
+++ b/ssl/ssl_error.c
@@ -272,6 +272,7 @@
{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 dfa7df4..714e320 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -565,8 +565,7 @@
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 6f3089d..7bcdf90 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3354,6 +3354,9 @@
/* 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 @@
{
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 81daa73..e6ebd7e 100644
--- a/ssl/tls1.h
+++ b/ssl/tls1.h
@@ -708,9 +708,6 @@
#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
