diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc index 1aa65219d78e..6d357078169d 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc @@ -56,10 +56,12 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str) return nullptr; } + bool success = true; + auto status = (*trust_domains) - ->iterate([&spiffe_data](const std::string& domain_name, - const Envoy::Json::Object& domain_object) -> bool { + ->iterate([&spiffe_data, &success](const std::string& domain_name, + const Envoy::Json::Object& domain_object) -> bool { if (spiffe_data->trust_bundle_stores.contains(domain_name)) { ENVOY_LOG(warn, "Duplicate domain '{}' in SPIFFE bundle map", domain_name); } else { @@ -72,7 +74,7 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str) if (!keys.ok() || keys->empty()) { ENVOY_LOG(error, "No keys found in SPIFFE bundle for domain '{}'", domain_name); - return false; + return (success = false); } ENVOY_LOG(info, "Found '{}' keys for domain '{}'", keys->size(), domain_name); @@ -83,18 +85,18 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str) if (!use.ok() || *use != "x509-svid") { ENVOY_LOG(error, "missing or invalid 'use' field found in cert for domain: '{}'", domain_name); - return false; + return (success = false); } const auto& certs = key->getStringArray("x5c"); - if (!certs.ok()) { - ENVOY_LOG(error, "missing 'x5c' field found in cert for domain: '{}'", domain_name); - return false; + if (!certs.ok() || (*certs).size() == 0) { + ENVOY_LOG(error, "missing 'x5c' field found in keys for domain: '{}'", domain_name); + return (success = false); } for (const auto& cert : *certs) { std::string decoded_cert = Envoy::Base64::decode(cert); if (decoded_cert.empty()) { ENVOY_LOG(error, "Invalid or empty cert decoded in domain '{}'", domain_name); - return false; + return (success = false); } const unsigned char* cert_data = @@ -104,13 +106,13 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str) ENVOY_LOG(error, "Failed to create x509 object while loading certs in domain '{}'", domain_name); - return false; + return (success = false); } if (X509_STORE_add_cert(spiffe_data->trust_bundle_stores[domain_name].get(), x509.get()) != 1) { ENVOY_LOG(error, "Failed to add x509 object while loading certs for domain '{}'", domain_name); - return false; + return (success = false); } spiffe_data->ca_certs.push_back(std::move(x509)); } @@ -119,7 +121,7 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str) return true; }); - if (!status.ok()) { + if (!status.ok() || !success) { return nullptr; } diff --git a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc index c9cf6cd0f348..4057133e1e05 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc @@ -86,23 +86,24 @@ class TestSPIFFEValidator : public testing::Test { return ss.str(); } - void initialize(std::string yaml) { + void initialize(std::string yaml, std::string trust_bundle_file = "") { envoy::config::core::v3::TypedExtensionConfig typed_conf; TestUtility::loadFromYaml(yaml, typed_conf); config_ = std::make_unique( typed_conf, allow_expired_certificate_, san_matchers_); - EXPECT_CALL(factory_context_.dispatcher_, createFilesystemWatcher_()) - .WillRepeatedly(testing::Invoke([] { - Filesystem::MockWatcher* mock_watcher = new NiceMock(); - EXPECT_CALL( - *mock_watcher, - addWatch(TestEnvironment::substitute( - "{{ test_rundir }}/test/common/tls/test_data/trust_bundles.json"), - _, _)) - .WillRepeatedly(testing::Return(absl::OkStatus())); - return mock_watcher; - })); + if (trust_bundle_file != "") { + EXPECT_CALL(factory_context_.dispatcher_, createFilesystemWatcher_()) + .WillRepeatedly(testing::Invoke([trust_bundle_file] { + Filesystem::MockWatcher* mock_watcher = new NiceMock(); + EXPECT_CALL(*mock_watcher, addWatch(TestEnvironment::substitute( + "{{ test_rundir }}/test/common/tls/test_data/" + + trust_bundle_file), + _, _)) + .WillRepeatedly(testing::Return(absl::OkStatus())); + return mock_watcher; + })); + } validator_ = std::make_unique(config_.get(), stats_, factory_context_); } @@ -775,36 +776,36 @@ name: envoy.tls.cert_validator.spiffe TEST_F(TestSPIFFEValidator, InvalidTrustBundleMapConfig) { { - EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( + EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( name: envoy.tls.cert_validator.spiffe typed_config: "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig trust_bundles: - filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_empty_keys.json" + filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_empty_keys.json" )EOF")), - EnvoyException, "Failed to load SPIFFE Bundle map""); + EnvoyException, "Failed to load SPIFFE Bundle map"); } { - EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( + EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( name: envoy.tls.cert_validator.spiffe typed_config: "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig trust_bundles: - filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_invalid_key.json" + filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_invalid_key.json" )EOF")), - EnvoyException, "Failed to load SPIFFE Bundle map""); + EnvoyException, "Failed to load SPIFFE Bundle map"); } { - EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( + EXPECT_THROW_WITH_MESSAGE(initialize(TestEnvironment::substitute(R"EOF( name: envoy.tls.cert_validator.spiffe typed_config: "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig trust_bundles: - filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_missing_use.json" + filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles_missing_use.json" )EOF")), - EnvoyException, "Failed to load SPIFFE Bundle map""); + EnvoyException, "Failed to load SPIFFE Bundle map"); } } @@ -890,7 +891,8 @@ name: envoy.tls.cert_validator.spiffe "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig trust_bundles: filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles.json" - )EOF")); + )EOF"), + "trust_bundles.json"); X509StorePtr store = X509_STORE_new(); SSLContextPtr ssl_ctx = SSL_CTX_new(TLS_method()); @@ -957,7 +959,8 @@ name: envoy.tls.cert_validator.spiffe "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig trust_bundles: filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles.json" - )EOF")); + )EOF"), + "trust_bundles.json"); TestSslExtendedSocketInfo info; // Chain contains workload, intermediate, and ca cert, so it should be accepted. @@ -979,7 +982,7 @@ name: envoy.tls.cert_validator.spiffe .status); } -TEST_F(TestSPIFFEValidator, TestDoVerifyCertChainSANMatchingTrustBundles) { +TEST_F(TestSPIFFEValidator, TestDoVerifyCertChainSANMatchingTrustBundleMapping) { const auto config = TestEnvironment::substitute(R"EOF( name: envoy.tls.cert_validator.spiffe typed_config: @@ -1003,7 +1006,7 @@ name: envoy.tls.cert_validator.spiffe envoy::type::matcher::v3::StringMatcher matcher; matcher.set_prefix("spiffe://lyft.com/"); setSanMatchers({matcher}); - initialize(config); + initialize(config, "trust_bundles.json"); ValidationResults results = validator().doVerifyCertChain( *cert_chain, info.createValidateResultCallback(), /*transport_socket_options=*/nullptr, *ssl_ctx, {}, false, ""); @@ -1014,53 +1017,7 @@ name: envoy.tls.cert_validator.spiffe envoy::type::matcher::v3::StringMatcher matcher; matcher.set_prefix("spiffe://example.com/"); setSanMatchers({matcher}); - initialize(config); - ValidationResults results = validator().doVerifyCertChain( - *cert_chain, info.createValidateResultCallback(), - /*transport_socket_options=*/nullptr, *ssl_ctx, {}, false, ""); - EXPECT_EQ(ValidationResults::ValidationStatus::Failed, results.status); - EXPECT_EQ(Envoy::Ssl::ClientValidationStatus::Failed, results.detailed_status); - EXPECT_EQ(1, stats().fail_verify_san_.value()); - stats().fail_verify_san_.reset(); - } -} - -TEST_F(TestSPIFFEValidator, TestDoVerifyCertChainSANMatchingTrustBundlesMapping) { - const auto config = TestEnvironment::substitute(R"EOF( -name: envoy.tls.cert_validator.spiffe -typed_config: - "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig - trust_bundles: - filename: "{{ test_rundir }}/test/common/tls/test_data/trust_bundles.json" - )EOF"); - - X509StorePtr store = X509_STORE_new(); - SSLContextPtr ssl_ctx = SSL_CTX_new(TLS_method()); - // URI SAN = spiffe://lyft.com/test-team - auto cert = readCertFromFile( - TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_uri_cert.pem")); - X509StoreContextPtr store_ctx = X509_STORE_CTX_new(); - EXPECT_TRUE(X509_STORE_CTX_init(store_ctx.get(), store.get(), cert.get(), nullptr)); - bssl::UniquePtr cert_chain(sk_X509_new_null()); - sk_X509_push(cert_chain.get(), cert.release()); - TestSslExtendedSocketInfo info; - info.setCertificateValidationStatus(Envoy::Ssl::ClientValidationStatus::NotValidated); - { - envoy::type::matcher::v3::StringMatcher matcher; - matcher.set_prefix("spiffe://lyft.com/"); - setSanMatchers({matcher}); - initialize(config); - ValidationResults results = validator().doVerifyCertChain( - *cert_chain, info.createValidateResultCallback(), - /*transport_socket_options=*/nullptr, *ssl_ctx, {}, false, ""); - EXPECT_EQ(ValidationResults::ValidationStatus::Successful, results.status); - EXPECT_EQ(Envoy::Ssl::ClientValidationStatus::Validated, results.detailed_status); - } - { - envoy::type::matcher::v3::StringMatcher matcher; - matcher.set_prefix("spiffe://example.com/"); - setSanMatchers({matcher}); - initialize(config); + initialize(config, "trust_bundles.json"); ValidationResults results = validator().doVerifyCertChain( *cert_chain, info.createValidateResultCallback(), /*transport_socket_options=*/nullptr, *ssl_ctx, {}, false, "");