refactor PemKeyCertPair

CMakeLists.txt

@@ -2117,6 +2117,7 @@ add_library(grpc_test_util
   test/core/util/subprocess_windows.cc
   test/core/util/test_config.cc
   test/core/util/test_tcp_server.cc
+  test/core/util/tls_utils.cc
   test/core/util/tracer_util.cc
   test/core/util/trickle_endpoint.cc
 )
@@ -11671,7 +11672,6 @@ if(gRPC_BUILD_TESTS)
 
 add_executable(grpc_tls_certificate_distributor_test
   test/core/security/grpc_tls_certificate_distributor_test.cc
-  test/core/security/tls_utils.cc
   third_party/googletest/googletest/src/gtest-all.cc
   third_party/googletest/googlemock/src/gmock-all.cc
 )
@@ -11710,7 +11710,6 @@ if(gRPC_BUILD_TESTS)
 
 add_executable(grpc_tls_certificate_provider_test
   test/core/security/grpc_tls_certificate_provider_test.cc
-  test/core/security/tls_utils.cc
   third_party/googletest/googletest/src/gtest-all.cc
   third_party/googletest/googlemock/src/gmock-all.cc
 )
@@ -11749,7 +11748,6 @@ if(gRPC_BUILD_TESTS)
 
 add_executable(grpc_tls_credentials_options_test
   test/core/security/grpc_tls_credentials_options_test.cc
-  test/core/security/tls_utils.cc
   third_party/googletest/googletest/src/gtest-all.cc
   third_party/googletest/googlemock/src/gmock-all.cc
 )

build_autogenerated.yaml

@@ -1441,6 +1441,7 @@ libs:
   - test/core/util/subprocess.h
   - test/core/util/test_config.h
   - test/core/util/test_tcp_server.h
+  - test/core/util/tls_utils.h
   - test/core/util/tracer_util.h
   - test/core/util/trickle_endpoint.h
   src:
@@ -1464,6 +1465,7 @@ libs:
   - test/core/util/subprocess_windows.cc
   - test/core/util/test_config.cc
   - test/core/util/test_tcp_server.cc
+  - test/core/util/tls_utils.cc
   - test/core/util/tracer_util.cc
   - test/core/util/trickle_endpoint.cc
   deps:
@@ -6190,11 +6192,9 @@ targets:
   gtest: true
   build: test
   language: c++
-  headers:
-  - test/core/security/tls_utils.h
+  headers: []
   src:
   - test/core/security/grpc_tls_certificate_distributor_test.cc
-  - test/core/security/tls_utils.cc
   deps:
   - grpc_test_util
   - grpc
@@ -6205,11 +6205,9 @@ targets:
   gtest: true
   build: test
   language: c++
-  headers:
-  - test/core/security/tls_utils.h
+  headers: []
   src:
   - test/core/security/grpc_tls_certificate_provider_test.cc
-  - test/core/security/tls_utils.cc
   deps:
   - grpc_test_util
   - grpc
@@ -6220,11 +6218,9 @@ targets:
   gtest: true
   build: test
   language: c++
-  headers:
-  - test/core/security/tls_utils.h
+  headers: []
   src:
   - test/core/security/grpc_tls_credentials_options_test.cc
-  - test/core/security/tls_utils.cc
   deps:
   - grpc_test_util
   - grpc

gRPC-Core.podspec

@@ -2058,6 +2058,8 @@ Pod::Spec.new do |s|
                       'test/core/util/test_config.h',
                       'test/core/util/test_tcp_server.cc',
                       'test/core/util/test_tcp_server.h',
+                      'test/core/util/tls_utils.cc',
+                      'test/core/util/tls_utils.h',
                       'test/core/util/tracer_util.cc',
                       'test/core/util/tracer_util.h',
                       'test/core/util/trickle_endpoint.cc',

grpc.gyp

@@ -1023,6 +1023,7 @@
         'test/core/util/subprocess_windows.cc',
         'test/core/util/test_config.cc',
         'test/core/util/test_tcp_server.cc',
