Skip to content

Commit

Permalink
Revert "Require peer certificate to be present when ca_cert_file is s…
Browse files Browse the repository at this point in the history
…et (#648)"

This reverts commit 1687644.
  • Loading branch information
mattklein123 committed Apr 1, 2017
1 parent bf3f23a commit 87c0282
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 45 deletions.
3 changes: 1 addition & 2 deletions docs/configuration/cluster_manager/cluster_ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ private_key_file

ca_cert_file
*(optional, string)* A file containing certificate authority certificates to use in verifying
a presented server certificate. If specified, a server must present a valid certificate or the
connection will be rejected.
a presented server certificate.

verify_certificate_hash
*(optional, string)* If specified, Envoy will verify (pin) the hash of the presented server
Expand Down
3 changes: 1 addition & 2 deletions docs/configuration/listeners/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ alt_alpn_protocols
ca_cert_file
*(optional, string)* A file containing certificate authority certificates to use in verifying
a presented client side certificate. If not specified and a client certificate is presented it
will not be verified. If specified, a client must present a valid certificate or the
connection will be rejected.
will not be verified.

verify_certificate_hash
*(optional, string)* If specified, Envoy will verify (pin) the hash of the presented client
Expand Down
5 changes: 0 additions & 5 deletions source/common/ssl/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,6 @@ bool ContextImpl::verifyPeer(SSL* ssl) const {

if (!cert.get()) {
stats_.no_certificate_.inc();

if (ca_cert_) {
// In case that ca_cert_ exists, reject the connection when peer certificate is not present.
verified = false;
}
}

if (!verify_subject_alt_name_list_.empty()) {
Expand Down
36 changes: 0 additions & 36 deletions test/common/ssl/context_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,40 +154,4 @@ TEST(SslContextImplTest, TestNoCert) {
EXPECT_EQ("", context->getCertChainInformation());
}

TEST(SslContextImplTest, TestVerifyPeer) {
std::string json = R"EOF(
{
"ca_cert_file": "test/common/ssl/test_data/ca.crt"
}
)EOF";

Json::ObjectPtr loader = TestEnvironment::jsonLoadFromString(json);
ContextConfigImpl cfg(*loader);
Runtime::MockLoader runtime;
ContextManagerImpl manager(runtime);
Stats::IsolatedStoreImpl store;
ClientContextPtr context(manager.createSslClientContext(store, cfg));
ClientContextImpl* contextImpl = dynamic_cast<ClientContextImpl*>(context.get());

SSL_CTX* ctx = SSL_CTX_new(SSLv23_server_method());
SSL* ssl = SSL_new(ctx);
SSL_SESSION* session = SSL_SESSION_new();
SSL_set_session(ssl, session);

// Test that verifyPeer returns false when peer certificate is not set.
EXPECT_FALSE(contextImpl->verifyPeer(ssl));

// Set peer certificate and make sure verifyPeer returns true.
FILE* fp = fopen("test/common/ssl/test_data/san_dns.crt", "r");
EXPECT_NE(fp, nullptr);
X509* cert = PEM_read_X509(fp, nullptr, nullptr, nullptr);
session->x509_peer = cert;
EXPECT_TRUE(contextImpl->verifyPeer(ssl));

fclose(fp);
SSL_SESSION_free(session);
SSL_free(ssl);
SSL_CTX_free(ctx);
}

} // Ssl

0 comments on commit 87c0282

Please sign in to comment.