ssl_verify_peer_cert: implement |SSL_VERIFY_NONE| as advertised.
Since SSL{,_CTX}_set_custom_verify take a |mode| parameter that may be
|SSL_VERIFY_NONE|, it should do what it says on the tin, which is to
perform verification and ignore the result.
Change-Id: I0d8490111fb199c6b325cc167cf205316ecd4b49
Reviewed-on: https://boringssl-review.googlesource.com/25224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 247a145..a44241a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -192,7 +192,9 @@
OPENSSL_EXPORT const SSL_METHOD *DTLS_method(void);
// TLS_with_buffers_method is like |TLS_method|, but avoids all use of
-// crypto/x509.
+// crypto/x509. All client connections created with |TLS_with_buffers_method|
+// will fail unless a certificate verifier is installed with
+// |SSL_set_custom_verify| or |SSL_CTX_set_custom_verify|.
OPENSSL_EXPORT const SSL_METHOD *TLS_with_buffers_method(void);
// DTLS_with_buffers_method is like |DTLS_method|, but avoids all use of
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index cdd1b32..43afc6c 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -334,6 +334,11 @@
hs->new_session->verify_result = X509_V_OK;
break;
case ssl_verify_invalid:
+ // If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the result.
+ if (ssl->verify_mode == SSL_VERIFY_NONE) {
+ ERR_clear_error();
+ ret = ssl_verify_ok;
+ }
hs->new_session->verify_result = X509_V_ERR_APPLICATION_VERIFICATION;
break;
case ssl_verify_retry:
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index a2a53f6..6c4282e 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -3141,6 +3141,83 @@
server_ctx.get()));
}
+TEST(SSLTest, BuffersFailWithoutCustomVerify) {
+ bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(client_ctx);
+ bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(server_ctx);
+
+ bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
+ ASSERT_TRUE(key);
+ bssl::UniquePtr<CRYPTO_BUFFER> leaf = GetChainTestCertificateBuffer();
+ ASSERT_TRUE(leaf);
+ std::vector<CRYPTO_BUFFER*> chain = { leaf.get() };
+ ASSERT_TRUE(SSL_CTX_set_chain_and_key(server_ctx.get(), &chain[0],
+ chain.size(), key.get(), nullptr));
+
+ // Without SSL_CTX_set_custom_verify(), i.e. with everything in the default
+ // configuration, certificate verification should fail.
+ bssl::UniquePtr<SSL> client, server;
+ ASSERT_FALSE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get()));
+
+ // Whereas with a verifier, the connection should succeed.
+ 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;
+ });
+ ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get()));
+}
+
+TEST(SSLTest, CustomVerify) {
+ bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(client_ctx);
+ bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(server_ctx);
+
+ bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
+ ASSERT_TRUE(key);
+ bssl::UniquePtr<CRYPTO_BUFFER> leaf = GetChainTestCertificateBuffer();
+ ASSERT_TRUE(leaf);
+ std::vector<CRYPTO_BUFFER*> chain = { leaf.get() };
+ ASSERT_TRUE(SSL_CTX_set_chain_and_key(server_ctx.get(), &chain[0],
+ chain.size(), key.get(), nullptr));
+
+ 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(),
+ server_ctx.get()));
+
+ // With SSL_VERIFY_PEER, ssl_verify_invalid should result in a dropped
+ // connection.
+ SSL_CTX_set_custom_verify(
+ client_ctx.get(), SSL_VERIFY_PEER,
+ [](SSL *ssl, uint8_t *out_alert) -> ssl_verify_result_t {
+ return ssl_verify_invalid;
+ });
+
+ ASSERT_FALSE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get()));
+
+ // But with SSL_VERIFY_NONE, ssl_verify_invalid should not cause a dropped
+ // connection.
+ SSL_CTX_set_custom_verify(
+ client_ctx.get(), SSL_VERIFY_NONE,
+ [](SSL *ssl, uint8_t *out_alert) -> ssl_verify_result_t {
+ return ssl_verify_invalid;
+ });
+
+ ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get()));
+}
+
TEST(SSLTest, ClientCABuffers) {
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_with_buffers_method()));
ASSERT_TRUE(client_ctx);
