From 13f1ebe8272c5463b5d83d656cb8f0c93c89cd94 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 20 Jul 2016 10:11:04 +0200 Subject: [PATCH] Factor out the client_cert_cb code. Share a bit more of it between TLS 1.2 and 1.3. Change-Id: I43c9dbf785a3d33db1793cffb0fdbd3af075cc89 Reviewed-on: https://boringssl-review.googlesource.com/8849 Reviewed-by: David Benjamin Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- include/openssl/ssl3.h | 1 - ssl/handshake_client.c | 50 +++++++----------------------------------- ssl/internal.h | 5 +++++ ssl/ssl_cert.c | 27 +++++++++++++++++++++++ ssl/ssl_stat.c | 6 ----- ssl/tls13_client.c | 19 +++++----------- 6 files changed, 45 insertions(+), 63 deletions(-) diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index beab5722..4439a385 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -322,7 +322,6 @@ OPENSSL_COMPILE_ASSERT( #define SSL3_ST_CW_CERT_A (0x170 | SSL_ST_CONNECT) #define SSL3_ST_CW_CERT_B (0x171 | SSL_ST_CONNECT) #define SSL3_ST_CW_CERT_C (0x172 | SSL_ST_CONNECT) -#define SSL3_ST_CW_CERT_D (0x173 | SSL_ST_CONNECT) #define SSL3_ST_CW_KEY_EXCH_A (0x180 | SSL_ST_CONNECT) #define SSL3_ST_CW_KEY_EXCH_B (0x181 | SSL_ST_CONNECT) #define SSL3_ST_CW_CERT_VRFY_A (0x190 | SSL_ST_CONNECT) diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index 4a82c497..8afc289a 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -328,7 +328,6 @@ int ssl3_connect(SSL *ssl) { case SSL3_ST_CW_CERT_A: case SSL3_ST_CW_CERT_B: case SSL3_ST_CW_CERT_C: - case SSL3_ST_CW_CERT_D: if (ssl->s3->tmp.cert_request) { ret = ssl3_send_client_certificate(ssl); if (ret <= 0) { @@ -1410,22 +1409,6 @@ static int ssl3_get_server_hello_done(SSL *ssl) { return 1; } -static int ssl_do_client_cert_cb(SSL *ssl, X509 **out_x509, - EVP_PKEY **out_pkey) { - if (ssl->ctx->client_cert_cb == NULL) { - return 0; - } - - int ret = ssl->ctx->client_cert_cb(ssl, out_x509, out_pkey); - if (ret <= 0) { - return ret; - } - - assert(*out_x509 != NULL); - assert(*out_pkey != NULL); - return 1; -} - static int ssl3_send_client_certificate(SSL *ssl) { if (ssl->state == SSL3_ST_CW_CERT_A) { /* Call cert_cb to update the certificate. */ @@ -1441,36 +1424,19 @@ static int ssl3_send_client_certificate(SSL *ssl) { } } - if (ssl_has_certificate(ssl)) { - ssl->state = SSL3_ST_CW_CERT_C; - } else { - ssl->state = SSL3_ST_CW_CERT_B; - } + ssl->state = SSL3_ST_CW_CERT_B; } if (ssl->state == SSL3_ST_CW_CERT_B) { /* Call client_cert_cb to update the certificate. */ - X509 *x509 = NULL; - EVP_PKEY *pkey = NULL; - int ret = ssl_do_client_cert_cb(ssl, &x509, &pkey); - if (ret < 0) { - ssl->rwstate = SSL_X509_LOOKUP; + int should_retry; + if (!ssl_do_client_cert_cb(ssl, &should_retry)) { + if (should_retry) { + ssl->rwstate = SSL_X509_LOOKUP; + } return -1; } - int setup_error = ret == 1 && (!SSL_use_certificate(ssl, x509) || - !SSL_use_PrivateKey(ssl, pkey)); - X509_free(x509); - EVP_PKEY_free(pkey); - if (setup_error) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return -1; - } - - ssl->state = SSL3_ST_CW_CERT_C; - } - - if (ssl->state == SSL3_ST_CW_CERT_C) { if (!ssl_has_certificate(ssl)) { ssl->s3->tmp.cert_request = 0; /* Without a client certificate, the handshake buffer may be released. */ @@ -1486,10 +1452,10 @@ static int ssl3_send_client_certificate(SSL *ssl) { if (!ssl3_output_cert_chain(ssl)) { return -1; } - ssl->state = SSL3_ST_CW_CERT_D; + ssl->state = SSL3_ST_CW_CERT_C; } - assert(ssl->state == SSL3_ST_CW_CERT_D); + assert(ssl->state == SSL3_ST_CW_CERT_C); return ssl->method->write_message(ssl); } diff --git a/ssl/internal.h b/ssl/internal.h index 867cfa29..eb8b8795 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -774,6 +774,11 @@ int ssl_add_client_CA_list(SSL *ssl, CBB *cbb); * error queue. */ int ssl_check_leaf_certificate(SSL *ssl, X509 *leaf); +/* ssl_do_client_cert_cb runs the client_cert_cb, if any, and returns one on + * success and zero on error. On error, it sets |*out_should_retry| to one if + * the callback failed and should be retried and zero otherwise. */ +int ssl_do_client_cert_cb(SSL *ssl, int *out_should_retry); + /* TLS 1.3 key derivation. */ diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 5138f15a..e81e83d8 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -639,6 +639,33 @@ int ssl_add_client_CA_list(SSL *ssl, CBB *cbb) { return CBB_flush(cbb); } +int ssl_do_client_cert_cb(SSL *ssl, int *out_should_retry) { + if (ssl_has_certificate(ssl) || ssl->ctx->client_cert_cb == NULL) { + return 1; + } + + X509 *x509 = NULL; + EVP_PKEY *pkey = NULL; + int ret = ssl->ctx->client_cert_cb(ssl, &x509, &pkey); + if (ret < 0) { + *out_should_retry = 1; + return 0; + } + + if (ret != 0) { + if (!SSL_use_certificate(ssl, x509) || + !SSL_use_PrivateKey(ssl, pkey)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + *out_should_retry = 0; + return 0; + } + } + + X509_free(x509); + EVP_PKEY_free(pkey); + return 1; +} + static int set_cert_store(X509_STORE **store_ptr, X509_STORE *new_store, int take_ref) { X509_STORE_free(*store_ptr); *store_ptr = new_store; diff --git a/ssl/ssl_stat.c b/ssl/ssl_stat.c index 0881b830..3fdc6e59 100644 --- a/ssl/ssl_stat.c +++ b/ssl/ssl_stat.c @@ -134,9 +134,6 @@ const char *SSL_state_string_long(const SSL *ssl) { case SSL3_ST_CW_CERT_C: return "SSLv3 write client certificate C"; - case SSL3_ST_CW_CERT_D: - return "SSLv3 write client certificate D"; - case SSL3_ST_CW_KEY_EXCH_A: return "SSLv3 write client key exchange A"; @@ -294,9 +291,6 @@ const char *SSL_state_string(const SSL *ssl) { case SSL3_ST_CW_CERT_C: return "3WCC_C"; - case SSL3_ST_CW_CERT_D: - return "3WCC_D"; - case SSL3_ST_CW_KEY_EXCH_A: return "3WCKEA"; diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index cf8b390e..1b5573ba 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c @@ -330,23 +330,14 @@ static enum ssl_hs_wait_t do_certificate_callback(SSL *ssl, SSL_HANDSHAKE *hs) { static enum ssl_hs_wait_t do_send_client_certificate(SSL *ssl, SSL_HANDSHAKE *hs) { - if (!ssl_has_certificate(ssl) && ssl->ctx->client_cert_cb != NULL) { - X509 *x509 = NULL; - EVP_PKEY *pkey = NULL; - int rv = ssl->ctx->client_cert_cb(ssl, &x509, &pkey); - if (rv < 0) { + /* Call client_cert_cb to update the certificate. */ + int should_retry; + if (!ssl_do_client_cert_cb(ssl, &should_retry)) { + if (should_retry) { hs->state = state_send_client_certificate; return ssl_hs_x509_lookup; } - - int setup_error = rv == 1 && (!SSL_use_certificate(ssl, x509) || - !SSL_use_PrivateKey(ssl, pkey)); - X509_free(x509); - EVP_PKEY_free(pkey); - if (setup_error) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } + return ssl_hs_error; } if (!tls13_prepare_certificate(ssl)) {