Option to reverify certs on resumption. Works in the 1.3 and 1.2 client handshakes, not implemented on the server for now. Creates an SSL_CTX option to reverify the server certificate on session resumption. Reverification only runs the client's certificate verify callback. Adds new states to the client handshakes: state_reverify_server_certificate in TLS 1.2, and state_server_certificate_reverify in TLS 1.3. Adds a negative test to make sure that by default we don't verify the certificate on resumption, and positive tests that make sure we do when the new option is set. Change-Id: I3a47ff3eacb3099df4db4c5bc57f7c801ceea8f1 Bug: chromium:347402 Reviewed-on: https://boringssl-review.googlesource.com/29984 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index a24af46..d616981 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -3521,6 +3521,13 @@ OPENSSL_EXPORT void SSL_CTX_set_dos_protection_cb( SSL_CTX *ctx, int (*cb)(const SSL_CLIENT_HELLO *)); +// SSL_CTX_set_reverify_on_resume configures whether the certificate +// verification callback will be used to reverify stored certificates +// when resuming a session. This only works with |SSL_CTX_set_custom_verify|. +// For now, this is incompatible with |SSL_VERIFY_NONE| mode, and is only +// respected on clients. +OPENSSL_EXPORT void SSL_CTX_set_reverify_on_resume(SSL_CTX *ctx, int enabled); + // SSL_ST_* are possible values for |SSL_state|, the bitmasks that make them up, // and some historical values for compatibility. Only |SSL_ST_INIT| and // |SSL_ST_OK| are ever returned.
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 4683ac5..8e5c62c 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -157,8 +157,7 @@ UniquePtr<SSL_HANDSHAKE> ssl_handshake_new(SSL *ssl) { UniquePtr<SSL_HANDSHAKE> hs = MakeUnique<SSL_HANDSHAKE>(ssl); - if (!hs || - !hs->transcript.Init()) { + if (!hs || !hs->transcript.Init()) { return nullptr; } hs->config = ssl->config.get(); @@ -373,6 +372,32 @@ return ret; } +// Verifies a stored certificate when resuming a session. A few things are +// different from verify_peer_cert: +// 1. We can't be renegotiating if we're resuming a session. +// 2. The session is immutable, so we don't support verify_mode == +// SSL_VERIFY_NONE +// 3. We don't call the OCSP callback. +// 4. We only support custom verify callbacks. +enum ssl_verify_result_t ssl_reverify_peer_cert(SSL_HANDSHAKE *hs) { + SSL *const ssl = hs->ssl; + assert(ssl->s3->established_session == nullptr); + assert(hs->config->verify_mode != SSL_VERIFY_NONE); + + uint8_t alert = SSL_AD_CERTIFICATE_UNKNOWN; + enum ssl_verify_result_t ret = ssl_verify_invalid; + if (hs->config->custom_verify_callback != nullptr) { + ret = hs->config->custom_verify_callback(ssl, &alert); + } + + if (ret == ssl_verify_invalid) { + OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED); + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); + } + + return ret; +} + uint16_t ssl_get_grease_value(SSL_HANDSHAKE *hs, enum ssl_grease_index_t index) { // Draw entropy for all GREASE values at once. This avoids calling @@ -452,8 +477,7 @@ } // Log the master secret, if logging is enabled. - if (!ssl_log_secret(ssl, "CLIENT_RANDOM", - session->master_key, + if (!ssl_log_secret(ssl, "CLIENT_RANDOM", session->master_key, session->master_key_length)) { return 0; }
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index de5d8e9..cb9b6de 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -182,6 +182,7 @@ state_read_server_certificate, state_read_certificate_status, state_verify_server_certificate, + state_reverify_server_certificate, state_read_server_key_exchange, state_read_certificate_request, state_read_server_hello_done, @@ -734,7 +735,12 @@ ssl->method->next_message(ssl); if (ssl->session != NULL) { - hs->state = state_read_session_ticket; + if (ssl->ctx->reverify_on_resume && + ssl_cipher_uses_certificate_auth(hs->new_cipher)) { + hs->state = state_reverify_server_certificate; + } else { + hs->state = state_read_session_ticket; + } return ssl_hs_ok; } @@ -868,6 +874,23 @@ return ssl_hs_ok; } +static enum ssl_hs_wait_t do_reverify_server_certificate(SSL_HANDSHAKE *hs) { + assert(hs->ssl->ctx->reverify_on_resume); + + switch (ssl_reverify_peer_cert(hs)) { + case ssl_verify_ok: + break; + case ssl_verify_invalid: + return ssl_hs_error; + case ssl_verify_retry: + hs->state = state_reverify_server_certificate; + return ssl_hs_certificate_verify; + } + + hs->state = state_read_session_ticket; + return ssl_hs_ok; +} + static enum ssl_hs_wait_t do_read_server_key_exchange(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; SSLMessage msg; @@ -1671,6 +1694,9 @@ case state_verify_server_certificate: ret = do_verify_server_certificate(hs); break; + case state_reverify_server_certificate: + ret = do_reverify_server_certificate(hs); + break; case state_read_server_key_exchange: ret = do_read_server_key_exchange(hs); break; @@ -1745,6 +1771,8 @@ return "TLS client read_certificate_status"; case state_verify_server_certificate: return "TLS client verify_server_certificate"; + case state_reverify_server_certificate: + return "TLS client reverify_server_certificate"; case state_read_server_key_exchange: return "TLS client read_server_key_exchange"; case state_read_certificate_request:
diff --git a/ssl/internal.h b/ssl/internal.h index cc3e075..46c5248 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1698,6 +1698,9 @@ // ssl_verify_peer_cert verifies the peer certificate for |hs|. enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs); +// ssl_reverify_peer_cert verifies the peer certificate for |hs| when resuming a +// session. +enum ssl_verify_result_t ssl_reverify_peer_cert(SSL_HANDSHAKE *hs); enum ssl_hs_wait_t ssl_get_finished(SSL_HANDSHAKE *hs); bool ssl_send_finished(SSL_HANDSHAKE *hs); @@ -2899,6 +2902,11 @@ // abort. int (*dos_protection_cb) (const SSL_CLIENT_HELLO *) = nullptr; + // Controls whether to verify certificates when resuming connections. They + // were already verified when the connection was first made, so the default is + // false. For now, this is only respected on clients, not servers. + bool reverify_on_resume = false; + // Maximum amount of data to send in one fragment. actual record size can be // more than this due to padding and MAC overheads. uint16_t max_send_fragment = SSL3_RT_MAX_PLAIN_LENGTH;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 36c26cd..d6181f3 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -2545,6 +2545,10 @@ ctx->dos_protection_cb = cb; } +void SSL_CTX_set_reverify_on_resume(SSL_CTX *ctx, int enabled) { + ctx->reverify_on_resume = !!enabled; +} + void SSL_set_renegotiate_mode(SSL *ssl, enum ssl_renegotiate_mode_t mode) { ssl->renegotiate_mode = mode;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index d1748dd..47c4513 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -4967,6 +4967,49 @@ shouldFail: true, expectedError: ":CERTIFICATE_VERIFY_FAILED:", }) + // Tests that although the verify callback fails on resumption, by default we don't call it. + tests = append(tests, testCase{ + testType: testType, + name: "CertificateVerificationDoesNotFailOnResume" + suffix, + config: Config{ + MaxVersion: vers.version, + Certificates: []Certificate{rsaCertificate}, + }, + tls13Variant: vers.tls13Variant, + flags: append([]string{"-on-resume-verify-fail"}, flags...), + resumeSession: true, + }) + if testType == clientTest && useCustomCallback { + tests = append(tests, testCase{ + testType: testType, + name: "CertificateVerificationFailsOnResume" + suffix, + config: Config{ + MaxVersion: vers.version, + Certificates: []Certificate{rsaCertificate}, + }, + tls13Variant: vers.tls13Variant, + flags: append([]string{ + "-on-resume-verify-fail", + "-reverify-on-resume", + }, flags...), + resumeSession: true, + shouldFail: true, + expectedError: ":CERTIFICATE_VERIFY_FAILED:", + }) + tests = append(tests, testCase{ + testType: testType, + name: "CertificateVerificationPassesOnResume" + suffix, + config: Config{ + MaxVersion: vers.version, + Certificates: []Certificate{rsaCertificate}, + }, + tls13Variant: vers.tls13Variant, + flags: append([]string{ + "-reverify-on-resume", + }, flags...), + resumeSession: true, + }) + } } }
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index d92cf72..fee4258 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -145,6 +145,7 @@ &TestConfig::install_cert_compression_algs }, { "-is-handshaker-supported", &TestConfig::is_handshaker_supported }, { "-handshaker-resume", &TestConfig::handshaker_resume }, + { "-reverify-on-resume", &TestConfig::reverify_on_resume }, }; const Flag<std::string> kStringFlags[] = { @@ -1491,6 +1492,9 @@ if (partial_write) { SSL_set_mode(ssl.get(), SSL_MODE_ENABLE_PARTIAL_WRITE); } + if (reverify_on_resume) { + SSL_CTX_set_reverify_on_resume(ssl_ctx, 1); + } if (no_tls13) { SSL_set_options(ssl.get(), SSL_OP_NO_TLSv1_3); }
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 6c9ac3e..7f9f442 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -166,6 +166,7 @@ bool decline_ocsp_callback = false; bool fail_ocsp_callback = false; bool install_cert_compression_algs = false; + bool reverify_on_resume = false; bool is_handshaker_supported = false; bool handshaker_resume = false; std::string handshaker_path;
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 40281a0..cf20403 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -40,6 +40,7 @@ state_read_certificate_request, state_read_server_certificate, state_read_server_certificate_verify, + state_server_certificate_reverify, state_read_server_finished, state_send_end_of_early_data, state_send_client_certificate, @@ -464,6 +465,10 @@ SSL *const ssl = hs->ssl; // CertificateRequest may only be sent in non-resumption handshakes. if (ssl->s3->session_reused) { + if (ssl->ctx->reverify_on_resume) { + hs->tls13_state = state_server_certificate_reverify; + return ssl_hs_ok; + } hs->tls13_state = state_read_server_finished; return ssl_hs_ok; } @@ -585,6 +590,21 @@ return ssl_hs_ok; } +static enum ssl_hs_wait_t do_server_certificate_reverify( + SSL_HANDSHAKE *hs) { + switch (ssl_reverify_peer_cert(hs)) { + case ssl_verify_ok: + break; + case ssl_verify_invalid: + return ssl_hs_error; + case ssl_verify_retry: + hs->tls13_state = state_server_certificate_reverify; + return ssl_hs_certificate_verify; + } + hs->tls13_state = state_read_server_finished; + return ssl_hs_ok; +} + static enum ssl_hs_wait_t do_read_server_finished(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; SSLMessage msg; @@ -754,6 +774,9 @@ case state_read_server_certificate_verify: ret = do_read_server_certificate_verify(hs); break; + case state_server_certificate_reverify: + ret = do_server_certificate_reverify(hs); + break; case state_read_server_finished: ret = do_read_server_finished(hs); break; @@ -804,6 +827,8 @@ return "TLS 1.3 client read_server_certificate"; case state_read_server_certificate_verify: return "TLS 1.3 client read_server_certificate_verify"; + case state_server_certificate_reverify: + return "TLS 1.3 client server_certificate_reverify"; case state_read_server_finished: return "TLS 1.3 client read_server_finished"; case state_send_end_of_early_data: