Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require peer certificate to be present when ca_cert_file is set #648

Merged
merged 2 commits into from
Mar 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/configuration/cluster_manager/cluster_ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ private_key_file

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

verify_certificate_hash
*(optional, string)* If specified, Envoy will verify (pin) the hash of the presented server
Expand Down
3 changes: 2 additions & 1 deletion docs/configuration/listeners/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ 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.
will not be verified. If specified, a client must present a valid certificate or the
connection will be rejected.

verify_certificate_hash
*(optional, string)* If specified, Envoy will verify (pin) the hash of the presented client
Expand Down
5 changes: 5 additions & 0 deletions source/common/ssl/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ 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: 36 additions & 0 deletions test/common/ssl/context_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,40 @@ 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