Minimum time file watcher (#32365)

Enforce a minimum value for the `refresh_interval_sec_` for the
`FileWatcherCertificateProvider`. There have been issues found when this
is set to 0, and the security team discussed and agreed that 0 should
not be a valid value for this use-case.

I made the `refresh_interval_sec_` public to make it easy to test - I
didn't immediately see an easy way around this. I found `FRIEND_TEST`
exists for accessing private members, but I didn't see that used
anywhere in grpc. If there is a better solution to this, please let me
know.
pull/32370/head
Gregory Cooke 2 years ago committed by GitHub
parent 420180c6d7
commit a944a06755
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 13
      src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc
  2. 2
      src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h
  3. 7
      test/core/security/grpc_tls_certificate_provider_test.cc

@ -117,6 +117,8 @@ gpr_timespec TimeoutSecondsToDeadline(int64_t seconds) {
} // namespace
static constexpr int64_t kMinimumFileWatcherRefreshIntervalSeconds = 1;
FileWatcherCertificateProvider::FileWatcherCertificateProvider(
std::string private_key_path, std::string identity_certificate_path,
std::string root_cert_path, int64_t refresh_interval_sec)
@ -125,6 +127,12 @@ FileWatcherCertificateProvider::FileWatcherCertificateProvider(
root_cert_path_(std::move(root_cert_path)),
refresh_interval_sec_(refresh_interval_sec),
distributor_(MakeRefCounted<grpc_tls_certificate_distributor>()) {
if (refresh_interval_sec_ < kMinimumFileWatcherRefreshIntervalSeconds) {
gpr_log(GPR_INFO,
"FileWatcherCertificateProvider refresh_interval_sec_ set to value "
"less than minimum. Overriding configured value to minimum.");
refresh_interval_sec_ = kMinimumFileWatcherRefreshIntervalSeconds;
}
// Private key and identity cert files must be both set or both unset.
GPR_ASSERT(private_key_path_.empty() == identity_certificate_path_.empty());
// Must be watching either root or identity certs.
@ -381,6 +389,11 @@ FileWatcherCertificateProvider::ReadIdentityKeyCertPairFromFiles(
return absl::nullopt;
}
int64_t FileWatcherCertificateProvider::TestOnlyGetRefreshIntervalSecond()
const {
return refresh_interval_sec_;
}
absl::StatusOr<bool> PrivateKeyAndCertificateMatch(
absl::string_view private_key, absl::string_view cert_chain) {
if (private_key.empty()) {

@ -151,6 +151,8 @@ class FileWatcherCertificateProvider final
UniqueTypeName type() const override;
int64_t TestOnlyGetRefreshIntervalSecond() const;
private:
struct WatcherInfo {
bool root_being_watched = false;

@ -488,6 +488,13 @@ TEST_F(GrpcTlsCertificateProviderTest,
CancelWatch(watcher_state_1);
}
TEST_F(GrpcTlsCertificateProviderTest,
FileWatcherCertificateProviderTooShortRefreshIntervalIsOverwritten) {
FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
CA_CERT_PATH, 0);
ASSERT_THAT(provider.TestOnlyGetRefreshIntervalSecond(), 1);
}
TEST_F(GrpcTlsCertificateProviderTest, FailedKeyCertMatchOnEmptyPrivateKey) {
absl::StatusOr<bool> status =
PrivateKeyAndCertificateMatch(/*private_key=*/"", cert_chain_);

Loading…
Cancel
Save