diff --git a/source/extensions/filters/http/gcp_authn/crypto_utils.cc b/source/extensions/filters/http/gcp_authn/crypto_utils.cc index a73dcfc27e160..49c6cd2ef3e1a 100644 --- a/source/extensions/filters/http/gcp_authn/crypto_utils.cc +++ b/source/extensions/filters/http/gcp_authn/crypto_utils.cc @@ -1,29 +1,16 @@ #include "source/extensions/filters/http/gcp_authn/crypto_utils.h" -#include #include #include -#include #include #include -#include #include -#include #include -#include #include -#include - -#include "envoy/api/api.h" -#include "envoy/secret/secret_provider.h" #include "source/common/common/base64.h" #include "source/common/common/logger.h" -#include "source/common/common/matchers.h" -#include "source/common/common/thread.h" -#include "source/common/config/datasource.h" -#include "source/common/tls/utility.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -34,27 +21,6 @@ namespace HttpFilters { namespace GcpAuthn { namespace { -std::vector getSubjectAltNames(X509* cert) { - std::vector sans = - Envoy::Extensions::TransportSockets::Tls::Utility::getSubjectAltNames(*cert, GEN_URI); - std::vector dns_sans = - Envoy::Extensions::TransportSockets::Tls::Utility::getSubjectAltNames(*cert, GEN_DNS); - sans.insert(sans.end(), dns_sans.begin(), dns_sans.end()); - return sans; -} - -bool validateSubjectAltNames(const std::vector& sans, - const std::vector& san_matchers) { - for (const auto& san : sans) { - for (const auto& matcher : san_matchers) { - if (matcher.match(san)) { - return true; - } - } - } - return false; -} - absl::StatusOr calculateFingerprint(X509* cert) { unsigned char* der = nullptr; int len = i2d_X509(cert, &der); @@ -73,47 +39,17 @@ absl::StatusOr calculateFingerprint(X509* cert) { } // namespace -absl::StatusOr getBase64EncodedCertificateFingerprint( - Secret::TlsCertificateConfigProviderSharedPtr tls_cert_provider, - const std::vector& san_matchers, Api::Api& api) { - // Config::DataSource::read() is blocking and should only be called on the - // main thread. - ASSERT_IS_MAIN_OR_TEST_THREAD(); - - if (san_matchers.empty()) { - return absl::InvalidArgumentError("SAN matchers are empty"); - } - - if (tls_cert_provider == nullptr) { - return absl::InvalidArgumentError("TLS certificate provider is null"); - } - const auto* secret = tls_cert_provider->secret(); - if (secret == nullptr) { - return absl::NotFoundError("Secret is null"); - } - - // Read certificate from secret. - const auto& cert_chain = secret->certificate_chain(); - auto file_content_or_error = Config::DataSource::read(cert_chain, true, api); - if (!file_content_or_error.ok()) { - return absl::InternalError("Failed to read certificate from data source"); +absl::StatusOr +CertFingerprinterImpl::getFingerprintFromPem(const std::string& pem) const { + if (pem.empty()) { + return absl::InvalidArgumentError("Certificate PEM content is empty"); } - std::string file_content = file_content_or_error.value(); - if (file_content.empty()) { - return absl::InvalidArgumentError("Certificate file content is empty"); - } - - bssl::UniquePtr bio(BIO_new_mem_buf(file_content.data(), file_content.size())); + bssl::UniquePtr bio(BIO_new_mem_buf(pem.data(), pem.size())); bssl::UniquePtr cert(PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr)); if (!cert) { - ENVOY_LOG_MISC(error, "Failed to parse certificate"); - return absl::InvalidArgumentError("Failed to parse certificate"); - } - - std::vector sans = getSubjectAltNames(cert.get()); - if (!validateSubjectAltNames(sans, san_matchers)) { - return absl::InvalidArgumentError("Subject Alternative Names do not match"); + ENVOY_LOG_MISC(error, "Failed to parse certificate from PEM"); + return absl::InvalidArgumentError("Failed to parse certificate from PEM"); } return calculateFingerprint(cert.get()); diff --git a/source/extensions/filters/http/gcp_authn/crypto_utils.h b/source/extensions/filters/http/gcp_authn/crypto_utils.h index 742dc2e429b3b..87f895957b0ec 100644 --- a/source/extensions/filters/http/gcp_authn/crypto_utils.h +++ b/source/extensions/filters/http/gcp_authn/crypto_utils.h @@ -1,13 +1,6 @@ #pragma once -#include #include -#include - -#include "envoy/api/api.h" -#include "envoy/secret/secret_provider.h" - -#include "source/common/common/matchers.h" #include "absl/status/statusor.h" @@ -16,9 +9,17 @@ namespace Extensions { namespace HttpFilters { namespace GcpAuthn { -absl::StatusOr getBase64EncodedCertificateFingerprint( - Secret::TlsCertificateConfigProviderSharedPtr tls_cert_provider, - const std::vector& san_matchers, Api::Api& api); +class CertFingerprinter { +public: + virtual ~CertFingerprinter() = default; + virtual absl::StatusOr getFingerprintFromPem(const std::string& pem) const = 0; +}; +using CertFingerprinterSharedPtr = std::shared_ptr; + +class CertFingerprinterImpl : public CertFingerprinter { +public: + absl::StatusOr getFingerprintFromPem(const std::string& pem) const override; +}; } // namespace GcpAuthn } // namespace HttpFilters diff --git a/test/extensions/filters/http/gcp_authn/BUILD b/test/extensions/filters/http/gcp_authn/BUILD index b444eb857230d..0336ae942fe77 100644 --- a/test/extensions/filters/http/gcp_authn/BUILD +++ b/test/extensions/filters/http/gcp_authn/BUILD @@ -119,12 +119,9 @@ envoy_extension_cc_test( extension_names = ["envoy.filters.http.gcp_authn"], deps = [ "//envoy/api:api_interface", - "//source/common/common:matchers_lib", "//source/extensions/filters/http/gcp_authn:crypto_utils", - "//test/mocks/secret:secret_mocks", "//test/test_common:environment_lib", "//test/test_common:utility_lib", "@abseil-cpp//absl/status", - "@envoy_api//envoy/extensions/transport_sockets/tls/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/http/gcp_authn/crypto_utils_test.cc b/test/extensions/filters/http/gcp_authn/crypto_utils_test.cc index acb73ca0d8028..ef97ae3880f3b 100644 --- a/test/extensions/filters/http/gcp_authn/crypto_utils_test.cc +++ b/test/extensions/filters/http/gcp_authn/crypto_utils_test.cc @@ -1,16 +1,7 @@ -#include -#include #include -#include #include "envoy/api/api.h" -#include "envoy/common/callback.h" -#include "envoy/extensions/transport_sockets/tls/v3/common.pb.h" -#include "envoy/extensions/transport_sockets/tls/v3/secret.pb.h" -#include "envoy/init/target.h" -#include "envoy/secret/secret_provider.h" -#include "source/common/common/matchers.h" #include "source/extensions/filters/http/gcp_authn/crypto_utils.h" #include "test/test_common/environment.h" @@ -21,37 +12,11 @@ #include "gtest/gtest.h" namespace Envoy { -namespace Secret { - -class MockTlsCertificateConfigProvider : public TlsCertificateConfigProvider { -public: - MockTlsCertificateConfigProvider(); - ~MockTlsCertificateConfigProvider() override; - - MOCK_METHOD(const envoy::extensions::transport_sockets::tls::v3::TlsCertificate*, secret, (), - (const)); - MOCK_METHOD(Common::CallbackHandlePtr, addValidationCallback, - (std::function)); - MOCK_METHOD(Common::CallbackHandlePtr, addUpdateCallback, (std::function)); - MOCK_METHOD(Common::CallbackHandlePtr, addRemoveCallback, (std::function)); - MOCK_METHOD(const Init::Target*, initTarget, ()); - MOCK_METHOD(void, start, ()); -}; - -MockTlsCertificateConfigProvider::MockTlsCertificateConfigProvider() = default; -MockTlsCertificateConfigProvider::~MockTlsCertificateConfigProvider() = default; - -} // namespace Secret - namespace Extensions { namespace HttpFilters { namespace GcpAuthn { namespace { -using testing::NiceMock; -using testing::Return; - class CryptoUtilsTest : public testing::Test { public: CryptoUtilsTest() : api_(Api::createApiForTest()) {} @@ -59,145 +24,31 @@ class CryptoUtilsTest : public testing::Test { Api::ApiPtr api_; }; -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintSuccess) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - +TEST_F(CryptoUtilsTest, GetFingerprintFromPemSuccess) { + CertFingerprinterImpl fingerprinter; std::string cert_path = TestEnvironment::runfilesPath("test/config/integration/certs/clientcert.pem"); - secret->mutable_certificate_chain()->set_filename(cert_path); - - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); + std::string cert_pem = TestEnvironment::readFileToStringForTest(cert_path); - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); + auto fingerprint = fingerprinter.getFingerprintFromPem(cert_pem); ASSERT_TRUE(fingerprint.ok()); - // SHA256 fingerprint of - // test/config/integration/certs/clientcert.pem // SPELLCHECKER(off) EXPECT_EQ(fingerprint.value(), "wH4U/EO5x7PZLxAE+R06ngcdnJOlivx2tMFDA646DzQ"); // SPELLCHECKER(on) } -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintSuccessDnsSan) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - std::string cert_path = - TestEnvironment::runfilesPath("test/config/integration/certs/upstreamlocalhostcert.pem"); - secret->mutable_certificate_chain()->set_filename(cert_path); - - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; - san_matchers.emplace_back(Matchers::StringMatcherImpl::createExactMatcher("localhost")); - - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - ASSERT_TRUE(fingerprint.ok()); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintNoSecret) { - auto secret_provider = std::make_shared>(); - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(nullptr)); - - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "Secret is null"); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintSanMismatch) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - std::string cert_path = - TestEnvironment::runfilesPath("test/config/integration/certs/clientcert.pem"); - secret->mutable_certificate_chain()->set_filename(cert_path); - - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://example.com/other-team")); - - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "Subject Alternative Names do not match"); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintEmptyMatchers) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - std::string cert_path = - TestEnvironment::runfilesPath("test/config/integration/certs/clientcert.pem"); - secret->mutable_certificate_chain()->set_filename(cert_path); - - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; // Empty - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "SAN matchers are empty"); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintNullProvider) { - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - auto fingerprint = getBase64EncodedCertificateFingerprint(nullptr, san_matchers, *api_); +TEST_F(CryptoUtilsTest, GetFingerprintFromPemEmpty) { + CertFingerprinterImpl fingerprinter; + auto fingerprint = fingerprinter.getFingerprintFromPem(""); EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "TLS certificate provider is null"); + EXPECT_EQ(fingerprint.status().message(), "Certificate PEM content is empty"); } -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintDataSourceReadFailure) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - secret->mutable_certificate_chain()->set_filename("/nonexistent_file.pem"); - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "Failed to read certificate from data source"); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintEmptyContent) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - secret->mutable_certificate_chain()->set_inline_string(""); - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); - EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "Certificate file content is empty"); -} - -TEST_F(CryptoUtilsTest, GetBase64EncodedCertificateFingerprintParseFailure) { - auto secret_provider = std::make_shared>(); - auto secret = std::make_unique(); - - secret->mutable_certificate_chain()->set_inline_string("invalid cert content"); - EXPECT_CALL(*secret_provider, secret()).WillRepeatedly(Return(secret.get())); - - std::vector san_matchers; - san_matchers.emplace_back( - Matchers::StringMatcherImpl::createExactMatcher("spiffe://lyft.com/frontend-team")); - auto fingerprint = getBase64EncodedCertificateFingerprint(secret_provider, san_matchers, *api_); +TEST_F(CryptoUtilsTest, GetFingerprintFromPemInvalid) { + CertFingerprinterImpl fingerprinter; + auto fingerprint = fingerprinter.getFingerprintFromPem("invalid cert content"); EXPECT_FALSE(fingerprint.ok()); - EXPECT_EQ(fingerprint.status().message(), "Failed to parse certificate"); + EXPECT_EQ(fingerprint.status().message(), "Failed to parse certificate from PEM"); } } // namespace