tls: enable allow_expired_certificate for SPIFFE validator (envoyprox…

…y#15426)

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>

source/common/ssl/certificate_validation_context_config_impl.cc

@@ -39,7 +39,7 @@ CertificateValidationContextConfigImpl::CertificateValidationContextConfigImpl(
                     config.custom_validator_config())
               : absl::nullopt),
       api_(api) {
-  if (ca_cert_.empty()) {
+  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",
                                        certificateRevocationListPath()));

source/extensions/transport_sockets/tls/cert_validator/BUILD

@@ -13,11 +13,13 @@ envoy_cc_library(
     srcs = [
         "default_validator.cc",
         "factory.cc",
+        "utility.cc",
     ],
     hdrs = [
         "cert_validator.h",
         "default_validator.h",
         "factory.h",
+        "utility.h",
         "well_known_names.h",
     ],
     external_deps = [

source/extensions/transport_sockets/tls/cert_validator/default_validator.cc

@@ -26,6 +26,7 @@
 
 #include "extensions/transport_sockets/tls/cert_validator/cert_validator.h"
 #include "extensions/transport_sockets/tls/cert_validator/factory.h"
+#include "extensions/transport_sockets/tls/cert_validator/utility.h"
 #include "extensions/transport_sockets/tls/cert_validator/well_known_names.h"
 #include "extensions/transport_sockets/tls/stats.h"
 #include "extensions/transport_sockets/tls/utility.h"
@@ -111,7 +112,7 @@ int DefaultCertValidator::initializeSslContexts(std::vector<SSL_CTX*> contexts,
       // the hood. Therefore, to ignore cert expiration, we need to set the callback
       // for X509_verify_cert to ignore that error.
       if (config_->allowExpiredCertificate()) {
-        X509_STORE_set_verify_cb(store, DefaultCertValidator::ignoreCertificateExpirationCallback);
+        X509_STORE_set_verify_cb(store, CertValidatorUtil::ignoreCertificateExpirationCallback);
       }
     }
   }
@@ -226,17 +227,6 @@ int DefaultCertValidator::doVerifyCertChain(
                                       : (validated != Envoy::Ssl::ClientValidationStatus::Failed);
 }
 
-int DefaultCertValidator::ignoreCertificateExpirationCallback(int ok, X509_STORE_CTX* store_ctx) {
-  if (!ok) {
-    int err = X509_STORE_CTX_get_error(store_ctx);
-    if (err == X509_V_ERR_CERT_HAS_EXPIRED || err == X509_V_ERR_CERT_NOT_YET_VALID) {
-      return 1;
-    }
-  }
-
-  return ok;
-}
-
 Envoy::Ssl::ClientValidationStatus DefaultCertValidator::verifyCertificate(
     X509* cert, const std::vector<std::string>& verify_san_list,
     const std::vector<Matchers::StringMatcherImpl>& subject_alt_name_matchers) {

source/extensions/transport_sockets/tls/cert_validator/default_validator.h

@@ -52,9 +52,7 @@ class DefaultCertValidator : public CertValidator {
   std::string getCaFileName() const override { return ca_file_path_; };
   Envoy::Ssl::CertificateDetailsPtr getCaCertInformation() const override;
 
-  // utility functions
-  static int ignoreCertificateExpirationCallback(int ok, X509_STORE_CTX* store_ctx);
-
+  // Utility functions.
   Envoy::Ssl::ClientValidationStatus
   verifyCertificate(X509* cert, const std::vector<std::string>& verify_san_list,
                     const std::vector<Matchers::StringMatcherImpl>& subject_alt_name_matchers);

source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc

@@ -1,30 +1,18 @@
 #include "extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h"
 
-#include <algorithm>
-#include <array>
-#include <cstdint>
-#include <deque>
-#include <functional>
-#include <string>
-#include <vector>
-
-#include "envoy/common/pure.h"
 #include "envoy/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.pb.h"
 #include "envoy/network/transport_socket.h"
 #include "envoy/registry/registry.h"
-#include "envoy/ssl/context.h"
 #include "envoy/ssl/context_config.h"
-#include "envoy/ssl/private_key/private_key.h"
 #include "envoy/ssl/ssl_socket_extended_info.h"
 
-#include "common/common/matchers.h"
-#include "common/common/regex.h"
 #include "common/config/datasource.h"
 #include "common/config/utility.h"
 #include "common/protobuf/message_validator_impl.h"
 #include "common/stats/symbol_table_impl.h"
 
 #include "extensions/transport_sockets/tls/cert_validator/factory.h"
+#include "extensions/transport_sockets/tls/cert_validator/utility.h"
 #include "extensions/transport_sockets/tls/cert_validator/well_known_names.h"
 #include "extensions/transport_sockets/tls/stats.h"
 #include "extensions/transport_sockets/tls/utility.h"
@@ -43,6 +31,7 @@ SPIFFEValidator::SPIFFEValidator(const Envoy::Ssl::CertificateValidationContextC
                                  SslStats& stats, TimeSource& time_source)
     : stats_(stats), time_source_(time_source) {
   ASSERT(config != nullptr);
+  allow_expired_certificate_ = config->allowExpiredCertificate();
 
   SPIFFEConfig message;
   Config::Utility::translateOpaqueConfig(config->customValidatorConfig().value().typed_config(),
@@ -159,6 +148,9 @@ int SPIFFEValidator::doVerifyCertChain(X509_STORE_CTX* store_ctx,
 
   // Set the trust bundle's certificate store on the context, and do the verification.
   store_ctx->ctx = trust_bundle;
+  if (allow_expired_certificate_) {
+    X509_STORE_CTX_set_verify_cb(store_ctx, CertValidatorUtil::ignoreCertificateExpirationCallback);
+  }
   auto ret = X509_verify_cert(store_ctx);
   if (ssl_extended_info) {
     ssl_extended_info->setCertificateValidationStatus(

source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.h

@@ -63,6 +63,7 @@ class SPIFFEValidator : public CertValidator {
   };
 
 private:
+  bool allow_expired_certificate_{false};
   std::vector<bssl::UniquePtr<X509>> ca_certs_;
   std::string ca_file_name_;
   absl::flat_hash_map<std::string, X509StorePtr> trust_bundle_stores_;

source/extensions/transport_sockets/tls/cert_validator/utility.cc

@@ -0,0 +1,21 @@
+#include "extensions/transport_sockets/tls/cert_validator/utility.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace TransportSockets {
+namespace Tls {
+
+int CertValidatorUtil::ignoreCertificateExpirationCallback(int ok, X509_STORE_CTX* store_ctx) {
+  if (!ok) {
+    int err = X509_STORE_CTX_get_error(store_ctx);
+    if (err == X509_V_ERR_CERT_HAS_EXPIRED || err == X509_V_ERR_CERT_NOT_YET_VALID) {
+      return 1;
+    }
+  }
+  return ok;
+}
+
+} // namespace Tls
+} // namespace TransportSockets
+} // namespace Extensions
+} // namespace Envoy

source/extensions/transport_sockets/tls/cert_validator/utility.h

@@ -0,0 +1,19 @@
+#pragma once
+
+#include "openssl/x509v3.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace TransportSockets {
+namespace Tls {
+
+class CertValidatorUtil {
+public:
+  // Callback for allow_expired_certificate option
+  static int ignoreCertificateExpirationCallback(int ok, X509_STORE_CTX* store_ctx);
+};
+
+} // namespace Tls
+} // namespace TransportSockets
+} // namespace Extensions
+} // namespace Envoy

test/config/utility.cc

@@ -1112,6 +1112,7 @@ void ConfigHelper::initializeTls(
     validation_context->add_verify_certificate_hash(
         options.expect_client_ecdsa_cert_ ? TEST_CLIENT_ECDSA_CERT_HASH : TEST_CLIENT_CERT_HASH);
   }
+  validation_context->set_allow_expired_certificate(options.allow_expired_certificate_);
 
   // We'll negotiate up to TLSv1.3 for the tests that care, but it really
   // depends on what the client sets.

test/config/utility.h

@@ -33,6 +33,11 @@ class ConfigHelper {
   using HttpConnectionManager =
       envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager;
   struct ServerSslOptions {
+    ServerSslOptions& setAllowExpiredCertificate(bool allow) {
+      allow_expired_certificate_ = allow;
+      return *this;
+    }
+
     ServerSslOptions& setRsaCert(bool rsa_cert) {
       rsa_cert_ = rsa_cert;
       return *this;
@@ -74,6 +79,7 @@ class ConfigHelper {
       return *this;
     }
 
+    bool allow_expired_certificate_{};
     envoy::config::core::v3::TypedExtensionConfig* custom_validator_config_;
     bool rsa_cert_{true};
     bool rsa_cert_ocsp_staple_{true};

test/extensions/transport_sockets/tls/cert_validator/BUILD

@@ -10,40 +10,50 @@ licenses(["notice"])  # Apache 2
 envoy_package()
 
 envoy_cc_test(
-    name = "factory_test",
+    name = "default_validator_test",
     srcs = [
-        "factory_test.cc",
+        "default_validator_test.cc",
+    ],
+    data = [
+        "//test/extensions/transport_sockets/tls/test_data:certs",
     ],
     deps = [
-        ":util",
         "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib",
+        "//test/extensions/transport_sockets/tls:ssl_test_utils",
+        "//test/extensions/transport_sockets/tls/cert_validator:test_common",
+        "//test/test_common:environment_lib",
+        "//test/test_common:test_runtime_lib",
     ],
 )
 
-envoy_cc_test_library(
-    name = "util",
-    hdrs = ["util.h"],
+envoy_cc_test(
+    name = "factory_test",
+    srcs = [
+        "factory_test.cc",
+    ],
     deps = [
-        "//include/envoy/ssl:context_config_interface",
-        "//include/envoy/ssl:ssl_socket_extended_info_interface",
-        "//source/common/common:macros",
-        "//test/test_common:utility_lib",
+        "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib",
+        "//test/extensions/transport_sockets/tls/cert_validator:test_common",
     ],
 )
 
 envoy_cc_test(
-    name = "default_validator_test",
+    name = "utility_test",
     srcs = [
-        "default_validator_test.cc",
-    ],
-    data = [
-        "//test/extensions/transport_sockets/tls/test_data:certs",
+        "utility_test.cc",
     ],
     deps = [
         "//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib",
-        "//test/extensions/transport_sockets/tls:ssl_test_utils",
-        "//test/extensions/transport_sockets/tls/cert_validator:util",
-        "//test/test_common:environment_lib",
-        "//test/test_common:test_runtime_lib",
+    ],
+)
+
+envoy_cc_test_library(
+    name = "test_common",
+    hdrs = ["test_common.h"],
+    deps = [
+        "//include/envoy/ssl:context_config_interface",
+        "//include/envoy/ssl:ssl_socket_extended_info_interface",
+        "//source/common/common:macros",
+        "//test/test_common:utility_lib",
     ],
 )

test/extensions/transport_sockets/tls/cert_validator/factory_test.cc

@@ -2,7 +2,7 @@
 
 #include "extensions/transport_sockets/tls/cert_validator/factory.h"
 
-#include "test/extensions/transport_sockets/tls/cert_validator/util.h"
+#include "test/extensions/transport_sockets/tls/cert_validator/test_common.h"
 
 #include "gtest/gtest.h"
 

test/extensions/transport_sockets/tls/cert_validator/spiffe/BUILD

@@ -23,7 +23,7 @@ envoy_extension_cc_test(
     deps = [
         "//source/extensions/transport_sockets/tls/cert_validator/spiffe:config",
         "//test/extensions/transport_sockets/tls:ssl_test_utils",
-        "//test/extensions/transport_sockets/tls/cert_validator:util",
+        "//test/extensions/transport_sockets/tls/cert_validator:test_common",
         "//test/test_common:environment_lib",
         "//test/test_common:simulated_time_system_lib",
         "//test/test_common:test_runtime_lib",

test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.cc

@@ -12,9 +12,12 @@ namespace Envoy {
 namespace Ssl {
 
 void SslSPIFFECertValidatorIntegrationTest::initialize() {
-  config_helper_.addSslConfig(
-      ConfigHelper::ServerSslOptions().setRsaCert(true).setTlsV13(true).setCustomValidatorConfig(
-          custom_validator_config_));
+  config_helper_.addSslConfig(ConfigHelper::ServerSslOptions()
+                                  .setRsaCert(true)
+                                  .setTlsV13(true)
+                                  .setRsaCertOcspStaple(false)
+                                  .setCustomValidatorConfig(custom_validator_config_)
+                                  .setAllowExpiredCertificate(allow_expired_cert_));
   HttpIntegrationTest::initialize();
 
   context_manager_ =
@@ -29,9 +32,10 @@ void SslSPIFFECertValidatorIntegrationTest::TearDown() {
 }
 
 Network::ClientConnectionPtr SslSPIFFECertValidatorIntegrationTest::makeSslClientConnection(
-    const ClientSslTransportOptions& options) {
+    const ClientSslTransportOptions& options, bool use_expired = false) {
   ClientSslTransportOptions modified_options{options};
   modified_options.setTlsVersion(tls_version_);
+  modified_options.use_expired_spiffe_cert_ = use_expired;
 
   Network::Address::InstanceConstSharedPtr address = getSslAddress(version_, lookupPort("http"));
   auto client_transport_socket_factory_ptr =
@@ -78,6 +82,60 @@ name: envoy.tls.cert_validator.spiffe
   checkVerifyErrorCouter(0);
 }
 
+// Client certificate has expired but the config allows expired certificates, so this case should
+// be accepted.
+TEST_P(SslSPIFFECertValidatorIntegrationTest, ServerRsaSPIFFEValidatorExpiredButAccepcepted) {
+  auto typed_conf = new envoy::config::core::v3::TypedExtensionConfig();
+  TestUtility::loadFromYaml(TestEnvironment::substitute(R"EOF(
+name: envoy.tls.cert_validator.spiffe
+typed_config:
+  "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig
+  trust_domains:
+    - name: example.com
+      trust_bundle:
+        filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem"
+  )EOF"),
+                            *typed_conf);
+  custom_validator_config_ = typed_conf;
+  allow_expired_cert_ = true;
+  ConnectionCreationFunction creator = [&]() -> Network::ClientConnectionPtr {
+    const bool use_expired_certificate = true;
+    return makeSslClientConnection({}, use_expired_certificate);
+  };
+  testRouterRequestAndResponseWithBody(1024, 512, false, false, &creator);
+  checkVerifyErrorCouter(0);
+}
+
+// Client certificate has expired and the config does NOT allow expired certificates, so this case
+// should be rejected.
+TEST_P(SslSPIFFECertValidatorIntegrationTest, ServerRsaSPIFFEValidatorExpiredAndRejected) {
+  auto typed_conf = new envoy::config::core::v3::TypedExtensionConfig();
+  TestUtility::loadFromYaml(TestEnvironment::substitute(R"EOF(
+name: envoy.tls.cert_validator.spiffe
+typed_config:
+  "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.SPIFFECertValidatorConfig
+  trust_domains:
+    - name: example.com
+      trust_bundle:
+        filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_cert.pem"
+  )EOF"),
+                            *typed_conf);
+  custom_validator_config_ = typed_conf;
+  // Explicitly specify "false" just in case and for clarity.
+  allow_expired_cert_ = false;
+  initialize();
+  auto conn = makeSslClientConnection({});
+  if (tls_version_ == envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_2) {
+    auto codec = makeRawHttpConnection(std::move(conn), absl::nullopt);
+    EXPECT_FALSE(codec->connected());
+  } else {
+    auto codec = makeHttpConnection(std::move(conn));
+    ASSERT_TRUE(codec->waitForDisconnect());
+    codec->close();
+  }
+  checkVerifyErrorCouter(1);
+}
+
 // clientcert.pem's san is "spiffe://lyft.com/frontend-team" so it should be rejected.
 TEST_P(SslSPIFFECertValidatorIntegrationTest, ServerRsaSPIFFEValidatorRejected1) {
   auto typed_conf = new envoy::config::core::v3::TypedExtensionConfig();

test/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator_integration_test.h

@@ -24,7 +24,7 @@ class SslSPIFFECertValidatorIntegrationTest
   void TearDown() override;
 
   virtual Network::ClientConnectionPtr
-  makeSslClientConnection(const ClientSslTransportOptions& options);
+  makeSslClientConnection(const ClientSslTransportOptions& options, bool use_expired);
   void checkVerifyErrorCouter(uint64_t value);
 
   static std::string ipClientVersionTestParamsToString(
@@ -39,6 +39,7 @@ class SslSPIFFECertValidatorIntegrationTest
   }
 
 protected:
+  bool allow_expired_cert_{};
   envoy::config::core::v3::TypedExtensionConfig* custom_validator_config_{nullptr};
   std::unique_ptr<ContextManager> context_manager_;
   const envoy::extensions::transport_sockets::tls::v3::TlsParameters::TlsProtocol tls_version_{