Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sds: certificate hot-reload for xDS gRPC connection #10163

Merged
merged 3 commits into from Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/envoy/filesystem/watcher.h
Expand Up @@ -30,6 +30,9 @@ class Watcher {
/**
* Add a file watch.
* @param path supplies the path to watch.
* If path is a file, callback is called on events for the given file.
* If path is a directory (ends with "/"), callback is called on events
* for the given directory.
* @param events supplies the events to watch.
* @param cb supplies the callback to invoke when a change occurs.
*/
Expand Down
11 changes: 8 additions & 3 deletions source/common/filesystem/inotify/watcher_impl.cc
Expand Up @@ -80,9 +80,14 @@ void WatcherImpl::onInotifyEvent() {
}

for (FileWatch& watch : callback_map_[file_event->wd].watches_) {
if (watch.file_ == file && (watch.events_ & events)) {
ENVOY_LOG(debug, "matched callback: file: {}", file);
watch.cb_(events);
if (watch.events_ & events) {
if (watch.file_ == file) {
ENVOY_LOG(debug, "matched callback: file: {}", file);
watch.cb_(events);
} else if (watch.file_.empty()) {
ENVOY_LOG(debug, "matched callback: directory: {}", file);
watch.cb_(events);
}
}
}

Expand Down
9 changes: 6 additions & 3 deletions source/common/filesystem/kqueue/watcher_impl.cc
Expand Up @@ -65,9 +65,6 @@ WatcherImpl::FileWatchPtr WatcherImpl::addWatch(absl::string_view path, uint32_t
watch->watching_dir_ = watching_dir;

u_int flags = NOTE_DELETE | NOTE_RENAME | NOTE_WRITE;
if (watching_dir) {
flags = NOTE_DELETE | NOTE_WRITE;
}

struct kevent event;
EV_SET(&event, watch_fd, EVFILT_VNODE, EV_ADD | EV_CLEAR, flags, 0,
Expand Down Expand Up @@ -109,6 +106,8 @@ void WatcherImpl::onKqueueEvent() {
ASSERT(file != nullptr);
ASSERT(watch_fd == file->fd_);

auto pathname = api_.fileSystem().splitPathFromFilename(file->file_);

if (file->watching_dir_) {
if (event.fflags & NOTE_DELETE) {
// directory was deleted
Expand All @@ -126,6 +125,10 @@ void WatcherImpl::onKqueueEvent() {
events |= Events::MovedTo;
}
}
} else if (pathname.file_.empty()) {
if (event.fflags & NOTE_WRITE) {
events |= Events::MovedTo;
}
} else {
// kqueue doesn't seem to work well with NOTE_RENAME and O_SYMLINK, so instead if we
// get a NOTE_DELETE on the symlink we check if there is another file with the same
Expand Down
70 changes: 67 additions & 3 deletions source/common/secret/sds_api.cc
Expand Up @@ -19,13 +19,15 @@ namespace Secret {
SdsApi::SdsApi(envoy::config::core::v3::ConfigSource sds_config, absl::string_view sds_config_name,
Config::SubscriptionFactory& subscription_factory, TimeSource& time_source,
ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats,
Init::Manager& init_manager, std::function<void()> destructor_cb)
Init::Manager& init_manager, std::function<void()> destructor_cb,
Event::Dispatcher& dispatcher, Api::Api& api)
: 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),
time_source_(time_source), secret_data_{sds_config_name_, "uninitialized",
time_source_.systemTime()} {
time_source_.systemTime()},
dispatcher_(dispatcher), api_(api) {
// 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
Expand All @@ -45,12 +47,36 @@ void SdsApi::onConfigUpdate(const Protobuf::RepeatedPtrField<ProtobufWkt::Any>&
fmt::format("Unexpected SDS secret (expecting {}): {}", sds_config_name_, secret.name()));
}

const uint64_t new_hash = MessageUtil::hash(secret);
uint64_t new_hash = MessageUtil::hash(secret);

if (new_hash != secret_hash_) {
validateConfig(secret);
secret_hash_ = new_hash;
setSecret(secret);
update_callback_manager_.runCallbacks();

// List DataSources that refer to files
auto files = getDataSourceFilenames();
if (!files.empty()) {
// Create new watch, also destroys the old watch if any.
watcher_ = dispatcher_.createFilesystemWatcher();
files_hash_ = getHashForFiles();
for (auto const& filename : files) {
// Watch for directory instead of file. This allows users to do atomic renames
// on directory level (e.g. Kubernetes secret update).
const auto result = api_.fileSystem().splitPathFromFilename(filename);
watcher_->addWatch(absl::StrCat(result.directory_, "/"),
Filesystem::Watcher::Events::MovedTo, [this](uint32_t) {
uint64_t new_hash = getHashForFiles();
if (new_hash != files_hash_) {
update_callback_manager_.runCallbacks();
files_hash_ = new_hash;
}
});
}
} else {
watcher_.reset(); // Destroy the old watch if any
}
}
secret_data_.last_updated_ = time_source_.systemTime();
secret_data_.version_info_ = version_info;
Expand Down Expand Up @@ -92,5 +118,43 @@ void SdsApi::initialize() {

SdsApi::SecretData SdsApi::secretData() { return secret_data_; }

uint64_t SdsApi::getHashForFiles() {
uint64_t hash = 0;
for (auto const& filename : getDataSourceFilenames()) {
hash = HashUtil::xxHash64(api_.fileSystem().fileReadToEnd(filename), hash);
}
return hash;
}

std::vector<std::string> TlsCertificateSdsApi::getDataSourceFilenames() {
std::vector<std::string> files;
if (tls_certificate_secrets_ && tls_certificate_secrets_->has_certificate_chain() &&
tls_certificate_secrets_->certificate_chain().specifier_case() ==
envoy::config::core::v3::DataSource::SpecifierCase::kFilename) {
files.push_back(tls_certificate_secrets_->certificate_chain().filename());
}
if (tls_certificate_secrets_ && tls_certificate_secrets_->has_private_key() &&
tls_certificate_secrets_->private_key().specifier_case() ==
envoy::config::core::v3::DataSource::SpecifierCase::kFilename) {
files.push_back(tls_certificate_secrets_->private_key().filename());
}
return files;
}

std::vector<std::string> CertificateValidationContextSdsApi::getDataSourceFilenames() {
std::vector<std::string> files;
if (certificate_validation_context_secrets_ &&
certificate_validation_context_secrets_->has_trusted_ca() &&
certificate_validation_context_secrets_->trusted_ca().specifier_case() ==
envoy::config::core::v3::DataSource::SpecifierCase::kFilename) {
files.push_back(certificate_validation_context_secrets_->trusted_ca().filename());
}
return files;
}

std::vector<std::string> TlsSessionTicketKeysSdsApi::getDataSourceFilenames() { return {}; }

std::vector<std::string> GenericSecretSdsApi::getDataSourceFilenames() { return {}; }

} // namespace Secret
} // namespace Envoy
46 changes: 33 additions & 13 deletions source/common/secret/sds_api.h
Expand Up @@ -44,7 +44,8 @@ class SdsApi : public Envoy::Config::SubscriptionBase<
SdsApi(envoy::config::core::v3::ConfigSource sds_config, absl::string_view sds_config_name,
Config::SubscriptionFactory& subscription_factory, TimeSource& time_source,
ProtobufMessage::ValidationVisitor& validation_visitor, Stats::Store& stats,
Init::Manager& init_manager, std::function<void()> destructor_cb);
Init::Manager& init_manager, std::function<void()> destructor_cb,
Event::Dispatcher& dispatcher, Api::Api& api);

SecretData secretData();

Expand All @@ -65,10 +66,13 @@ class SdsApi : public Envoy::Config::SubscriptionBase<
return MessageUtil::anyConvert<envoy::extensions::transport_sockets::tls::v3::Secret>(resource)
.name();
}
virtual std::vector<std::string> getDataSourceFilenames() PURE;

private:
void validateUpdateSize(int num_resources);
void initialize();
uint64_t getHashForFiles();

Init::TargetImpl init_target_;
Stats::Store& stats_;

Expand All @@ -77,11 +81,15 @@ class SdsApi : public Envoy::Config::SubscriptionBase<
const std::string sds_config_name_;

uint64_t secret_hash_;
uint64_t files_hash_;
Cleanup clean_up_;
ProtobufMessage::ValidationVisitor& validation_visitor_;
Config::SubscriptionFactory& subscription_factory_;
TimeSource& time_source_;
SecretData secret_data_;
Event::Dispatcher& dispatcher_;
Api::Api& api_;
std::unique_ptr<Filesystem::Watcher> watcher_;
};

class TlsCertificateSdsApi;
Expand Down Expand Up @@ -110,16 +118,18 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider
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);
*secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(),
secret_provider_context.api());
}

