Skip to content

Commit

Permalink
tls: allow cert validation by only leaf trusted CA's CRL (#18289)
Browse files Browse the repository at this point in the history
Commit Message: Allow cert validation by only leaf trusted CAs CRL
Additional Description: Close #18268. In the previous implementation, we don't have availability to validate certs when all trusted CAs don't have their own CRLs if any trusted CAs have that. This feature allows validating even if all trusted CAs don't have CRLs.
Risk Level: Low
Testing: Unit
Docs Changes: Required
Release Notes: Required

Signed-off-by: Shikugawa <rei@tetrate.io>
  • Loading branch information
Shikugawa committed Nov 2, 2021
1 parent ca13cce commit 56e8c45
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 6 deletions.
13 changes: 11 additions & 2 deletions api/envoy/extensions/transport_sockets/tls/v3/common.proto
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ message CertificateProviderPluginInstance {
string certificate_name = 2;
}

// [#next-free-field: 14]
// [#next-free-field: 15]
message CertificateValidationContext {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.auth.CertificateValidationContext";
Expand Down Expand Up @@ -292,6 +292,9 @@ message CertificateValidationContext {
// that if a CRL is provided for any certificate authority in a trust chain, a CRL must be
// provided for all certificate authorities in that chain. Failure to do so will result in
// verification failure for both revoked and unrevoked certificates from that chain.
// The behavior of requiring all certificates to contain CRLs if any do can be altered by
// setting :ref:`only_verify_leaf_cert_crl <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.only_verify_leaf_cert_crl>`
// true. If set to true, only the final certificate in the chain undergoes CRL verification.
//
// See :ref:`the TLS overview <arch_overview_ssl_enabling_verification>` for a list of common
// system CA locations.
Expand Down Expand Up @@ -417,7 +420,9 @@ message CertificateValidationContext {
// for any certificate authority in a trust chain, a CRL must be provided
// for all certificate authorities in that chain. Failure to do so will
// result in verification failure for both revoked and unrevoked certificates
// from that chain.
// from that chain. This default behavior can be altered by setting
// :ref:`only_verify_leaf_cert_crl <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.only_verify_leaf_cert_crl>` to
// true.
config.core.v3.DataSource crl = 7;

// If specified, Envoy will not reject expired certificates.
Expand All @@ -433,4 +438,8 @@ message CertificateValidationContext {
// Refer to the documentation for the specified validator. If you do not want a custom validation algorithm, do not set this field.
// [#extension-category: envoy.tls.cert_validator]
config.core.v3.TypedExtensionConfig custom_validator_config = 12;

// If this option is set to true, only the certificate at the end of the
// certificate chain will be subject to validation by :ref:`CRL <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.crl>`.
bool only_verify_leaf_cert_crl = 14;
}
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ New Features
* thrift_proxy: add upstream metrics to show decoding errors and whether exception is from local or remote, e.g. ``cluster.cluster_name.thrift.upstream_resp_exception_remote``.
* thrift_proxy: add host level success/error metrics where success is a reply of type success and error is any other response to a call.
* thrift_proxy: support subset lb when using request or route metadata.
* tls: added support for only verifying the leaf CRL in the certificate chain with :ref:`only_verify_leaf_cert_crl <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.only_verify_leaf_cert_crl>`.
* transport_socket: added :ref:`envoy.transport_sockets.tcp_stats <envoy_v3_api_msg_extensions.transport_sockets.tcp_stats.v3.Config>` which generates additional statistics gathered from the OS TCP stack.
* udp: add support for multiple listener filters.
* upstream: added the ability to :ref:`configure max connection duration <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_connection_duration>` for upstream clusters.
Expand Down
5 changes: 5 additions & 0 deletions envoy/ssl/certificate_validation_context_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class CertificateValidationContextConfig {
* @return a reference to the api object.
*/
virtual Api::Api& api() const PURE;

/**
* @return whether to validate certificate chain with all CRL or not.
*/
virtual bool onlyVerifyLeafCertificateCrl() const PURE;
};

using CertificateValidationContextConfigPtr = std::unique_ptr<CertificateValidationContextConfig>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
? absl::make_optional<envoy::config::core::v3::TypedExtensionConfig>(
config.custom_validator_config())
: absl::nullopt),
api_(api) {
api_(api), only_verify_leaf_cert_crl_(config.only_verify_leaf_cert_crl()) {
if (ca_cert_.empty() && custom_validator_config_ == absl::nullopt) {
if (!certificate_revocation_list_.empty()) {
throw EnvoyException(fmt::format("Failed to load CRL from {} without trusted CA",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte

Api::Api& api() const override { return api_; }

bool onlyVerifyLeafCertificateCrl() const override { return only_verify_leaf_cert_crl_; }

private:
const std::string ca_cert_;
const std::string ca_cert_path_;
Expand All @@ -61,6 +63,7 @@ class CertificateValidationContextConfigImpl : public CertificateValidationConte
TrustChainVerification trust_chain_verification_;
const absl::optional<envoy::config::core::v3::TypedExtensionConfig> custom_validator_config_;
Api::Api& api_;
const bool only_verify_leaf_cert_crl_;
};

} // namespace Ssl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ int DefaultCertValidator::initializeSslContexts(std::vector<SSL_CTX*> contexts,
absl::StrCat("Failed to load trusted CA certificates from ", config_->caCertPath()));
}
if (has_crl) {
X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
X509_STORE_set_flags(store, config_->onlyVerifyLeafCertificateCrl()
? X509_V_FLAG_CRL_CHECK
: X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
}
verify_mode = SSL_VERIFY_PEER;
verify_trusted_ca_ = true;
Expand Down Expand Up @@ -136,8 +138,9 @@ int DefaultCertValidator::initializeSslContexts(std::vector<SSL_CTX*> contexts,
X509_STORE_add_crl(store, item->crl);
}
}

X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
X509_STORE_set_flags(store, config_->onlyVerifyLeafCertificateCrl()
? X509_V_FLAG_CRL_CHECK
: X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class TestCertificateValidationContextConfig
}

Api::Api& api() const override { return *api_; }
bool onlyVerifyLeafCertificateCrl() const override { return false; }

private:
bool allow_expired_certificate_{false};
Expand Down
79 changes: 79 additions & 0 deletions test/extensions/transport_sockets/tls/ssl_socket_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4610,6 +4610,85 @@ TEST_P(SslSocketTest, RevokedIntermediateCertificateCRLInTrustedCA) {
testUtil(complete_unrevoked_test_options.setExpectedSerialNumber(TEST_SAN_DNS4_CERT_SERIAL));
}

TEST_P(SslSocketTest, NotRevokedLeafCertificateOnlyLeafCRLValidation) {
// The test checks that revoked certificate will makes the validation success even if we set
// only_verify_leaf_cert_crl to true.
//
// Trust chain contains:
// - Root authority certificate (i.e., ca_cert.pem)
// - Intermediate authority certificate (i.e., intermediate_ca_cert.pem)
// - Intermediate authority certificate revocation list (i.e., intermediate_ca_cert.crl)
//
// Trust chain omits (But this test will succeed):
// - Root authority certificate revocation list (i.e., ca_cert.crl)
const std::string incomplete_server_ctx_yaml = R"EOF(
common_tls_context:
tls_certificates:
certificate_chain:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_key.pem"
validation_context:
trusted_ca:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/intermediate_ca_cert_chain_with_crl.pem"
only_verify_leaf_cert_crl: true
)EOF";

// This should succeed, since the certificate has not been revoked.
const std::string unrevoked_client_ctx_yaml = R"EOF(
common_tls_context:
tls_certificates:
certificate_chain:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns4_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns4_key.pem"
)EOF";

TestUtilOptions complete_unrevoked_test_options(unrevoked_client_ctx_yaml,
incomplete_server_ctx_yaml, true, GetParam());
testUtil(complete_unrevoked_test_options.setExpectedSerialNumber(TEST_SAN_DNS4_CERT_SERIAL));
}

TEST_P(SslSocketTest, RevokedLeafCertificateOnlyLeafCRLValidation) {
// The test checks that revoked certificate will makes the validation fails even if we set
// only_verify_leaf_cert_crl to true.
//
// Trust chain contains:
// - Root authority certificate (i.e., ca_cert.pem)
// - Intermediate authority certificate (i.e., intermediate_ca_cert.pem)
// - Intermediate authority certificate revocation list (i.e., intermediate_ca_cert.crl)
//
// Trust chain omits (But this test will succeed):
// - Root authority certificate revocation list (i.e., ca_cert.crl)
const std::string incomplete_server_ctx_yaml = R"EOF(
common_tls_context:
tls_certificates:
certificate_chain:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/unittest_key.pem"
validation_context:
trusted_ca:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/intermediate_ca_cert_chain_with_crl.pem"
only_verify_leaf_cert_crl: true
)EOF";

// This should fail, since the certificate has been revoked.
const std::string revoked_client_ctx_yaml = R"EOF(
common_tls_context:
tls_certificates:
certificate_chain:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns3_cert.pem"
private_key:
filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/san_dns3_key.pem"
)EOF";

TestUtilOptions complete_revoked_test_options(revoked_client_ctx_yaml, incomplete_server_ctx_yaml,
false, GetParam());
testUtil(complete_revoked_test_options.setExpectedServerStats("ssl.fail_verify_error")
.setExpectedVerifyErrorCode(X509_V_ERR_CERT_REVOKED));
}

TEST_P(SslSocketTest, GetRequestedServerName) {
envoy::config::listener::v3::Listener listener;
envoy::config::listener::v3::FilterChain* filter_chain = listener.add_filter_chains();
Expand Down
1 change: 1 addition & 0 deletions test/mocks/ssl/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ class MockCertificateValidationContextConfig : public CertificateValidationConte
MOCK_METHOD(envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext::
TrustChainVerification,
trustChainVerification, (), (const));
MOCK_METHOD(bool, onlyVerifyLeafCertificateCrl, (), (const));
};

class MockPrivateKeyMethodManager : public PrivateKeyMethodManager {
Expand Down

0 comments on commit 56e8c45

Please sign in to comment.