Merge pull request #21779 from grpc/zhen_cleanup_namecheck

[TLS Lib Clean-up] Add host name check For tls library

src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h

@@ -273,7 +273,8 @@ struct grpc_tls_credentials_options
 
  private:
   grpc_ssl_client_certificate_request_type cert_request_type_;
-  grpc_tls_server_verification_option server_verification_option_;
+  grpc_tls_server_verification_option server_verification_option_ =
+      GRPC_TLS_SERVER_VERIFICATION;
   grpc_core::RefCountedPtr<grpc_tls_key_materials_config> key_materials_config_;
   grpc_core::RefCountedPtr<grpc_tls_credential_reload_config>
       credential_reload_config_;

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

@@ -112,6 +112,18 @@ grpc_status_code TlsFetchKeyMaterials(
   return status;
 }
 
+grpc_error* TlsCheckHostName(const char* peer_name, const tsi_peer* peer) {
+  /* Check the peer name if specified. */
+  if (peer_name != nullptr && !grpc_ssl_host_matches_name(peer, peer_name)) {
+    char* msg;
+    gpr_asprintf(&msg, "Peer name %s is not in peer certificate", peer_name);
+    grpc_error* error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
+    gpr_free(msg);
+    return error;
+  }
+  return GRPC_ERROR_NONE;
+}
+
 TlsChannelSecurityConnector::TlsChannelSecurityConnector(
     grpc_core::RefCountedPtr<grpc_channel_credentials> channel_creds,
     grpc_core::RefCountedPtr<grpc_call_credentials> request_metadata_creds,
@@ -180,6 +192,17 @@ void TlsChannelSecurityConnector::check_peer(
       grpc_ssl_peer_to_auth_context(&peer, GRPC_TLS_TRANSPORT_SECURITY_TYPE);
   const TlsCredentials* creds =
       static_cast<const TlsCredentials*>(channel_creds());
+  if (creds->options().server_verification_option() ==
+      GRPC_TLS_SERVER_VERIFICATION) {
+    /* Do the default host name check if specifying the target name. */
+    error = TlsCheckHostName(target_name, &peer);
+    if (error != GRPC_ERROR_NONE) {
+      grpc_core::ExecCtx::Run(DEBUG_LOCATION, on_peer_checked, error);
+      tsi_peer_destruct(&peer);
+      return;
+    }
+  }
+  /* Do the custom server authorization check, if specified by the user. */
   const grpc_tls_server_authorization_check_config* config =
       creds->options().server_authorization_check_config();
   /* If server authorization config is not null, use it to perform

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

@@ -144,13 +144,17 @@ class TlsServerSecurityConnector final : public grpc_server_security_connector {
   grpc_core::RefCountedPtr<grpc_tls_key_materials_config> key_materials_config_;
 };
 
-// Exposed for testing only.
+// ---- Functions below are exposed for testing only -----------------------
 grpc_status_code TlsFetchKeyMaterials(
     const grpc_core::RefCountedPtr<grpc_tls_key_materials_config>&
         key_materials_config,
     const grpc_tls_credentials_options& options, bool server_config,
     grpc_ssl_certificate_config_reload_status* status);
 
+// TlsCheckHostName checks if |peer_name| matches the identity information
+// contained in |peer|. This is AKA hostname check.
+grpc_error* TlsCheckHostName(const char* peer_name, const tsi_peer* peer);
+
 }  // namespace grpc_core
 
 #endif /* GRPC_CORE_LIB_SECURITY_SECURITY_CONNECTOR_TLS_TLS_SECURITY_CONNECTOR_H \

test/core/end2end/fixtures/h2_tls.cc

@@ -16,16 +16,13 @@
  *
  */
 
-#include "test/core/end2end/end2end_tests.h"
-
-#include <stdio.h>
-#include <string.h>
-
 #include <grpc/grpc_security.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
-
 #include <grpc/support/string_util.h>
+#include <stdio.h>
+#include <string.h>
+
 #include "src/core/lib/channel/channel_args.h"
 #include "src/core/lib/gpr/env.h"
 #include "src/core/lib/gpr/string.h"
@@ -37,6 +34,7 @@
 #include "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h"
 #include "src/core/lib/security/security_connector/ssl_utils_config.h"
 #include "test/core/end2end/data/ssl_test_data.h"
+#include "test/core/end2end/end2end_tests.h"
 #include "test/core/util/port.h"
 #include "test/core/util/test_config.h"
 
@@ -193,6 +191,7 @@ static int server_cred_reload_sync(void* /*config_user_data*/,
 static grpc_channel_credentials* create_tls_channel_credentials(
     fullstack_secure_fixture_data* ffd) {
   grpc_tls_credentials_options* options = grpc_tls_credentials_options_create();
+  options->set_server_verification_option(GRPC_TLS_SERVER_VERIFICATION);
   /* Set credential reload config. */
   grpc_tls_credential_reload_config* reload_config =
       grpc_tls_credential_reload_config_create(nullptr, client_cred_reload_sync,

test/core/security/BUILD

@@ -269,6 +269,8 @@ grpc_cc_test(
         "//:gpr",
         "//:grpc",
         "//:grpc_secure",
+        "//:tsi",
+        "//:tsi_interface",
         "//test/core/end2end:ssl_test_data",
         "//test/core/util:grpc_test_util",
     ],

test/core/security/tls_security_connector_test.cc

@@ -16,16 +16,17 @@
  *
  */
 
-#include <stdlib.h>
-#include <string.h>
+#include "src/core/lib/security/security_connector/tls/tls_security_connector.h"
 
 #include <gmock/gmock.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 #include <gtest/gtest.h>
+#include <stdlib.h>
+#include <string.h>
 
-#include "src/core/lib/security/security_connector/tls/tls_security_connector.h"
+#include "src/core/tsi/transport_security.h"
 #include "test/core/end2end/data/ssl_test_data.h"
 #include "test/core/util/test_config.h"
 
@@ -254,6 +255,35 @@ TEST_F(TlsSecurityConnectorTest, CreateChannelSecurityConnectorFailInit) {
   EXPECT_EQ(connector, nullptr);
 }
 
+TEST_F(TlsSecurityConnectorTest, TlsCheckHostNameSuccess) {
+  const char* target_name = "foo.test.google.fr";
+  tsi_peer peer;
+  GPR_ASSERT(tsi_construct_peer(1, &peer) == TSI_OK);
+  GPR_ASSERT(tsi_construct_string_peer_property_from_cstring(
+                 TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, target_name,
+                 &peer.properties[0]) == TSI_OK);
+  grpc_error* error = grpc_core::TlsCheckHostName(target_name, &peer);
+  tsi_peer_destruct(&peer);
+  EXPECT_EQ(error, GRPC_ERROR_NONE);
+  GRPC_ERROR_UNREF(error);
+  options_->Unref();
+}
+
+TEST_F(TlsSecurityConnectorTest, TlsCheckHostNameFail) {
+  const char* target_name = "foo.test.google.fr";
+  const char* another_name = "bar.test.google.fr";
+  tsi_peer peer;
+  GPR_ASSERT(tsi_construct_peer(1, &peer) == TSI_OK);
+  GPR_ASSERT(tsi_construct_string_peer_property_from_cstring(
+                 TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, another_name,
+                 &peer.properties[0]) == TSI_OK);
+  grpc_error* error = grpc_core::TlsCheckHostName(target_name, &peer);
+  tsi_peer_destruct(&peer);
+  EXPECT_NE(error, GRPC_ERROR_NONE);
+  GRPC_ERROR_UNREF(error);
+  options_->Unref();
+}
+
 TEST_F(TlsSecurityConnectorTest, CreateServerSecurityConnectorSuccess) {
   SetOptions(SUCCESS);
   auto cred = std::unique_ptr<grpc_server_credentials>(