TlsCertificateSdsApi(const envoy::config::core::v3::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<void()> destructor_cb)
Init::Manager& init_manager, std::function<void()> destructor_cb,
Event::Dispatcher& dispatcher, Api::Api& api)
: SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor,
stats, init_manager, std::move(destructor_cb)) {}
stats, init_manager, std::move(destructor_cb), dispatcher, api) {}

// SecretProvider
const envoy::extensions::transport_sockets::tls::v3::TlsCertificate* secret() const override {
Expand All @@ -144,6 +154,7 @@ class TlsCertificateSdsApi : public SdsApi, public TlsCertificateConfigProvider
secret.tls_certificate());
}
void validateConfig(const envoy::extensions::transport_sockets::tls::v3::Secret&) override {}
std::vector<std::string> getDataSourceFilenames() override;

private:
TlsCertificatePtr tls_certificate_secrets_;
Expand All @@ -168,17 +179,19 @@ class CertificateValidationContextSdsApi : public SdsApi,
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);
*secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(),
secret_provider_context.api());
}
CertificateValidationContextSdsApi(const envoy::config::core::v3::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<void()> destructor_cb)
std::function<void()> destructor_cb,
Event::Dispatcher& dispatcher, Api::Api& api)
: SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor,
stats, init_manager, std::move(destructor_cb)) {}
stats, init_manager, std::move(destructor_cb), dispatcher, api) {}