+        'test/core/util/tls_utils.cc',
         'test/core/util/tracer_util.cc',
         'test/core/util/trickle_endpoint.cc',
       ],

src/core/lib/security/credentials/tls/grpc_tls_certificate_distributor.cc

@@ -337,12 +337,7 @@ void grpc_tls_identity_pairs_add_pair(grpc_tls_identity_pairs* pairs,
   GPR_ASSERT(pairs != nullptr);
   GPR_ASSERT(private_key != nullptr);
   GPR_ASSERT(cert_chain != nullptr);
-  grpc_ssl_pem_key_cert_pair* ssl_pair =
-      static_cast<grpc_ssl_pem_key_cert_pair*>(
-          gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-  ssl_pair->private_key = gpr_strdup(private_key);
-  ssl_pair->cert_chain = gpr_strdup(cert_chain);
-  pairs->pem_key_cert_pairs.emplace_back(ssl_pair);
+  pairs->pem_key_cert_pairs.emplace_back(private_key, cert_chain);
 }
 
 void grpc_tls_identity_pairs_destroy(grpc_tls_identity_pairs* pairs) {

src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc

@@ -332,13 +332,8 @@ FileWatcherCertificateProvider::ReadIdentityKeyCertPairFromFiles(
     }
     std::string private_key(StringViewFromSlice(key_slice.slice));
     std::string cert_chain(StringViewFromSlice(cert_slice.slice));
-    grpc_ssl_pem_key_cert_pair* ssl_pair =
-        static_cast<grpc_ssl_pem_key_cert_pair*>(
-            gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-    ssl_pair->private_key = gpr_strdup(private_key.c_str());
-    ssl_pair->cert_chain = gpr_strdup(cert_chain.c_str());
     PemKeyCertPairList identity_pairs;
-    identity_pairs.emplace_back(ssl_pair);
+    identity_pairs.emplace_back(private_key, cert_chain);
     // Checking the last modification of identity files before reading.
     time_t identity_key_ts_after =
         GetModificationTime(private_key_path.c_str());

src/core/lib/security/security_connector/ssl_utils.h

@@ -145,13 +145,8 @@ class DefaultSslRootStore {
 
 class PemKeyCertPair {
  public:
-  // Construct from the C struct.  We steal its members and then immediately
-  // free it.
-  explicit PemKeyCertPair(grpc_ssl_pem_key_cert_pair* pair)
-      : private_key_(const_cast<char*>(pair->private_key)),
-        cert_chain_(const_cast<char*>(pair->cert_chain)) {
-    gpr_free(pair);
-  }
+  PemKeyCertPair(absl::string_view private_key, absl::string_view cert_chain)
+      : private_key_(private_key), cert_chain_(cert_chain) {}
 
   // Movable.
   PemKeyCertPair(PemKeyCertPair&& other) noexcept {
@@ -166,30 +161,28 @@ class PemKeyCertPair {
 
   // Copyable.
   PemKeyCertPair(const PemKeyCertPair& other)
-      : private_key_(gpr_strdup(other.private_key())),
-        cert_chain_(gpr_strdup(other.cert_chain())) {}
+      : private_key_(other.private_key()), cert_chain_(other.cert_chain()) {}
   PemKeyCertPair& operator=(const PemKeyCertPair& other) {
-    private_key_ = grpc_core::UniquePtr<char>(gpr_strdup(other.private_key()));
-    cert_chain_ = grpc_core::UniquePtr<char>(gpr_strdup(other.cert_chain()));
+    private_key_ = other.private_key();
+    cert_chain_ = other.cert_chain();
     return *this;
   }
 
   bool operator==(const PemKeyCertPair& other) const {
-    return std::strcmp(this->private_key(), other.private_key()) == 0 &&
-           std::strcmp(this->cert_chain(), other.cert_chain()) == 0;
+    return this->private_key() == other.private_key() &&
+           this->cert_chain() == other.cert_chain();
   }
 
-  char* private_key() const { return private_key_.get(); }
-  char* cert_chain() const { return cert_chain_.get(); }
+  const std::string& private_key() const { return private_key_; }
+  const std::string& cert_chain() const { return cert_chain_; }
 
  private:
-  grpc_core::UniquePtr<char> private_key_;
-  grpc_core::UniquePtr<char> cert_chain_;
+  std::string private_key_;
+  std::string cert_chain_;
 };
 
 typedef absl::InlinedVector<grpc_core::PemKeyCertPair, 1> PemKeyCertPairList;
 
 }  // namespace grpc_core
 
-#endif /* GRPC_CORE_LIB_SECURITY_SECURITY_CONNECTOR_SSL_UTILS_H \
-        */
+#endif  // GRPC_CORE_LIB_SECURITY_SECURITY_CONNECTOR_SSL_UTILS_H

src/core/lib/security/security_connector/tls/tls_security_connector.cc

@@ -55,10 +55,12 @@ tsi_ssl_pem_key_cert_pair* ConvertToTsiPemKeyCertPair(
         gpr_zalloc(num_key_cert_pairs * sizeof(tsi_ssl_pem_key_cert_pair)));
   }
   for (size_t i = 0; i < num_key_cert_pairs; i++) {
-    GPR_ASSERT(cert_pair_list[i].private_key() != nullptr);
-    GPR_ASSERT(cert_pair_list[i].cert_chain() != nullptr);
-    tsi_pairs[i].cert_chain = gpr_strdup(cert_pair_list[i].cert_chain());
-    tsi_pairs[i].private_key = gpr_strdup(cert_pair_list[i].private_key());
+    GPR_ASSERT(!cert_pair_list[i].private_key().empty());
+    GPR_ASSERT(!cert_pair_list[i].cert_chain().empty());
+    tsi_pairs[i].cert_chain =
+        gpr_strdup(cert_pair_list[i].cert_chain().c_str());
+    tsi_pairs[i].private_key =
+        gpr_strdup(cert_pair_list[i].private_key().c_str());
   }
   return tsi_pairs;
 }

test/core/security/BUILD

@@ -287,15 +287,6 @@ grpc_cc_test(
     ],
 )
 
-grpc_cc_library(
-    name = "tls_utils",
-    srcs = ["tls_utils.cc"],
-    hdrs = ["tls_utils.h"],
-    language = "C++",
-    visibility = ["//test/cpp:__subpackages__"],
-    deps = ["//:grpc"],
-)
-
 grpc_cc_test(
     name = "tls_security_connector_test",
     srcs = ["tls_security_connector_test.cc"],
@@ -337,7 +328,6 @@ grpc_cc_test(
     external_deps = ["gtest"],
     language = "C++",
     deps = [
-        ":tls_utils",
         "//:gpr",
         "//:grpc",
         "//:grpc_secure",
@@ -351,7 +341,6 @@ grpc_cc_test(
     external_deps = ["gtest"],
     language = "C++",
     deps = [
-        ":tls_utils",
         "//:gpr",
         "//:grpc",
         "//:grpc_secure",
@@ -374,7 +363,6 @@ grpc_cc_test(
     external_deps = ["gtest"],
     language = "C++",
     deps = [
-        ":tls_utils",
         "//:gpr",
         "//:grpc",
         "//:grpc_secure",

test/core/security/grpc_tls_certificate_distributor_test.cc

@@ -28,8 +28,8 @@
 #include <thread>
 
 #include "src/core/lib/slice/slice_internal.h"
-#include "test/core/security/tls_utils.h"
 #include "test/core/util/test_config.h"
+#include "test/core/util/tls_utils.h"
 
 namespace grpc_core {
 

test/core/security/grpc_tls_certificate_provider_test.cc

@@ -29,8 +29,8 @@
 #include "src/core/lib/gpr/tmpfile.h"
 #include "src/core/lib/iomgr/load_file.h"
 #include "src/core/lib/slice/slice_internal.h"
-#include "test/core/security/tls_utils.h"
 #include "test/core/util/test_config.h"
+#include "test/core/util/tls_utils.h"
 
 #define CA_CERT_PATH "src/core/tsi/test_creds/ca.pem"
 #define SERVER_CERT_PATH "src/core/tsi/test_creds/server1.pem"

test/core/security/grpc_tls_credentials_options_test.cc

@@ -28,8 +28,8 @@
 #include "src/core/lib/iomgr/load_file.h"
 #include "src/core/lib/security/credentials/tls/tls_credentials.h"
 #include "src/core/lib/security/security_connector/tls/tls_security_connector.h"
-#include "test/core/security/tls_utils.h"
 #include "test/core/util/test_config.h"
+#include "test/core/util/tls_utils.h"
 
 #define CA_CERT_PATH "src/core/tsi/test_creds/ca.pem"
 #define SERVER_CERT_PATH "src/core/tsi/test_creds/server1.pem"

test/core/security/tls_security_connector_test.cc

@@ -75,18 +75,8 @@ class TlsSecurityConnectorTest : public ::testing::Test {
         std::string(grpc_core::StringViewFromSlice(cert_slice_1));
     std::string identity_cert_0 =
         std::string(grpc_core::StringViewFromSlice(cert_slice_0));
-    grpc_ssl_pem_key_cert_pair* ssl_pair_1 =
-        static_cast<grpc_ssl_pem_key_cert_pair*>(
-            gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-    ssl_pair_1->private_key = gpr_strdup(identity_key_1.c_str());
-    ssl_pair_1->cert_chain = gpr_strdup(identity_cert_1.c_str());
-    identity_pairs_1_.emplace_back(ssl_pair_1);
-    grpc_ssl_pem_key_cert_pair* ssl_pair_0 =
-        static_cast<grpc_ssl_pem_key_cert_pair*>(
-            gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-    ssl_pair_0->private_key = gpr_strdup(identity_key_0.c_str());
-    ssl_pair_0->cert_chain = gpr_strdup(identity_cert_0.c_str());
-    identity_pairs_0_.emplace_back(ssl_pair_0);
+    identity_pairs_1_.emplace_back(identity_key_1, identity_cert_1);
+    identity_pairs_0_.emplace_back(identity_key_0, identity_cert_0);
     grpc_slice_unref(ca_slice_1);
     grpc_slice_unref(ca_slice_0);
     grpc_slice_unref(cert_slice_1);

test/core/util/BUILD

@@ -87,8 +87,8 @@ grpc_cc_library(
 
 grpc_cc_library(
     name = "grpc_test_util",
-    srcs = [],
-    hdrs = [],
+    srcs = ["tls_utils.cc"],
+    hdrs = ["tls_utils.h"],
     language = "C++",
     deps = [
         ":grpc_test_util_base",

test/core/util/tls_utils.cc

@@ -14,7 +14,7 @@
 // limitations under the License.
 //
 
-#include "test/core/security/tls_utils.h"
+#include "test/core/util/tls_utils.h"
 
 #include "src/core/lib/gpr/tmpfile.h"
 #include "src/core/lib/iomgr/load_file.h"
@@ -55,19 +55,12 @@ std::string TmpFile::CreateTmpFileAndWriteData(
   return name_to_return;
 }
 
-PemKeyCertPairList MakeCertKeyPairs(const char* private_key,
-                                    const char* certs) {
-  if (strcmp(private_key, "") == 0 && strcmp(certs, "") == 0) {
+PemKeyCertPairList MakeCertKeyPairs(absl::string_view private_key,
+                                    absl::string_view certs) {
+  if (private_key.empty() && certs.empty()) {
     return {};
   }
-  grpc_ssl_pem_key_cert_pair* ssl_pair =
-      static_cast<grpc_ssl_pem_key_cert_pair*>(
-          gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-  ssl_pair->private_key = gpr_strdup(private_key);
-  ssl_pair->cert_chain = gpr_strdup(certs);
-  PemKeyCertPairList pem_key_cert_pairs;
-  pem_key_cert_pairs.emplace_back(ssl_pair);
-  return pem_key_cert_pairs;
+  return PemKeyCertPairList{PemKeyCertPair(private_key, certs)};
 }
 
 std::string GetFileContents(const char* path) {

test/core/util/tls_utils.h

@@ -38,7 +38,8 @@ class TmpFile {
   std::string name_;
 };
 
-PemKeyCertPairList MakeCertKeyPairs(const char* private_key, const char* certs);
+PemKeyCertPairList MakeCertKeyPairs(absl::string_view private_key,
+                                    absl::string_view certs);
 
 std::string GetFileContents(const char* path);
 

test/core/xds/BUILD

@@ -76,6 +76,7 @@ grpc_cc_test(
     deps = [
         "//:gpr",
         "//:grpc",
+        "//:grpc_secure",
         "//test/core/util:grpc_test_util",
     ],
 )

test/core/xds/xds_certificate_provider_test.cc

@@ -21,6 +21,7 @@
 
 #include "src/core/ext/xds/xds_certificate_provider.h"
 #include "test/core/util/test_config.h"
+#include "test/core/util/tls_utils.h"
 
 namespace grpc_core {
 namespace testing {
@@ -35,25 +36,12 @@ constexpr const char* kIdentityCert2 = "identity_cert_2_contents";
 constexpr const char* kRootErrorMessage = "root_error_message";
 constexpr const char* kIdentityErrorMessage = "identity_error_message";
 
-PemKeyCertPairList MakeKeyCertPairs(const char* private_key,
-                                    const char* certs) {
-  if (strcmp(private_key, "") == 0 && strcmp(certs, "") == 0) {
-    return {};
-  }
-  grpc_ssl_pem_key_cert_pair* ssl_pair =
-      static_cast<grpc_ssl_pem_key_cert_pair*>(
-          gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-  ssl_pair->private_key = gpr_strdup(private_key);
-  ssl_pair->cert_chain = gpr_strdup(certs);
-  return PemKeyCertPairList{PemKeyCertPair(ssl_pair)};
-}
-
 PemKeyCertPairList MakeKeyCertPairsType1() {
-  return MakeKeyCertPairs(kIdentityCert1PrivateKey, kIdentityCert1);
+  return MakeCertKeyPairs(kIdentityCert1PrivateKey, kIdentityCert1);
 }
 
 PemKeyCertPairList MakeKeyCertPairsType2() {
-  return MakeKeyCertPairs(kIdentityCert2PrivateKey, kIdentityCert2);
+  return MakeCertKeyPairs(kIdentityCert2PrivateKey, kIdentityCert2);
 }
 
 class TestCertificatesWatcher

test/cpp/end2end/xds_end2end_test.cc

@@ -1353,12 +1353,8 @@ std::string ReadFile(const char* file_path) {
 
 grpc_core::PemKeyCertPairList ReadTlsIdentityPair(const char* key_path,
                                                   const char* cert_path) {
-  grpc_ssl_pem_key_cert_pair* ssl_pair =
-      static_cast<grpc_ssl_pem_key_cert_pair*>(
-          gpr_malloc(sizeof(grpc_ssl_pem_key_cert_pair)));
-  ssl_pair->private_key = gpr_strdup(ReadFile(key_path).c_str());
-  ssl_pair->cert_chain = gpr_strdup(ReadFile(cert_path).c_str());
-  return grpc_core::PemKeyCertPairList{grpc_core::PemKeyCertPair(ssl_pair)};
+  return grpc_core::PemKeyCertPairList{
+      grpc_core::PemKeyCertPair(ReadFile(key_path), ReadFile(cert_path))};
 }
 
 // Based on StaticDataCertificateProvider, but provides alternate certificates