Addressed comments

envoyproxy · Sep 26, 2024 · 06a982e · 06a982e
1 parent 78ad7dc
commit 06a982e
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 13 deletions.
diff --git a/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto b/api/envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto
@@ -80,11 +80,21 @@ message SPIFFECertValidatorConfig {
     config.core.v3.DataSource trust_bundle = 2;
   }
 
+  enum TrustBundleFormat {
+    TRUST_BUNDLE_MAP = 0;
+  }
+
+  message TrustBundlesSource {
+    TrustBundleFormat format = 1;
+    config.core.v3.DataSource source = 2;
+  }
+
   // This field specifies trust domains used for validating incoming X.509-SVID(s).
   repeated TrustDomain trust_domains = 1 [(validate.rules).repeated = {min_items: 1}];
 
-  // This field specifies a trust domain mapping as a json object. Mutually
-  // excluse with trust_domains.
-  config.core.v3.DataSource trust_bundle_map = 2;
+  // This field specifies  as a json object. If both
+  // trust_bundle_map and trust_domains are specified, trust_bundle_map will
+  // take precedence.
+  TrustBundlesSource trust_bundles = 2;
 
 }
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
@@ -33,7 +33,7 @@ namespace Tls {
 
 using SPIFFEConfig = envoy::extensions::transport_sockets::tls::v3::SPIFFECertValidatorConfig;
 
-std::shared_ptr<SpiffeData> SPIFFEValidator::loadTrustBundleMap() {
+std::shared_ptr<SpiffeData> SPIFFEValidator::loadTrustBundles() {
     std::ifstream file(trust_bundle_file_name_);
     if (file.fail()) {
       ENVOY_LOG(error, "Failed to open SPIFFE bundle map file '{}'", trust_bundle_file_name_);
@@ -152,7 +152,7 @@ void SPIFFEValidator::initializeCertificateRefresh(Server::Configuration::Common
     THROW_IF_NOT_OK(
         file_watcher_->addWatch(trust_bundle_file_name_, Filesystem::Watcher::Events::Modified, [this](uint32_t) {
             ENVOY_LOG(info, "Updating SPIFFE bundle map from file '{}'", trust_bundle_file_name_);
-            if (auto new_trust_bundle = loadTrustBundleMap()) {
+            if (auto new_trust_bundle = loadTrustBundles()) {
                 updateSpiffeDataAsync(new_trust_bundle);
             } else {
                 ENVOY_LOG(error, "Failed to load SPIFFE bundle map from '{}'", trust_bundle_file_name_);
@@ -190,19 +190,19 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC
   }
 
   const auto n_trust_domains = message.trust_domains().size();
-  if (message.has_trust_bundle_map() && n_trust_domains > 0 ) {
-      throw EnvoyException(
-          "Cannot configure both trust_domains and trust_bundle_map...");
-  }
 
   tls_->set([](Event::Dispatcher&) {
     return std::make_shared<ThreadLocalSpiffeState>();
   });
 
   // If a trust bundle map is provided, use that...
-  if (message.has_trust_bundle_map()) {
-    trust_bundle_file_name_ = message.trust_bundle_map().filename();
-    spiffe_data_ = loadTrustBundleMap();
+  if (message.has_trust_bundles()) {
+    if (!message.trust_bundles().source().has_filename()) {
+      throw EnvoyException("SPIFFE Bundle DataSource requires a filename");
+    }
+    trust_bundle_file_name_ = message.trust_bundles().source().filename();
+    bundle_format_ = message.trust_bundles().format();
+    spiffe_data_ = loadTrustBundles();
     if (!spiffe_data_) {
       throw EnvoyException("Failed to load SPIFFE Bundle map");
     }

diff --git a/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h b/source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h
@@ -12,6 +12,7 @@
 #include "envoy/ssl/context_config.h"
 #include "envoy/ssl/private_key/private_key.h"
 #include "envoy/ssl/ssl_socket_extended_info.h"
+#include "envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.pb.h"
 
 #include "source/common/common/logger.h"
 #include "source/common/common/c_smart_ptr.h"
@@ -31,6 +32,7 @@ namespace TransportSockets {
 namespace Tls {
 
 using X509StorePtr = CSmartPtr<X509_STORE, X509_STORE_free>;
+using SPIFFEConfig = envoy::extensions::transport_sockets::tls::v3::SPIFFECertValidatorConfig;
 
 struct SpiffeData {
   absl::flat_hash_map<std::string, CSmartPtr<X509_STORE, X509_STORE_free>> trust_bundle_stores;
@@ -98,7 +100,7 @@ class SPIFFEValidator : public CertValidator, Logger::Loggable<Logger::Id::secre
                                             std::string& error_details);
 
   void initializeCertificateRefresh(Server::Configuration::CommonFactoryContext& context);
-  std::shared_ptr<SpiffeData> loadTrustBundleMap();
+  std::shared_ptr<SpiffeData> loadTrustBundles();
 
   class ThreadLocalSpiffeState : public Envoy::ThreadLocal::ThreadLocalObject {
   public:
@@ -140,6 +142,8 @@ class SPIFFEValidator : public CertValidator, Logger::Loggable<Logger::Id::secre
   ThreadLocal::TypedSlotPtr<ThreadLocalSpiffeState> tls_;
   std::string ca_file_name_;
   std::string trust_bundle_file_name_;
+  SPIFFEConfig::TrustBundleFormat bundle_format_;
+
   std::shared_ptr<SpiffeData> spiffe_data_;
   std::vector<SanMatcherPtr> subject_alt_name_matchers_{};
   Event::Dispatcher& main_thread_dispatcher_;