Skip to content

Commit

Permalink
Fix tests and exception handling
Browse files Browse the repository at this point in the history 
Signed-off-by: Brian Sonnenberg <[email protected]>
  • Loading branch information
@briansonnenberg
briansonnenberg committed Dec 11, 2024
1 parent c7d5d18 commit cb2c9d5
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 83 deletions.
24 changes: 13 additions & 11 deletions source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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 =
Expand All @@ -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));
}
Expand All @@ -119,7 +121,7 @@ SPIFFEValidator::parseTrustBundles(const std::string& trust_bundle_mapping_str)
return true;
});

if (!status.ok()) {
if (!status.ok() || !success) {
return nullptr;
}

Expand Down
101 changes: 29 additions & 72 deletions test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestCertificateValidationContextConfig>(
typed_conf, allow_expired_certificate_, san_matchers_);

EXPECT_CALL(factory_context_.dispatcher_, createFilesystemWatcher_())
.WillRepeatedly(testing::Invoke([] {
Filesystem::MockWatcher* mock_watcher = new NiceMock<Filesystem::MockWatcher>();
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<Filesystem::MockWatcher>();
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<SPIFFEValidator>(config_.get(), stats_, factory_context_);
}
Expand Down Expand Up @@ -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");
}
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand All @@ -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, "");
Expand All @@ -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<STACK_OF(X509)> 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, "");
Expand Down

0 comments on commit cb2c9d5

Please sign in to comment.