// SecretProvider
const envoy::extensions::transport_sockets::tls::v3::CertificateValidationContext*
Expand Down Expand Up @@ -210,6 +223,7 @@ class CertificateValidationContextSdsApi : public SdsApi,
validateConfig(const envoy::extensions::transport_sockets::tls::v3::Secret& secret) override {
validation_callback_manager_.runCallbacks(secret.validation_context());
}
std::vector<std::string> getDataSourceFilenames() override;

private:
CertificateValidationContextPtr certificate_validation_context_secrets_;
Expand All @@ -236,7 +250,8 @@ class TlsSessionTicketKeysSdsApi : public SdsApi, public TlsSessionTicketKeysCon
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);
*secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(),
secret_provider_context.api());
}

TlsSessionTicketKeysSdsApi(const envoy::config::core::v3::ConfigSource& sds_config,
Expand All @@ -245,9 +260,10 @@ class TlsSessionTicketKeysSdsApi : public SdsApi, public TlsSessionTicketKeysCon
TimeSource& time_source,
ProtobufMessage::ValidationVisitor& validation_visitor,
Stats::Store& stats, Init::Manager& init_manager,
std::function<void()> destructor_cb)
std::function<void()> destructor_cb, Event::Dispatcher& dispatcher,
Api::Api& api)
: SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor,
stats, init_manager, std::move(destructor_cb)) {}
stats, init_manager, std::move(destructor_cb), dispatcher, api) {}

