diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 769fbf7c..1f6ca313 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -563,6 +563,7 @@ typedef struct ssl3_state_st { /* extra state */ #define SSL3_ST_CW_FLUSH (0x100 | SSL_ST_CONNECT) #define SSL3_ST_FALSE_START (0x101 | SSL_ST_CONNECT) +#define SSL3_ST_VERIFY_SERVER_CERT (0x102 | SSL_ST_CONNECT) /* write to server */ #define SSL3_ST_CW_CLNT_HELLO_A (0x110 | SSL_ST_CONNECT) #define SSL3_ST_CW_CLNT_HELLO_B (0x111 | SSL_ST_CONNECT) diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c index 6edf2c39..a3c1c37c 100644 --- a/ssl/d1_clnt.c +++ b/ssl/d1_clnt.c @@ -260,7 +260,7 @@ int dtls1_connect(SSL *s) { 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 @@ int dtls1_connect(SSL *s) { 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 @@ int dtls1_connect(SSL *s) { 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 d26fd6d0..5e0dcb60 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1119,6 +1119,7 @@ int ssl3_get_server_key_exchange(SSL *s); 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 485c29f9..6e82fff6 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -276,7 +276,7 @@ int ssl3_connect(SSL *s) { 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 @@ int ssl3_connect(SSL *s) { 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 @@ int ssl3_connect(SSL *s) { 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 @@ err: } 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 @@ int ssl3_get_server_certificate(SSL *s) { 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 @@ int ssl_do_client_cert_cb(SSL *s, X509 **px509, EVP_PKEY **ppkey) { } 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 97044ab4..eb5bdae5 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -301,10 +301,29 @@ static int SelectCertificateCallback(const struct ssl_early_callback_ctx *ctx) { 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 @@ static ScopedSSL_CTX SetupCtx(const TestConfig *config) { 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 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume) { } } + 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 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx, } 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 8124382e..3c077bf4 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -2624,6 +2624,7 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol) "-enable-ocsp-stapling", "-expect-ocsp-response", base64.StdEncoding.EncodeToString(testOCSPResponse), + "-verify-peer", }, }) @@ -2637,6 +2638,34 @@ func addStateMachineCoverageTests(async, splitHandshake bool, protocol protocol) }, }) + 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 7a87fd58..ad14ed31 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -93,6 +93,9 @@ const Flag kBoolFlags[] = { { "-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 kStringFlags[] = { diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index a41af49e..57760958 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -92,6 +92,9 @@ struct TestConfig { 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);