Add tests for what happens when no certificate is configured We have ssl_has_certificate and ssl_has_private_key calls scattered throughout libssl, but they're never actually tested. The checks are also a little subtle because of cert->chain's weird representation of the leaf being missing but a chain configured. In hindsight, possibly we should have made them separate fields, but it's too late now. We'd have to get rid of SSL_CTX_get0_chain and SSL_get0_chain. Normally we don't bother with these functions, under the "you should know what you configured" theory, but one caller needed it recently in https://boringssl-review.googlesource.com/c/boringssl/+/66087 The tests also confirm that most of the ssl_has_private_key calls, other than the one in ssl_has_certificate, are redundant. The ssl_has_certificate calls are also in an odd place. This will all get shuffled around with SSL_CREDENTIAL, so set up tests first. Bug: 249 Change-Id: If1bb7097a15649e593886c3c22e2cc829a853830 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66508 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index d7953e5..bc0c842 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -9097,5 +9097,90 @@ EXPECT_FALSE(SSL_get_privatekey(ssl2.get())); } +// Test that the server handshake cleanly fails if it had no certificate +// configured, at all versions. +TEST_P(SSLVersionTest, NoCertOrKey) { + bssl::UniquePtr<X509> cert = GetChainTestCertificate(); + ASSERT_TRUE(cert); + bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey(); + ASSERT_TRUE(key); + bssl::UniquePtr<X509> intermediate = GetChainTestIntermediate(); + ASSERT_TRUE(intermediate); + bssl::UniquePtr<STACK_OF(X509)> chain(sk_X509_new_null()); + ASSERT_TRUE(chain); + ASSERT_TRUE(bssl::PushToStack(chain.get(), std::move(intermediate))); + + const struct { + bool has_cert; + bool has_key; + bool has_chain; + } kTests[] = { + // If nothing is configured, there is unambiguously no certificate. + {/*has_cert=*/false, /*has_key=*/false, /*has_chain=*/false}, + + // If only one of the key and certificate is configured, it is still treated + // as if there is no certificate. + {/*has_cert=*/true, /*has_key=*/false, /*has_chain=*/false}, + {/*has_cert=*/false, /*has_key=*/true, /*has_chain=*/false}, + + // The key and intermediates may be configured, but without a leaf there is + // no certificate. This case is interesting because we internally store the + // chain with a somewhat fragile null fist entry. + {/*has_cert=*/false, /*has_key=*/true, /*has_chain=*/true}, + }; + for (const auto &t : kTests) { + SCOPED_TRACE(testing::Message() << "has_cert = " << t.has_cert); + SCOPED_TRACE(testing::Message() << "has_key = " << t.has_key); + SCOPED_TRACE(testing::Message() << "has_chain = " << t.has_chain); + for (bool client : {false, true}) { + SCOPED_TRACE(testing::Message() << "client = " << client); + + EXPECT_NO_FATAL_FAILURE(ResetContexts()); + if (client) { + // Request client certificates from the server. + SSL_CTX_set_verify(server_ctx_.get(), SSL_VERIFY_PEER, nullptr); + SSL_CTX_set_cert_verify_callback(client_ctx_.get(), VerifySucceed, + nullptr); + } else { + // Recreate the server context. ResetContexts automatically adds server + // certificates. + server_ctx_ = CreateContext(); + ASSERT_TRUE(server_ctx_); + } + + SSL_CTX *ctx = client ? client_ctx_.get() : server_ctx_.get(); + if (t.has_cert) { + ASSERT_TRUE(SSL_CTX_use_certificate(ctx, cert.get())); + } + if (t.has_key) { + ASSERT_TRUE(SSL_CTX_use_PrivateKey(ctx, key.get())); + } + if (t.has_chain) { + ASSERT_TRUE(SSL_CTX_set1_chain(ctx, chain.get())); + } + + // In each of these cases, |SSL_CTX_check_private_key| should report the + // certificate was not configured. + EXPECT_FALSE(SSL_CTX_check_private_key(ctx)); + ERR_clear_error(); + + if (client) { + // The client should cleanly handshake without asserting a certificate. + EXPECT_TRUE(Connect()); + EXPECT_FALSE(SSL_get0_peer_certificates(server_.get())); + } else { + // Servers cannot be anonymous. The connection should fail. + EXPECT_FALSE(Connect()); + // Depending on the TLS version, this should either appear as + // NO_SHARED_CIPHER (TLS 1.2) or NO_CERTIFICATE_SET (TLS 1.3). + uint32_t err = ERR_get_error(); + if (!ErrorEquals(err, ERR_LIB_SSL, SSL_R_NO_SHARED_CIPHER)) { + EXPECT_TRUE(ErrorEquals(err, ERR_LIB_SSL, SSL_R_NO_CERTIFICATE_SET)); + } + } + } + } +} + } // namespace BSSL_NAMESPACE_END