commit a5177cb319f5ed5cb28f72bd564b4d6d6d355975
author David Benjamin <davidben@google.com> Fri Mar 11 19:38:17 2016 -0500
committer David Benjamin <davidben@google.com> Mon Mar 14 23:12:35 2016 +0000
tree 59d182cd1851ff019696776be6742db2d444d8eb
parent 6b6e0b20893e2be0e68af605a60ffa2cbb0ffa64

Use a less tedious pattern for X509_NAME.

Also fix a long/unsigned-long cast. (ssl_get_message returns long. It really
shouldn't, but ssl_get_message needs much more work than just a long -> size_t
change, so leave it as long for now.)

Change-Id: Ice8741f62a138c0f35ca735eedb541440f57e114
Reviewed-on: https://boringssl-review.googlesource.com/7457
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index fee0b51..3035d34 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1306,17 +1306,12 @@
 
 int ssl3_get_certificate_request(SSL *ssl) {
   int ok, ret = 0;
-  unsigned long n;
   X509_NAME *xn = NULL;
   STACK_OF(X509_NAME) *ca_sk = NULL;
-  CBS cbs;
-  CBS certificate_types;
-  CBS certificate_authorities;
-  const uint8_t *data;
 
-  n = ssl->method->ssl_get_message(ssl, SSL3_ST_CR_CERT_REQ_A,
-                                   SSL3_ST_CR_CERT_REQ_B, -1,
-                                   ssl->max_cert_list, ssl_hash_message, &ok);
+  long n = ssl->method->ssl_get_message(
+      ssl, SSL3_ST_CR_CERT_REQ_A, SSL3_ST_CR_CERT_REQ_B, -1, ssl->max_cert_list,
+      ssl_hash_message, &ok);
 
   if (!ok) {
     return n;
@@ -1338,6 +1333,7 @@
     goto err;
   }
 
+  CBS cbs;
   CBS_init(&cbs, ssl->init_msg, n);
 
   ca_sk = sk_X509_NAME_new(ca_dn_cmp);
@@ -1347,6 +1343,7 @@
   }
 
   /* get the certificate types */
+  CBS certificate_types;
   if (!CBS_get_u8_length_prefixed(&cbs, &certificate_types)) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
     OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
@@ -1370,6 +1367,7 @@
   }
 
   /* get the CA RDNs */
+  CBS certificate_authorities;
   if (!CBS_get_u16_length_prefixed(&cbs, &certificate_authorities)) {
     ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
     OPENSSL_PUT_ERROR(SSL, SSL_R_LENGTH_MISMATCH);
@@ -1385,25 +1383,13 @@
       goto err;
     }
 
-    data = CBS_data(&distinguished_name);
-
+    const uint8_t *data = CBS_data(&distinguished_name);
     /* A u16 length cannot overflow a long. */
     xn = d2i_X509_NAME(NULL, &data, (long)CBS_len(&distinguished_name));
-    if (xn == NULL) {
+    if (xn == NULL ||
+        data != CBS_data(&distinguished_name) + CBS_len(&distinguished_name)) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-      OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB);
-      goto err;
-    }
-
-    if (!CBS_skip(&distinguished_name, data - CBS_data(&distinguished_name))) {
-      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
-      goto err;
-    }
-
-    if (CBS_len(&distinguished_name) != 0) {
-      ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
-      OPENSSL_PUT_ERROR(SSL, SSL_R_CA_DN_LENGTH_MISMATCH);
+      OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
       goto err;
     }