From 32f1d5db367f01342eb8efbb90ef7914e37bf704 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Sat, 22 Jun 2019 04:32:39 +0000 Subject: [PATCH 01/27] sds config dump single cmt due to dco... Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/BUILD | 1 + api/envoy/admin/v2alpha/config_dump.proto | 24 +++ source/common/secret/BUILD | 1 + source/common/secret/sds_api.cc | 13 +- source/common/secret/sds_api.h | 27 +++- source/common/secret/secret_manager_impl.cc | 53 +++++++ source/common/secret/secret_manager_impl.h | 18 ++- source/server/config_validation/server.cc | 2 +- source/server/server.cc | 5 +- test/common/secret/BUILD | 1 + .../common/secret/secret_manager_impl_test.cc | 139 +++++++++++++++++- test/integration/integration_admin_test.cc | 7 +- test/mocks/config/mocks.cc | 2 + test/mocks/config/mocks.h | 6 +- test/mocks/server/mocks.cc | 9 +- test/mocks/server/mocks.h | 10 +- 16 files changed, 284 insertions(+), 34 deletions(-) diff --git a/api/envoy/admin/v2alpha/BUILD b/api/envoy/admin/v2alpha/BUILD index 0cbbb7f0d3895..aa35aa7c8d7d6 100644 --- a/api/envoy/admin/v2alpha/BUILD +++ b/api/envoy/admin/v2alpha/BUILD @@ -11,6 +11,7 @@ api_proto_library_internal( "//envoy/api/v2:lds", "//envoy/api/v2:rds", "//envoy/api/v2:srds", + "//envoy/api/v2/auth:cert", "//envoy/config/bootstrap/v2:bootstrap", ], ) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index ba5b36df36c7c..d05dbc5aafc9a 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -6,6 +6,7 @@ option java_outer_classname = "ConfigDumpProto"; option java_multiple_files = true; option java_package = "io.envoyproxy.envoy.admin.v2alpha"; +import "envoy/api/v2/auth/cert.proto"; import "envoy/api/v2/cds.proto"; import "envoy/api/v2/lds.proto"; import "envoy/api/v2/rds.proto"; @@ -219,3 +220,26 @@ message ScopedRoutesConfigDump { repeated DynamicScopedRouteConfigs dynamic_scoped_route_configs = 2 [(gogoproto.nullable) = false]; } + +// Config dump for SDS. +message SecretsConfigDump { + // TODO: does it make sense to include a veresion_info here? what criteria we aggregated on? + // string version_info; + + message DynamicSecret { + // The name assigned to the secret. + string name = 1; + + // Per resource version information. + string version_info = 2; + + // The timestamp when the Cluster was last updated. + google.protobuf.Timestamp last_updated = 3; + + // Security sensitive information should be stripped. + envoy.api.v2.auth.Secret secret = 4; + } + + // List of all the secrets fetched via SDS. + repeated DynamicSecret dynamic_secrets = 1; +} diff --git a/source/common/secret/BUILD b/source/common/secret/BUILD index ca30d94ddfb38..b584cd57c7daa 100644 --- a/source/common/secret/BUILD +++ b/source/common/secret/BUILD @@ -19,6 +19,7 @@ envoy_cc_library( "//include/envoy/server:transport_socket_config_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "@envoy_api//envoy/admin/v2alpha:config_dump_cc", "@envoy_api//envoy/api/v2/auth:cert_cc", ], ) diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index aa4d4442747c3..7626433b9a25d 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -11,13 +11,15 @@ namespace Envoy { namespace Secret { SdsApi::SdsApi(envoy::api::v2::core::ConfigSource sds_config, absl::string_view sds_config_name, - Config::SubscriptionFactory& subscription_factory, + Config::SubscriptionFactory& subscription_factory, TimeSource& time_source, ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, Init::Manager& init_manager, std::function destructor_cb) : init_target_(fmt::format("SdsApi {}", sds_config_name), [this] { initialize(); }), stats_(stats), sds_config_(std::move(sds_config)), sds_config_name_(sds_config_name), secret_hash_(0), clean_up_(std::move(destructor_cb)), validation_visitor_(validation_visitor), - subscription_factory_(subscription_factory) { + subscription_factory_(subscription_factory), + time_source_(time_source), secret_data_{sds_config_name_, "uninitialized", + time_source_.systemTime()} { // TODO(JimmyCYJ): Implement chained_init_manager, so that multiple init_manager // can be chained together to behave as one init_manager. In that way, we let // two listeners which share same SdsApi to register at separate init managers, and @@ -26,7 +28,7 @@ SdsApi::SdsApi(envoy::api::v2::core::ConfigSource sds_config, absl::string_view } void SdsApi::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, - const std::string&) { + const std::string& version_info) { validateUpdateSize(resources.size()); auto secret = MessageUtil::anyConvert(resources[0], validation_visitor_); @@ -44,7 +46,8 @@ void SdsApi::onConfigUpdate(const Protobuf::RepeatedPtrField& setSecret(secret); update_callback_manager_.runCallbacks(); } - + secret_data_.last_updated_ = time_source_.systemTime(); + secret_data_.version_info_ = version_info; init_target_.ready(); } @@ -79,5 +82,7 @@ void SdsApi::initialize() { subscription_->start({sds_config_name_}); } +SdsApi::SecretData SdsApi::secretData() { return secret_data_; } + } // namespace Secret } // namespace Envoy diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index e05e3e5c03029..b6fde31396626 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -32,11 +32,21 @@ namespace Secret { */ class SdsApi : public Config::SubscriptionCallbacks { public: + struct SecretData { + const std::string resource_name; + std::string version_info_; + SystemTime last_updated_; + }; + SdsApi(envoy::api::v2::core::ConfigSource sds_config, absl::string_view sds_config_name, - Config::SubscriptionFactory& subscription_factory, + Config::SubscriptionFactory& subscription_factory, TimeSource& time_source, ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, Init::Manager& init_manager, std::function destructor_cb); + // TODO: what's the thread model of the admin::config_dump vs xDS config updates? + // any lock mechanism needed? + SecretData secretData(); + protected: // Creates new secrets. virtual void setSecret(const envoy::api::v2::auth::Secret&) PURE; @@ -68,6 +78,8 @@ class SdsApi : public Config::SubscriptionCallbacks { Cleanup clean_up_; ProtobufMessage::ValidationVisitor& validation_visitor_; Config::SubscriptionFactory& subscription_factory_; + TimeSource& time_source_; + SecretData secret_data_; }; class TlsCertificateSdsApi; @@ -90,17 +102,18 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider Config::Utility::checkLocalInfo("TlsCertificateSdsApi", secret_provider_context.localInfo()); return std::make_shared( sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), + secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), *secret_provider_context.initManager(), destructor_cb); } TlsCertificateSdsApi(const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, - Config::SubscriptionFactory& subscription_factory, + Config::SubscriptionFactory& subscription_factory, TimeSource& time_source, ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, Init::Manager& init_manager, std::function destructor_cb) - : SdsApi(sds_config, sds_config_name, subscription_factory, validation_visitor, stats, - init_manager, std::move(destructor_cb)) {} + : SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor, + stats, init_manager, std::move(destructor_cb)) {} // SecretProvider const envoy::api::v2::auth::TlsCertificate* secret() const override { @@ -138,17 +151,19 @@ class CertificateValidationContextSdsApi : public SdsApi, secret_provider_context.localInfo()); return std::make_shared( sds_config, sds_config_name, secret_provider_context.clusterManager().subscriptionFactory(), + secret_provider_context.dispatcher().timeSource(), secret_provider_context.messageValidationVisitor(), secret_provider_context.stats(), *secret_provider_context.initManager(), destructor_cb); } CertificateValidationContextSdsApi(const envoy::api::v2::core::ConfigSource& sds_config, const std::string& sds_config_name, Config::SubscriptionFactory& subscription_factory, + TimeSource& time_source, ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, Init::Manager& init_manager, std::function destructor_cb) - : SdsApi(sds_config, sds_config_name, subscription_factory, validation_visitor, stats, - init_manager, std::move(destructor_cb)) {} + : SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor, + stats, init_manager, std::move(destructor_cb)) {} // SecretProvider const envoy::api::v2::auth::CertificateValidationContext* secret() const override { diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index aed0c374255c4..272b6bdd2c881 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -1,8 +1,10 @@ #include "common/secret/secret_manager_impl.h" +#include "envoy/admin/v2alpha/config_dump.pb.h" #include "envoy/common/exception.h" #include "common/common/assert.h" +#include "common/common/logger.h" #include "common/secret/sds_api.h" #include "common/secret/secret_provider_impl.h" #include "common/ssl/certificate_validation_context_config_impl.h" @@ -11,6 +13,9 @@ namespace Envoy { namespace Secret { +SecretManagerImpl::SecretManagerImpl(Server::ConfigTracker& config_tracker) + : config_tracker_entry_(config_tracker.add("secrets", [this] { return dumpSecretConfigs(); })) { +} void SecretManagerImpl::addStaticSecret(const envoy::api::v2::auth::Secret& secret) { switch (secret.type_case()) { case envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate: { @@ -79,5 +84,53 @@ SecretManagerImpl::findOrCreateCertificateValidationContextProvider( secret_provider_context); } +// TODO: question, what's the handling of static inlined stuff? they're just internal constructs +// not needed to be exposed, maybe? +ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { + auto config_dump = std::make_unique(); + auto providers = certificate_providers_.allSecretProviders(); + for (const auto& cert_secrets : providers) { + const auto& secret_data = cert_secrets->secretData(); + const auto& tls_cert = cert_secrets->secret(); + const auto& dynamic_secret = config_dump->mutable_dynamic_secrets()->Add(); + const auto& secret = dynamic_secret->mutable_secret(); + + ProtobufWkt::Timestamp last_updated_ts; + TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); + dynamic_secret->set_version_info(secret_data.version_info_); + *dynamic_secret->mutable_last_updated() = last_updated_ts; + secret->set_name(secret_data.resource_name); + // TODO(incfly): this means the config is registered but Envoy hasn't received a + // valid push from SDS server yet... should we still dump it somehow? + if (!tls_cert) { + continue; + } + auto tls_certificate = secret->mutable_tls_certificate(); + tls_certificate->MergeFrom(*tls_cert); + tls_certificate->clear_private_key(); + tls_certificate->clear_password(); + } + + // Handling validation Context provided via SDS. + auto context_secret_provider = validation_context_providers_.allSecretProviders(); + for (const auto& validation_context_secret : context_secret_provider) { + auto secret_data = validation_context_secret->secretData(); + auto validation_context = validation_context_secret->secret(); + auto dynamic_secret = config_dump->mutable_dynamic_secrets()->Add(); + auto secret = dynamic_secret->mutable_secret(); + ProtobufWkt::Timestamp last_updated_ts; + TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); + dynamic_secret->set_version_info(secret_data.version_info_); + *dynamic_secret->mutable_last_updated() = last_updated_ts; + secret->set_name(secret_data.resource_name); + if (!validation_context) { + continue; + } + auto dump_context = secret->mutable_validation_context(); + dump_context->MergeFrom(*validation_context); + } + return config_dump; +} + } // namespace Secret } // namespace Envoy diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index aa7ee6e4b2154..f4139d140c105 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -14,8 +14,9 @@ namespace Envoy { namespace Secret { -class SecretManagerImpl : public SecretManager { +class SecretManagerImpl : public SecretManager, public Logger::Loggable { public: + SecretManagerImpl(Server::ConfigTracker& config_tracker); void addStaticSecret(const envoy::api::v2::auth::Secret& secret) override; TlsCertificateConfigProviderSharedPtr @@ -42,6 +43,8 @@ class SecretManagerImpl : public SecretManager { Server::Configuration::TransportSocketFactoryContext& secret_provider_context) override; private: + ProtobufTypes::MessagePtr dumpSecretConfigs(); + template class DynamicSecretProviders : public Logger::Loggable { public: @@ -68,6 +71,17 @@ class SecretManagerImpl : public SecretManager { return secret_provider; } + std::vector> allSecretProviders() { + std::vector> providers; + for (const auto& secret_entry : dynamic_secret_providers_) { + std::shared_ptr secret_provider = secret_entry.second.lock(); + if (secret_provider) { + providers.push_back(std::move(secret_provider)); + } + } + return providers; + } + private: // Removes dynamic secret provider which has been deleted. void removeDynamicSecretProvider(const std::string& map_key) { @@ -91,6 +105,8 @@ class SecretManagerImpl : public SecretManager { // map hash code of SDS config source and SdsApi object. DynamicSecretProviders certificate_providers_; DynamicSecretProviders validation_context_providers_; + + Server::ConfigTracker::EntryOwnerPtr config_tracker_entry_; }; } // namespace Secret diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 96faf5f81bd6a..431d1de7a318b 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -90,7 +90,7 @@ void ValidationInstance::initialize(const Options& options, listener_manager_ = std::make_unique(*this, *this, *this, false); thread_local_.registerThread(*dispatcher_, true); runtime_loader_ = component_factory.createRuntime(*this, initial_config); - secret_manager_ = std::make_unique(); + secret_manager_ = std::make_unique(admin().getConfigTracker()); ssl_context_manager_ = std::make_unique(api_->timeSource()); cluster_manager_factory_ = std::make_unique( diff --git a/source/server/server.cc b/source/server/server.cc index 3f52053471983..96969108331de 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -55,8 +55,7 @@ InstanceImpl::InstanceImpl(const Options& options, Event::TimeSystem& time_syste ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system, std::unique_ptr process_context) - : secret_manager_(std::make_unique()), shutdown_(false), - options_(options), time_source_(time_system), restarter_(restarter), + : shutdown_(false), options_(options), time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), thread_local_(tls), api_(new Api::Impl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), @@ -309,6 +308,8 @@ void InstanceImpl::initialize(const Options& options, loadServerFlags(initial_config.flagsPath()); + secret_manager_ = std::make_unique(admin_->getConfigTracker()); + // Initialize the overload manager early so other modules can register for actions. overload_manager_ = std::make_unique( *dispatcher_, stats_store_, thread_local_, bootstrap_.overload_manager(), diff --git a/test/common/secret/BUILD b/test/common/secret/BUILD index 2a354658e4d32..19712797f54af 100644 --- a/test/common/secret/BUILD +++ b/test/common/secret/BUILD @@ -22,6 +22,7 @@ envoy_cc_test( "//test/mocks/server:server_mocks", "//test/test_common:environment_lib", "//test/test_common:registry_lib", + "//test/test_common:simulated_time_system_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 95daa82d0d78c..4fcd686238ba0 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -1,15 +1,19 @@ #include +#include "envoy/admin/v2alpha/config_dump.pb.h" #include "envoy/api/v2/auth/cert.pb.h" #include "envoy/common/exception.h" +#include "common/common/logger.h" #include "common/secret/sds_api.h" #include "common/secret/secret_manager_impl.h" #include "common/ssl/certificate_validation_context_config_impl.h" #include "common/ssl/tls_certificate_config_impl.h" +#include "test/mocks/event/mocks.h" #include "test/mocks/server/mocks.h" #include "test/test_common/environment.h" +#include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -22,11 +26,24 @@ namespace Envoy { namespace Secret { namespace { -class SecretManagerImplTest : public testing::Test { +class SecretManagerImplTest : public testing::Test, public Logger::Loggable { protected: SecretManagerImplTest() : api_(Api::createApiForTest()) {} + void checkConfigDump(const std::string& expected_dump_yaml) { + auto message_ptr = config_tracker_.config_tracker_callbacks_["secrets"](); + const auto& secrets_config_dump = + dynamic_cast(*message_ptr); + envoy::admin::v2alpha::SecretsConfigDump expected_secrets_config_dump; + TestUtility::loadFromYaml(expected_dump_yaml, expected_secrets_config_dump); + EXPECT_EQ(expected_secrets_config_dump.DebugString(), secrets_config_dump.DebugString()); + } + + void setupSecretProviderContext() {} + Api::ApiPtr api_; + testing::NiceMock config_tracker_; + Event::SimulatedTimeSystem time_system_; }; // Validate that secret manager adds static TLS certificate secret successfully. @@ -42,7 +59,7 @@ name: "abc.com" filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_key.pem" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); secret_manager->addStaticSecret(secret_config); ASSERT_EQ(secret_manager->findStaticTlsCertificateProvider("undefined"), nullptr); @@ -75,7 +92,7 @@ TEST_F(SecretManagerImplTest, DuplicateStaticTlsCertificateSecret) { filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/selfsigned_key.pem" )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); secret_manager->addStaticSecret(secret_config); ASSERT_NE(secret_manager->findStaticTlsCertificateProvider("abc.com"), nullptr); @@ -94,7 +111,7 @@ TEST_F(SecretManagerImplTest, CertificateValidationContextSecretLoadSuccess) { allow_expired_certificate: true )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); secret_manager->addStaticSecret(secret_config); ASSERT_EQ(secret_manager->findStaticCertificateValidationContextProvider("undefined"), nullptr); @@ -119,7 +136,7 @@ TEST_F(SecretManagerImplTest, DuplicateStaticCertificateValidationContextSecret) allow_expired_certificate: true )EOF"; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); secret_manager->addStaticSecret(secret_config); ASSERT_NE(secret_manager->findStaticCertificateValidationContextProvider("abc.com"), nullptr); @@ -142,7 +159,7 @@ name: "abc.com" TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), secret_config); - std::unique_ptr secret_manager(new SecretManagerImpl()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); EXPECT_THROW_WITH_MESSAGE(secret_manager->addStaticSecret(secret_config), EnvoyException, "Secret type not implemented"); @@ -150,7 +167,7 @@ name: "abc.com" TEST_F(SecretManagerImplTest, SdsDynamicSecretUpdateSuccess) { Server::MockInstance server; - std::unique_ptr secret_manager(std::make_unique()); + std::unique_ptr secret_manager(new SecretManagerImpl(config_tracker_)); NiceMock secret_context; @@ -168,6 +185,7 @@ TEST_F(SecretManagerImplTest, SdsDynamicSecretUpdateSuccess) { })); EXPECT_CALL(secret_context, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); EXPECT_CALL(secret_context, localInfo()).WillOnce(ReturnRef(local_info)); auto secret_provider = @@ -199,6 +217,113 @@ name: "abc.com" tls_config.privateKey()); } +TEST_F(SecretManagerImplTest, ConfigDumpHandler) { + Server::MockInstance server; + auto secret_manager = std::make_unique(config_tracker_); + time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); + + NiceMock secret_context; + + envoy::api::v2::core::ConfigSource config_source; + NiceMock local_info; + NiceMock dispatcher; + NiceMock random; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillRepeatedly(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); + EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); + + auto secret_provider = + secret_manager->findOrCreateTlsCertificateProvider(config_source, "abc.com", secret_context); + const std::string yaml = + R"EOF( +name: "abc.com" +tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" +)EOF"; + envoy::api::v2::auth::Secret typed_secret; + TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), typed_secret); + Protobuf::RepeatedPtrField secret_resources; + secret_resources.Add()->PackFrom(typed_secret); + init_target_handle->initialize(init_watcher); + secret_context.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(secret_resources, + "keycert-v1"); + Ssl::TlsCertificateConfigImpl tls_config(*secret_provider->secret(), *api_); + EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_CERT_CHAIN", tls_config.certificateChain()); + EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY", tls_config.privateKey()); + + // Private key is removed. + const std::string expected_secrets_config_dump = R"EOF( +dynamic_secrets: + version_info: "keycert-v1" + last_updated: + seconds: 1234567891 + nanos: 234000000 + secret: + name: "abc.com" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" +)EOF"; + checkConfigDump(expected_secrets_config_dump); + + // Add a dynamic tls validatoin context provider. + time_system_.setSystemTime(std::chrono::milliseconds(1234567899000)); + auto context_secret_provider = secret_manager->findOrCreateCertificateValidationContextProvider( + config_source, "abc.com.validation", secret_context); + const std::string validation_yaml = R"EOF( +name: "abc.com.validation" +validation_context: + trusted_ca: + inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" +)EOF"; + TestUtility::loadFromYaml(TestEnvironment::substitute(validation_yaml), typed_secret); + secret_resources.Clear(); + secret_resources.Add()->PackFrom(typed_secret); + + // TODO(incfly): different config source is needed here... + // add helper function in the test class for easier add the test. + // uint64_t hash = MessageUtil::hash(config_source); + init_target_handle->initialize(init_watcher); + secret_context.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( + secret_resources, "validation-context-v1"); + Ssl::CertificateValidationContextConfigImpl cert_validation_context( + *context_secret_provider->secret(), *api_); + EXPECT_EQ("DUMMY_INLINE_STRING_TRUSTED_CA", cert_validation_context.caCert()); + const std::string updated_config_dump = R"EOF( +dynamic_secrets: +- version_info: "keycert-v1" + last_updated: + seconds: 1234567891 + nanos: 234000000 + secret: + name: "abc.com" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" +- version_info: "validation-context-v1" + last_updated: + seconds: 1234567899 + secret: + name: "abc.com.validation" + validation_context: + trusted_ca: + inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" +)EOF"; + checkConfigDump(updated_config_dump); +} + } // namespace } // namespace Secret } // namespace Envoy diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 137e972e00558..e7b37f1df42cf 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -393,7 +393,8 @@ TEST_P(IntegrationAdminTest, Admin) { "type.googleapis.com/envoy.admin.v2alpha.ClustersConfigDump", "type.googleapis.com/envoy.admin.v2alpha.ListenersConfigDump", "type.googleapis.com/envoy.admin.v2alpha.ScopedRoutesConfigDump", - "type.googleapis.com/envoy.admin.v2alpha.RoutesConfigDump"}; + "type.googleapis.com/envoy.admin.v2alpha.RoutesConfigDump", + "type.googleapis.com/envoy.admin.v2alpha.SecretsConfigDump"}; for (const Json::ObjectSharedPtr& obj_ptr : json->getObjectArray("configs")) { EXPECT_TRUE(expected_types[index].compare(obj_ptr->getString("@type")) == 0); @@ -403,12 +404,14 @@ TEST_P(IntegrationAdminTest, Admin) { // Validate we can parse as proto. envoy::admin::v2alpha::ConfigDump config_dump; TestUtility::loadFromJson(response->body(), config_dump); - EXPECT_EQ(5, config_dump.configs_size()); + EXPECT_EQ(6, config_dump.configs_size()); // .. and that we can unpack one of the entries. envoy::admin::v2alpha::RoutesConfigDump route_config_dump; config_dump.configs(4).UnpackTo(&route_config_dump); EXPECT_EQ("route_config_0", route_config_dump.static_route_configs(0).route_config().name()); + + // TODO(incfly) do we need to add parsing and data input for the sds? } TEST_P(IntegrationAdminTest, AdminOnDestroyCallbacks) { diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index c5bee0ccb920e..b912fa28b9cf6 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -17,6 +17,8 @@ MockSubscriptionFactory::MockSubscriptionFactory() { auto ret = std::make_unique>(); subscription_ = ret.get(); callbacks_ = &callbacks; + // uint64_t hash = MessageUtil::hash(config); + // callbacks_map_[hash] = &callbacks; return ret; })); ON_CALL(*this, messageValidationVisitor()) diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 8396fff4fc312..fb26c1ac3050a 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -47,7 +47,8 @@ class MockSubscription : public Subscription { MOCK_METHOD1(updateResources, void(const std::set& update_to_these_names)); }; -class MockSubscriptionFactory : public SubscriptionFactory { +class MockSubscriptionFactory : public SubscriptionFactory, + public Logger::Loggable { public: MockSubscriptionFactory(); ~MockSubscriptionFactory() override; @@ -59,6 +60,9 @@ class MockSubscriptionFactory : public SubscriptionFactory { MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); MockSubscription* subscription_{}; + + // map from the config hash to the callbacks. + // std::unordered_map callbacks_map_; SubscriptionCallbacks* callbacks_{}; }; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 12e2ac5c3169d..63968ebf74114 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -124,9 +124,9 @@ MockWorker::MockWorker() { MockWorker::~MockWorker() = default; MockInstance::MockInstance() - : secret_manager_(new Secret::SecretManagerImpl()), cluster_manager_(timeSource()), - ssl_context_manager_(timeSource()), singleton_manager_(new Singleton::ManagerImpl( - Thread::threadFactoryForTest().currentThreadId())), + : cluster_manager_(timeSource()), ssl_context_manager_(timeSource()), + singleton_manager_( + new Singleton::ManagerImpl(Thread::threadFactoryForTest().currentThreadId())), grpc_context_(stats_store_.symbolTable()), http_context_(stats_store_.symbolTable()) { ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_store_)); @@ -197,8 +197,7 @@ MockFactoryContext::MockFactoryContext() MockFactoryContext::~MockFactoryContext() = default; -MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() - : secret_manager_(new Secret::SecretManagerImpl()) { +MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() { ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, messageValidationVisitor()) diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index fde44c23ac7c4..9d449d2388726 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -340,11 +340,10 @@ class MockInstance : public Instance { MockInstance(); ~MockInstance(); - Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } - MOCK_METHOD0(admin, Admin&()); MOCK_METHOD0(api, Api::Api&()); MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); + MOCK_METHOD0(secretManager, Secret::SecretManager&()); MOCK_METHOD0(sslContextManager, Ssl::ContextManager&()); MOCK_METHOD0(dispatcher, Event::Dispatcher&()); MOCK_METHOD0(dnsResolver, Network::DnsResolverSharedPtr()); @@ -380,7 +379,7 @@ class MockInstance : public Instance { TimeSource& timeSource() override { return time_system_; } - std::unique_ptr secret_manager_; + testing::NiceMock secret_manager_; testing::NiceMock thread_local_; NiceMock stats_store_; std::shared_ptr> dns_resolver_{ @@ -489,9 +488,10 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MockTransportSocketFactoryContext(); ~MockTransportSocketFactoryContext(); - Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } + // Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } MOCK_METHOD0(admin, Server::Admin&()); + MOCK_METHOD0(secretManager, Secret::SecretManager&()); MOCK_METHOD0(sslContextManager, Ssl::ContextManager&()); MOCK_CONST_METHOD0(statsScope, Stats::Scope&()); MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); @@ -507,7 +507,7 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MOCK_METHOD0(api, Api::Api&()); testing::NiceMock cluster_manager_; - std::unique_ptr secret_manager_; + testing::NiceMock secret_manager_; testing::NiceMock api_; }; From c017b073390f328e7cc8124b4860352a6fae9992 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Sat, 22 Jun 2019 04:38:08 +0000 Subject: [PATCH 02/27] spelling check. Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index d05dbc5aafc9a..fba7a12c69537 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -223,9 +223,8 @@ message ScopedRoutesConfigDump { // Config dump for SDS. message SecretsConfigDump { - // TODO: does it make sense to include a veresion_info here? what criteria we aggregated on? - // string version_info; - + // DynamicSecret contains secret information fetched via SDS. + // The security sensitive information will trimmed, for example, private key. message DynamicSecret { // The name assigned to the secret. string name = 1; From 4269751bb4a7b8174feab6a8513496fdff16066e Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 24 Jun 2019 21:05:31 +0000 Subject: [PATCH 03/27] fix the sds_api_test. Signed-off-by: Jianfei Hu --- test/common/secret/sds_api_test.cc | 42 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 7a2d0abe50d9a..0894390cc5d27 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -42,6 +42,7 @@ class SdsApiTest : public testing::Test { NiceMock subscription_factory_; NiceMock init_manager_; NiceMock init_watcher_; + Event::GlobalTimeSystem time_system_; Init::TargetHandlePtr init_target_handle_; }; @@ -52,8 +53,8 @@ TEST_F(SdsApiTest, BasicTest) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); initialize(); } @@ -62,8 +63,8 @@ TEST_F(SdsApiTest, BasicTest) { TEST_F(SdsApiTest, DynamicTlsCertificateUpdateSuccess) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); initialize(); NiceMock secret_callback; auto handle = @@ -104,9 +105,9 @@ class PartialMockSds : public SdsApi { public: PartialMockSds(NiceMock& server, NiceMock& init_manager, envoy::api::v2::core::ConfigSource& config_source, - Config::SubscriptionFactory& subscription_factory) - : SdsApi(config_source, "abc.com", subscription_factory, validation_visitor_, server.stats(), - init_manager, []() {}) {} + Config::SubscriptionFactory& subscription_factory, TimeSource& time_source) + : SdsApi(config_source, "abc.com", subscription_factory, time_source, validation_visitor_, + server.stats(), init_manager, []() {}) {} MOCK_METHOD2(onConfigUpdate, void(const Protobuf::RepeatedPtrField&, const std::string&)); @@ -137,7 +138,8 @@ TEST_F(SdsApiTest, Delta) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - PartialMockSds sds(server, init_manager_, config_source, subscription_factory_); + Event::GlobalTimeSystem time_system; + PartialMockSds sds(server, init_manager_, config_source, subscription_factory_, time_system); initialize(); EXPECT_CALL(sds, onConfigUpdate(RepeatedProtoEq(for_matching), "version1")); subscription_factory_.callbacks_->onConfigUpdate(resources, {}, "ignored"); @@ -155,8 +157,8 @@ TEST_F(SdsApiTest, Delta) { TEST_F(SdsApiTest, DeltaUpdateSuccess) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); NiceMock secret_callback; auto handle = @@ -200,8 +202,8 @@ TEST_F(SdsApiTest, DynamicCertificateValidationContextUpdateSuccess) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; CertificateValidationContextSdsApi sds_api(config_source, "abc.com", subscription_factory_, - validation_visitor_, server.stats(), init_manager_, - []() {}); + time_system_, validation_visitor_, server.stats(), + init_manager_, []() {}); NiceMock secret_callback; auto handle = @@ -252,8 +254,8 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; CertificateValidationContextSdsApi sds_api(config_source, "abc.com", subscription_factory_, - validation_visitor_, server.stats(), init_manager_, - []() {}); + time_system_, validation_visitor_, server.stats(), + init_manager_, []() {}); NiceMock secret_callback; auto handle = @@ -321,8 +323,8 @@ TEST_F(SdsApiTest, DefaultCertificateValidationContextTest) { TEST_F(SdsApiTest, EmptyResource) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); Protobuf::RepeatedPtrField secret_resources; @@ -336,8 +338,8 @@ TEST_F(SdsApiTest, EmptyResource) { TEST_F(SdsApiTest, SecretUpdateWrongSize) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); std::string yaml = R"EOF( @@ -365,8 +367,8 @@ TEST_F(SdsApiTest, SecretUpdateWrongSize) { TEST_F(SdsApiTest, SecretUpdateWrongSecretName) { NiceMock server; envoy::api::v2::core::ConfigSource config_source; - TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, validation_visitor_, - server.stats(), init_manager_, []() {}); + TlsCertificateSdsApi sds_api(config_source, "abc.com", subscription_factory_, time_system_, + validation_visitor_, server.stats(), init_manager_, []() {}); std::string yaml = R"EOF( From a8e2724e72973c816b4f74a73a9c5a87e940063e Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 24 Jun 2019 23:17:17 +0000 Subject: [PATCH 04/27] fix mock test and config mocks comments. Signed-off-by: Jianfei Hu --- test/mocks/config/mocks.cc | 2 -- test/mocks/config/mocks.h | 3 --- test/mocks/server/mocks.h | 6 +++--- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index b912fa28b9cf6..c5bee0ccb920e 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -17,8 +17,6 @@ MockSubscriptionFactory::MockSubscriptionFactory() { auto ret = std::make_unique>(); subscription_ = ret.get(); callbacks_ = &callbacks; - // uint64_t hash = MessageUtil::hash(config); - // callbacks_map_[hash] = &callbacks; return ret; })); ON_CALL(*this, messageValidationVisitor()) diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index fb26c1ac3050a..fa9c0e47e9baf 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -60,9 +60,6 @@ class MockSubscriptionFactory : public SubscriptionFactory, MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); MockSubscription* subscription_{}; - - // map from the config hash to the callbacks. - // std::unordered_map callbacks_map_; SubscriptionCallbacks* callbacks_{}; }; diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 9d449d2388726..4f0b71c7e76fd 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -340,10 +340,11 @@ class MockInstance : public Instance { MockInstance(); ~MockInstance(); + Secret::SecretManager& secretManager() override { return secret_manager_; } + MOCK_METHOD0(admin, Admin&()); MOCK_METHOD0(api, Api::Api&()); MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); - MOCK_METHOD0(secretManager, Secret::SecretManager&()); MOCK_METHOD0(sslContextManager, Ssl::ContextManager&()); MOCK_METHOD0(dispatcher, Event::Dispatcher&()); MOCK_METHOD0(dnsResolver, Network::DnsResolverSharedPtr()); @@ -488,10 +489,9 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MockTransportSocketFactoryContext(); ~MockTransportSocketFactoryContext(); - // Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } + Secret::SecretManager& secretManager() override { return secret_manager_; } MOCK_METHOD0(admin, Server::Admin&()); - MOCK_METHOD0(secretManager, Secret::SecretManager&()); MOCK_METHOD0(sslContextManager, Ssl::ContextManager&()); MOCK_CONST_METHOD0(statsScope, Stats::Scope&()); MOCK_METHOD0(clusterManager, Upstream::ClusterManager&()); From fabae9ed59fbad570246becbe2561469f5532b68 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 24 Jun 2019 23:29:41 +0000 Subject: [PATCH 05/27] rewording config_dump and secret manager impl todo. Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 9 +++++---- source/common/secret/secret_manager_impl.cc | 4 ++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index fba7a12c69537..2dc093f3f8265 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -221,7 +221,7 @@ message ScopedRoutesConfigDump { [(gogoproto.nullable) = false]; } -// Config dump for SDS. +// Envoys SDS implementation fills this message with all secrets fetched dynamically via SDS. message SecretsConfigDump { // DynamicSecret contains secret information fetched via SDS. // The security sensitive information will trimmed, for example, private key. @@ -229,13 +229,14 @@ message SecretsConfigDump { // The name assigned to the secret. string name = 1; - // Per resource version information. + // This is the per-resource version information. string version_info = 2; - // The timestamp when the Cluster was last updated. + // The timestamp when the secret was last updated. google.protobuf.Timestamp last_updated = 3; - // Security sensitive information should be stripped. + // The actual secret information. Security sensitive information is stripped. + // For example, private key in a TLS cert. envoy.api.v2.auth.Secret secret = 4; } diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 272b6bdd2c881..8b6ad69dd69f1 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -107,6 +107,10 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { } auto tls_certificate = secret->mutable_tls_certificate(); tls_certificate->MergeFrom(*tls_cert); + + // We clear private key and password to avoid information leaking.j + // TODO(incfly): switch to more generic scrubbing mechanism once + // https://github.com/envoyproxy/envoy/issues/4757 is resolved. tls_certificate->clear_private_key(); tls_certificate->clear_password(); } From 350e7c8ba79e881dfd890216f79adfb30f6141bd Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 25 Jun 2019 07:49:27 +0000 Subject: [PATCH 06/27] fix all tests in context_impl_test by changing to impl. Signed-off-by: Jianfei Hu --- .../transport_sockets/tls/context_impl_test.cc | 8 ++++++++ test/mocks/config/mocks.h | 3 +-- test/mocks/server/mocks.cc | 4 +++- test/mocks/server/mocks.h | 9 +++++---- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/test/extensions/transport_sockets/tls/context_impl_test.cc b/test/extensions/transport_sockets/tls/context_impl_test.cc index f987590cfae29..993290814ae80 100644 --- a/test/extensions/transport_sockets/tls/context_impl_test.cc +++ b/test/extensions/transport_sockets/tls/context_impl_test.cc @@ -748,9 +748,11 @@ TEST_F(ClientContextConfigImplTest, SecretNotReady) { NiceMock local_info; Stats::IsolatedStoreImpl stats; NiceMock init_manager; + NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); sds_secret_configs->set_name("abc.com"); @@ -778,9 +780,11 @@ TEST_F(ClientContextConfigImplTest, ValidationContextNotReady) { NiceMock local_info; Stats::IsolatedStoreImpl stats; NiceMock init_manager; + NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); sds_secret_configs->set_name("abc.com"); @@ -1079,9 +1083,11 @@ TEST_F(ServerContextConfigImplTest, SecretNotReady) { NiceMock local_info; Stats::IsolatedStoreImpl stats; NiceMock init_manager; + NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_tls_certificate_sds_secret_configs()->Add(); sds_secret_configs->set_name("abc.com"); @@ -1109,9 +1115,11 @@ TEST_F(ServerContextConfigImplTest, ValidationContextNotReady) { NiceMock local_info; Stats::IsolatedStoreImpl stats; NiceMock init_manager; + NiceMock dispatcher; EXPECT_CALL(factory_context_, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context_, stats()).WillOnce(ReturnRef(stats)); EXPECT_CALL(factory_context_, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context_, dispatcher()).WillOnce(ReturnRef(dispatcher)); auto sds_secret_configs = tls_context.mutable_common_tls_context()->mutable_validation_context_sds_secret_config(); sds_secret_configs->set_name("abc.com"); diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index fa9c0e47e9baf..8396fff4fc312 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -47,8 +47,7 @@ class MockSubscription : public Subscription { MOCK_METHOD1(updateResources, void(const std::set& update_to_these_names)); }; -class MockSubscriptionFactory : public SubscriptionFactory, - public Logger::Loggable { +class MockSubscriptionFactory : public SubscriptionFactory { public: MockSubscriptionFactory(); ~MockSubscriptionFactory() override; diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 63968ebf74114..5164f8d24b188 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -128,7 +128,8 @@ MockInstance::MockInstance() singleton_manager_( new Singleton::ManagerImpl(Thread::threadFactoryForTest().currentThreadId())), grpc_context_(stats_store_.symbolTable()), http_context_(stats_store_.symbolTable()) { - ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); + secret_manager_.reset(new Secret::SecretManagerImpl(admin_.getConfigTracker())), + ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_store_)); ON_CALL(*this, grpcContext()).WillByDefault(ReturnRef(grpc_context_)); ON_CALL(*this, httpContext()).WillByDefault(ReturnRef(http_context_)); @@ -202,6 +203,7 @@ MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() { ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); + secret_manager_.reset(new Secret::SecretManagerImpl(config_tracker_)); } MockTransportSocketFactoryContext::~MockTransportSocketFactoryContext() = default; diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 4f0b71c7e76fd..e8e0701015d62 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -340,7 +340,7 @@ class MockInstance : public Instance { MockInstance(); ~MockInstance(); - Secret::SecretManager& secretManager() override { return secret_manager_; } + Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } MOCK_METHOD0(admin, Admin&()); MOCK_METHOD0(api, Api::Api&()); @@ -380,7 +380,7 @@ class MockInstance : public Instance { TimeSource& timeSource() override { return time_system_; } - testing::NiceMock secret_manager_; + std::unique_ptr secret_manager_; testing::NiceMock thread_local_; NiceMock stats_store_; std::shared_ptr> dns_resolver_{ @@ -489,7 +489,7 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MockTransportSocketFactoryContext(); ~MockTransportSocketFactoryContext(); - Secret::SecretManager& secretManager() override { return secret_manager_; } + Secret::SecretManager& secretManager() override { return *(secret_manager_.get()); } MOCK_METHOD0(admin, Server::Admin&()); MOCK_METHOD0(sslContextManager, Ssl::ContextManager&()); @@ -507,8 +507,9 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MOCK_METHOD0(api, Api::Api&()); testing::NiceMock cluster_manager_; - testing::NiceMock secret_manager_; + std::unique_ptr secret_manager_; testing::NiceMock api_; + testing::NiceMock config_tracker_; }; class MockListenerFactoryContext : public MockFactoryContext, public ListenerFactoryContext { From a24bcc8cb786914bb27d1d9cd086785b6aa71042 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 25 Jun 2019 16:50:46 +0000 Subject: [PATCH 07/27] fix spelling... Signed-off-by: Jianfei Hu --- test/common/secret/secret_manager_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 4fcd686238ba0..3f8c27a0bd4de 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -278,7 +278,7 @@ name: "abc.com" )EOF"; checkConfigDump(expected_secrets_config_dump); - // Add a dynamic tls validatoin context provider. + // Add a dynamic tls validation context provider. time_system_.setSystemTime(std::chrono::milliseconds(1234567899000)); auto context_secret_provider = secret_manager->findOrCreateCertificateValidationContextProvider( config_source, "abc.com.validation", secret_context); From d5e329b06a366deb759016c523e5406511df6e13 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 25 Jun 2019 18:13:40 +0000 Subject: [PATCH 08/27] fix the clang tidy and ssl_socket mock setup. Signed-off-by: Jianfei Hu --- test/extensions/transport_sockets/tls/ssl_socket_test.cc | 4 ++++ test/mocks/server/mocks.cc | 7 ++++--- test/mocks/server/mocks.h | 4 ++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index f7395a7a518e9..6b5bf647a47dd 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -3710,6 +3710,8 @@ TEST_P(SslSocketTest, DownstreamNotReadySslSocket) { NiceMock local_info; testing::NiceMock factory_context; NiceMock init_manager; + NiceMock dispatcher; + EXPECT_CALL(factory_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); @@ -3744,9 +3746,11 @@ TEST_P(SslSocketTest, UpstreamNotReadySslSocket) { NiceMock local_info; testing::NiceMock factory_context; NiceMock init_manager; + NiceMock dispatcher; EXPECT_CALL(factory_context, localInfo()).WillOnce(ReturnRef(local_info)); EXPECT_CALL(factory_context, stats()).WillOnce(ReturnRef(stats_store)); EXPECT_CALL(factory_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(factory_context, dispatcher()).WillOnce(ReturnRef(dispatcher)); envoy::api::v2::auth::UpstreamTlsContext tls_context; auto sds_secret_configs = diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 5164f8d24b188..60fb50147e434 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -124,7 +124,8 @@ MockWorker::MockWorker() { MockWorker::~MockWorker() = default; MockInstance::MockInstance() - : cluster_manager_(timeSource()), ssl_context_manager_(timeSource()), + : secret_manager_(std::make_unique(admin_.getConfigTracker())), + cluster_manager_(timeSource()), ssl_context_manager_(timeSource()), singleton_manager_( new Singleton::ManagerImpl(Thread::threadFactoryForTest().currentThreadId())), grpc_context_(stats_store_.symbolTable()), http_context_(stats_store_.symbolTable()) { @@ -198,12 +199,12 @@ MockFactoryContext::MockFactoryContext() MockFactoryContext::~MockFactoryContext() = default; -MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() { +MockTransportSocketFactoryContext::MockTransportSocketFactoryContext() + : secret_manager_(std::make_unique(config_tracker_)) { ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(ReturnRef(ProtobufMessage::getStrictValidationVisitor())); - secret_manager_.reset(new Secret::SecretManagerImpl(config_tracker_)); } MockTransportSocketFactoryContext::~MockTransportSocketFactoryContext() = default; diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index e8e0701015d62..d3e76e72d7a74 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -380,7 +380,6 @@ class MockInstance : public Instance { TimeSource& timeSource() override { return time_system_; } - std::unique_ptr secret_manager_; testing::NiceMock thread_local_; NiceMock stats_store_; std::shared_ptr> dns_resolver_{ @@ -388,6 +387,7 @@ class MockInstance : public Instance { testing::NiceMock api_; testing::NiceMock admin_; Event::GlobalTimeSystem time_system_; + std::unique_ptr secret_manager_; testing::NiceMock cluster_manager_; Thread::MutexBasicLockable access_log_lock_; testing::NiceMock runtime_loader_; @@ -507,9 +507,9 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext { MOCK_METHOD0(api, Api::Api&()); testing::NiceMock cluster_manager_; - std::unique_ptr secret_manager_; testing::NiceMock api_; testing::NiceMock config_tracker_; + std::unique_ptr secret_manager_; }; class MockListenerFactoryContext : public MockFactoryContext, public ListenerFactoryContext { From 9343b62083234ff0bc15ce87ade7388c7f6bf641 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 25 Jun 2019 22:50:13 +0000 Subject: [PATCH 09/27] fix double reset issue. testing rocks! Signed-off-by: Jianfei Hu --- test/mocks/server/mocks.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 60fb50147e434..0764789ddaab5 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -129,8 +129,7 @@ MockInstance::MockInstance() singleton_manager_( new Singleton::ManagerImpl(Thread::threadFactoryForTest().currentThreadId())), grpc_context_(stats_store_.symbolTable()), http_context_(stats_store_.symbolTable()) { - secret_manager_.reset(new Secret::SecretManagerImpl(admin_.getConfigTracker())), - ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); + ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_store_)); ON_CALL(*this, grpcContext()).WillByDefault(ReturnRef(grpc_context_)); ON_CALL(*this, httpContext()).WillByDefault(ReturnRef(http_context_)); From 7082ff75bb27a9b2df02b88e855b7285610d751b Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Thu, 11 Jul 2019 16:03:06 +0000 Subject: [PATCH 10/27] change to add warming secrets. Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 9 +++- source/common/secret/secret_manager_impl.cc | 49 ++++++++++--------- .../common/secret/secret_manager_impl_test.cc | 4 +- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index 2dc093f3f8265..ff19154714958 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -240,6 +240,11 @@ message SecretsConfigDump { envoy.api.v2.auth.Secret secret = 4; } - // List of all the secrets fetched via SDS. - repeated DynamicSecret dynamic_secrets = 1; + // The dynamically loaded active secrets. These are secrets that are available to service + // clusters or listeners. + repeated DynamicSecret dynamic_active_secrets = 1; + + // The dynamically loaded warming secrets. These are secrets that are currently undergoing + // warming in preparation to service clusters or listeners. + repeated DynamicSecret dynamic_warming_secrets = 2; } diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 8b6ad69dd69f1..69f5770340373 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -84,52 +84,55 @@ SecretManagerImpl::findOrCreateCertificateValidationContextProvider( secret_provider_context); } -// TODO: question, what's the handling of static inlined stuff? they're just internal constructs -// not needed to be exposed, maybe? ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { auto config_dump = std::make_unique(); auto providers = certificate_providers_.allSecretProviders(); for (const auto& cert_secrets : providers) { const auto& secret_data = cert_secrets->secretData(); const auto& tls_cert = cert_secrets->secret(); - const auto& dynamic_secret = config_dump->mutable_dynamic_secrets()->Add(); - const auto& secret = dynamic_secret->mutable_secret(); + ::envoy::admin::v2alpha::SecretsConfigDump_DynamicSecret* dump_secret; + bool secret_ready = tls_cert != nullptr; + if (secret_ready) { + dump_secret = config_dump->mutable_dynamic_active_secrets()->Add(); + } else { + dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); + } + auto secret = dump_secret->mutable_secret(); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); - dynamic_secret->set_version_info(secret_data.version_info_); - *dynamic_secret->mutable_last_updated() = last_updated_ts; + dump_secret->set_version_info(secret_data.version_info_); + *dump_secret->mutable_last_updated() = last_updated_ts; secret->set_name(secret_data.resource_name); - // TODO(incfly): this means the config is registered but Envoy hasn't received a - // valid push from SDS server yet... should we still dump it somehow? - if (!tls_cert) { - continue; - } - auto tls_certificate = secret->mutable_tls_certificate(); - tls_certificate->MergeFrom(*tls_cert); - + if (secret_ready) { + auto tls_certificate = secret->mutable_tls_certificate(); + tls_certificate->MergeFrom(*tls_cert); // We clear private key and password to avoid information leaking.j // TODO(incfly): switch to more generic scrubbing mechanism once // https://github.com/envoyproxy/envoy/issues/4757 is resolved. tls_certificate->clear_private_key(); tls_certificate->clear_password(); + } } // Handling validation Context provided via SDS. auto context_secret_provider = validation_context_providers_.allSecretProviders(); for (const auto& validation_context_secret : context_secret_provider) { - auto secret_data = validation_context_secret->secretData(); - auto validation_context = validation_context_secret->secret(); - auto dynamic_secret = config_dump->mutable_dynamic_secrets()->Add(); - auto secret = dynamic_secret->mutable_secret(); + const auto& secret_data = validation_context_secret->secretData(); + const auto& validation_context = validation_context_secret->secret(); + ::envoy::admin::v2alpha::SecretsConfigDump_DynamicSecret* dump_secret; + bool secret_ready = validation_context != nullptr; + if (secret_ready) { + dump_secret = config_dump->mutable_dynamic_active_secrets()->Add(); + } else { + dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); + } + auto secret = dump_secret->mutable_secret(); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); - dynamic_secret->set_version_info(secret_data.version_info_); - *dynamic_secret->mutable_last_updated() = last_updated_ts; + dump_secret->set_version_info(secret_data.version_info_); + *dump_secret->mutable_last_updated() = last_updated_ts; secret->set_name(secret_data.resource_name); - if (!validation_context) { - continue; - } auto dump_context = secret->mutable_validation_context(); dump_context->MergeFrom(*validation_context); } diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 3f8c27a0bd4de..f742d79b2bcb4 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -265,7 +265,7 @@ name: "abc.com" // Private key is removed. const std::string expected_secrets_config_dump = R"EOF( -dynamic_secrets: +dynamic_active_secrets: version_info: "keycert-v1" last_updated: seconds: 1234567891 @@ -302,7 +302,7 @@ name: "abc.com.validation" *context_secret_provider->secret(), *api_); EXPECT_EQ("DUMMY_INLINE_STRING_TRUSTED_CA", cert_validation_context.caCert()); const std::string updated_config_dump = R"EOF( -dynamic_secrets: +dynamic_active_secrets: - version_info: "keycert-v1" last_updated: seconds: 1234567891 From f72888b48c8c4f0cc07f663071f5e5f93a86b850 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Thu, 11 Jul 2019 16:48:19 +0000 Subject: [PATCH 11/27] warming case unit test added. Signed-off-by: Jianfei Hu --- source/common/secret/secret_manager_impl.cc | 16 ++--- .../common/secret/secret_manager_impl_test.cc | 61 ++++++++++++++++++- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 69f5770340373..94a02b87e799a 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -107,11 +107,11 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { if (secret_ready) { auto tls_certificate = secret->mutable_tls_certificate(); tls_certificate->MergeFrom(*tls_cert); - // We clear private key and password to avoid information leaking.j - // TODO(incfly): switch to more generic scrubbing mechanism once - // https://github.com/envoyproxy/envoy/issues/4757 is resolved. - tls_certificate->clear_private_key(); - tls_certificate->clear_password(); + // We clear private key and password to avoid information leaking.j + // TODO(incfly): switch to more generic scrubbing mechanism once + // https://github.com/envoyproxy/envoy/issues/4757 is resolved. + tls_certificate->clear_private_key(); + tls_certificate->clear_password(); } } @@ -133,8 +133,10 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { dump_secret->set_version_info(secret_data.version_info_); *dump_secret->mutable_last_updated() = last_updated_ts; secret->set_name(secret_data.resource_name); - auto dump_context = secret->mutable_validation_context(); - dump_context->MergeFrom(*validation_context); + if (secret_ready) { + auto dump_context = secret->mutable_validation_context(); + dump_context->MergeFrom(*validation_context); + } } return config_dump; } diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index f742d79b2bcb4..f6a3eea532a02 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -292,9 +292,6 @@ name: "abc.com.validation" secret_resources.Clear(); secret_resources.Add()->PackFrom(typed_secret); - // TODO(incfly): different config source is needed here... - // add helper function in the test class for easier add the test. - // uint64_t hash = MessageUtil::hash(config_source); init_target_handle->initialize(init_watcher); secret_context.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate( secret_resources, "validation-context-v1"); @@ -324,6 +321,64 @@ name: "abc.com.validation" checkConfigDump(updated_config_dump); } +TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { + Server::MockInstance server; + auto secret_manager = std::make_unique(config_tracker_); + time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); + + NiceMock secret_context; + + envoy::api::v2::core::ConfigSource config_source; + NiceMock local_info; + NiceMock dispatcher; + NiceMock random; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillRepeatedly(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); + EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); + + auto secret_provider = + secret_manager->findOrCreateTlsCertificateProvider(config_source, "abc.com", secret_context); + const std::string expected_secrets_config_dump = R"EOF( +dynamic_warming_secrets: + version_info: "uninitialized" + last_updated: + seconds: 1234567891 + nanos: 234000000 + secret: + name: "abc.com" + )EOF"; + checkConfigDump(expected_secrets_config_dump); + + time_system_.setSystemTime(std::chrono::milliseconds(1234567899000)); + auto context_secret_provider = secret_manager->findOrCreateCertificateValidationContextProvider( + config_source, "abc.com.validation", secret_context); + init_target_handle->initialize(init_watcher); + const std::string updated_config_dump = R"EOF( +dynamic_warming_secrets: +- version_info: "uninitialized" + last_updated: + seconds: 1234567891 + nanos: 234000000 + secret: + name: "abc.com" +- version_info: "uninitialized" + last_updated: + seconds: 1234567899 + secret: + name: "abc.com.validation" +)EOF"; + checkConfigDump(updated_config_dump); +} + } // namespace } // namespace Secret } // namespace Envoy From d68b3f54bfa5ac7259cd1f8572e2949ccb18739b Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Thu, 11 Jul 2019 16:52:17 +0000 Subject: [PATCH 12/27] simplify context var. Signed-off-by: Jianfei Hu --- source/common/secret/secret_manager_impl.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 94a02b87e799a..6fb1223b305e6 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -134,8 +134,7 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { *dump_secret->mutable_last_updated() = last_updated_ts; secret->set_name(secret_data.resource_name); if (secret_ready) { - auto dump_context = secret->mutable_validation_context(); - dump_context->MergeFrom(*validation_context); + secret->mutable_validation_context()->MergeFrom(*validation_context); } } return config_dump; From 40f83ed3911c6e1f2d583b9351a1132ecaf30494 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Fri, 12 Jul 2019 17:49:24 +0000 Subject: [PATCH 13/27] change to pw and key redacted Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 22 ++++++++++++++++--- source/common/secret/secret_manager_impl.cc | 10 ++++++--- source/common/secret/secret_manager_impl.h | 2 +- .../common/secret/secret_manager_impl_test.cc | 15 ++++++++++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index ff19154714958..ffe962ea4c4d9 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -56,7 +56,7 @@ message ListenersConfigDump { // will be "". string version_info = 1; - // Describes a statically loaded cluster. + // Describes a statically loaded listener. message StaticListener { // The listener config. envoy.api.v2.Listener listener = 1; @@ -240,11 +240,27 @@ message SecretsConfigDump { envoy.api.v2.auth.Secret secret = 4; } + // StaticSecret specifies statically loaded secret. + message StaticSecret { + // The name assigned to the secret. + string name = 1; + + // The timestamp when the Listener was last updated. + google.protobuf.Timestamp last_updated = 2; + + // The secret config. + envoy.api.v2.auth.Secret secret = 3; + } + + + // The statically loaded secrets. + repeated StaticSecret static_secrets = 1; + // The dynamically loaded active secrets. These are secrets that are available to service // clusters or listeners. - repeated DynamicSecret dynamic_active_secrets = 1; + repeated DynamicSecret dynamic_active_secrets = 2; // The dynamically loaded warming secrets. These are secrets that are currently undergoing // warming in preparation to service clusters or listeners. - repeated DynamicSecret dynamic_warming_secrets = 2; + repeated DynamicSecret dynamic_warming_secrets = 3; } diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 6fb1223b305e6..bf5f64a08d6e2 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -107,11 +107,15 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { if (secret_ready) { auto tls_certificate = secret->mutable_tls_certificate(); tls_certificate->MergeFrom(*tls_cert); - // We clear private key and password to avoid information leaking.j + // We clear private key and password to avoid information leaking. // TODO(incfly): switch to more generic scrubbing mechanism once // https://github.com/envoyproxy/envoy/issues/4757 is resolved. - tls_certificate->clear_private_key(); - tls_certificate->clear_password(); + if (tls_certificate->has_private_key()) { + tls_certificate->mutable_private_key()->set_inline_string("[redacted]"); + } + if (tls_certificate->has_password()) { + tls_certificate->mutable_password()->set_inline_string("[redacted]"); + } } } diff --git a/source/common/secret/secret_manager_impl.h b/source/common/secret/secret_manager_impl.h index f4139d140c105..7e3da018ecc8d 100644 --- a/source/common/secret/secret_manager_impl.h +++ b/source/common/secret/secret_manager_impl.h @@ -14,7 +14,7 @@ namespace Envoy { namespace Secret { -class SecretManagerImpl : public SecretManager, public Logger::Loggable { +class SecretManagerImpl : public SecretManager { public: SecretManagerImpl(Server::ConfigTracker& config_tracker); void addStaticSecret(const envoy::api::v2::auth::Secret& secret) override; diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index f6a3eea532a02..f7b779a8ec44e 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -251,6 +251,8 @@ name: "abc.com" inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" private_key: inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" + password: + inline_string: "DUMMY_PASSWORD" )EOF"; envoy::api::v2::auth::Secret typed_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), typed_secret); @@ -263,7 +265,7 @@ name: "abc.com" EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_CERT_CHAIN", tls_config.certificateChain()); EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY", tls_config.privateKey()); - // Private key is removed. + // Private key and password are removed. const std::string expected_secrets_config_dump = R"EOF( dynamic_active_secrets: version_info: "keycert-v1" @@ -275,6 +277,10 @@ name: "abc.com" tls_certificate: certificate_chain: inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "[redacted]" + password: + inline_string: "[redacted]" )EOF"; checkConfigDump(expected_secrets_config_dump); @@ -309,6 +315,10 @@ name: "abc.com.validation" tls_certificate: certificate_chain: inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "[redacted]" + password: + inline_string: "[redacted]" - version_info: "validation-context-v1" last_updated: seconds: 1234567899 @@ -379,6 +389,9 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { checkConfigDump(updated_config_dump); } +TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticSecrets) { +} + } // namespace } // namespace Secret } // namespace Envoy From 022db7b635badde9eb221f9a2f811171da9a445d Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Fri, 12 Jul 2019 18:28:24 +0000 Subject: [PATCH 14/27] wip finish test case for static, fail... Signed-off-by: Jianfei Hu --- source/common/secret/secret_manager_impl.cc | 29 +++++++++++- .../common/secret/secret_manager_impl_test.cc | 45 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index bf5f64a08d6e2..0573dd858a200 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -86,6 +86,33 @@ SecretManagerImpl::findOrCreateCertificateValidationContextProvider( ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { auto config_dump = std::make_unique(); + // Handle static tls key/cert providers. + for (const auto& cert_iter : static_tls_certificate_providers_) { + const auto& tls_cert = cert_iter.second; + auto dump_secret = config_dump->mutable_static_secrets()->Add(); + dump_secret->set_name(cert_iter.first); + dump_secret->set_name(cert_iter.first); + if (!tls_cert.get()) { + continue; + } + auto dump_tls_cert = dump_secret->mutable_secret()->mutable_tls_certificate(); + dump_tls_cert->MergeFrom(*tls_cert.get()->secret()); + // TODO: same util function to handle the key clean. + } + + // Handle static certificate validation context providers. + for (const auto& context_iter : static_certificate_validation_context_providers_) { + const auto& validation_context = context_iter.second; + auto dump_secret = config_dump->mutable_static_secrets()->Add(); + dump_secret->set_name(context_iter.first); + if (!validation_context.get()) { + continue; + } + dump_secret->mutable_secret()->mutable_validation_context()->MergeFrom( + *validation_context.get()->secret()); + } + + // Handle dynamic cert providers. auto providers = certificate_providers_.allSecretProviders(); for (const auto& cert_secrets : providers) { const auto& secret_data = cert_secrets->secretData(); @@ -119,7 +146,7 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { } } - // Handling validation Context provided via SDS. + // Handling dynamic cert validation context providers. auto context_secret_provider = validation_context_providers_.allSecretProviders(); for (const auto& validation_context_secret : context_secret_provider) { const auto& secret_data = validation_context_secret->secretData(); diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index f7b779a8ec44e..babdac2020b15 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -390,6 +390,51 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { } TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticSecrets) { + Server::MockInstance server; + auto secret_manager = std::make_unique(config_tracker_); + time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); + + NiceMock secret_context; + + envoy::api::v2::core::ConfigSource config_source; + NiceMock local_info; + NiceMock dispatcher; + NiceMock random; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillRepeatedly(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); + EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); + + const std::string tls_certificate = + R"EOF( +name: "abc.com" +tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" + password: + inline_string: "DUMMY_PASSWORD" +)EOF"; + envoy::api::v2::auth::Secret tls_cert_secret; + TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate), tls_cert_secret); + secret_manager->addStaticSecret(tls_cert_secret); + const std::string expected_config_dump = R"EOF( +static_secrets: +- name: "abc.com" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" +)EOF"; + checkConfigDump(expected_config_dump); } } // namespace From da5d481cc76d3b842ef3d752d2161f84106795fc Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 15 Jul 2019 19:02:35 +0000 Subject: [PATCH 15/27] all tests pass, update to set_name for all. Signed-off-by: Jianfei Hu --- source/common/secret/secret_manager_impl.cc | 57 +++++++++++-------- .../common/secret/secret_manager_impl_test.cc | 30 +++++++--- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 0573dd858a200..171a19bc71990 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -84,35 +84,51 @@ SecretManagerImpl::findOrCreateCertificateValidationContextProvider( secret_provider_context); } +// We clear private key and password to avoid information leaking. +// TODO(incfly): switch to more generic scrubbing mechanism once +// https://github.com/envoyproxy/envoy/issues/4757 is resolved. +void redactSecret(::envoy::api::v2::auth::Secret* secret) { + if (secret && secret->type_case() == envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate) { + auto tls_certificate = secret->mutable_tls_certificate(); + if (tls_certificate->has_private_key()) { + tls_certificate->mutable_private_key()->set_inline_string("[redacted]"); + } + if (tls_certificate->has_password()) { + tls_certificate->mutable_password()->set_inline_string("[redacted]"); + } + } +} + ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { auto config_dump = std::make_unique(); // Handle static tls key/cert providers. for (const auto& cert_iter : static_tls_certificate_providers_) { const auto& tls_cert = cert_iter.second; - auto dump_secret = config_dump->mutable_static_secrets()->Add(); - dump_secret->set_name(cert_iter.first); - dump_secret->set_name(cert_iter.first); + auto static_secret = config_dump->mutable_static_secrets()->Add(); + static_secret->set_name(cert_iter.first); if (!tls_cert.get()) { continue; } - auto dump_tls_cert = dump_secret->mutable_secret()->mutable_tls_certificate(); - dump_tls_cert->MergeFrom(*tls_cert.get()->secret()); - // TODO: same util function to handle the key clean. + auto dump_secret = static_secret->mutable_secret(); + dump_secret->set_name(cert_iter.first); + dump_secret->mutable_tls_certificate()->MergeFrom(*tls_cert.get()->secret()); + redactSecret(dump_secret); } // Handle static certificate validation context providers. for (const auto& context_iter : static_certificate_validation_context_providers_) { const auto& validation_context = context_iter.second; - auto dump_secret = config_dump->mutable_static_secrets()->Add(); - dump_secret->set_name(context_iter.first); + auto static_secret = config_dump->mutable_static_secrets()->Add(); + static_secret->set_name(context_iter.first); if (!validation_context.get()) { continue; } - dump_secret->mutable_secret()->mutable_validation_context()->MergeFrom( - *validation_context.get()->secret()); + auto dump_secret = static_secret->mutable_secret(); + dump_secret->set_name(context_iter.first); + dump_secret->mutable_validation_context()->MergeFrom(*validation_context.get()->secret()); } - // Handle dynamic cert providers. + // Handle dynamic tls_certificate providers. auto providers = certificate_providers_.allSecretProviders(); for (const auto& cert_secrets : providers) { const auto& secret_data = cert_secrets->secretData(); @@ -124,26 +140,18 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { } else { dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); } + dump_secret->set_name(secret_data.resource_name); auto secret = dump_secret->mutable_secret(); - + secret->set_name(secret_data.resource_name); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); dump_secret->set_version_info(secret_data.version_info_); *dump_secret->mutable_last_updated() = last_updated_ts; secret->set_name(secret_data.resource_name); if (secret_ready) { - auto tls_certificate = secret->mutable_tls_certificate(); - tls_certificate->MergeFrom(*tls_cert); - // We clear private key and password to avoid information leaking. - // TODO(incfly): switch to more generic scrubbing mechanism once - // https://github.com/envoyproxy/envoy/issues/4757 is resolved. - if (tls_certificate->has_private_key()) { - tls_certificate->mutable_private_key()->set_inline_string("[redacted]"); - } - if (tls_certificate->has_password()) { - tls_certificate->mutable_password()->set_inline_string("[redacted]"); - } + secret->mutable_tls_certificate()->MergeFrom(*tls_cert); } + redactSecret(secret); } // Handling dynamic cert validation context providers. @@ -158,12 +166,13 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { } else { dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); } + dump_secret->set_name(secret_data.resource_name); auto secret = dump_secret->mutable_secret(); + secret->set_name(secret_data.resource_name); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); dump_secret->set_version_info(secret_data.version_info_); *dump_secret->mutable_last_updated() = last_updated_ts; - secret->set_name(secret_data.resource_name); if (secret_ready) { secret->mutable_validation_context()->MergeFrom(*validation_context); } diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index babdac2020b15..dc1b7ece74a2c 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -268,6 +268,7 @@ name: "abc.com" // Private key and password are removed. const std::string expected_secrets_config_dump = R"EOF( dynamic_active_secrets: +- name: "abc.com" version_info: "keycert-v1" last_updated: seconds: 1234567891 @@ -289,7 +290,7 @@ name: "abc.com" auto context_secret_provider = secret_manager->findOrCreateCertificateValidationContextProvider( config_source, "abc.com.validation", secret_context); const std::string validation_yaml = R"EOF( -name: "abc.com.validation" +name: "abc.com" validation_context: trusted_ca: inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" @@ -306,7 +307,8 @@ name: "abc.com.validation" EXPECT_EQ("DUMMY_INLINE_STRING_TRUSTED_CA", cert_validation_context.caCert()); const std::string updated_config_dump = R"EOF( dynamic_active_secrets: -- version_info: "keycert-v1" +- name: "abc.com" + version_info: "keycert-v1" last_updated: seconds: 1234567891 nanos: 234000000 @@ -319,7 +321,8 @@ name: "abc.com.validation" inline_string: "[redacted]" password: inline_string: "[redacted]" -- version_info: "validation-context-v1" +- name: "abc.com.validation" + version_info: "validation-context-v1" last_updated: seconds: 1234567899 secret: @@ -359,6 +362,7 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { secret_manager->findOrCreateTlsCertificateProvider(config_source, "abc.com", secret_context); const std::string expected_secrets_config_dump = R"EOF( dynamic_warming_secrets: +- name: "abc.com" version_info: "uninitialized" last_updated: seconds: 1234567891 @@ -374,13 +378,15 @@ TEST_F(SecretManagerImplTest, ConfigDumpHandlerWarmingSecrets) { init_target_handle->initialize(init_watcher); const std::string updated_config_dump = R"EOF( dynamic_warming_secrets: -- version_info: "uninitialized" +- name: "abc.com" + version_info: "uninitialized" last_updated: seconds: 1234567891 nanos: 234000000 secret: name: "abc.com" -- version_info: "uninitialized" +- name: "abc.com.validation" + version_info: "uninitialized" last_updated: seconds: 1234567899 secret: @@ -429,10 +435,16 @@ name: "abc.com" secret_manager->addStaticSecret(tls_cert_secret); const std::string expected_config_dump = R"EOF( static_secrets: -- name: "abc.com" - tls_certificate: - certificate_chain: - inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" +- name: "abc.com" + secret: + name: "abc.com" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "[redacted]" + password: + inline_string: "[redacted]" )EOF"; checkConfigDump(expected_config_dump); } From 742fa012f2c7d31397da5a419187031e3e73743f Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 15 Jul 2019 19:12:21 +0000 Subject: [PATCH 16/27] one more test for static validaiton context Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 1 - .../common/secret/secret_manager_impl_test.cc | 48 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index ffe962ea4c4d9..d166907aa7476 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -252,7 +252,6 @@ message SecretsConfigDump { envoy.api.v2.auth.Secret secret = 3; } - // The statically loaded secrets. repeated StaticSecret static_secrets = 1; diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index dc1b7ece74a2c..51b9a24e27ea1 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -290,7 +290,7 @@ name: "abc.com" auto context_secret_provider = secret_manager->findOrCreateCertificateValidationContextProvider( config_source, "abc.com.validation", secret_context); const std::string validation_yaml = R"EOF( -name: "abc.com" +name: "abc.com.validation" validation_context: trusted_ca: inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" @@ -449,6 +449,52 @@ name: "abc.com" checkConfigDump(expected_config_dump); } +TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticValidationContext) { + Server::MockInstance server; + auto secret_manager = std::make_unique(config_tracker_); + time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); + + NiceMock secret_context; + + envoy::api::v2::core::ConfigSource config_source; + NiceMock local_info; + NiceMock dispatcher; + NiceMock random; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillRepeatedly(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); + EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); + + const std::string validation_context = + R"EOF( +name: "abc.com.validation" +validation_context: + trusted_ca: + inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" +)EOF"; + envoy::api::v2::auth::Secret validation_secret; + TestUtility::loadFromYaml(TestEnvironment::substitute(validation_context), validation_secret); + secret_manager->addStaticSecret(validation_secret); + const std::string expected_config_dump = R"EOF( +static_secrets: +- name: "abc.com.validation" + secret: + name: "abc.com.validation" + validation_context: + trusted_ca: + inline_string: "DUMMY_INLINE_STRING_TRUSTED_CA" +)EOF"; + checkConfigDump(expected_config_dump); +} + } // namespace } // namespace Secret } // namespace Envoy From ec6449ea18fabb22e28e1251b0b1e1f06d99ed99 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 15 Jul 2019 19:19:14 +0000 Subject: [PATCH 17/27] remove threadmodel todo question. Signed-off-by: Jianfei Hu --- source/common/secret/sds_api.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index b6fde31396626..ce9f97b70757a 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -43,8 +43,6 @@ class SdsApi : public Config::SubscriptionCallbacks { ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats, Init::Manager& init_manager, std::function destructor_cb); - // TODO: what's the thread model of the admin::config_dump vs xDS config updates? - // any lock mechanism needed? SecretData secretData(); protected: From 51a64ad8689bb3429756d292756d0d2ee0db5fb8 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Sun, 21 Jul 2019 07:04:49 +0000 Subject: [PATCH 18/27] add integration test for sds config dump. Signed-off-by: Jianfei Hu --- test/config/utility.cc | 10 ++++++++++ test/integration/integration_admin_test.cc | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/test/config/utility.cc b/test/config/utility.cc index b49638a6ca010..cecf66ca568cb 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -31,6 +31,15 @@ const std::string ConfigHelper::BASE_CONFIG = R"EOF( lds_config: path: /dev/null static_resources: + secrets: + - name: "secret_static_0" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES" + private_key: + inline_string: "DUMMY_INLINE_BYTES" + password: + inline_string: "DUMMY_INLINE_BYTES" clusters: name: cluster_0 hosts: @@ -703,4 +712,5 @@ void EdsHelper::setEdsAndWait( RELEASE_ASSERT( update_successes_ == server_stats.counter("cluster.cluster_0.update_success")->value(), ""); } + } // namespace Envoy diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index e7b37f1df42cf..2af1c1f069df5 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -411,7 +411,9 @@ TEST_P(IntegrationAdminTest, Admin) { config_dump.configs(4).UnpackTo(&route_config_dump); EXPECT_EQ("route_config_0", route_config_dump.static_route_configs(0).route_config().name()); - // TODO(incfly) do we need to add parsing and data input for the sds? + envoy::admin::v2alpha::SecretsConfigDump secret_config_dump; + config_dump.configs(5).UnpackTo(&secret_config_dump); + EXPECT_EQ("secret_static_0", secret_config_dump.static_secrets(0).name()); } TEST_P(IntegrationAdminTest, AdminOnDestroyCallbacks) { From 44ace74d17212c4bac007237fd3922ac7cf8e592 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 01:19:18 +0000 Subject: [PATCH 19/27] clang format fix. Signed-off-by: Jianfei Hu --- source/server/server.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index bcae3bf1f0d21..e3432967fa0ba 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -56,10 +56,10 @@ InstanceImpl::InstanceImpl(const Options& options, Event::TimeSystem& time_syste ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system, std::unique_ptr process_context) - : workers_started_(false), shutdown_(false), options_(options), - time_source_(time_system), restarter_(restarter), - start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), - thread_local_(tls), api_(new Api::Impl(thread_factory, store, time_system, file_system)), + : workers_started_(false), shutdown_(false), options_(options), time_source_(time_system), + restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), + stats_store_(store), thread_local_(tls), + api_(new Api::Impl(thread_factory, store, time_system, file_system)), dispatcher_(api_->allocateDispatcher()), singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())), handler_(new ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), From 031da5cf453202cc199f1a027709d54eaa9bfdb1 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 01:24:55 +0000 Subject: [PATCH 20/27] update comments in configdump.proto. Signed-off-by: Jianfei Hu --- api/envoy/admin/v2alpha/config_dump.proto | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto index d166907aa7476..8ff5ba8fa102a 100644 --- a/api/envoy/admin/v2alpha/config_dump.proto +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -224,7 +224,6 @@ message ScopedRoutesConfigDump { // Envoys SDS implementation fills this message with all secrets fetched dynamically via SDS. message SecretsConfigDump { // DynamicSecret contains secret information fetched via SDS. - // The security sensitive information will trimmed, for example, private key. message DynamicSecret { // The name assigned to the secret. string name = 1; @@ -235,20 +234,23 @@ message SecretsConfigDump { // The timestamp when the secret was last updated. google.protobuf.Timestamp last_updated = 3; - // The actual secret information. Security sensitive information is stripped. - // For example, private key in a TLS cert. + // The actual secret information. + // Security sensitive information is redacted (replaced with "[redacted]") for + // private keys and passwords in TLS certificates. envoy.api.v2.auth.Secret secret = 4; } - // StaticSecret specifies statically loaded secret. + // StaticSecret specifies statically loaded secret in bootstrap. message StaticSecret { // The name assigned to the secret. string name = 1; - // The timestamp when the Listener was last updated. + // The timestamp when the secret was last updated. google.protobuf.Timestamp last_updated = 2; - // The secret config. + // The actual secret information. + // Security sensitive information is redacted (replaced with "[redacted]") for + // private keys and passwords in TLS certificates. envoy.api.v2.auth.Secret secret = 3; } From aef1787241f0b550b2686985cce326a4e989081c Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 17:33:39 +0000 Subject: [PATCH 21/27] address comments. Signed-off-by: Jianfei Hu --- test/common/secret/secret_manager_impl_test.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 51b9a24e27ea1..03bdf33bc6758 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -264,6 +264,7 @@ name: "abc.com" Ssl::TlsCertificateConfigImpl tls_config(*secret_provider->secret(), *api_); EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_CERT_CHAIN", tls_config.certificateChain()); EXPECT_EQ("DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY", tls_config.privateKey()); + EXPECT_EQ("DUMMY_PASSWORD", tls_config.password()); // Private key and password are removed. const std::string expected_secrets_config_dump = R"EOF( @@ -432,9 +433,26 @@ name: "abc.com" )EOF"; envoy::api::v2::auth::Secret tls_cert_secret; TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate), tls_cert_secret); + secret_manager->addStaticSecret(tls_cert_secret); + TestUtility::loadFromYaml(TestEnvironment::substitute(R"EOF( +name: "abc.com.nopassword" +tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" +)EOF"), tls_cert_secret); secret_manager->addStaticSecret(tls_cert_secret); const std::string expected_config_dump = R"EOF( static_secrets: +- name: "abc.com.nopassword" + secret: + name: "abc.com.nopassword" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "[redacted]" - name: "abc.com" secret: name: "abc.com" From 67373f20d34f640f1de9abc679efe831b3dba4f7 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 17:38:24 +0000 Subject: [PATCH 22/27] format fix. Signed-off-by: Jianfei Hu --- test/common/secret/secret_manager_impl_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index 03bdf33bc6758..c46b44ebecb0a 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -441,7 +441,8 @@ name: "abc.com.nopassword" inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" private_key: inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" -)EOF"), tls_cert_secret); +)EOF"), + tls_cert_secret); secret_manager->addStaticSecret(tls_cert_secret); const std::string expected_config_dump = R"EOF( static_secrets: From 62b9635e8439c1a7a8456aaecda14aed0053c787 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 23:13:47 +0000 Subject: [PATCH 23/27] address comments, not redact for filename. Signed-off-by: Jianfei Hu --- source/common/secret/sds_api.h | 2 +- source/common/secret/secret_manager_impl.cc | 32 ++++++++++----------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/source/common/secret/sds_api.h b/source/common/secret/sds_api.h index ce9f97b70757a..a99fafb6c8122 100644 --- a/source/common/secret/sds_api.h +++ b/source/common/secret/sds_api.h @@ -33,7 +33,7 @@ namespace Secret { class SdsApi : public Config::SubscriptionCallbacks { public: struct SecretData { - const std::string resource_name; + const std::string resource_name_; std::string version_info_; SystemTime last_updated_; }; diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 171a19bc71990..9b892429c8a62 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -90,10 +90,12 @@ SecretManagerImpl::findOrCreateCertificateValidationContextProvider( void redactSecret(::envoy::api::v2::auth::Secret* secret) { if (secret && secret->type_case() == envoy::api::v2::auth::Secret::TypeCase::kTlsCertificate) { auto tls_certificate = secret->mutable_tls_certificate(); - if (tls_certificate->has_private_key()) { + if (tls_certificate->has_private_key() && tls_certificate->private_key().specifier_case() != + envoy::api::v2::core::DataSource::kFilename) { tls_certificate->mutable_private_key()->set_inline_string("[redacted]"); } - if (tls_certificate->has_password()) { + if (tls_certificate->has_password() && tls_certificate->password().specifier_case() != + envoy::api::v2::core::DataSource::kFilename) { tls_certificate->mutable_password()->set_inline_string("[redacted]"); } } @@ -106,9 +108,7 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { const auto& tls_cert = cert_iter.second; auto static_secret = config_dump->mutable_static_secrets()->Add(); static_secret->set_name(cert_iter.first); - if (!tls_cert.get()) { - continue; - } + ASSERT(tls_cert != nullptr); auto dump_secret = static_secret->mutable_secret(); dump_secret->set_name(cert_iter.first); dump_secret->mutable_tls_certificate()->MergeFrom(*tls_cert.get()->secret()); @@ -120,34 +120,32 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { const auto& validation_context = context_iter.second; auto static_secret = config_dump->mutable_static_secrets()->Add(); static_secret->set_name(context_iter.first); - if (!validation_context.get()) { - continue; - } + ASSERT(validation_context != nullptr); auto dump_secret = static_secret->mutable_secret(); dump_secret->set_name(context_iter.first); dump_secret->mutable_validation_context()->MergeFrom(*validation_context.get()->secret()); } // Handle dynamic tls_certificate providers. - auto providers = certificate_providers_.allSecretProviders(); + const auto providers = certificate_providers_.allSecretProviders(); for (const auto& cert_secrets : providers) { const auto& secret_data = cert_secrets->secretData(); const auto& tls_cert = cert_secrets->secret(); ::envoy::admin::v2alpha::SecretsConfigDump_DynamicSecret* dump_secret; - bool secret_ready = tls_cert != nullptr; + const bool secret_ready = tls_cert != nullptr; if (secret_ready) { dump_secret = config_dump->mutable_dynamic_active_secrets()->Add(); } else { dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); } - dump_secret->set_name(secret_data.resource_name); + dump_secret->set_name(secret_data.resource_name_); auto secret = dump_secret->mutable_secret(); - secret->set_name(secret_data.resource_name); + secret->set_name(secret_data.resource_name_); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); dump_secret->set_version_info(secret_data.version_info_); *dump_secret->mutable_last_updated() = last_updated_ts; - secret->set_name(secret_data.resource_name); + secret->set_name(secret_data.resource_name_); if (secret_ready) { secret->mutable_tls_certificate()->MergeFrom(*tls_cert); } @@ -155,20 +153,20 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { } // Handling dynamic cert validation context providers. - auto context_secret_provider = validation_context_providers_.allSecretProviders(); + const auto context_secret_provider = validation_context_providers_.allSecretProviders(); for (const auto& validation_context_secret : context_secret_provider) { const auto& secret_data = validation_context_secret->secretData(); const auto& validation_context = validation_context_secret->secret(); ::envoy::admin::v2alpha::SecretsConfigDump_DynamicSecret* dump_secret; - bool secret_ready = validation_context != nullptr; + const bool secret_ready = validation_context != nullptr; if (secret_ready) { dump_secret = config_dump->mutable_dynamic_active_secrets()->Add(); } else { dump_secret = config_dump->mutable_dynamic_warming_secrets()->Add(); } - dump_secret->set_name(secret_data.resource_name); + dump_secret->set_name(secret_data.resource_name_); auto secret = dump_secret->mutable_secret(); - secret->set_name(secret_data.resource_name); + secret->set_name(secret_data.resource_name_); ProtobufWkt::Timestamp last_updated_ts; TimestampUtil::systemClockToTimestamp(secret_data.last_updated_, last_updated_ts); dump_secret->set_version_info(secret_data.version_info_); From 9fb8f423f48c9d17de966a58b025392cdc0bc1fa Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Mon, 22 Jul 2019 23:16:27 +0000 Subject: [PATCH 24/27] get rid of uni ptr get. Signed-off-by: Jianfei Hu --- source/common/secret/secret_manager_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/secret/secret_manager_impl.cc b/source/common/secret/secret_manager_impl.cc index 9b892429c8a62..411f71ae6d0bf 100644 --- a/source/common/secret/secret_manager_impl.cc +++ b/source/common/secret/secret_manager_impl.cc @@ -111,7 +111,7 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { ASSERT(tls_cert != nullptr); auto dump_secret = static_secret->mutable_secret(); dump_secret->set_name(cert_iter.first); - dump_secret->mutable_tls_certificate()->MergeFrom(*tls_cert.get()->secret()); + dump_secret->mutable_tls_certificate()->MergeFrom(*tls_cert->secret()); redactSecret(dump_secret); } @@ -123,7 +123,7 @@ ProtobufTypes::MessagePtr SecretManagerImpl::dumpSecretConfigs() { ASSERT(validation_context != nullptr); auto dump_secret = static_secret->mutable_secret(); dump_secret->set_name(context_iter.first); - dump_secret->mutable_validation_context()->MergeFrom(*validation_context.get()->secret()); + dump_secret->mutable_validation_context()->MergeFrom(*validation_context->secret()); } // Handle dynamic tls_certificate providers. From 5130216ec9c713e3d3cc408e98ab873d66a9e8ca Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 23 Jul 2019 00:44:41 +0000 Subject: [PATCH 25/27] add test for filename no redact. Signed-off-by: Jianfei Hu --- .../common/secret/secret_manager_impl_test.cc | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/test/common/secret/secret_manager_impl_test.cc b/test/common/secret/secret_manager_impl_test.cc index c46b44ebecb0a..b3a6105fb11f2 100644 --- a/test/common/secret/secret_manager_impl_test.cc +++ b/test/common/secret/secret_manager_impl_test.cc @@ -468,13 +468,62 @@ name: "abc.com.nopassword" checkConfigDump(expected_config_dump); } -TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticValidationContext) { +TEST_F(SecretManagerImplTest, ConfigDumpNotRedactFilenamePrivateKey) { Server::MockInstance server; auto secret_manager = std::make_unique(config_tracker_); time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); - NiceMock secret_context; + envoy::api::v2::core::ConfigSource config_source; + NiceMock local_info; + NiceMock dispatcher; + NiceMock random; + Stats::IsolatedStoreImpl stats; + NiceMock init_manager; + NiceMock init_watcher; + Init::TargetHandlePtr init_target_handle; + EXPECT_CALL(init_manager, add(_)) + .WillRepeatedly(Invoke([&init_target_handle](const Init::Target& target) { + init_target_handle = target.createHandle("test"); + })); + EXPECT_CALL(secret_context, stats()).WillRepeatedly(ReturnRef(stats)); + EXPECT_CALL(secret_context, initManager()).WillRepeatedly(Return(&init_manager)); + EXPECT_CALL(secret_context, dispatcher()).WillRepeatedly(ReturnRef(dispatcher)); + EXPECT_CALL(secret_context, localInfo()).WillRepeatedly(ReturnRef(local_info)); + const std::string tls_certificate = R"EOF( +name: "abc.com" +tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "DUMMY_INLINE_BYTES_FOR_PRIVATE_KEY" + password: + filename: "/etc/certs/password" +)EOF"; + envoy::api::v2::auth::Secret tls_cert_secret; + TestUtility::loadFromYaml(TestEnvironment::substitute(tls_certificate), tls_cert_secret); + secret_manager->addStaticSecret(tls_cert_secret); + const std::string expected_config_dump = R"EOF( +static_secrets: +- name: "abc.com" + secret: + name: "abc.com" + tls_certificate: + certificate_chain: + inline_string: "DUMMY_INLINE_BYTES_FOR_CERT_CHAIN" + private_key: + inline_string: "[redacted]" + password: + filename: "/etc/certs/password" +)EOF"; + checkConfigDump(expected_config_dump); +} + +TEST_F(SecretManagerImplTest, ConfigDumpHandlerStaticValidationContext) { + Server::MockInstance server; + auto secret_manager = std::make_unique(config_tracker_); + time_system_.setSystemTime(std::chrono::milliseconds(1234567891234)); + NiceMock secret_context; envoy::api::v2::core::ConfigSource config_source; NiceMock local_info; NiceMock dispatcher; From e0f12dd0d572595298bff49a28135a193a6b7eca Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 23 Jul 2019 23:55:34 +0000 Subject: [PATCH 26/27] update the version history. Signed-off-by: Jianfei Hu --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index e38946186a4e3..7c4799ce08c93 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -8,6 +8,7 @@ Version history * config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. * listeners: added :ref:`HTTP inspector listener filter `. * http: added the ability to reject HTTP/1.1 requests with invalid HTTP header values, using the runtime feature `envoy.reloadable_features.strict_header_validation`. +* admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump `. 1.11.0 (July 11, 2019) ====================== From 52938d192a9701db76af7f9c6a8c6c22ee61eb56 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Wed, 24 Jul 2019 00:35:41 +0000 Subject: [PATCH 27/27] lint fix and use the right rst syntax. Signed-off-by: Jianfei Hu --- docs/root/intro/version_history.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 7c4799ce08c93..04e8694abb771 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -4,11 +4,11 @@ Version history 1.12.0 (pending) ================ * admin: added ability to configure listener :ref:`socket options `. +* admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump `. * config: async data access for local and remote data source. * config: changed the default value of :ref:`initial_fetch_timeout ` from 0s to 15s. This is a change in behaviour in the sense that Envoy will move to the next initialization phase, even if the first config is not delivered in 15s. Refer to :ref:`initialization process ` for more details. * listeners: added :ref:`HTTP inspector listener filter `. * http: added the ability to reject HTTP/1.1 requests with invalid HTTP header values, using the runtime feature `envoy.reloadable_features.strict_header_validation`. -* admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump `. 1.11.0 (July 11, 2019) ======================