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>
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; }