From 0d1730ddf1a9462a8c270ac5bad1a6851eb35ae9 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 15 Jun 2017 23:24:25 -0400 Subject: [PATCH] Squash together states in the TLS 1.2 server Certificate flight. We've got an asynchronous ServerKeyExchange state in the middle that complicates things a bit, but this is still a little tighter. BUG=128 Change-Id: I4ee2e3b85e677c9555d2fbddd387c12d41ab2b54 Reviewed-on: https://boringssl-review.googlesource.com/17250 Commit-Queue: Steven Valdez Reviewed-by: Steven Valdez CQ-Verified: CQ bot account: commit-bot@chromium.org --- include/openssl/ssl3.h | 2 - ssl/handshake_server.c | 107 ++++++++++++++--------------------------- ssl/ssl_stat.c | 6 --- 3 files changed, 37 insertions(+), 78 deletions(-) diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 9fbd4ef9..4ff61c5e 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -345,7 +345,6 @@ OPENSSL_COMPILE_ASSERT( #define SSL3_ST_SW_CERT_A (0x140 | SSL_ST_ACCEPT) #define SSL3_ST_SW_KEY_EXCH_A (0x150 | SSL_ST_ACCEPT) #define SSL3_ST_SW_KEY_EXCH_B (0x151 | SSL_ST_ACCEPT) -#define SSL3_ST_SW_CERT_REQ_A (0x160 | SSL_ST_ACCEPT) #define SSL3_ST_SW_SRVR_DONE_A (0x170 | SSL_ST_ACCEPT) /* read from client */ #define SSL3_ST_SR_CERT_A (0x180 | SSL_ST_ACCEPT) @@ -359,7 +358,6 @@ OPENSSL_COMPILE_ASSERT( /* write to client */ #define SSL3_ST_SW_FINISHED_A (0x1E0 | SSL_ST_ACCEPT) -#define SSL3_ST_SW_CERT_STATUS_A (0x200 | SSL_ST_ACCEPT) #define SSL3_MT_HELLO_REQUEST 0 #define SSL3_MT_CLIENT_HELLO 1 diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index b59e3b2e..2915e2ea 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -175,9 +175,7 @@ static int ssl3_select_certificate(SSL_HANDSHAKE *hs); static int ssl3_select_parameters(SSL_HANDSHAKE *hs); static int ssl3_send_server_hello(SSL_HANDSHAKE *hs); static int ssl3_send_server_certificate(SSL_HANDSHAKE *hs); -static int ssl3_send_certificate_status(SSL_HANDSHAKE *hs); static int ssl3_send_server_key_exchange(SSL_HANDSHAKE *hs); -static int ssl3_send_certificate_request(SSL_HANDSHAKE *hs); static int ssl3_send_server_hello_done(SSL_HANDSHAKE *hs); static int ssl3_get_client_certificate(SSL_HANDSHAKE *hs); static int ssl3_get_client_key_exchange(SSL_HANDSHAKE *hs); @@ -262,21 +260,9 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { break; case SSL3_ST_SW_CERT_A: - if (ssl_cipher_uses_certificate_auth(hs->new_cipher)) { - ret = ssl3_send_server_certificate(hs); - if (ret <= 0) { - goto end; - } - } - hs->state = SSL3_ST_SW_CERT_STATUS_A; - break; - - case SSL3_ST_SW_CERT_STATUS_A: - if (hs->certificate_status_expected) { - ret = ssl3_send_certificate_status(hs); - if (ret <= 0) { - goto end; - } + ret = ssl3_send_server_certificate(hs); + if (ret <= 0) { + goto end; } hs->state = SSL3_ST_SW_KEY_EXCH_A; break; @@ -294,16 +280,6 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { } } - hs->state = SSL3_ST_SW_CERT_REQ_A; - break; - - case SSL3_ST_SW_CERT_REQ_A: - if (hs->cert_request) { - ret = ssl3_send_certificate_request(hs); - if (ret <= 0) { - goto end; - } - } hs->state = SSL3_ST_SW_SRVR_DONE_A; break; @@ -1071,6 +1047,10 @@ static int ssl3_send_server_hello(SSL_HANDSHAKE *hs) { static int ssl3_send_server_certificate(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; + if (!ssl_cipher_uses_certificate_auth(hs->new_cipher)) { + return 1; + } + if (!ssl_has_certificate(ssl)) { OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_SET); return -1; @@ -1079,23 +1059,21 @@ static int ssl3_send_server_certificate(SSL_HANDSHAKE *hs) { if (!ssl3_output_cert_chain(ssl)) { return -1; } - return 1; -} -static int ssl3_send_certificate_status(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; - CBB cbb, body, ocsp_response; - if (!ssl->method->init_message(ssl, &cbb, &body, - SSL3_MT_CERTIFICATE_STATUS) || - !CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) || - !CBB_add_u24_length_prefixed(&body, &ocsp_response) || - !CBB_add_bytes(&ocsp_response, - CRYPTO_BUFFER_data(ssl->cert->ocsp_response), - CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) || - !ssl_add_message_cbb(ssl, &cbb)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - CBB_cleanup(&cbb); - return -1; + if (hs->certificate_status_expected) { + CBB cbb, body, ocsp_response; + if (!ssl->method->init_message(ssl, &cbb, &body, + SSL3_MT_CERTIFICATE_STATUS) || + !CBB_add_u8(&body, TLSEXT_STATUSTYPE_ocsp) || + !CBB_add_u24_length_prefixed(&body, &ocsp_response) || + !CBB_add_bytes(&ocsp_response, + CRYPTO_BUFFER_data(ssl->cert->ocsp_response), + CRYPTO_BUFFER_len(ssl->cert->ocsp_response)) || + !ssl_add_message_cbb(ssl, &cbb)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + CBB_cleanup(&cbb); + return -1; + } } return 1; @@ -1250,26 +1228,28 @@ err: return -1; } -static int ssl3_send_certificate_request(SSL_HANDSHAKE *hs) { +static int ssl3_send_server_hello_done(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - CBB cbb, body, cert_types, sigalgs_cbb; - if (!ssl->method->init_message(ssl, &cbb, &body, - SSL3_MT_CERTIFICATE_REQUEST) || - !CBB_add_u8_length_prefixed(&body, &cert_types) || - !CBB_add_u8(&cert_types, SSL3_CT_RSA_SIGN) || - (ssl->version >= TLS1_VERSION && - !CBB_add_u8(&cert_types, TLS_CT_ECDSA_SIGN))) { - goto err; - } + CBB cbb, body; - if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) { - if (!CBB_add_u16_length_prefixed(&body, &sigalgs_cbb) || - !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb)) { + if (hs->cert_request) { + CBB cert_types, sigalgs_cbb; + if (!ssl->method->init_message(ssl, &cbb, &body, + SSL3_MT_CERTIFICATE_REQUEST) || + !CBB_add_u8_length_prefixed(&body, &cert_types) || + !CBB_add_u8(&cert_types, SSL3_CT_RSA_SIGN) || + (ssl3_protocol_version(ssl) >= TLS1_VERSION && + !CBB_add_u8(&cert_types, TLS_CT_ECDSA_SIGN)) || + (ssl3_protocol_version(ssl) >= TLS1_2_VERSION && + (!CBB_add_u16_length_prefixed(&body, &sigalgs_cbb) || + !tls12_add_verify_sigalgs(ssl, &sigalgs_cbb))) || + !ssl_add_client_CA_list(ssl, &body) || + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } } - if (!ssl_add_client_CA_list(ssl, &body) || + if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO_DONE) || !ssl_add_message_cbb(ssl, &cbb)) { goto err; } @@ -1282,19 +1262,6 @@ err: return -1; } -static int ssl3_send_server_hello_done(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; - CBB cbb, body; - if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO_DONE) || - !ssl_add_message_cbb(ssl, &cbb)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - CBB_cleanup(&cbb); - return -1; - } - - return 1; -} - static int ssl3_get_client_certificate(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; assert(hs->cert_request); diff --git a/ssl/ssl_stat.c b/ssl/ssl_stat.c index bb7216c2..09f59e54 100644 --- a/ssl/ssl_stat.c +++ b/ssl/ssl_stat.c @@ -182,9 +182,6 @@ const char *SSL_state_string_long(const SSL *ssl) { case SSL3_ST_SW_KEY_EXCH_A: return "SSLv3 write key exchange A"; - case SSL3_ST_SW_CERT_REQ_A: - return "SSLv3 write certificate request A"; - case SSL3_ST_SW_SRVR_DONE_A: return "SSLv3 write server done A"; @@ -291,9 +288,6 @@ const char *SSL_state_string(const SSL *ssl) { case SSL3_ST_SW_KEY_EXCH_B: return "3WSKEB"; - case SSL3_ST_SW_CERT_REQ_A: - return "3WCR_A"; - case SSL3_ST_SW_SRVR_DONE_A: return "3WSD_A";