[tls] AddValidateRootCertificates and ValidateIdentityKeyCertPairs AP…

…Is to TLS cert provider.
matthewstevenson88 · Nov 4, 2024 · 045b123 · 045b123
1 parent 7f664c6
commit 045b123
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 6 deletions.
diff --git a/include/grpcpp/security/tls_certificate_provider.h b/include/grpcpp/security/tls_certificate_provider.h
@@ -71,11 +71,15 @@ class GRPCXX_DLL StaticDataCertificateProvider
 
   grpc_tls_certificate_provider* c_provider() override { return c_provider_; }
 
-  // Returns an OK status if the following conditions hold:
+  // Returns an OK status if the both of the following conditions hold:
   // - the root certificates consist of one or more valid PEM blocks, and
   // - every identity key-cert pair has a certificate chain that consists of
   //   valid PEM blocks and has a private key is a valid PEM block.
+  // Separate validation APIs are provided to validate the above conditions
+  // individually.
   absl::Status ValidateCredentials() const;
+  absl::Status ValidateRootCertificates() const;
+  absl::Status ValidateIdentityKeyCertPairs() const;
 
  private:
   grpc_tls_certificate_provider* c_provider_ = nullptr;
@@ -127,13 +131,17 @@ class GRPCXX_DLL FileWatcherCertificateProvider final
 
   grpc_tls_certificate_provider* c_provider() override { return c_provider_; }
 
-  // Returns an OK status if the following conditions hold:
+  // Returns an OK status if the both of the following conditions hold:
   // - the currently-loaded root certificates, if any, consist of one or more
   //   valid PEM blocks, and
   // - every currently-loaded identity key-cert pair, if any, has a certificate
   //   chain that consists of valid PEM blocks and has a private key is a valid
   //   PEM block.
+  // Separate validation APIs are provided to validate the above conditions
+  // individually.
   absl::Status ValidateCredentials() const;
+  absl::Status ValidateRootCertificates() const;
+  absl::Status ValidateIdentityKeyCertPairs() const;
 
  private:
   grpc_tls_certificate_provider* c_provider_ = nullptr;

diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc
@@ -45,7 +45,7 @@
 namespace grpc_core {
 namespace {
 
-absl::Status ValidateRootCertificates(absl::string_view root_certificates) {
+absl::Status ValidateRootCertificateSet(absl::string_view root_certificates) {
   if (root_certificates.empty()) return absl::OkStatus();
   absl::StatusOr<std::vector<X509*>> parsed_roots =
       ParsePemCertificateChain(root_certificates);
@@ -142,7 +142,8 @@ UniqueTypeName StaticDataCertificateProvider::type() const {
 }
 
 absl::Status StaticDataCertificateProvider::ValidateCredentials() const {
-  absl::Status status = ValidateRootCertificates(root_certificate_);
+  MutexLock lock(&mu_);
+  absl::Status status = ValidateRootCertificateSet(root_certificate_);
   if (!status.ok()) {
     return status;
   }
@@ -156,6 +157,23 @@ absl::Status StaticDataCertificateProvider::ValidateCredentials() const {
   return absl::OkStatus();
 }
 
+absl::Status StaticDataCertificateProvider::ValidatePemKeyCertPairList() const {
+  MutexLock lock(&mu_);
+  for (const PemKeyCertPair& pair : pem_key_cert_pairs_) {
+    absl::Status status =
+        ValidatePemKeyCertPair(pair.cert_chain(), pair.private_key());
+    if (!status.ok()) {
+      return status;
+    }
+  }
+  return absl::OkStatus();
+}
+
+absl::Status StaticDataCertificateProvider::ValidateRootCertificates() const {
+  MutexLock lock(&mu_);
+  return ValidateRootCertificateSet(root_certificate_);
+}
+
 namespace {
 
 gpr_timespec TimeoutSecondsToDeadline(int64_t seconds) {
@@ -262,7 +280,7 @@ UniqueTypeName FileWatcherCertificateProvider::type() const {
 
 absl::Status FileWatcherCertificateProvider::ValidateCredentials() const {
   MutexLock lock(&mu_);
-  absl::Status status = ValidateRootCertificates(root_certificate_);
+  absl::Status status = ValidateRootCertificateSet(root_certificate_);
   if (!status.ok()) {
     return status;
   }
@@ -276,6 +294,24 @@ absl::Status FileWatcherCertificateProvider::ValidateCredentials() const {
   return absl::OkStatus();
 }
 
+absl::Status FileWatcherCertificateProvider::ValidatePemKeyCertPairList()
+    const {
+  MutexLock lock(&mu_);
+  for (const PemKeyCertPair& pair : pem_key_cert_pairs_) {
+    absl::Status status =
+        ValidatePemKeyCertPair(pair.cert_chain(), pair.private_key());
+    if (!status.ok()) {
+      return status;
+    }
+  }
+  return absl::OkStatus();
+}
+
+absl::Status FileWatcherCertificateProvider::ValidateRootCertificates() const {
+  MutexLock lock(&mu_);
+  return ValidateRootCertificateSet(root_certificate_);
+}
+
 void FileWatcherCertificateProvider::ForceUpdate() {
   absl::optional<std::string> root_certificate;
   absl::optional<PemKeyCertPairList> pem_key_cert_pairs;

diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h
@@ -107,6 +107,8 @@ class StaticDataCertificateProvider final
   UniqueTypeName type() const override;
 
   absl::Status ValidateCredentials() const;
+  absl::Status ValidatePemKeyCertPairList() const;
+  absl::Status ValidateRootCertificates() const;
 
  private:
   struct WatcherInfo {
@@ -124,7 +126,7 @@ class StaticDataCertificateProvider final
   std::string root_certificate_;
   PemKeyCertPairList pem_key_cert_pairs_;
   // Guards members below.
-  Mutex mu_;
+  mutable Mutex mu_;
   // Stores each cert_name we get from the distributor callback and its watcher
   // information.
   std::map<std::string, WatcherInfo> watcher_info_;
@@ -148,6 +150,8 @@ class FileWatcherCertificateProvider final
   UniqueTypeName type() const override;
 
   absl::Status ValidateCredentials() const;
+  absl::Status ValidatePemKeyCertPairList() const;
+  absl::Status ValidateRootCertificates() const;
 
   int64_t TestOnlyGetRefreshIntervalSecond() const;
 

diff --git a/src/cpp/common/tls_certificate_provider.cc b/src/cpp/common/tls_certificate_provider.cc
@@ -52,6 +52,21 @@ absl::Status StaticDataCertificateProvider::ValidateCredentials() const {
   return provider->ValidateCredentials();
 }
 
+absl::Status StaticDataCertificateProvider::ValidateIdentityKeyCertPairs()
+    const {
+  auto* provider =
+      grpc_core::DownCast<grpc_core::StaticDataCertificateProvider*>(
+          c_provider_);
+  return provider->ValidatePemKeyCertPairList();
+}
+
+absl::Status StaticDataCertificateProvider::ValidateRootCertificates() const {
+  auto* provider =
+      grpc_core::DownCast<grpc_core::StaticDataCertificateProvider*>(
+          c_provider_);
+  return provider->ValidateRootCertificates();
+}
+
 FileWatcherCertificateProvider::FileWatcherCertificateProvider(
     const std::string& private_key_path,
     const std::string& identity_certificate_path,
@@ -73,5 +88,20 @@ absl::Status FileWatcherCertificateProvider::ValidateCredentials() const {
   return provider->ValidateCredentials();
 }
 
+absl::Status FileWatcherCertificateProvider::ValidateRootCertificates() const {
+  auto* provider =
+      grpc_core::DownCast<grpc_core::FileWatcherCertificateProvider*>(
+          c_provider_);
+  return provider->ValidateRootCertificates();
+}
+
+absl::Status FileWatcherCertificateProvider::ValidateIdentityKeyCertPairs()
+    const {
+  auto* provider =
+      grpc_core::DownCast<grpc_core::FileWatcherCertificateProvider*>(
+          c_provider_);
+  return provider->ValidatePemKeyCertPairList();
+}
+
 }  // namespace experimental
 }  // namespace grpc
diff --git a/test/core/security/grpc_tls_certificate_provider_test.cc b/test/core/security/grpc_tls_certificate_provider_test.cc
@@ -269,6 +269,38 @@ TEST_F(GrpcTlsCertificateProviderTest,
             absl::NotFoundError("No private key found."));
 }
 
+TEST_F(GrpcTlsCertificateProviderTest,
+       StaticDataCertificateProviderWithRootCertificatesValidationSuccess) {
+  StaticDataCertificateProvider provider(
+      root_cert_, MakeCertKeyPairs(private_key_.c_str(), cert_chain_.c_str()));
+  EXPECT_EQ(provider.ValidateRootCertificates(), absl::OkStatus());
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       StaticDataCertificateProviderWithRootCertificatesValidationFailure) {
+  StaticDataCertificateProvider provider(
+      malformed_cert_,
+      MakeCertKeyPairs(private_key_.c_str(), cert_chain_.c_str()));
+  EXPECT_EQ(provider.ValidateRootCertificates(),
+            absl::FailedPreconditionError("Invalid PEM."));
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       StaticDataCertificateProviderWithPemKeyCertPairListValidationSuccess) {
+  StaticDataCertificateProvider provider(
+      root_cert_, MakeCertKeyPairs(private_key_.c_str(), cert_chain_.c_str()));
+  EXPECT_EQ(provider.ValidatePemKeyCertPairList(), absl::OkStatus());
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       StaticDataCertificateProviderWithPemKeyCertPairListValidationFailure) {
+  StaticDataCertificateProvider provider(
+      root_cert_,
+      MakeCertKeyPairs(private_key_.c_str(), malformed_cert_.c_str()));
+  EXPECT_EQ(provider.ValidatePemKeyCertPairList(),
+            absl::FailedPreconditionError("Invalid PEM."));
+}
+
 TEST_F(GrpcTlsCertificateProviderTest,
        FileWatcherCertificateProviderWithGoodPaths) {
   FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
@@ -328,6 +360,36 @@ TEST_F(GrpcTlsCertificateProviderTest,
             absl::NotFoundError("No private key found."));
 }
 
+TEST_F(GrpcTlsCertificateProviderTest,
+       FileWatcherCertificateProviderWithRootCertificateValidationSuccess) {
+  FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
+                                          CA_CERT_PATH, 1);
+  EXPECT_EQ(provider.ValidateRootCertificates(), absl::OkStatus());
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       FileWatcherCertificateProviderWithRootCertificateValidationFailure) {
+  FileWatcherCertificateProvider provider(SERVER_KEY_PATH_2, SERVER_CERT_PATH_2,
+                                          MALFORMED_CERT_PATH, 1);
+  EXPECT_EQ(provider.ValidateRootCertificates(),
+            absl::FailedPreconditionError("Invalid PEM."));
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       FileWatcherCertificateProviderWithPemKeyCertPairListValidationSuccess) {
+  FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
+                                          CA_CERT_PATH, 1);
+  EXPECT_EQ(provider.ValidatePemKeyCertPairList(), absl::OkStatus());
+}
+
+TEST_F(GrpcTlsCertificateProviderTest,
+       FileWatcherCertificateProviderWithPemKeyCertPairListValidationFailure) {
+  FileWatcherCertificateProvider provider(
+      MALFORMED_KEY_PATH, SERVER_CERT_PATH_2, CA_CERT_PATH_2, 1);
+  EXPECT_EQ(provider.ValidatePemKeyCertPairList(),
+            absl::NotFoundError("No private key found."));
+}
+
 TEST_F(GrpcTlsCertificateProviderTest,
        FileWatcherCertificateProviderWithBadPaths) {
   FileWatcherCertificateProvider provider(INVALID_PATH, INVALID_PATH,

diff --git a/test/cpp/server/credentials_test.cc b/test/cpp/server/credentials_test.cc
@@ -125,6 +125,8 @@ TEST(CredentialsTest,
   key_cert_pair.certificate_chain = GetFileContents(SERVER_CERT_PATH);
   StaticDataCertificateProvider provider(root_certificates, {key_cert_pair});
   EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
+  EXPECT_EQ(provider.ValidateRootCertificates(), absl::OkStatus());
+  EXPECT_EQ(provider.ValidateIdentityKeyCertPairs(), absl::OkStatus());
 }
 
 TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedRoot) {
@@ -142,6 +144,8 @@ TEST(CredentialsTest,
   FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
                                           CA_CERT_PATH, 1);
   EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
+  EXPECT_EQ(provider.ValidateRootCertificates(), absl::OkStatus());
+  EXPECT_EQ(provider.ValidateIdentityKeyCertPairs(), absl::OkStatus());
 }
 
 TEST(CredentialsTest, FileWatcherCertificateProviderWithMalformedRoot) {