fix memory leaks in ssl credential reload.

CMakeLists.txt

@@ -484,6 +484,7 @@ add_dependencies(buildtests_c h2_sockpair+trace_test)
 add_dependencies(buildtests_c h2_sockpair_1byte_test)
 add_dependencies(buildtests_c h2_spiffe_test)
 add_dependencies(buildtests_c h2_ssl_test)
+add_dependencies(buildtests_c h2_ssl_cred_reload_test)
 add_dependencies(buildtests_c h2_ssl_proxy_test)
 if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
 add_dependencies(buildtests_c h2_uds_test)
@@ -17679,6 +17680,41 @@ target_link_libraries(h2_ssl_test
 endif (gRPC_BUILD_TESTS)
 if (gRPC_BUILD_TESTS)
 
+add_executable(h2_ssl_cred_reload_test
+  test/core/end2end/fixtures/h2_ssl_cred_reload.cc
+)
+
+
+target_include_directories(h2_ssl_cred_reload_test
+  PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
+  PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
+  PRIVATE ${_gRPC_SSL_INCLUDE_DIR}
+  PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR}
+  PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR}
+  PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR}
+  PRIVATE ${_gRPC_CARES_INCLUDE_DIR}
+  PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR}
+  PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
+  PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR}
+)
+
+target_link_libraries(h2_ssl_cred_reload_test
+  ${_gRPC_ALLTARGETS_LIBRARIES}
+  end2end_tests
+  grpc_test_util
+  grpc
+  gpr
+)
+
+  # avoid dependency on libstdc++
+  if (_gRPC_CORE_NOSTDCXX_FLAGS)
+    set_target_properties(h2_ssl_cred_reload_test PROPERTIES LINKER_LANGUAGE C)
+    target_compile_options(h2_ssl_cred_reload_test PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${_gRPC_CORE_NOSTDCXX_FLAGS}>)
+  endif()
+
+endif (gRPC_BUILD_TESTS)
+if (gRPC_BUILD_TESTS)
+
 add_executable(h2_ssl_proxy_test
   test/core/end2end/fixtures/h2_ssl_proxy.cc
 )

Makefile

@@ -1316,6 +1316,7 @@ h2_sockpair+trace_test: $(BINDIR)/$(CONFIG)/h2_sockpair+trace_test
 h2_sockpair_1byte_test: $(BINDIR)/$(CONFIG)/h2_sockpair_1byte_test
 h2_spiffe_test: $(BINDIR)/$(CONFIG)/h2_spiffe_test
 h2_ssl_test: $(BINDIR)/$(CONFIG)/h2_ssl_test
+h2_ssl_cred_reload_test: $(BINDIR)/$(CONFIG)/h2_ssl_cred_reload_test
 h2_ssl_proxy_test: $(BINDIR)/$(CONFIG)/h2_ssl_proxy_test
 h2_uds_test: $(BINDIR)/$(CONFIG)/h2_uds_test
 inproc_test: $(BINDIR)/$(CONFIG)/inproc_test
@@ -1579,6 +1580,7 @@ buildtests_c: privatelibs_c \
   $(BINDIR)/$(CONFIG)/h2_sockpair_1byte_test \
   $(BINDIR)/$(CONFIG)/h2_spiffe_test \
   $(BINDIR)/$(CONFIG)/h2_ssl_test \
+  $(BINDIR)/$(CONFIG)/h2_ssl_cred_reload_test \
   $(BINDIR)/$(CONFIG)/h2_ssl_proxy_test \
   $(BINDIR)/$(CONFIG)/h2_uds_test \
   $(BINDIR)/$(CONFIG)/inproc_test \
@@ -20667,6 +20669,38 @@ endif
 endif
 
 
