Skip to content

Commit

Permalink
tls: support for ECDSA P-384 and P-521 certificates (#10855)
Browse files Browse the repository at this point in the history
Signed-off-by: anitabyte <[email protected]>
  • Loading branch information
anitabyte committed Sep 27, 2024
1 parent 39851cd commit d1821b2
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 25 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ bug_fixes:
- area: tracing
change: |
Fixed a bug where the OpenTelemetry tracer exports the OTLP request even when no spans are present.
- area: tls
change: |
Added support for P-384 and P-521 curves for server certificates.
removed_config_or_runtime:
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`
Expand Down
6 changes: 3 additions & 3 deletions docs/root/intro/arch_overview/security/ssl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,15 @@ Certificate selection
---------------------

:ref:`DownstreamTlsContexts <envoy_v3_api_msg_extensions.transport_sockets.tls.v3.DownstreamTlsContext>` support multiple TLS
certificates. These may be a mix of RSA and P-256 ECDSA certificates for multiple server name patterns.
certificates. These may be a mix of RSA and ECDSA certificates for multiple server name patterns.

Certificate config/loading rules:

* DNS SANs or Subject Common Name is extracted as server name pattern to match SNI during handshake. Subject Common Name is not used if DNS SANs are present in the certificate.
* FQDN like "test.example.com" and wildcard like "\*.example.com" are valid at the same time, which will be loaded
as two different server name patterns.
* If multiple certificates of a particular type (RSA or ECDSA) are specified for the same name or name pattern, the first one loaded is used for that name.
* Non-P-256 server ECDSA certificates are rejected.
* Non-P-256, P-384 or P-521 server ECDSA certificates are rejected.
* Static and SDS certificates may not be mixed in a given :ref:`DownstreamTlsContext
<envoy_v3_api_msg_extensions.transport_sockets.tls.v3.DownstreamTlsContext>`.

Expand All @@ -144,7 +144,7 @@ Certificate selection rules:
is false or true.
* Full scan execuates OCSP and key type checking on each cert which is the same as described above in exact SNI matching.
It falls back to the first cert in the whole list if there is no cert selected.
* Currently only two kinds of key type are supported, RSA or ECDSA. If the client supports P-256 ECDSA, the P-256 ECDSA certificate
* Currently only two kinds of key type are supported, RSA or ECDSA. If the client supports P-256, P384 or P-521 ECDSA, the P-256, P384 or P-521 ECDSA certificate
is preferred over RSA. The certificate that it falls back to might result in a failed handshake. For instance, a client only supports
RSA certificates and the certificate only support ECDSA.
* The final selected certificate must adhere to the OCSP policy. If no such certificate is found, the connection is refused.
Expand Down
10 changes: 6 additions & 4 deletions source/common/tls/context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,18 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
ctx.is_ecdsa_ = pkey_id == EVP_PKEY_EC;
switch (pkey_id) {
case EVP_PKEY_EC: {
// We only support P-256 ECDSA today.
// We only support P-256, P384 or P-521 ECDSA today.
const EC_KEY* ecdsa_public_key = EVP_PKEY_get0_EC_KEY(public_key.get());
// Since we checked the key type above, this should be valid.
ASSERT(ecdsa_public_key != nullptr);
const EC_GROUP* ecdsa_group = EC_KEY_get0_group(ecdsa_public_key);
if (ecdsa_group == nullptr ||
EC_GROUP_get_curve_name(ecdsa_group) != NID_X9_62_prime256v1) {
(EC_GROUP_get_curve_name(ecdsa_group) != NID_X9_62_prime256v1 &&
EC_GROUP_get_curve_name(ecdsa_group) != NID_secp384r1 &&
EC_GROUP_get_curve_name(ecdsa_group) != NID_secp521r1)) {
creation_status = absl::InvalidArgumentError(
fmt::format("Failed to load certificate chain from {}, only P-256 "
"ECDSA certificates are supported",
fmt::format("Failed to load certificate chain from {}, only P-256, "
"P384 or P-521 ECDSA certificates are supported",
ctx.cert_chain_file_path_));
return;
}
Expand Down
4 changes: 3 additions & 1 deletion source/common/tls/server_context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,9 @@ bool ServerContextImpl::isClientEcdsaCapable(const SSL_CLIENT_HELLO& ssl_client_
CBS_len(&signature_algorithms_ext) != 0) {
return false;
}
if (cbsContainsU16(signature_algorithms, SSL_SIGN_ECDSA_SECP256R1_SHA256)) {
if (cbsContainsU16(signature_algorithms, SSL_SIGN_ECDSA_SECP256R1_SHA256) ||
cbsContainsU16(signature_algorithms, SSL_SIGN_ECDSA_SECP384R1_SHA384) ||
cbsContainsU16(signature_algorithms, SSL_SIGN_ECDSA_SECP521R1_SHA512)) {
return true;
}
}
Expand Down
38 changes: 22 additions & 16 deletions test/common/tls/context_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1408,8 +1408,8 @@ TEST_F(ClientContextConfigImplTest, P256EcdsaCert) {
auto cleanup = cleanUpHelper(context);
}

// Validate that non-P256 ECDSA certs are rejected.
TEST_F(ClientContextConfigImplTest, NonP256EcdsaCert) {
// Validate that P384 ECDSA certs load.
TEST_F(ClientContextConfigImplTest, P384EcdsaCert) {
envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context;
const std::string tls_certificate_yaml = R"EOF(
certificate_chain:
Expand All @@ -1421,16 +1421,12 @@ TEST_F(ClientContextConfigImplTest, NonP256EcdsaCert) {
*tls_context.mutable_common_tls_context()->add_tls_certificates());
auto client_context_config = *ClientContextConfigImpl::create(tls_context, factory_context_);
Stats::IsolatedStoreImpl store;
EXPECT_THAT(manager_.createSslClientContext(*store.rootScope(), *client_context_config)
.status()
.message(),
testing::ContainsRegex(
"Failed to load certificate chain from .*selfsigned_ecdsa_p384_cert.pem, "
"only P-256 ECDSA certificates are supported"));
auto context = *manager_.createSslClientContext(*store.rootScope(), *client_context_config);
auto cleanup = cleanUpHelper(context);
}

// Validate that non-P256 ECDSA certs are rejected loaded from `pkcs12`.
TEST_F(ClientContextConfigImplTest, NonP256EcdsaPkcs12) {
// Validate that P384 ECDSA certs are loaded from `pkcs12`.
TEST_F(ClientContextConfigImplTest, P384EcdsaPkcs12) {
envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context;
const std::string tls_certificate_yaml = R"EOF(
pkcs12:
Expand All @@ -1440,12 +1436,22 @@ TEST_F(ClientContextConfigImplTest, NonP256EcdsaPkcs12) {
*tls_context.mutable_common_tls_context()->add_tls_certificates());
auto client_context_config = *ClientContextConfigImpl::create(tls_context, factory_context_);
Stats::IsolatedStoreImpl store;
EXPECT_THAT(manager_.createSslClientContext(*store.rootScope(), *client_context_config)
.status()
.message(),
testing::ContainsRegex(
"Failed to load certificate chain from .*selfsigned_ecdsa_p384_certkey.p12, "
"only P-256 ECDSA certificates are supported"));
}

TEST_F(ClientContextConfigImplTest, P521EcdsaCert) {
envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context;
const std::string tls_certificate_yaml = R"EOF(
certificate_chain:
filename: "{{ test_rundir }}/test/common/tls/test_data/selfsigned_ecdsa_p521_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/common/tls/test_data/selfsigned_ecdsa_p521_key.pem"
)EOF";
TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate_yaml),
*tls_context.mutable_common_tls_context()->add_tls_certificates());
auto client_context_config = *ClientContextConfigImpl::create(tls_context, factory_context_);
Stats::IsolatedStoreImpl store;
auto context = *manager_.createSslClientContext(*store.rootScope(), *client_context_config);
auto cleanup = cleanUpHelper(context);
}

// Multiple TLS certificates are not yet supported.
Expand Down
5 changes: 4 additions & 1 deletion test/common/tls/test_data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ There are 15 identities:
using the config *selfsigned_cert.cfg*. *selfsigned_ecdsa_p256_key.pem* is
its private key.
- **Self-signed ECDSA P-384**: The self-signed certificate *selfsigned_ecdsa_p384_cert.pem*,
using the config *selfsigned_cert.cfg*. *selfsigned_ecdsa_p256_key.pem* is
using the config *selfsigned_cert.cfg*. *selfsigned_ecdsa_p384_key.pem* is
its private key.
- **Self-signed ECDSA P-521**: The self-signed certificate *selfsigned_ecdsa_p521_cert.pem*,
using the config *selfsigned_cert.cfg*. *selfsigned_ecdsa_p521_key.pem* is
its private key.
- **Expired**: A self-signed, expired certificate *expired_cert.pem*,
using the config *selfsigned_cert.cfg*. *expired_key.pem* is its private
Expand Down
7 changes: 7 additions & 0 deletions test/common/tls/test_data/certs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,13 @@ generate_ecdsa_key selfsigned_ecdsa_p384 secp384r1
generate_selfsigned_x509_cert selfsigned_ecdsa_p384
rm -f selfsigned_ecdsa_p384_cert.cfg

# Generate selfsigned_ecdsa_p521_cert.pem.
cp -f selfsigned_cert.cfg selfsigned_ecdsa_p521_cert.cfg
generate_ecdsa_key selfsigned_ecdsa_p521 secp521r1
generate_selfsigned_x509_cert selfsigned_ecdsa_p521
rm -f selfsigned_ecdsa_p521_cert.cfg


# Generate selfsigned_ecdsa_p384_certkey.p12 with no password.
openssl pkcs12 -export -out selfsigned_ecdsa_p384_certkey.p12 -inkey selfsigned_ecdsa_p384_key.pem -in selfsigned_ecdsa_p384_cert.pem -keypbe NONE -certpbe NONE -nomaciter -passout pass:

Expand Down
19 changes: 19 additions & 0 deletions test/common/tls/test_data/selfsigned_ecdsa_p521_cert.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIDHDCCAn2gAwIBAgIUfFEHcYA6qzTSMJKdKpDbr8j7lHgwCgYIKoZIzj0EAwIw
ejELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNh
biBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsMEEx5ZnQgRW5naW5l
ZXJpbmcxFDASBgNVBAMMC1Rlc3QgU2VydmVyMB4XDTI0MDkyNzE3NDI1OFoXDTI2
MDkyNzE3NDI1OFowejELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWEx
FjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsM
EEx5ZnQgRW5naW5lZXJpbmcxFDASBgNVBAMMC1Rlc3QgU2VydmVyMIGbMBAGByqG
SM49AgEGBSuBBAAjA4GGAAQAWMQGu6Rr2C1Ydc015kP2EwOz/xLh7yikFk8edJyc
WA9WwyXPGWFtpYl4LAlFH/kIHDnOsYnM/DJngqF1fOb8sngB1dEL+93YRvINvjmG
iBQ9qHg+/kEJLW5NOx53Dc1a4oia3Ey7b2/Gkm8A+CzN2CLbMzLHTesfgSSIP7D7
YNWlNAujgZ0wgZowDAYDVR0TAQH/BAIwADALBgNVHQ8EBAMCBeAwHQYDVR0lBBYw
FAYIKwYBBQUHAwIGCCsGAQUFBwMBMB4GA1UdEQQXMBWCE3NlcnZlcjEuZXhhbXBs
ZS5jb20wHQYDVR0OBBYEFLwTH64c7Glg6D61fFVfM+DHhYq7MB8GA1UdIwQYMBaA
FLwTH64c7Glg6D61fFVfM+DHhYq7MAoGCCqGSM49BAMCA4GMADCBiAJCAfuwJBMT
L9CAMp2CVxgkmNk19Ku6idjQ59mXB/UOdpDU4SSvhlZBNgXZNHehe1syTa35clGr
OV8T5KE80Yui8C2ZAkIBooI1HiRQ+Vrzs5WyJ/F6pVLqT9iIw/p0mIhoCtfTUsi4
RWQXJfm3OWqnYHhHba5riEQsUoTJV9bb4RdAUQrDlJc=
-----END CERTIFICATE-----
14 changes: 14 additions & 0 deletions test/common/tls/test_data/selfsigned_ecdsa_p521_cert_info.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#pragma once

// NOLINT(namespace-envoy)
// This file is auto-generated by certs.sh.
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_256_HASH[] =
"d893db88874c259f607cdd819a0b12a4852898ae7f80f9cf67c0f244e67f5b0d";
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_1_HASH[] =
"6f60b7fa1ee64a9dcf90a34f851bd39281b653a0";
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_SPKI[] =
"WWdJYW4tEaF/LBVKVcQSNO1vwerCpB2p7dw6GPksFlE=";
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_SERIAL[] =
"7c510771803aab34d230929d2a90dbafc8fb9478";
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_NOT_BEFORE[] = "Sep 27 17:42:58 2024 GMT";
constexpr char TEST_SELFSIGNED_ECDSA_P521_CERT_NOT_AFTER[] = "Sep 27 17:42:58 2026 GMT";
10 changes: 10 additions & 0 deletions test/common/tls/test_data/selfsigned_ecdsa_p521_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-----BEGIN EC PARAMETERS-----
BgUrgQQAIw==
-----END EC PARAMETERS-----
-----BEGIN EC PRIVATE KEY-----
MIHcAgEBBEIBvfR0YP6beGkBRT+6pwru4XdQYiVp6wHgYT4kVlmrJTk9QYRFtkLs
Tc+jEB6y7akyGuaAcwj1ZZe06YE4t8eyXCqgBwYFK4EEACOhgYkDgYYABABYxAa7
pGvYLVh1zTXmQ/YTA7P/EuHvKKQWTx50nJxYD1bDJc8ZYW2liXgsCUUf+QgcOc6x
icz8MmeCoXV85vyyeAHV0Qv73dhG8g2+OYaIFD2oeD7+QQktbk07HncNzVriiJrc
TLtvb8aSbwD4LM3YItszMsdN6x+BJIg/sPtg1aU0Cw==
-----END EC PRIVATE KEY-----

0 comments on commit d1821b2

Please sign in to comment.