// SecretProvider
const envoy::extensions::transport_sockets::tls::v3::TlsSessionTicketKeys*
Expand Down Expand Up @@ -280,6 +296,7 @@ class TlsSessionTicketKeysSdsApi : public SdsApi, public TlsSessionTicketKeysCon
validateConfig(const envoy::extensions::transport_sockets::tls::v3::Secret& secret) override {
validation_callback_manager_.runCallbacks(secret.session_ticket_keys());
}
std::vector<std::string> getDataSourceFilenames() override;

private:
Secret::TlsSessionTicketKeysPtr tls_session_ticket_keys_;
Expand All @@ -304,16 +321,18 @@ class GenericSecretSdsApi : public SdsApi, public GenericSecretConfigProvider {
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);
*secret_provider_context.initManager(), destructor_cb, secret_provider_context.dispatcher(),
secret_provider_context.api());
}

GenericSecretSdsApi(const envoy::config::core::v3::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<void()> destructor_cb)
Init::Manager& init_manager, std::function<void()> destructor_cb,
Event::Dispatcher& dispatcher, Api::Api& api)
: SdsApi(sds_config, sds_config_name, subscription_factory, time_source, validation_visitor,
stats, init_manager, std::move(destructor_cb)) {}
stats, init_manager, std::move(destructor_cb), dispatcher, api) {}

// SecretProvider
const envoy::extensions::transport_sockets::tls::v3::GenericSecret* secret() const override {
Expand All @@ -337,6 +356,7 @@ class GenericSecretSdsApi : public SdsApi, public GenericSecretConfigProvider {
validateConfig(const envoy::extensions::transport_sockets::tls::v3::Secret& secret) override {
validation_callback_manager_.runCallbacks(secret.generic_secret());
}
std::vector<std::string> getDataSourceFilenames() override;

private:
GenericSecretPtr generic_secret;
Expand Down
30 changes: 30 additions & 0 deletions test/common/filesystem/watcher_impl_test.cc
Expand Up @@ -151,5 +151,35 @@ TEST_F(WatcherImplTest, RootDirectoryPath) {
#endif
}

TEST_F(WatcherImplTest, SymlinkAtomicRename) {
Filesystem::WatcherPtr watcher = dispatcher_->createFilesystemWatcher();

TestEnvironment::createPath(TestEnvironment::temporaryPath("envoy_test"));
TestEnvironment::createPath(TestEnvironment::temporaryPath("envoy_test/..timestamp1"));
{ std::ofstream file(TestEnvironment::temporaryPath("envoy_test/..timestamp1/watched_file")); }

TestEnvironment::createSymlink(TestEnvironment::temporaryPath("envoy_test/..timestamp1"),
TestEnvironment::temporaryPath("envoy_test/..data"));
TestEnvironment::createSymlink(TestEnvironment::temporaryPath("envoy_test/..data/watched_file"),
TestEnvironment::temporaryPath("envoy_test/watched_file"));

WatchCallback callback;
EXPECT_CALL(callback, called(Watcher::Events::MovedTo));
watcher->addWatch(TestEnvironment::temporaryPath("envoy_test/"), Watcher::Events::MovedTo,
[&](uint32_t events) -> void {
callback.called(events);
dispatcher_->exit();
});

TestEnvironment::createPath(TestEnvironment::temporaryPath("envoy_test/..timestamp2"));
{ std::ofstream file(TestEnvironment::temporaryPath("envoy_test/..timestamp2/watched_file")); }
TestEnvironment::createSymlink(TestEnvironment::temporaryPath("envoy_test/..timestamp2"),
TestEnvironment::temporaryPath("envoy_test/..tmp"));
TestEnvironment::renameFile(TestEnvironment::temporaryPath("envoy_test/..tmp"),
TestEnvironment::temporaryPath("envoy_test/..data"));

dispatcher_->run(Event::Dispatcher::RunType::Block);
}

} // namespace Filesystem
} // namespace Envoy