+H2_SSL_CRED_RELOAD_TEST_SRC = \
+    test/core/end2end/fixtures/h2_ssl_cred_reload.cc \
+
+H2_SSL_CRED_RELOAD_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(H2_SSL_CRED_RELOAD_TEST_SRC))))
+ifeq ($(NO_SECURE),true)
+
+# You can't build secure targets if you don't have OpenSSL.
+
+$(BINDIR)/$(CONFIG)/h2_ssl_cred_reload_test: openssl_dep_error
+
+else
+
+
+
+$(BINDIR)/$(CONFIG)/h2_ssl_cred_reload_test: $(H2_SSL_CRED_RELOAD_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libend2end_tests.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a
+	$(E) "[LD]      Linking $@"
+	$(Q) mkdir -p `dirname $@`
+	$(Q) $(LD) $(LDFLAGS) $(H2_SSL_CRED_RELOAD_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libend2end_tests.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/h2_ssl_cred_reload_test
+
+endif
+
+$(OBJDIR)/$(CONFIG)/test/core/end2end/fixtures/h2_ssl_cred_reload.o:  $(LIBDIR)/$(CONFIG)/libend2end_tests.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a
+
+deps_h2_ssl_cred_reload_test: $(H2_SSL_CRED_RELOAD_TEST_OBJS:.o=.dep)
+
+ifneq ($(NO_SECURE),true)
+ifneq ($(NO_DEPS),true)
+-include $(H2_SSL_CRED_RELOAD_TEST_OBJS:.o=.dep)
+endif
+endif
+
+
 H2_SSL_PROXY_TEST_SRC = \
     test/core/end2end/fixtures/h2_ssl_proxy.cc \
 

src/core/lib/security/security_connector/ssl/ssl_security_connector.cc

@@ -314,7 +314,6 @@ class grpc_ssl_server_security_connector
   bool try_fetch_ssl_server_credentials() {
     grpc_ssl_server_certificate_config* certificate_config = nullptr;
     bool status;
-
     if (!has_cert_config_fetcher()) return false;
 
     grpc_ssl_server_credentials* server_creds =
@@ -374,7 +373,9 @@ class grpc_ssl_server_security_connector
     options.num_alpn_protocols = static_cast<uint16_t>(num_alpn_protocols);
     tsi_result result = tsi_create_ssl_server_handshaker_factory_with_options(
         &options, &new_handshaker_factory);
-    gpr_free((void*)options.pem_key_cert_pairs);
+    grpc_tsi_ssl_pem_key_cert_pairs_destroy(
+        const_cast<tsi_ssl_pem_key_cert_pair*>(options.pem_key_cert_pairs),
+        options.num_key_cert_pairs);
     gpr_free((void*)alpn_protocol_strings);
 
     if (result != TSI_OK) {

test/core/end2end/fixtures/h2_ssl_cred_reload.cc

@@ -0,0 +1,208 @@
+/*
+ *
+ * Copyright 2015 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include "test/core/end2end/end2end_tests.h"
+
+#include <stdio.h>
+#include <string.h>
+
+#include <grpc/support/alloc.h>
+#include <grpc/support/log.h>
+
+#include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/gpr/env.h"
+#include "src/core/lib/gpr/host_port.h"
+#include "src/core/lib/gpr/string.h"
+#include "src/core/lib/gpr/tmpfile.h"
+#include "src/core/lib/security/credentials/credentials.h"
+#include "test/core/end2end/data/ssl_test_data.h"
+#include "test/core/util/port.h"
+#include "test/core/util/test_config.h"
+
+typedef struct fullstack_secure_fixture_data {
+  char* localaddr;
+  bool server_credential_reloaded;
+} fullstack_secure_fixture_data;
+
+static grpc_ssl_certificate_config_reload_status
+ssl_server_certificate_config_callback(
+    void* user_data, grpc_ssl_server_certificate_config** config) {
+  if (config == nullptr) {
+    return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL;
+  }
+  fullstack_secure_fixture_data* ffd =
+      static_cast<fullstack_secure_fixture_data*>(user_data);
+  if (!ffd->server_credential_reloaded) {
+    grpc_ssl_pem_key_cert_pair pem_key_cert_pair = {test_server1_key,
+                                                    test_server1_cert};
+    *config = grpc_ssl_server_certificate_config_create(test_root_cert,
+                                                        &pem_key_cert_pair, 1);
+    ffd->server_credential_reloaded = true;
+    return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW;
+  } else {
+    return GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED;
+  }
+}
+
+static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack(
+    grpc_channel_args* client_args, grpc_channel_args* server_args) {
+  grpc_end2end_test_fixture f;
+  int port = grpc_pick_unused_port_or_die();
+  fullstack_secure_fixture_data* ffd =
+      static_cast<fullstack_secure_fixture_data*>(
+          gpr_malloc(sizeof(fullstack_secure_fixture_data)));
+  memset(&f, 0, sizeof(f));
+  gpr_join_host_port(&ffd->localaddr, "localhost", port);
+
+  f.fixture_data = ffd;
+  f.cq = grpc_completion_queue_create_for_next(nullptr);
+  f.shutdown_cq = grpc_completion_queue_create_for_pluck(nullptr);
+
+  return f;
+}
+
+static void process_auth_failure(void* state, grpc_auth_context* ctx,
+                                 const grpc_metadata* md, size_t md_count,
+                                 grpc_process_auth_metadata_done_cb cb,
+                                 void* user_data) {
+  GPR_ASSERT(state == nullptr);
+  cb(user_data, nullptr, 0, nullptr, 0, GRPC_STATUS_UNAUTHENTICATED, nullptr);
+}
+
+static void chttp2_init_client_secure_fullstack(
+    grpc_end2end_test_fixture* f, grpc_channel_args* client_args,
+    grpc_channel_credentials* creds) {
+  fullstack_secure_fixture_data* ffd =
+      static_cast<fullstack_secure_fixture_data*>(f->fixture_data);
+  f->client =
+      grpc_secure_channel_create(creds, ffd->localaddr, client_args, nullptr);
+  GPR_ASSERT(f->client != nullptr);
+  grpc_channel_credentials_release(creds);
+}
+
+static void chttp2_init_server_secure_fullstack(
+    grpc_end2end_test_fixture* f, grpc_channel_args* server_args,
+    grpc_server_credentials* server_creds) {
+  fullstack_secure_fixture_data* ffd =
+      static_cast<fullstack_secure_fixture_data*>(f->fixture_data);
+  if (f->server) {
+    grpc_server_destroy(f->server);
+  }
+  ffd->server_credential_reloaded = false;
+  f->server = grpc_server_create(server_args, nullptr);
+  grpc_server_register_completion_queue(f->server, f->cq, nullptr);
+  GPR_ASSERT(grpc_server_add_secure_http2_port(f->server, ffd->localaddr,
+                                               server_creds));
+  grpc_server_credentials_release(server_creds);
+  grpc_server_start(f->server);
+}
+
+void chttp2_tear_down_secure_fullstack(grpc_end2end_test_fixture* f) {
+  fullstack_secure_fixture_data* ffd =
+      static_cast<fullstack_secure_fixture_data*>(f->fixture_data);
+  gpr_free(ffd->localaddr);
+  gpr_free(ffd);
+}
+
+static void chttp2_init_client_simple_ssl_secure_fullstack(
+    grpc_end2end_test_fixture* f, grpc_channel_args* client_args) {
+  grpc_channel_credentials* ssl_creds =
+      grpc_ssl_credentials_create(nullptr, nullptr, nullptr, nullptr);
+  grpc_arg ssl_name_override = {
+      GRPC_ARG_STRING,
+      const_cast<char*>(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG),
+      {const_cast<char*>("foo.test.google.fr")}};
+  grpc_channel_args* new_client_args =
+      grpc_channel_args_copy_and_add(client_args, &ssl_name_override, 1);
+  chttp2_init_client_secure_fullstack(f, new_client_args, ssl_creds);
+  grpc_channel_args_destroy(new_client_args);
+}
+
+static int fail_server_auth_check(grpc_channel_args* server_args) {
+  size_t i;
+  if (server_args == nullptr) return 0;
+  for (i = 0; i < server_args->num_args; i++) {
+    if (strcmp(server_args->args[i].key, FAIL_AUTH_CHECK_SERVER_ARG_NAME) ==
+        0) {
+      return 1;
+    }
+  }
+  return 0;
+}
+
+static void chttp2_init_server_simple_ssl_secure_fullstack(
+    grpc_end2end_test_fixture* f, grpc_channel_args* server_args) {
+  grpc_ssl_server_credentials_options* options =
+      grpc_ssl_server_credentials_create_options_using_config_fetcher(
+          GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE,
+          ssl_server_certificate_config_callback, f->fixture_data);
+  grpc_server_credentials* ssl_creds =
+      grpc_ssl_server_credentials_create_with_options(options);
+  if (fail_server_auth_check(server_args)) {
+    grpc_auth_metadata_processor processor = {process_auth_failure, nullptr,
+                                              nullptr};
+    grpc_server_credentials_set_auth_metadata_processor(ssl_creds, processor);
+  }
+  chttp2_init_server_secure_fullstack(f, server_args, ssl_creds);
+}
+
+/* All test configurations */
+
+static grpc_end2end_test_config configs[] = {
+    {"chttp2/simple_ssl_fullstack",
+     FEATURE_MASK_SUPPORTS_DELAYED_CONNECTION |
+         FEATURE_MASK_SUPPORTS_PER_CALL_CREDENTIALS |
+         FEATURE_MASK_SUPPORTS_CLIENT_CHANNEL |
+         FEATURE_MASK_SUPPORTS_AUTHORITY_HEADER,
+     "foo.test.google.fr", chttp2_create_fixture_secure_fullstack,
+     chttp2_init_client_simple_ssl_secure_fullstack,
+     chttp2_init_server_simple_ssl_secure_fullstack,
+     chttp2_tear_down_secure_fullstack},
+};
+
+int main(int argc, char** argv) {
+  size_t i;
+  FILE* roots_file;
+  size_t roots_size = strlen(test_root_cert);
+  char* roots_filename;
+
+  grpc::testing::TestEnvironment env(argc, argv);
+  grpc_end2end_tests_pre_init();
+
+  /* Set the SSL roots env var. */
+  roots_file = gpr_tmpfile("chttp2_simple_ssl_fullstack_test", &roots_filename);
+  GPR_ASSERT(roots_filename != nullptr);
+  GPR_ASSERT(roots_file != nullptr);
+  GPR_ASSERT(fwrite(test_root_cert, 1, roots_size, roots_file) == roots_size);
+  fclose(roots_file);
+  gpr_setenv(GRPC_DEFAULT_SSL_ROOTS_FILE_PATH_ENV_VAR, roots_filename);
+
+  grpc_init();
+
+  for (i = 0; i < sizeof(configs) / sizeof(*configs); i++) {
+    grpc_end2end_tests(argc, argv, configs[i]);
+  }
+
+  grpc_shutdown();
+
+  /* Cleanup. */
+  remove(roots_filename);
+  gpr_free(roots_filename);
+
+  return 0;
+}

test/core/end2end/gen_build_yaml.py

@@ -74,6 +74,7 @@ END2END_FIXTURES = {
     'h2_sockpair+trace': socketpair_unsecure_fixture_options._replace(
         ci_mac=False, tracing=True, large_writes=False, exclude_iomgrs=['uv']),
     'h2_ssl': default_secure_fixture_options,
+    'h2_ssl_cred_reload': default_secure_fixture_options,
     'h2_spiffe': default_secure_fixture_options,
     'h2_local_uds': local_fixture_options,
     'h2_local_ipv4': local_fixture_options,

test/core/end2end/generate_tests.bzl

@@ -87,6 +87,7 @@ END2END_FIXTURES = {
         client_channel = False,
     ),
     "h2_ssl": _fixture_options(secure = True),
+    "h2_ssl_cred_reload": _fixture_options(secure = True),
     "h2_spiffe": _fixture_options(secure = True),
     "h2_local_uds": _fixture_options(secure = True, dns_resolver = False, _platforms = ["linux", "mac", "posix"]),
     "h2_local_ipv4": _fixture_options(secure = True, dns_resolver = False, _platforms = ["linux", "mac", "posix"]),
@@ -150,6 +151,7 @@ END2END_NOSEC_FIXTURES = {
         client_channel = False,
     ),
     "h2_ssl": _fixture_options(secure = False),
+    "h2_ssl_cred_reload": _fixture_options(secure = False),
     "h2_ssl_proxy": _fixture_options(includes_proxy = True, secure = False),
     "h2_uds": _fixture_options(
         dns_resolver = False,

tools/run_tests/generated/sources_and_headers.json

@@ -5672,6 +5672,23 @@
     "third_party": false, 
     "type": "target"
   }, 
+  {
+    "deps": [
+      "end2end_tests", 
+      "gpr", 
+      "grpc", 
+      "grpc_test_util"
+    ], 
+    "headers": [], 
+    "is_filegroup": false, 
+    "language": "c", 
+    "name": "h2_ssl_cred_reload_test", 
+    "src": [
+      "test/core/end2end/fixtures/h2_ssl_cred_reload.cc"
+    ], 
+    "third_party": false, 
+    "type": "target"
+  }, 
   {
     "deps": [
       "end2end_tests",