From 5c900c8c4555a9de24c07a5d544b5f6aebb39252 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 13 Jul 2016 23:03:26 -0400 Subject: [PATCH] Factor out certificate list parsing. This is already duplicated between client and server and otherwise will get duplicated yet again for TLS 1.3. Change-Id: Ia8a352f9bc76fab0f88c1629d08a1da4c13d2510 Reviewed-on: https://boringssl-review.googlesource.com/8778 Reviewed-by: David Benjamin --- ssl/handshake_client.c | 79 +++++++--------------------- ssl/handshake_server.c | 116 +++++++++++++---------------------------- ssl/internal.h | 9 ++++ ssl/ssl_cert.c | 54 +++++++++++++++++++ 4 files changed, 116 insertions(+), 142 deletions(-) diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index a110aeae..267e164d 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -966,91 +966,48 @@ err: } static int ssl3_get_server_certificate(SSL *ssl) { - int al, ret = -1; - X509 *x = NULL; - STACK_OF(X509) *sk = NULL; - EVP_PKEY *pkey = NULL; - CBS cbs, certificate_list; - const uint8_t *data; - - int msg_ret = + int ret = ssl->method->ssl_get_message(ssl, SSL3_MT_CERTIFICATE, ssl_hash_message); - if (msg_ret <= 0) { - return msg_ret; + if (ret <= 0) { + return ret; } + CBS cbs; CBS_init(&cbs, ssl->init_msg, ssl->init_num); - - sk = sk_X509_new_null(); - if (sk == NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + uint8_t alert; + STACK_OF(X509) *chain = ssl_parse_cert_chain(ssl, &alert, NULL, &cbs); + if (chain == NULL) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); goto err; } - if (!CBS_get_u24_length_prefixed(&cbs, &certificate_list) || - CBS_len(&certificate_list) == 0 || - CBS_len(&cbs) != 0) { - al = SSL_AD_DECODE_ERROR; + if (sk_X509_num(chain) == 0 || CBS_len(&cbs) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - goto f_err; - } - - while (CBS_len(&certificate_list) > 0) { - CBS certificate; - if (!CBS_get_u24_length_prefixed(&certificate_list, &certificate)) { - al = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH); - goto f_err; - } - /* A u24 length cannot overflow a long. */ - data = CBS_data(&certificate); - x = d2i_X509(NULL, &data, (long)CBS_len(&certificate)); - if (x == NULL) { - al = SSL_AD_BAD_CERTIFICATE; - OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB); - goto f_err; - } - if (data != CBS_data(&certificate) + CBS_len(&certificate)) { - al = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH); - goto f_err; - } - if (!sk_X509_push(sk, x)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - goto err; - } - x = NULL; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; } - X509 *leaf = sk_X509_value(sk, 0); + X509 *leaf = sk_X509_value(chain, 0); if (!ssl3_check_leaf_certificate(ssl, leaf)) { - al = SSL_AD_ILLEGAL_PARAMETER; - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + goto err; } /* NOTE: Unlike the server half, the client's copy of |cert_chain| includes * the leaf. */ sk_X509_pop_free(ssl->session->cert_chain, X509_free); - ssl->session->cert_chain = sk; - sk = NULL; + ssl->session->cert_chain = chain; X509_free(ssl->session->peer); ssl->session->peer = X509_up_ref(leaf); ssl->session->verify_result = ssl->verify_result; - ret = 1; - - if (0) { - f_err: - ssl3_send_alert(ssl, SSL3_AL_FATAL, al); - } + return 1; err: - EVP_PKEY_free(pkey); - X509_free(x); - sk_X509_pop_free(sk, X509_free); - return ret; + sk_X509_pop_free(chain, X509_free); + return -1; } static int ssl3_get_cert_status(SSL *ssl) { diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index c041bb82..96b86238 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -165,7 +165,6 @@ #include #include #include -#include #include #include "internal.h" @@ -1247,13 +1246,6 @@ static int ssl3_send_server_hello_done(SSL *ssl) { } static int ssl3_get_client_certificate(SSL *ssl) { - int al, ret = -1; - X509 *x = NULL; - STACK_OF(X509) *sk = NULL; - SHA256_CTX sha256; - CBS certificate_msg, certificate_list; - int is_first_certificate = 1; - assert(ssl->s3->tmp.cert_request); int msg_ret = ssl->method->ssl_get_message(ssl, -1, ssl_hash_message); @@ -1268,120 +1260,82 @@ static int ssl3_get_client_certificate(SSL *ssl) { if ((ssl->verify_mode & SSL_VERIFY_PEER) && (ssl->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)) { OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE); - al = SSL_AD_HANDSHAKE_FAILURE; - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + return -1; } ssl->s3->tmp.reuse_message = 1; return 1; } - al = SSL_AD_UNEXPECTED_MESSAGE; OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE); - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE); + return -1; } + CBS certificate_msg; CBS_init(&certificate_msg, ssl->init_msg, ssl->init_num); - - sk = sk_X509_new_null(); - if (sk == NULL) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + uint8_t alert; + STACK_OF(X509) *chain = ssl_parse_cert_chain( + ssl, &alert, + ssl->ctx->retain_only_sha256_of_client_certs ? ssl->session->peer_sha256 + : NULL, + &certificate_msg); + if (chain == NULL) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); goto err; } - if (!CBS_get_u24_length_prefixed(&certificate_msg, &certificate_list) || - CBS_len(&certificate_msg) != 0) { - al = SSL_AD_DECODE_ERROR; + if (CBS_len(&certificate_msg) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - goto f_err; - } - - while (CBS_len(&certificate_list) > 0) { - CBS certificate; - const uint8_t *data; - - if (!CBS_get_u24_length_prefixed(&certificate_list, &certificate)) { - al = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - goto f_err; - } - - if (is_first_certificate && ssl->ctx->retain_only_sha256_of_client_certs) { - /* If this is the first certificate, and we don't want to keep peer - * certificates in memory, then we hash it right away. */ - SHA256_Init(&sha256); - SHA256_Update(&sha256, CBS_data(&certificate), CBS_len(&certificate)); - SHA256_Final(ssl->session->peer_sha256, &sha256); - ssl->session->peer_sha256_valid = 1; - } - is_first_certificate = 0; - - /* A u24 length cannot overflow a long. */ - data = CBS_data(&certificate); - x = d2i_X509(NULL, &data, (long)CBS_len(&certificate)); - if (x == NULL) { - al = SSL_AD_BAD_CERTIFICATE; - OPENSSL_PUT_ERROR(SSL, ERR_R_ASN1_LIB); - goto f_err; - } - if (data != CBS_data(&certificate) + CBS_len(&certificate)) { - al = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH); - goto f_err; - } - if (!sk_X509_push(sk, x)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - goto err; - } - x = NULL; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + goto err; } - if (sk_X509_num(sk) <= 0) { + if (sk_X509_num(chain) == 0) { /* No client certificate so the handshake buffer may be discarded. */ ssl3_free_handshake_buffer(ssl); /* TLS does not mind 0 certs returned */ if (ssl->version == SSL3_VERSION) { - al = SSL_AD_HANDSHAKE_FAILURE; OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATES_RETURNED); - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + goto err; } else if ((ssl->verify_mode & SSL_VERIFY_PEER) && (ssl->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)) { /* Fail for TLS only if we required a certificate */ OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE); - al = SSL_AD_HANDSHAKE_FAILURE; - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + goto err; } } else { - if (ssl_verify_cert_chain(ssl, sk) <= 0) { - al = ssl_verify_alarm_type(ssl->verify_result); + /* The hash would have been filled in. */ + if (ssl->ctx->retain_only_sha256_of_client_certs) { + ssl->session->peer_sha256_valid = 1; + } + + if (ssl_verify_cert_chain(ssl, chain) <= 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED); - goto f_err; + ssl3_send_alert(ssl, SSL3_AL_FATAL, + ssl_verify_alarm_type(ssl->verify_result)); + goto err; } } X509_free(ssl->session->peer); - ssl->session->peer = sk_X509_shift(sk); + ssl->session->peer = sk_X509_shift(chain); ssl->session->verify_result = ssl->verify_result; sk_X509_pop_free(ssl->session->cert_chain, X509_free); - ssl->session->cert_chain = sk; + ssl->session->cert_chain = chain; /* Inconsistency alert: cert_chain does *not* include the peer's own * certificate, while we do include it in s3_clnt.c */ - sk = NULL; - - ret = 1; - - if (0) { - f_err: - ssl3_send_alert(ssl, SSL3_AL_FATAL, al); - } + return 1; err: - X509_free(x); - sk_X509_pop_free(sk, X509_free); - return ret; + sk_X509_pop_free(chain, X509_free); + return -1; } static int ssl3_get_client_key_exchange(SSL *ssl) { diff --git a/ssl/internal.h b/ssl/internal.h index 81ac76e9..a6567ab7 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -738,6 +738,15 @@ void ssl_write_buffer_clear(SSL *ssl); * configured and zero otherwise. */ int ssl_has_certificate(const SSL *ssl); +/* ssl_parse_cert_chain parses a certificate list from |cbs| in the format used + * by a TLS Certificate message. On success, it returns a newly-allocated + * |X509| list and advances |cbs|. Otherwise, it returns NULL and sets + * |*out_alert| to an alert to send to the peer. If the list is non-empty and + * |out_leaf_sha256| is non-NULL, it writes the SHA-256 hash of the leaf to + * |out_leaf_sha256|. */ +STACK_OF(X509) *ssl_parse_cert_chain(SSL *ssl, uint8_t *out_alert, + uint8_t *out_leaf_sha256, CBS *cbs); + /* ssl_add_cert_to_cbb adds |x509| to |cbb|. It returns one on success and zero * on error. */ int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509); diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index aab86108..952bf452 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -122,6 +122,7 @@ #include #include #include +#include #include #include @@ -421,6 +422,59 @@ int ssl_has_certificate(const SSL *ssl) { return ssl->cert->x509 != NULL && ssl_has_private_key(ssl); } +STACK_OF(X509) *ssl_parse_cert_chain(SSL *ssl, uint8_t *out_alert, + uint8_t *out_leaf_sha256, CBS *cbs) { + STACK_OF(X509) *ret = sk_X509_new_null(); + if (ret == NULL) { + *out_alert = SSL_AD_INTERNAL_ERROR; + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + return NULL; + } + + X509 *x = NULL; + CBS certificate_list; + if (!CBS_get_u24_length_prefixed(cbs, &certificate_list)) { + *out_alert = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + goto err; + } + + while (CBS_len(&certificate_list) > 0) { + CBS certificate; + if (!CBS_get_u24_length_prefixed(&certificate_list, &certificate)) { + *out_alert = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_LENGTH_MISMATCH); + goto err; + } + + /* Retain the hash of the leaf certificate if requested. */ + if (sk_X509_num(ret) == 0 && out_leaf_sha256 != NULL) { + SHA256(CBS_data(&certificate), CBS_len(&certificate), out_leaf_sha256); + } + + /* A u24 length cannot overflow a long. */ + const uint8_t *data = CBS_data(&certificate); + x = d2i_X509(NULL, &data, (long)CBS_len(&certificate)); + if (x == NULL || data != CBS_data(&certificate) + CBS_len(&certificate)) { + *out_alert = SSL_AD_DECODE_ERROR; + goto err; + } + if (!sk_X509_push(ret, x)) { + *out_alert = SSL_AD_INTERNAL_ERROR; + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + goto err; + } + x = NULL; + } + + return ret; + +err: + X509_free(x); + sk_X509_pop_free(ret, X509_free); + return NULL; +} + int ssl_add_cert_to_cbb(CBB *cbb, X509 *x509) { int len = i2d_X509(x509, NULL); if (len < 0) {