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:
