Strip port in peer name check

This string comes from an authority field, which is allowed to contain a
':' port (see https://tools.ietf.org/html/rfc3986#section-3.2).

We need to strip it before performing host name verification.

2 files changed, 23 insertions, 4 deletions

diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c
index 60064dc83d..0dc37fa73c 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -338,6 +338,24 @@ static grpc_security_status ssl_server_create_handshaker(
   return ssl_create_handshaker(c->handshaker_factory, 0, NULL, handshaker);
 }
 
+static int ssl_host_matches_name(const tsi_peer *peer,
+                                 const char *peer_name) {
+  char *allocated_name = NULL;
+  int r;
+
+  if (strchr(peer_name, ':') != NULL) {
+    char *ignored_port;
+    gpr_split_host_port(peer_name, &allocated_name, &ignored_port);
+    gpr_free(ignored_port);
+    peer_name = allocated_name;
+    if (!peer_name) return 0;
+  }
+  
+  r = tsi_ssl_peer_matches_name(peer, peer_name);
+  gpr_free(allocated_name);
+  return r;
+}
+
 static grpc_security_status ssl_check_peer(const char *peer_name,
                                            const tsi_peer *peer) {
   /* Check the ALPN. */
@@ -359,10 +377,11 @@ static grpc_security_status ssl_check_peer(const char *peer_name,
 
   /* Check the peer name if specified. */
   if (peer_name != NULL &&
-      !tsi_ssl_peer_matches_name(peer, peer_name)) {
+      !ssl_host_matches_name(peer, peer_name)) {
     gpr_log(GPR_ERROR, "Peer name %s is not in peer certificate", peer_name);
     return GRPC_SECURITY_ERROR;
   }
+
   return GRPC_SECURITY_OK;
 }
 
@@ -398,7 +417,7 @@ static grpc_security_status ssl_channel_check_call_host(
   grpc_ssl_channel_security_context *c =
       (grpc_ssl_channel_security_context *)ctx;
 
-  if (tsi_ssl_peer_matches_name(&c->peer, host)) return GRPC_SECURITY_OK;
+  if (ssl_host_matches_name(&c->peer, host)) return GRPC_SECURITY_OK;
 
   /* If the target name was overridden, then the original target_name was
      'checked' transitively during the previous peer check at the end of the
diff --git a/test/core/end2end/tests/simple_request.c b/test/core/end2end/tests/simple_request.c
index dac82535be..5b36a19f3d 100644
--- a/test/core/end2end/tests/simple_request.c
+++ b/test/core/end2end/tests/simple_request.c
@@ -122,7 +122,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) {
   int was_cancelled = 2;
 
   c = grpc_channel_create_call(f.client, f.client_cq, "/foo",
-                               "foo.test.google.fr", deadline);
+                               "foo.test.google.fr:1234", deadline);
   GPR_ASSERT(c);
 
   grpc_metadata_array_init(&initial_metadata_recv);
@@ -178,7 +178,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) {
   GPR_ASSERT(status == GRPC_STATUS_UNIMPLEMENTED);
   GPR_ASSERT(0 == strcmp(details, "xyz"));
   GPR_ASSERT(0 == strcmp(call_details.method, "/foo"));
-  GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr"));
+  GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr:1234"));
   GPR_ASSERT(was_cancelled == 0);
 
   gpr_free(details);