Add async certificate verification callback.
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.
Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 9efbf0a..10c10a2 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -173,7 +173,6 @@
static int ssl3_get_server_hello(SSL_HANDSHAKE *hs);
static int ssl3_get_server_certificate(SSL_HANDSHAKE *hs);
static int ssl3_get_cert_status(SSL_HANDSHAKE *hs);
-static int ssl3_verify_server_cert(SSL_HANDSHAKE *hs);
static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs);
static int ssl3_get_certificate_request(SSL_HANDSHAKE *hs);
static int ssl3_get_server_hello_done(SSL_HANDSHAKE *hs);
@@ -292,9 +291,16 @@
case SSL3_ST_VERIFY_SERVER_CERT:
if (ssl_cipher_uses_certificate_auth(hs->new_cipher)) {
- ret = ssl3_verify_server_cert(hs);
- if (ret <= 0) {
- goto end;
+ switch (ssl_verify_peer_cert(hs)) {
+ case ssl_verify_ok:
+ break;
+ case ssl_verify_invalid:
+ ret = -1;
+ goto end;
+ case ssl_verify_retry:
+ ssl->rwstate = SSL_CERTIFICATE_VERIFY;
+ ret = -1;
+ goto end;
}
}
hs->state = SSL3_ST_CR_KEY_EXCH_A;
@@ -1185,15 +1191,6 @@
return 1;
}
-static int ssl3_verify_server_cert(SSL_HANDSHAKE *hs) {
- SSL *const ssl = hs->ssl;
- if (!ssl->ctx->x509_method->session_verify_cert_chain(hs->new_session, ssl)) {
- return -1;
- }
-
- return 1;
-}
-
static int ssl3_get_server_key_exchange(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
EC_KEY *ecdh = NULL;
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index ee5358c..00ac549 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -282,6 +282,23 @@
goto end;
}
}
+ hs->state = SSL3_ST_VERIFY_CLIENT_CERT;
+ break;
+
+ case SSL3_ST_VERIFY_CLIENT_CERT:
+ if (sk_CRYPTO_BUFFER_num(hs->new_session->certs) > 0) {
+ switch (ssl_verify_peer_cert(hs)) {
+ case ssl_verify_ok:
+ break;
+ case ssl_verify_invalid:
+ ret = -1;
+ goto end;
+ case ssl_verify_retry:
+ ssl->rwstate = SSL_CERTIFICATE_VERIFY;
+ ret = -1;
+ goto end;
+ }
+ }
hs->state = SSL3_ST_SR_KEY_EXCH_A;
break;
@@ -1264,10 +1281,6 @@
hs->new_session->peer_sha256_valid = 1;
}
- if (!ssl->ctx->x509_method->session_verify_cert_chain(hs->new_session, ssl)) {
- return -1;
- }
-
return 1;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index ba98783..e2a3af4 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1007,6 +1007,7 @@
ssl_hs_early_data_rejected,
ssl_hs_read_end_of_early_data,
ssl_hs_read_change_cipher_spec,
+ ssl_hs_certificate_verify,
};
struct ssl_handshake_st {
@@ -1341,6 +1342,9 @@
const SSL_EXTENSION_TYPE *ext_types,
size_t num_ext_types, int ignore_unknown);
+/* ssl_verify_peer_cert verifies the peer certificate for |hs|. */
+enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs);
+
/* SSLKEYLOGFILE functions. */
@@ -1597,7 +1601,8 @@
/* session_verify_cert_chain verifies the certificate chain in |session|,
* sets |session->verify_result| and returns one on success or zero on
* error. */
- int (*session_verify_cert_chain)(SSL_SESSION *session, SSL *ssl);
+ int (*session_verify_cert_chain)(SSL_SESSION *session, SSL *ssl,
+ uint8_t *out_alert);
/* hs_flush_cached_ca_names drops any cached |X509_NAME|s from |hs|. */
void (*hs_flush_cached_ca_names)(SSL_HANDSHAKE *hs);
@@ -1990,6 +1995,9 @@
int (*verify_callback)(int ok,
X509_STORE_CTX *ctx); /* fail if callback returns 0 */
+ enum ssl_verify_result_t (*custom_verify_callback)(SSL *ssl,
+ uint8_t *out_alert);
+
void (*info_callback)(const SSL *ssl, int type, int value);
/* Server-only: psk_identity_hint is the identity hint to send in
diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
index 79f71fa..85de99c 100644
--- a/ssl/s3_both.cc
+++ b/ssl/s3_both.cc
@@ -830,3 +830,34 @@
return 1;
}
+
+enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs) {
+ SSL *const ssl = hs->ssl;
+ uint8_t alert = SSL_AD_CERTIFICATE_UNKNOWN;
+ enum ssl_verify_result_t ret;
+ if (ssl->custom_verify_callback != nullptr) {
+ ret = ssl->custom_verify_callback(ssl, &alert);
+ switch (ret) {
+ case ssl_verify_ok:
+ hs->new_session->verify_result = X509_V_OK;
+ break;
+ case ssl_verify_invalid:
+ hs->new_session->verify_result = X509_V_ERR_APPLICATION_VERIFICATION;
+ break;
+ case ssl_verify_retry:
+ break;
+ }
+ } else {
+ ret = ssl->ctx->x509_method->session_verify_cert_chain(hs->new_session, ssl,
+ &alert)
+ ? ssl_verify_ok
+ : ssl_verify_invalid;
+ }
+
+ if (ret == ssl_verify_invalid) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ }
+
+ return ret;
+}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 7441925..b2d5f02 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -392,6 +392,7 @@
ssl->msg_callback_arg = ctx->msg_callback_arg;
ssl->verify_mode = ctx->verify_mode;
ssl->verify_callback = ctx->default_verify_callback;
+ ssl->custom_verify_callback = ctx->custom_verify_callback;
ssl->retain_only_sha256_of_client_certs =
ctx->retain_only_sha256_of_client_certs;
@@ -984,6 +985,9 @@
case SSL_EARLY_DATA_REJECTED:
return SSL_ERROR_EARLY_DATA_REJECTED;
+
+ case SSL_CERTIFICATE_VERIFY:
+ return SSL_ERROR_WANT_CERTIFICATE_VERIFY;
}
return SSL_ERROR_SYSCALL;
@@ -1554,12 +1558,22 @@
return TLSEXT_NAMETYPE_host_name;
}
-void SSL_CTX_enable_signed_cert_timestamps(SSL_CTX *ctx) {
- ctx->signed_cert_timestamps_enabled = 1;
+void SSL_CTX_set_custom_verify(
+ SSL_CTX *ctx, int mode,
+ enum ssl_verify_result_t (*callback)(SSL *ssl, uint8_t *out_alert)) {
+ ctx->verify_mode = mode;
+ ctx->custom_verify_callback = callback;
}
-void SSL_CTX_i_promise_to_verify_certs_after_the_handshake(SSL_CTX *ctx) {
- ctx->i_promise_to_verify_certs_after_the_handshake = 1;
+void SSL_set_custom_verify(
+ SSL *ssl, int mode,
+ enum ssl_verify_result_t (*callback)(SSL *ssl, uint8_t *out_alert)) {
+ ssl->verify_mode = mode;
+ ssl->custom_verify_callback = callback;
+}
+
+void SSL_CTX_enable_signed_cert_timestamps(SSL_CTX *ctx) {
+ ctx->signed_cert_timestamps_enabled = 1;
}
void SSL_enable_signed_cert_timestamps(SSL *ssl) {
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 2c648ac..640718a 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -3276,7 +3276,11 @@
ASSERT_TRUE(SSL_CTX_set_chain_and_key(server_ctx.get(), &chain[0],
chain.size(), key.get(), nullptr));
- SSL_CTX_i_promise_to_verify_certs_after_the_handshake(client_ctx.get());
+ SSL_CTX_set_custom_verify(
+ client_ctx.get(), SSL_VERIFY_PEER,
+ [](SSL *ssl, uint8_t *out_alert) -> ssl_verify_result_t {
+ return ssl_verify_ok;
+ });
bssl::UniquePtr<SSL> client, server;
ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
diff --git a/ssl/ssl_x509.cc b/ssl/ssl_x509.cc
index 77fc0e2..125e105 100644
--- a/ssl/ssl_x509.cc
+++ b/ssl/ssl_x509.cc
@@ -620,7 +620,9 @@
}
static int ssl_crypto_x509_session_verify_cert_chain(SSL_SESSION *session,
- SSL *ssl) {
+ SSL *ssl,
+ uint8_t *out_alert) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
STACK_OF(X509) *const cert_chain = session->x509_chain;
if (cert_chain == NULL || sk_X509_num(cert_chain) == 0) {
return 0;
@@ -666,8 +668,7 @@
/* If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the result. */
if (verify_ret <= 0 && ssl->verify_mode != SSL_VERIFY_NONE) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, ssl_verify_alarm_type(ctx.error));
- OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
+ *out_alert = ssl_verify_alarm_type(ctx.error);
goto err;
}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index cd846be..2e98780 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -112,6 +112,7 @@
bool alpn_select_done = false;
bool is_resume = false;
bool early_callback_ready = false;
+ bool custom_verify_ready = false;
};
static void TestStateExFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
@@ -684,27 +685,52 @@
return 1;
}
-static int VerifySucceed(X509_STORE_CTX *store_ctx, void *arg) {
- SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx,
- SSL_get_ex_data_X509_STORE_CTX_idx());
+static bool CheckVerifyCallback(SSL *ssl) {
const TestConfig *config = GetTestConfig(ssl);
-
if (!config->expected_ocsp_response.empty()) {
const uint8_t *data;
size_t len;
SSL_get0_ocsp_response(ssl, &data, &len);
if (len == 0) {
fprintf(stderr, "OCSP response not available in verify callback\n");
- return 0;
+ return false;
}
}
+ return true;
+}
+
+static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) {
+ SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx,
+ SSL_get_ex_data_X509_STORE_CTX_idx());
+ const TestConfig *config = GetTestConfig(ssl);
+ if (!CheckVerifyCallback(ssl)) {
+ return 0;
+ }
+
+ if (config->verify_fail) {
+ store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION;
+ return 0;
+ }
+
return 1;
}
-static int VerifyFail(X509_STORE_CTX *store_ctx, void *arg) {
- store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION;
- return 0;
+static ssl_verify_result_t CustomVerifyCallback(SSL *ssl, uint8_t *out_alert) {
+ const TestConfig *config = GetTestConfig(ssl);
+ if (!CheckVerifyCallback(ssl)) {
+ return ssl_verify_invalid;
+ }
+
+ if (config->async && !GetTestState(ssl)->custom_verify_ready) {
+ return ssl_verify_retry;
+ }
+
+ if (config->verify_fail) {
+ return ssl_verify_invalid;
+ }
+
+ return ssl_verify_ok;
}
static int NextProtosAdvertisedCallback(SSL *ssl, const uint8_t **out,
@@ -1139,10 +1165,8 @@
return nullptr;
}
- if (config->verify_fail) {
- SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifyFail, NULL);
- } else {
- SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifySucceed, NULL);
+ if (!config->use_custom_verify_callback) {
+ SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), CertVerifyCallback, NULL);
}
if (!config->signed_cert_timestamps.empty() &&
@@ -1270,6 +1294,9 @@
case SSL_ERROR_WANT_PRIVATE_KEY_OPERATION:
test_state->private_key_retries++;
return true;
+ case SSL_ERROR_WANT_CERTIFICATE_VERIFY:
+ test_state->custom_verify_ready = true;
+ return true;
default:
return false;
}
@@ -1763,20 +1790,23 @@
if (!config->use_old_client_cert_callback) {
SSL_set_cert_cb(ssl.get(), CertCallback, nullptr);
}
+ int mode = SSL_VERIFY_NONE;
if (config->require_any_client_certificate) {
- SSL_set_verify(ssl.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
- NULL);
+ mode = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
}
if (config->verify_peer) {
- SSL_set_verify(ssl.get(), SSL_VERIFY_PEER, NULL);
+ mode = SSL_VERIFY_PEER;
}
if (config->verify_peer_if_no_obc) {
// Set SSL_VERIFY_FAIL_IF_NO_PEER_CERT so testing whether client
// certificates were requested is easy.
- SSL_set_verify(ssl.get(),
- SSL_VERIFY_PEER | SSL_VERIFY_PEER_IF_NO_OBC |
- SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
- NULL);
+ mode = SSL_VERIFY_PEER | SSL_VERIFY_PEER_IF_NO_OBC |
+ SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
+ }
+ if (config->use_custom_verify_callback) {
+ SSL_set_custom_verify(ssl.get(), mode, CustomVerifyCallback);
+ } else if (mode != SSL_VERIFY_NONE) {
+ SSL_set_verify(ssl.get(), mode, NULL);
}
if (config->false_start) {
SSL_set_mode(ssl.get(), SSL_MODE_ENABLE_FALSE_START);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index e526576..8ced44d 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4553,47 +4553,49 @@
if config.protocol == dtls && !vers.hasDTLS {
continue
}
- for _, testType := range []testType{clientTest, serverTest} {
- suffix := "-Client"
- if testType == serverTest {
- suffix = "-Server"
- }
- suffix += "-" + vers.name
+ for _, useCustomCallback := range []bool{false, true} {
+ for _, testType := range []testType{clientTest, serverTest} {
+ suffix := "-Client"
+ if testType == serverTest {
+ suffix = "-Server"
+ }
+ suffix += "-" + vers.name
+ if useCustomCallback {
+ suffix += "-CustomCallback"
+ }
- flag := "-verify-peer"
- if testType == serverTest {
- flag = "-require-any-client-certificate"
- }
+ flags := []string{"-verify-peer"}
+ if testType == serverTest {
+ flags = append(flags, "-require-any-client-certificate")
+ }
+ if useCustomCallback {
+ flags = append(flags, "-use-custom-verify-callback")
+ }
- tests = append(tests, testCase{
- testType: testType,
- name: "CertificateVerificationSucceed" + suffix,
- config: Config{
- MaxVersion: vers.version,
- Certificates: []Certificate{rsaCertificate},
- },
- tls13Variant: vers.tls13Variant,
- flags: []string{
- flag,
- "-expect-verify-result",
- },
- resumeSession: true,
- })
- tests = append(tests, testCase{
- testType: testType,
- name: "CertificateVerificationFail" + suffix,
- config: Config{
- MaxVersion: vers.version,
- Certificates: []Certificate{rsaCertificate},
- },
- tls13Variant: vers.tls13Variant,
- flags: []string{
- flag,
- "-verify-fail",
- },
- shouldFail: true,
- expectedError: ":CERTIFICATE_VERIFY_FAILED:",
- })
+ tests = append(tests, testCase{
+ testType: testType,
+ name: "CertificateVerificationSucceed" + suffix,
+ config: Config{
+ MaxVersion: vers.version,
+ Certificates: []Certificate{rsaCertificate},
+ },
+ tls13Variant: vers.tls13Variant,
+ flags: append([]string{"-expect-verify-result"}, flags...),
+ resumeSession: true,
+ })
+ tests = append(tests, testCase{
+ testType: testType,
+ name: "CertificateVerificationFail" + suffix,
+ config: Config{
+ MaxVersion: vers.version,
+ Certificates: []Certificate{rsaCertificate},
+ },
+ tls13Variant: vers.tls13Variant,
+ flags: append([]string{"-verify-fail"}, flags...),
+ shouldFail: true,
+ expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ })
+ }
}
// By default, the client is in a soft fail mode where the peer
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index f925504..fa7dfe1 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -129,6 +129,7 @@
{ "-handshake-twice", &TestConfig::handshake_twice },
{ "-allow-unknown-alpn-protos", &TestConfig::allow_unknown_alpn_protos },
{ "-enable-ed25519", &TestConfig::enable_ed25519 },
+ { "-use-custom-verify-callback", &TestConfig::use_custom_verify_callback },
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index e157936..1e5912e 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -144,6 +144,7 @@
bool handshake_twice = false;
bool allow_unknown_alpn_protos = false;
bool enable_ed25519 = false;
+ bool use_custom_verify_callback = false;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_initial,
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index 83ff78a..6a6c3c6 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -102,6 +102,11 @@
hs->wait = ssl_hs_ok;
return -1;
+ case ssl_hs_certificate_verify:
+ ssl->rwstate = SSL_CERTIFICATE_VERIFY;
+ hs->wait = ssl_hs_ok;
+ return -1;
+
case ssl_hs_early_data_rejected:
ssl->rwstate = SSL_EARLY_DATA_REJECTED;
/* Cause |SSL_write| to start failing immediately. */
@@ -357,12 +362,6 @@
}
hs->new_session->peer_sha256_valid = retain_sha256;
-
- if (!ssl->ctx->x509_method->session_verify_cert_chain(hs->new_session,
- ssl)) {
- goto err;
- }
-
ret = 1;
err:
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 7f961bf..9153dd7 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -494,6 +494,16 @@
static enum ssl_hs_wait_t do_process_server_certificate_verify(
SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
+ switch (ssl_verify_peer_cert(hs)) {
+ case ssl_verify_ok:
+ break;
+ case ssl_verify_invalid:
+ return ssl_hs_error;
+ case ssl_verify_retry:
+ hs->tls13_state = state_process_server_certificate_verify;
+ return ssl_hs_certificate_verify;
+ }
+
if (!ssl_check_message_type(ssl, SSL3_MT_CERTIFICATE_VERIFY) ||
!tls13_process_certificate_verify(hs) ||
!ssl_hash_current_message(hs)) {
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 4e66016..dd7e86d 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -766,6 +766,16 @@
return ssl_hs_ok;
}
+ switch (ssl_verify_peer_cert(hs)) {
+ case ssl_verify_ok:
+ break;
+ case ssl_verify_invalid:
+ return ssl_hs_error;
+ case ssl_verify_retry:
+ hs->tls13_state = state_process_client_certificate_verify;
+ return ssl_hs_certificate_verify;
+ }
+
if (!ssl_check_message_type(ssl, SSL3_MT_CERTIFICATE_VERIFY) ||
!tls13_process_certificate_verify(hs) ||
!ssl_hash_current_message(hs)) {
diff --git a/ssl/tls_method.cc b/ssl/tls_method.cc
index d039b7d..0bcdf92 100644
--- a/ssl/tls_method.cc
+++ b/ssl/tls_method.cc
@@ -240,15 +240,9 @@
}
static void ssl_noop_x509_session_clear(SSL_SESSION *session) {}
static int ssl_noop_x509_session_verify_cert_chain(SSL_SESSION *session,
- SSL *ssl) {
- if (!ssl->ctx->i_promise_to_verify_certs_after_the_handshake) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNKNOWN_CA);
- OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
- return 0;
- }
-
- session->verify_result = X509_V_OK;
- return 1;
+ SSL *ssl,
+ uint8_t *out_alert) {
+ return 0;
}
static void ssl_noop_x509_hs_flush_cached_ca_names(SSL_HANDSHAKE *hs) {}
