Skip to content

Commit

Permalink
ssl: add support for multiple CAs for client certificates.
Browse files Browse the repository at this point in the history
Partially fixes envoyproxy#1220.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
  • Loading branch information
PiotrSikora committed Jul 12, 2017
1 parent 6f6b7a2 commit b199a59
Show file tree
Hide file tree
Showing 15 changed files with 211 additions and 6 deletions.
1 change: 1 addition & 0 deletions include/envoy/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ envoy_package()
envoy_cc_library(
name = "connection_interface",
hdrs = ["connection.h"],
external_deps = ["ssl"],
)

envoy_cc_library(
Expand Down
7 changes: 7 additions & 0 deletions include/envoy/ssl/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "envoy/common/pure.h"

#include "openssl/ssl.h"

namespace Envoy {
namespace Ssl {

Expand Down Expand Up @@ -42,6 +44,11 @@ class Connection {
* certificate, or no SAN field, or no URI.
**/
virtual std::string uriSanPeerCertificate() PURE;

/**
* @return underlying SSL object for use with BoringSSL.
**/
virtual SSL* rawSslForTest() PURE;
};

} // namespace Ssl
Expand Down
1 change: 1 addition & 0 deletions source/common/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ envoy_cc_library(
name = "connection_lib",
srcs = ["connection_impl.cc"],
hdrs = ["connection_impl.h"],
external_deps = ["ssl"],
deps = [
":context_lib",
"//source/common/common:assert_lib",
Expand Down
3 changes: 3 additions & 0 deletions source/common/ssl/connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "common/network/connection_impl.h"
#include "common/ssl/context_impl.h"

#include "openssl/ssl.h"

namespace Envoy {
namespace Ssl {

Expand All @@ -29,6 +31,7 @@ class ConnectionImpl : public Network::ConnectionImpl, public Connection {
std::string sha256PeerCertificateDigest() override;
std::string subjectPeerCertificate() override;
std::string uriSanPeerCertificate() override;
SSL* rawSslForTest() override { return ssl_.get(); }

private:
PostIoAction doHandshake();
Expand Down
14 changes: 9 additions & 5 deletions source/common/ssl/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ ContextImpl::ContextImpl(ContextManagerImpl& parent, Stats::Scope& scope, Contex
throw EnvoyException(
fmt::format("Failed to load verify locations file {}", config.caCertFile()));
}

// This will send an acceptable CA list to browsers which will prevent pop ups.
rc = SSL_CTX_add_client_CA(ctx_.get(), ca_cert_.get());
RELEASE_ASSERT(1 == rc);

verify_mode = SSL_VERIFY_PEER;
}

Expand Down Expand Up @@ -351,6 +346,15 @@ bssl::UniquePtr<SSL> ClientContextImpl::newSsl() const {
ServerContextImpl::ServerContextImpl(ContextManagerImpl& parent, Stats::Scope& scope,
ContextConfig& config, Runtime::Loader& runtime)
: ContextImpl(parent, scope, config), runtime_(runtime) {
if (!config.caCertFile().empty()) {
bssl::UniquePtr<STACK_OF(X509_NAME)> list(SSL_load_client_CA_file(config.caCertFile().c_str()));
if (nullptr == list) {
throw EnvoyException(fmt::format("Failed to load client CA file {}", config.caCertFile()));
}
SSL_CTX_set_client_CA_list(ctx_.get(), list.release());
// SSL_CTX_set_verify() was already set in ContextImpl::ContextImpl().
}

parsed_alt_alpn_protocols_ = parseAlpnProtocols(config.altAlpnProtocols());

if (!parsed_alpn_protocols_.empty()) {
Expand Down
1 change: 1 addition & 0 deletions test/common/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ envoy_cc_test(
"gen_unittest_certs.sh",
"//test/common/ssl/test_data:certs",
],
external_deps = ["ssl"],
deps = [
"//source/common/buffer:buffer_lib",
"//source/common/event:dispatcher_includes",
Expand Down
75 changes: 75 additions & 0 deletions test/common/ssl/connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "openssl/ssl.h"

namespace Envoy {
using testing::Invoke;
Expand Down Expand Up @@ -263,6 +264,80 @@ TEST_P(SslConnectionImplTest, FailedClientAuthHashVerification) {
GetParam());
}

TEST_P(SslConnectionImplTest, ClientAuthMultipleCAs) {
Stats::IsolatedStoreImpl stats_store;
Runtime::MockLoader runtime;

std::string server_ctx_json = R"EOF(
{
"cert_chain_file": "{{ test_tmpdir }}/unittestcert.pem",
"private_key_file": "{{ test_tmpdir }}/unittestkey.pem",
"ca_cert_file": "{{ test_rundir }}/test/common/ssl/test_data/ca_certificates.pem"
}
)EOF";

Json::ObjectSharedPtr server_ctx_loader = TestEnvironment::jsonLoadFromString(server_ctx_json);
ContextConfigImpl server_ctx_config(*server_ctx_loader);
ContextManagerImpl manager(runtime);
ServerContextPtr server_ctx(manager.createSslServerContext(stats_store, server_ctx_config));

Event::DispatcherImpl dispatcher;
Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(GetParam()), true);
Network::MockListenerCallbacks callbacks;
Network::MockConnectionHandler connection_handler;
Network::ListenerPtr listener =
dispatcher.createSslListener(connection_handler, *server_ctx, socket, callbacks, stats_store,
Network::ListenerOptions::listenerOptionsWithBindToPort());

std::string client_ctx_json = R"EOF(
{
"cert_chain_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_cert.pem",
"private_key_file": "{{ test_rundir }}/test/common/ssl/test_data/no_san_key.pem"
}
)EOF";

Json::ObjectSharedPtr client_ctx_loader = TestEnvironment::jsonLoadFromString(client_ctx_json);
ContextConfigImpl client_ctx_config(*client_ctx_loader);
ClientContextPtr client_ctx(manager.createSslClientContext(stats_store, client_ctx_config));
Network::ClientConnectionPtr client_connection =
dispatcher.createSslClientConnection(*client_ctx, socket.localAddress());

Ssl::Connection* ssl_connection = client_connection->ssl();
ASSERT(ssl_connection != nullptr);
SSL* ssl = ssl_connection->rawSslForTest();
ASSERT(ssl != nullptr);
SSL_set_cert_cb(ssl,
[](SSL* ssl, void*) -> int {
STACK_OF(X509_NAME)* list = SSL_get_client_CA_list(ssl);
EXPECT_NE(nullptr, list);
EXPECT_EQ(2U, sk_X509_NAME_num(list));
return 1;
},
nullptr);

client_connection->connect();

Network::ConnectionPtr server_connection;
Network::MockConnectionCallbacks server_connection_callbacks;
EXPECT_CALL(callbacks, onNewConnection_(_))
.WillOnce(Invoke([&](Network::ConnectionPtr& conn) -> void {
server_connection = std::move(conn);
server_connection->addConnectionCallbacks(server_connection_callbacks);
}));

EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected))
.WillOnce(Invoke([&](uint32_t) -> void {
server_connection->close(Network::ConnectionCloseType::NoFlush);
client_connection->close(Network::ConnectionCloseType::NoFlush);
dispatcher.exit();
}));
EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose));

dispatcher.run(Event::Dispatcher::RunType::Block);

EXPECT_EQ(1UL, stats_store.counter("ssl.handshake").value());
}

TEST_P(SslConnectionImplTest, SslError) {
Stats::IsolatedStoreImpl stats_store;
Runtime::MockLoader runtime;
Expand Down
5 changes: 4 additions & 1 deletion test/common/ssl/test_data/README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
# What are the identities, certificates and keys
There are 5 identities:
There are 6 identities:
- **CA**: Certificate Authority for **No SAN**, **SAN With URI** and **SAN With
DNS**. It has the self-signed certificate *ca_cert.pem*. *ca_key.pem* is its
private key.
-- **Fake CA**: Fake Certificate Authority used to validate verification logic.
- It has the self-signed certificate *fake_ca_cert.pem"*. *fake_ca_key.pem" is
- its private key.
- **No SAN**: It has the certificate *no_san_cert.pem*, signed by the **CA**.
The certificate does not have SAN field. *no_san_key.pem* is its private key.
- **SAN With URI**: It has the certificate *san_uri_cert.pem*, which is signed
Expand Down
36 changes: 36 additions & 0 deletions test/common/ssl/test_data/ca_certificates.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-----BEGIN CERTIFICATE-----
MIICzTCCAjagAwIBAgIJAPAEjHA3MX2BMA0GCSqGSIb3DQEBCwUAMHYxCzAJBgNV
BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNp
c2NvMQ0wCwYDVQQKEwRMeWZ0MRkwFwYDVQQLExBMeWZ0IEVuZ2luZWVyaW5nMRAw
DgYDVQQDEwdGYWtlIENBMB4XDTE3MDcwOTAxMzkzMloXDTI3MDcwNzAxMzkzMlow
djELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
biBGcmFuY2lzY28xDTALBgNVBAoTBEx5ZnQxGTAXBgNVBAsTEEx5ZnQgRW5naW5l
ZXJpbmcxEDAOBgNVBAMTB0Zha2UgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
AoGBAK/89R7j0kwhGh7NDAiuhlVmN1Hh73sGRzDBgON/ZfmJSgkvZIwQ4+hu0wZ6
wjN0am1iIKyp8O1OkmKurcfCRQM2QGiIUI7v7rSa0GzqENoEsQGXVk8/yhwD8Ys9
IPLTWXLVyh49yh5FCs6nt2/LrIuZX/K2cZQx+VuLCcg5sTllAgMBAAGjYzBhMA8G
A1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMB0GA1UdDgQWBBRRsE37S/H/
lCd1pWH4sCn9qrON3TAfBgNVHSMEGDAWgBRRsE37S/H/lCd1pWH4sCn9qrON3TAN
BgkqhkiG9w0BAQsFAAOBgQAvFbQm1PrqeWlZssPDPfI5dlo2KEL9VedIlThqDU4z
MStNR/Tfw2A0WtjOyjm2R0viqLu8JdDe8umUqO9DkRhZOaUtLgQEkaQn75z8WQPk
Sj+kenx0v3GMrPZMAsELiPG5PcSk8l5prDgU7VkzNvoCtypCsB7RQRyvZs52Qih3
Ug==
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIICzTCCAjagAwIBAgIJAOrzsOodDleaMA0GCSqGSIb3DQEBCwUAMHYxCzAJBgNV
BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNp
c2NvMQ0wCwYDVQQKEwRMeWZ0MRkwFwYDVQQLExBMeWZ0IEVuZ2luZWVyaW5nMRAw
DgYDVQQDEwdUZXN0IENBMB4XDTE3MDcwOTAxMzkzMloXDTI3MDcwNzAxMzkzMlow
djELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
biBGcmFuY2lzY28xDTALBgNVBAoTBEx5ZnQxGTAXBgNVBAsTEEx5ZnQgRW5naW5l
ZXJpbmcxEDAOBgNVBAMTB1Rlc3QgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
AoGBAJuJh8N5TheTHLKOxsLSAfiIu9VDeKPsV98KRJJaYCMoaof3j9wBs65HzIat
AunuV4DVZZ2c/x7/v741oWadYd3yqL7XSzQaeBvhXi+wv3g17FYrdxaowG7cfmsh
gCp7/9TRW0bRGL6Qp6od/u62L8dprdHXxnck/+sZMupam9YrAgMBAAGjYzBhMA8G
A1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMB0GA1UdDgQWBBQ7eKRRTxaE
kxxIKHoMrSuWQcp9eTAfBgNVHSMEGDAWgBQ7eKRRTxaEkxxIKHoMrSuWQcp9eTAN
BgkqhkiG9w0BAQsFAAOBgQCN00/2k9k8HNeJ8eYuFH10jnc+td7+OaYWpRSEKCS7
ux3KAu0UFt90mojEMClt4Y6uP4oXTWbRzMzAgQHldHU8Gkj8tYnv7mToX7Bh/xdc
19epzjCmo/4Q6+16GZZvltiFjkkHSZEVI5ggljy1QdMIPRegsKKmX9mjZSCSSXD6
SA==
-----END CERTIFICATE-----
9 changes: 9 additions & 0 deletions test/common/ssl/test_data/certs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,23 @@ set -e

# Uncomment the following lines if you want to regenerate the private keys.
# openssl genrsa -out ca_key.pem 1024
# openssl genrsa -out fake_ca_key.pem 1024
# openssl genrsa -out no_san_key.pem 1024
# openssl genrsa -out san_dns_key.pem 1024
# openssl genrsa -out san_uri_key.pem 1024
# openssl genrsa -out selfsigned_key.pem 1024

# Generate ca_cert.pem.
openssl req -new -key ca_key.pem -out ca_cert.csr -config ca_cert.cfg -batch -sha256
openssl x509 -req -days 3650 -in ca_cert.csr -signkey ca_key.pem -out ca_cert.pem -extensions v3_ca -extfile ca_cert.cfg

# Generate fake_ca_cert.pem.
openssl req -new -key fake_ca_key.pem -out fake_ca_cert.csr -config fake_ca_cert.cfg -batch -sha256
openssl x509 -req -days 3650 -in fake_ca_cert.csr -signkey fake_ca_key.pem -out fake_ca_cert.pem -extensions v3_ca -extfile fake_ca_cert.cfg

# Concatenate Fake CA (fake_ca_cert.pem) and Test CA (ca_cert.pem) to create CA file with multiple entries.
cat fake_ca_cert.pem ca_cert.pem > ca_certificates.pem

# Generate no_san_cert.pem.
openssl req -new -key no_san_key.pem -out no_san_cert.csr -config no_san_cert.cfg -batch -sha256
openssl x509 -req -days 730 -in no_san_cert.csr -sha256 -CA ca_cert.pem -CAkey ca_key.pem -CAcreateserial -out no_san_cert.pem -extensions v3_ca -extfile no_san_cert.cfg
Expand Down
29 changes: 29 additions & 0 deletions test/common/ssl/test_data/fake_ca_cert.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
[req]
distinguished_name = req_distinguished_name
req_extensions = v3_req

[req_distinguished_name]
countryName = US
countryName_default = US
stateOrProvinceName = California
stateOrProvinceName_default = California
localityName = San Francisco
localityName_default = San Francisco
organizationName = Lyft
organizationName_default = Lyft
organizationalUnitName = Lyft Engineering
organizationalUnitName_default = Lyft Engineering
commonName = Fake CA
commonName_default = Fake CA
commonName_max = 64

[v3_req]
basicConstraints = CA:TRUE
keyUsage = critical, cRLSign, keyCertSign
subjectKeyIdentifier = hash

[v3_ca]
basicConstraints = critical, CA:TRUE
keyUsage = critical, cRLSign, keyCertSign
subjectKeyIdentifier = hash
authorityKeyIdentifier = keyid:always
18 changes: 18 additions & 0 deletions test/common/ssl/test_data/fake_ca_cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-----BEGIN CERTIFICATE-----
MIICzTCCAjagAwIBAgIJAPAEjHA3MX2BMA0GCSqGSIb3DQEBCwUAMHYxCzAJBgNV
BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNp
c2NvMQ0wCwYDVQQKEwRMeWZ0MRkwFwYDVQQLExBMeWZ0IEVuZ2luZWVyaW5nMRAw
DgYDVQQDEwdGYWtlIENBMB4XDTE3MDcwOTAxMzkzMloXDTI3MDcwNzAxMzkzMlow
djELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
biBGcmFuY2lzY28xDTALBgNVBAoTBEx5ZnQxGTAXBgNVBAsTEEx5ZnQgRW5naW5l
ZXJpbmcxEDAOBgNVBAMTB0Zha2UgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
AoGBAK/89R7j0kwhGh7NDAiuhlVmN1Hh73sGRzDBgON/ZfmJSgkvZIwQ4+hu0wZ6
wjN0am1iIKyp8O1OkmKurcfCRQM2QGiIUI7v7rSa0GzqENoEsQGXVk8/yhwD8Ys9
IPLTWXLVyh49yh5FCs6nt2/LrIuZX/K2cZQx+VuLCcg5sTllAgMBAAGjYzBhMA8G
A1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMB0GA1UdDgQWBBRRsE37S/H/
lCd1pWH4sCn9qrON3TAfBgNVHSMEGDAWgBRRsE37S/H/lCd1pWH4sCn9qrON3TAN
BgkqhkiG9w0BAQsFAAOBgQAvFbQm1PrqeWlZssPDPfI5dlo2KEL9VedIlThqDU4z
MStNR/Tfw2A0WtjOyjm2R0viqLu8JdDe8umUqO9DkRhZOaUtLgQEkaQn75z8WQPk
Sj+kenx0v3GMrPZMAsELiPG5PcSk8l5prDgU7VkzNvoCtypCsB7RQRyvZs52Qih3
Ug==
-----END CERTIFICATE-----
15 changes: 15 additions & 0 deletions test/common/ssl/test_data/fake_ca_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-----BEGIN RSA PRIVATE KEY-----
MIICXAIBAAKBgQCv/PUe49JMIRoezQwIroZVZjdR4e97BkcwwYDjf2X5iUoJL2SM
EOPobtMGesIzdGptYiCsqfDtTpJirq3HwkUDNkBoiFCO7+60mtBs6hDaBLEBl1ZP
P8ocA/GLPSDy01ly1coePcoeRQrOp7dvy6yLmV/ytnGUMflbiwnIObE5ZQIDAQAB
AoGAMwxLBdTLsW2AqCKONQ56xNbLgSLqVmMxz0Cx5EuWBtX99cAbiE117nwHFkAR
iN9h56ypEayRyCQcbieBHQqK/bJfWZ6qembwyFsqnha//i8vGvUInfkla0/b+E+V
o4V/+lNNYK6FIJCPayu/VjadTmRQ6YFJp0b+sJK/7fJBEhkCQQDWuSNopTqAz0Dg
EIq4KngCYFdky+AICSKH2MNv35M9yFHiycojoOxkY93t93XI0IMEpw8X/Yi4tQzy
1V4p2rJvAkEA0dGbETtTxMEkOTASAoX580TdvuU2ciBP16AeEyXOSpCQwjPMTnDZ
dcgLe0IQgERVDKNfNnLaJnmBgX5IXbArawJAUdZKKo4e2A0lusBTPpHhH3a8mEwj
3Kwunvv8sNzTb46ztu4VvrKMpk5xvUq9d4YRCWrqk8grncpTXxH3S5hwvQJBAM5F
f2owZgkfS+pA3OPILNNBF7LtClqmc3frPMXcO/NILlgNrkRLYy4MjWUOrMQ86VP0
ZM1VmiuK9ouEx8X7RKMCQB4NOyFDGIP0IKNCrLKZcMG2c3utcd23CbwH70BgH9kt
2SOc0pEXdbuVLtgofLS/dULXj7gyDekYejZzqYeY/1E=
-----END RSA PRIVATE KEY-----
1 change: 1 addition & 0 deletions test/mocks/ssl/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ envoy_cc_mock(
name = "ssl_mocks",
srcs = ["mocks.cc"],
hdrs = ["mocks.h"],
external_deps = ["ssl"],
deps = [
"//include/envoy/ssl:connection_interface",
"//include/envoy/ssl:context_config_interface",
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/ssl/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "envoy/stats/stats.h"

#include "gmock/gmock.h"
#include "openssl/ssl.h"

namespace Envoy {
namespace Ssl {
Expand Down Expand Up @@ -43,6 +44,7 @@ class MockConnection : public Connection {
MOCK_METHOD0(sha256PeerCertificateDigest, std::string());
MOCK_METHOD0(subjectPeerCertificate, std::string());
MOCK_METHOD0(uriSanPeerCertificate, std::string());
MOCK_METHOD0(rawSslForTest, SSL*());
};

class MockClientContext : public ClientContext {
Expand Down

0 comments on commit b199a59

Please sign in to comment.