Wait for CertificateStatus message to verify certificate.
Applications may require the stapled OCSP response in order to verify
the certificate within the verification callback.
Change-Id: I8002e527f90c3ce7b6a66e3203c0a68371aac5ec
Reviewed-on: https://boringssl-review.googlesource.com/5730
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 6edf2c3..a3c1c37 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -260,7 +260,7 @@
if (s->s3->tmp.certificate_status_expected) {
s->state = SSL3_ST_CR_CERT_STATUS_A;
} else {
- s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->state = SSL3_ST_VERIFY_SERVER_CERT;
}
} else {
skip = 1;
@@ -269,6 +269,16 @@
s->init_num = 0;
break;
+ case SSL3_ST_VERIFY_SERVER_CERT:
+ ret = ssl3_verify_server_cert(s);
+ if (ret <= 0) {
+ goto end;
+ }
+
+ s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->init_num = 0;
+ break;
+
case SSL3_ST_CR_KEY_EXCH_A:
case SSL3_ST_CR_KEY_EXCH_B:
ret = ssl3_get_server_key_exchange(s);
@@ -418,7 +428,7 @@
if (ret <= 0) {
goto end;
}
- s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->state = SSL3_ST_VERIFY_SERVER_CERT;
s->init_num = 0;
break;
diff --git a/ssl/internal.h b/ssl/internal.h
index d26fd6d..5e0dcb6 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1119,6 +1119,7 @@
int ssl3_get_server_certificate(SSL *s);
int ssl3_send_next_proto(SSL *s);
int ssl3_send_channel_id(SSL *s);
+int ssl3_verify_server_cert(SSL *s);
/* some server-only functions */
int ssl3_get_initial_bytes(SSL *s);
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 485c29f..6e82fff 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -276,7 +276,7 @@
if (s->s3->tmp.certificate_status_expected) {
s->state = SSL3_ST_CR_CERT_STATUS_A;
} else {
- s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->state = SSL3_ST_VERIFY_SERVER_CERT;
}
} else {
skip = 1;
@@ -285,6 +285,16 @@
s->init_num = 0;
break;
+ case SSL3_ST_VERIFY_SERVER_CERT:
+ ret = ssl3_verify_server_cert(s);
+ if (ret <= 0) {
+ goto end;
+ }
+
+ s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->init_num = 0;
+ break;
+
case SSL3_ST_CR_KEY_EXCH_A:
case SSL3_ST_CR_KEY_EXCH_B:
ret = ssl3_get_server_key_exchange(s);
@@ -468,7 +478,7 @@
if (ret <= 0) {
goto end;
}
- s->state = SSL3_ST_CR_KEY_EXCH_A;
+ s->state = SSL3_ST_VERIFY_SERVER_CERT;
s->init_num = 0;
break;
@@ -946,7 +956,7 @@
}
int ssl3_get_server_certificate(SSL *s) {
- int al, i, ok, ret = -1;
+ int al, ok, ret = -1;
unsigned long n;
X509 *x = NULL;
STACK_OF(X509) *sk = NULL;
@@ -1004,14 +1014,6 @@
x = NULL;
}
- i = ssl_verify_cert_chain(s, sk);
- if (s->verify_mode != SSL_VERIFY_NONE && i <= 0) {
- al = ssl_verify_alarm_type(s->verify_result);
- OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
- goto f_err;
- }
- ERR_clear_error(); /* but we keep s->verify_result */
-
X509 *leaf = sk_X509_value(sk, 0);
if (!ssl3_check_certificate_for_cipher(leaf, s->s3->tmp.new_cipher)) {
al = SSL_AD_ILLEGAL_PARAMETER;
@@ -2197,3 +2199,17 @@
}
return i;
}
+
+int ssl3_verify_server_cert(SSL *s) {
+ int ret = ssl_verify_cert_chain(s, s->session->cert_chain);
+ if (s->verify_mode != SSL_VERIFY_NONE && ret <= 0) {
+ int al = ssl_verify_alarm_type(s->verify_result);
+ ssl3_send_alert(s, SSL3_AL_FATAL, al);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED);
+ } else {
+ ret = 1;
+ ERR_clear_error(); /* but we keep s->verify_result */
+ }
+
+ return ret;
+}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 97044ab..eb5bdae 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -301,10 +301,29 @@
return 1;
}
-static int SkipVerify(int preverify_ok, X509_STORE_CTX *store_ctx) {
+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());
+ const TestConfig *config = GetConfigPtr(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 1;
}
+static int VerifyFail(X509_STORE_CTX *store_ctx, void *arg) {
+ store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION;
+ return 0;
+}
+
static int NextProtosAdvertisedCallback(SSL *ssl, const uint8_t **out,
unsigned int *out_len, void *arg) {
const TestConfig *config = GetConfigPtr(ssl);
@@ -675,6 +694,12 @@
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);
+ }
+
return ssl_ctx;
}
@@ -908,6 +933,17 @@
}
}
+ if (config->expect_verify_result) {
+ int expected_verify_result = config->verify_fail ?
+ X509_V_ERR_APPLICATION_VERIFICATION :
+ X509_V_OK;
+
+ if (SSL_get_verify_result(ssl) != expected_verify_result) {
+ fprintf(stderr, "Wrong certificate verification result\n");
+ return false;
+ }
+ }
+
if (!config->is_server) {
/* Clients should expect a peer certificate chain iff this was not a PSK
* cipher suite. */
@@ -955,7 +991,10 @@
}
if (config->require_any_client_certificate) {
SSL_set_verify(ssl.get(), SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
- SkipVerify);
+ NULL);
+ }
+ if (config->verify_peer) {
+ SSL_set_verify(ssl.get(), SSL_VERIFY_PEER, 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 8124382..3c077bf 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2624,6 +2624,7 @@
"-enable-ocsp-stapling",
"-expect-ocsp-response",
base64.StdEncoding.EncodeToString(testOCSPResponse),
+ "-verify-peer",
},
})
@@ -2637,6 +2638,34 @@
},
})
+ tests = append(tests, testCase{
+ testType: clientTest,
+ name: "CertificateVerificationSucceed",
+ flags: []string{
+ "-verify-peer",
+ },
+ })
+
+ tests = append(tests, testCase{
+ testType: clientTest,
+ name: "CertificateVerificationFail",
+ flags: []string{
+ "-verify-fail",
+ "-verify-peer",
+ },
+ shouldFail: true,
+ expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ })
+
+ tests = append(tests, testCase{
+ testType: clientTest,
+ name: "CertificateVerificationSoftFail",
+ flags: []string{
+ "-verify-fail",
+ "-expect-verify-result",
+ },
+ })
+
if protocol == tls {
tests = append(tests, testCase{
name: "Renegotiate-Client",
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 7a87fd5..ad14ed3 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -93,6 +93,9 @@
{ "-check-close-notify", &TestConfig::check_close_notify },
{ "-shim-shuts-down", &TestConfig::shim_shuts_down },
{ "-microsoft-big-sslv3-buffer", &TestConfig::microsoft_big_sslv3_buffer },
+ { "-verify-fail", &TestConfig::verify_fail },
+ { "-verify-peer", &TestConfig::verify_peer },
+ { "-expect-verify-result", &TestConfig::expect_verify_result }
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index a41af49..5776095 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -92,6 +92,9 @@
bool check_close_notify = false;
bool shim_shuts_down = false;
bool microsoft_big_sslv3_buffer = false;
+ bool verify_fail = false;
+ bool verify_peer = false;
+ bool expect_verify_result = false;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_config);
