From 1fb7a841333658eee482801f0428fc297e979042 Mon Sep 17 00:00:00 2001
From: Yihua Zhang <yihuaz@google.com>
Date: Mon, 1 Apr 2019 08:48:35 -0700
Subject: [PATCH] fix ALPN issues.

---
 .../ssl/ssl_security_connector.cc             | 17 ++------
 test/core/security/security_connector_test.cc | 43 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc
index 8a00bbb82ed..ad492f361aa 100644
--- a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc
+++ b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc
@@ -44,24 +44,15 @@ namespace {
 grpc_error* ssl_check_peer(
     const char* peer_name, const tsi_peer* peer,
     grpc_core::RefCountedPtr<grpc_auth_context>* auth_context) {
-#if TSI_OPENSSL_ALPN_SUPPORT
-  /* Check the ALPN if ALPN is supported. */
-  const tsi_peer_property* p =
-      tsi_peer_get_property_by_name(peer, TSI_SSL_ALPN_SELECTED_PROTOCOL);
-  if (p == nullptr) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-        "Cannot check peer: missing selected ALPN property.");
-  }
-  if (!grpc_chttp2_is_alpn_version_supported(p->value.data, p->value.length)) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-        "Cannot check peer: invalid ALPN value.");
+  grpc_error* error = grpc_ssl_check_alpn(peer);
+  if (error != GRPC_ERROR_NONE) {
+    return error;
   }
-#endif /* TSI_OPENSSL_ALPN_SUPPORT */
   /* 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);
+    error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
     gpr_free(msg);
     return error;
   }
diff --git a/test/core/security/security_connector_test.cc b/test/core/security/security_connector_test.cc
index 2a31763c73c..496f064439c 100644
--- a/test/core/security/security_connector_test.cc
+++ b/test/core/security/security_connector_test.cc
@@ -36,6 +36,10 @@
 #include "src/core/tsi/transport_security.h"
 #include "test/core/util/test_config.h"
 
+#ifndef TSI_OPENSSL_ALPN_SUPPORT
+#define TSI_OPENSSL_ALPN_SUPPORT 1
+#endif
+
 static int check_transport_security_type(const grpc_auth_context* ctx) {
   grpc_auth_property_iterator it = grpc_auth_context_find_properties_by_name(
       ctx, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME);
@@ -432,6 +436,43 @@ static void test_default_ssl_roots(void) {
   gpr_free(roots_env_var_file_path);
 }
 
+static void test_peer_alpn_check(void) {
+#if TSI_OPENSSL_ALPN_SUPPORT
+  tsi_peer peer;
+  const char* alpn = "grpc";
+  const char* wrong_alpn = "wrong";
+  // peer does not have a TSI_SSL_ALPN_SELECTED_PROTOCOL property.
+  GPR_ASSERT(tsi_construct_peer(1, &peer) == TSI_OK);
+  GPR_ASSERT(tsi_construct_string_peer_property("wrong peer property name",
+                                                alpn, strlen(alpn),
+                                                &peer.properties[0]) == TSI_OK);
+  grpc_error* error = grpc_ssl_check_alpn(&peer);
+  GPR_ASSERT(error != GRPC_ERROR_NONE);
+  tsi_peer_destruct(&peer);
+  GRPC_ERROR_UNREF(error);
+  // peer has a TSI_SSL_ALPN_SELECTED_PROTOCOL property but with an incorrect
+  // property value.
+  GPR_ASSERT(tsi_construct_peer(1, &peer) == TSI_OK);
+  GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL,
+                                                wrong_alpn, strlen(wrong_alpn),
+                                                &peer.properties[0]) == TSI_OK);
+  error = grpc_ssl_check_alpn(&peer);
+  GPR_ASSERT(error != GRPC_ERROR_NONE);
+  tsi_peer_destruct(&peer);
+  GRPC_ERROR_UNREF(error);
+  // peer has a TSI_SSL_ALPN_SELECTED_PROTOCOL property with a correct property
+  // value.
+  GPR_ASSERT(tsi_construct_peer(1, &peer) == TSI_OK);
+  GPR_ASSERT(tsi_construct_string_peer_property(TSI_SSL_ALPN_SELECTED_PROTOCOL,
+                                                alpn, strlen(alpn),
+                                                &peer.properties[0]) == TSI_OK);
+  GPR_ASSERT(grpc_ssl_check_alpn(&peer) == GRPC_ERROR_NONE);
+  tsi_peer_destruct(&peer);
+#else
+  GPR_ASSERT(grpc_ssl_check_alpn(nullptr) == GRPC_ERROR_NONE);
+#endif
+}
+
 int main(int argc, char** argv) {
   grpc::testing::TestEnvironment env(argc, argv);
   grpc_init();
@@ -443,7 +484,7 @@ int main(int argc, char** argv) {
   test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context();
   test_ipv6_address_san();
   test_default_ssl_roots();
-
+  test_peer_alpn_check();
   grpc_shutdown();
   return 0;
 }