Merge pull request #18469 from yihuazhang/revert_ssl_security_connector

Revert the changes in ssl_security_connector.cc made in #18115

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

@@ -41,6 +41,33 @@
 #include "src/core/tsi/transport_security.h"
 
 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.");
+  }
+#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);
+    gpr_free(msg);
+    return error;
+  }
+  *auth_context = grpc_ssl_peer_to_auth_context(peer);
+  return GRPC_ERROR_NONE;
+}
 
 class grpc_ssl_channel_security_connector final
     : public grpc_channel_security_connector {
@@ -69,10 +96,34 @@ class grpc_ssl_channel_security_connector final
   }
 
   grpc_security_status InitializeHandshakerFactory(
-      const grpc_ssl_config* config, tsi_ssl_session_cache* ssl_session_cache) {
-    return grpc_ssl_tsi_client_handshaker_factory_init(
-        config->pem_key_cert_pair, config->pem_root_certs, ssl_session_cache,
-        &client_handshaker_factory_);
+      const grpc_ssl_config* config, const char* pem_root_certs,
+      const tsi_ssl_root_certs_store* root_store,
+      tsi_ssl_session_cache* ssl_session_cache) {
+    bool has_key_cert_pair =
+        config->pem_key_cert_pair != nullptr &&
+        config->pem_key_cert_pair->private_key != nullptr &&
+        config->pem_key_cert_pair->cert_chain != nullptr;
+    tsi_ssl_client_handshaker_options options;
+    GPR_DEBUG_ASSERT(pem_root_certs != nullptr);
+    options.pem_root_certs = pem_root_certs;
+    options.root_store = root_store;
+    options.alpn_protocols =
+        grpc_fill_alpn_protocol_strings(&options.num_alpn_protocols);
+    if (has_key_cert_pair) {
+      options.pem_key_cert_pair = config->pem_key_cert_pair;
+    }
+    options.cipher_suites = grpc_get_ssl_cipher_suites();
+    options.session_cache = ssl_session_cache;
+    const tsi_result result =
+        tsi_create_ssl_client_handshaker_factory_with_options(
+            &options, &client_handshaker_factory_);
+    gpr_free((void*)options.alpn_protocols);
+    if (result != TSI_OK) {
+      gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
+              tsi_result_to_string(result));
+      return GRPC_SECURITY_ERROR;
+    }
+    return GRPC_SECURITY_OK;
   }
 
   void add_handshakers(grpc_pollset_set* interested_parties,
@@ -99,35 +150,29 @@ class grpc_ssl_channel_security_connector final
     const char* target_name = overridden_target_name_ != nullptr
                                   ? overridden_target_name_
                                   : target_name_;
-    grpc_error* error = grpc_ssl_check_alpn(&peer);
-    if (error == GRPC_ERROR_NONE) {
-      error = grpc_ssl_check_peer_name(target_name, &peer);
-      if (error == GRPC_ERROR_NONE) {
-        if (verify_options_->verify_peer_callback != nullptr) {
-          const tsi_peer_property* p =
-              tsi_peer_get_property_by_name(&peer, TSI_X509_PEM_CERT_PROPERTY);
-          if (p == nullptr) {
-            error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-                "Cannot check peer: missing pem cert property.");
-          } else {
-            char* peer_pem =
-                static_cast<char*>(gpr_malloc(p->value.length + 1));
-            memcpy(peer_pem, p->value.data, p->value.length);
-            peer_pem[p->value.length] = '\0';
-            int callback_status = verify_options_->verify_peer_callback(
-                target_name, peer_pem,
-                verify_options_->verify_peer_callback_userdata);
-            gpr_free(peer_pem);
-            if (callback_status) {
-              char* msg;
-              gpr_asprintf(&msg, "Verify peer callback returned a failure (%d)",
-                           callback_status);
-              error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
-              gpr_free(msg);
-            }
-          }
+    grpc_error* error = ssl_check_peer(target_name, &peer, auth_context);
+    if (error == GRPC_ERROR_NONE &&
+        verify_options_->verify_peer_callback != nullptr) {
+      const tsi_peer_property* p =
+          tsi_peer_get_property_by_name(&peer, TSI_X509_PEM_CERT_PROPERTY);
+      if (p == nullptr) {
+        error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+            "Cannot check peer: missing pem cert property.");
+      } else {
+        char* peer_pem = static_cast<char*>(gpr_malloc(p->value.length + 1));
+        memcpy(peer_pem, p->value.data, p->value.length);
+        peer_pem[p->value.length] = '\0';
+        int callback_status = verify_options_->verify_peer_callback(
+            target_name, peer_pem,
+            verify_options_->verify_peer_callback_userdata);
+        gpr_free(peer_pem);
+        if (callback_status) {
+          char* msg;
+          gpr_asprintf(&msg, "Verify peer callback returned a failure (%d)",
+                       callback_status);
+          error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
+          gpr_free(msg);
         }
-        *auth_context = grpc_ssl_peer_to_auth_context(&peer);
       }
     }
     GRPC_CLOSURE_SCHED(on_peer_checked, error);
@@ -139,16 +184,34 @@ class grpc_ssl_channel_security_connector final
         reinterpret_cast<const grpc_ssl_channel_security_connector*>(other_sc);
     int c = channel_security_connector_cmp(other);
     if (c != 0) return c;
-    return grpc_ssl_cmp_target_name(target_name_, other->target_name_,
-                                    overridden_target_name_,
-                                    other->overridden_target_name_);
+    c = strcmp(target_name_, other->target_name_);
+    if (c != 0) return c;
+    return (overridden_target_name_ == nullptr ||
+            other->overridden_target_name_ == nullptr)
+               ? GPR_ICMP(overridden_target_name_,
+                          other->overridden_target_name_)
+               : strcmp(overridden_target_name_,
+                        other->overridden_target_name_);
   }
 
   bool check_call_host(const char* host, grpc_auth_context* auth_context,
                        grpc_closure* on_call_host_checked,
                        grpc_error** error) override {
-    return grpc_ssl_check_call_host(host, target_name_, overridden_target_name_,
-                                    auth_context, on_call_host_checked, error);
+    grpc_security_status status = GRPC_SECURITY_ERROR;
+    tsi_peer peer = grpc_shallow_peer_from_ssl_auth_context(auth_context);
+    if (grpc_ssl_host_matches_name(&peer, host)) status = 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
+       handshake. */
+    if (overridden_target_name_ != nullptr && strcmp(host, target_name_) == 0) {
+      status = GRPC_SECURITY_OK;
+    }
+    if (status != GRPC_SECURITY_OK) {
+      *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+          "call host does not match SSL server name");
+    }
+    grpc_shallow_peer_destruct(&peer);
+    return true;
   }
 
   void cancel_check_call_host(grpc_closure* on_call_host_checked,
@@ -185,25 +248,43 @@ class grpc_ssl_server_security_connector
   }
 
   grpc_security_status InitializeHandshakerFactory() {
-    grpc_security_status retval = GRPC_SECURITY_OK;
     if (has_cert_config_fetcher()) {
       // Load initial credentials from certificate_config_fetcher:
       if (!try_fetch_ssl_server_credentials()) {
         gpr_log(GPR_ERROR,
                 "Failed loading SSL server credentials from fetcher.");
-        retval = GRPC_SECURITY_ERROR;
+        return GRPC_SECURITY_ERROR;
       }
     } else {
       auto* server_credentials =
           static_cast<const grpc_ssl_server_credentials*>(server_creds());
-      retval = grpc_ssl_tsi_server_handshaker_factory_init(
-          server_credentials->config().pem_key_cert_pairs,
-          server_credentials->config().num_key_cert_pairs,
-          server_credentials->config().pem_root_certs,
-          server_credentials->config().client_certificate_request,
-          &server_handshaker_factory_);
+      size_t num_alpn_protocols = 0;
+      const char** alpn_protocol_strings =
+          grpc_fill_alpn_protocol_strings(&num_alpn_protocols);
+      tsi_ssl_server_handshaker_options options;
+      options.pem_key_cert_pairs =
+          server_credentials->config().pem_key_cert_pairs;
+      options.num_key_cert_pairs =
+          server_credentials->config().num_key_cert_pairs;
+      options.pem_client_root_certs =
+          server_credentials->config().pem_root_certs;
+      options.client_certificate_request =
+          grpc_get_tsi_client_certificate_request_type(
+              server_credentials->config().client_certificate_request);
+      options.cipher_suites = grpc_get_ssl_cipher_suites();
+      options.alpn_protocols = alpn_protocol_strings;
+      options.num_alpn_protocols = static_cast<uint16_t>(num_alpn_protocols);
+      const tsi_result result =
+          tsi_create_ssl_server_handshaker_factory_with_options(
+              &options, &server_handshaker_factory_);
+      gpr_free((void*)alpn_protocol_strings);
+      if (result != TSI_OK) {
+        gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
+                tsi_result_to_string(result));
+        return GRPC_SECURITY_ERROR;
+      }
     }
-    return retval;
+    return GRPC_SECURITY_OK;
   }
 
   void add_handshakers(grpc_pollset_set* interested_parties,
@@ -225,8 +306,7 @@ class grpc_ssl_server_security_connector
   void check_peer(tsi_peer peer, grpc_endpoint* ep,
                   grpc_core::RefCountedPtr<grpc_auth_context>* auth_context,
                   grpc_closure* on_peer_checked) override {
-    grpc_error* error = grpc_ssl_check_alpn(&peer);
-    *auth_context = grpc_ssl_peer_to_auth_context(&peer);
+    grpc_error* error = ssl_check_peer(nullptr, &peer, auth_context);
     tsi_peer_destruct(&peer);
     GRPC_CLOSURE_SCHED(on_peer_checked, error);
   }
@@ -243,7 +323,9 @@ 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 =
         static_cast<grpc_ssl_server_credentials*>(this->mutable_server_creds());
     grpc_ssl_certificate_config_reload_status cb_result =
@@ -260,6 +342,7 @@ class grpc_ssl_server_security_connector
               "use previously-loaded credentials.");
       status = false;
     }
+
     if (certificate_config != nullptr) {
       grpc_ssl_server_certificate_config_destroy(certificate_config);
     }
@@ -278,18 +361,34 @@ class grpc_ssl_server_security_connector
               "config.");
       return false;
     }
-    tsi_ssl_pem_key_cert_pair* pem_key_cert_pairs =
-        grpc_convert_grpc_to_tsi_cert_pairs(config->pem_key_cert_pairs,
-                                            config->num_key_cert_pairs);
-    const grpc_ssl_server_credentials* server_credentials =
-        static_cast<const grpc_ssl_server_credentials*>(this->server_creds());
+    gpr_log(GPR_DEBUG, "Using new server certificate config (%p).", config);
+
+    size_t num_alpn_protocols = 0;
+    const char** alpn_protocol_strings =
+        grpc_fill_alpn_protocol_strings(&num_alpn_protocols);
     tsi_ssl_server_handshaker_factory* new_handshaker_factory = nullptr;
-    grpc_security_status retval = grpc_ssl_tsi_server_handshaker_factory_init(
-        pem_key_cert_pairs, config->num_key_cert_pairs, config->pem_root_certs,
-        server_credentials->config().client_certificate_request,
-        &new_handshaker_factory);
-    gpr_free(pem_key_cert_pairs);
-    if (retval != GRPC_SECURITY_OK) {
+    const grpc_ssl_server_credentials* server_creds =
+        static_cast<const grpc_ssl_server_credentials*>(this->server_creds());
+    GPR_DEBUG_ASSERT(config->pem_root_certs != nullptr);
+    tsi_ssl_server_handshaker_options options;
+    options.pem_key_cert_pairs = grpc_convert_grpc_to_tsi_cert_pairs(
+        config->pem_key_cert_pairs, config->num_key_cert_pairs);
+    options.num_key_cert_pairs = config->num_key_cert_pairs;
+    options.pem_client_root_certs = config->pem_root_certs;
+    options.client_certificate_request =
+        grpc_get_tsi_client_certificate_request_type(
+            server_creds->config().client_certificate_request);
+    options.cipher_suites = grpc_get_ssl_cipher_suites();
+    options.alpn_protocols = alpn_protocol_strings;
+    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);
+    gpr_free((void*)alpn_protocol_strings);
+
+    if (result != TSI_OK) {
+      gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
+              tsi_result_to_string(result));
       return false;
     }
     set_server_handshaker_factory(new_handshaker_factory);
@@ -319,17 +418,28 @@ grpc_ssl_channel_security_connector_create(
     gpr_log(GPR_ERROR, "An ssl channel needs a config and a target name.");
     return nullptr;
   }
-  if (config->pem_root_certs == nullptr &&
-      grpc_core::DefaultSslRootStore::GetPemRootCerts() == nullptr) {
-    gpr_log(GPR_ERROR, "Could not get pem root certs.");
-    return nullptr;
+
+  const char* pem_root_certs;
+  const tsi_ssl_root_certs_store* root_store;
+  if (config->pem_root_certs == nullptr) {
+    // Use default root certificates.
+    pem_root_certs = grpc_core::DefaultSslRootStore::GetPemRootCerts();
+    if (pem_root_certs == nullptr) {
+      gpr_log(GPR_ERROR, "Could not get default pem root certs.");
+      return nullptr;
+    }
+    root_store = grpc_core::DefaultSslRootStore::GetRootStore();
+  } else {
+    pem_root_certs = config->pem_root_certs;
+    root_store = nullptr;
   }
+
   grpc_core::RefCountedPtr<grpc_ssl_channel_security_connector> c =
       grpc_core::MakeRefCounted<grpc_ssl_channel_security_connector>(
           std::move(channel_creds), std::move(request_metadata_creds), config,
           target_name, overridden_target_name);
-  const grpc_security_status result =
-      c->InitializeHandshakerFactory(config, ssl_session_cache);
+  const grpc_security_status result = c->InitializeHandshakerFactory(
+      config, pem_root_certs, root_store, ssl_session_cache);
   if (result != GRPC_SECURITY_OK) {
     return nullptr;
   }

src/core/tsi/ssl_transport_security.cc

@@ -344,24 +344,18 @@ static tsi_result add_subject_alt_names_properties_to_peer(
     size_t subject_alt_name_count) {
   size_t i;
   tsi_result result = TSI_OK;
+
   /* Reset for DNS entries filtering. */
   peer->property_count -= subject_alt_name_count;
+
   for (i = 0; i < subject_alt_name_count; i++) {
     GENERAL_NAME* subject_alt_name =
         sk_GENERAL_NAME_value(subject_alt_names, TSI_SIZE_AS_SIZE(i));
-    if (subject_alt_name->type == GEN_DNS ||
-        subject_alt_name->type == GEN_EMAIL ||
-        subject_alt_name->type == GEN_URI) {
+    /* Filter out the non-dns entries names. */
+    if (subject_alt_name->type == GEN_DNS) {
       unsigned char* name = nullptr;
       int name_size;
-      if (subject_alt_name->type == GEN_DNS) {
-        name_size = ASN1_STRING_to_UTF8(&name, subject_alt_name->d.dNSName);
-      } else if (subject_alt_name->type == GEN_EMAIL) {
-        name_size = ASN1_STRING_to_UTF8(&name, subject_alt_name->d.rfc822Name);
-      } else {
-        name_size = ASN1_STRING_to_UTF8(
-            &name, subject_alt_name->d.uniformResourceIdentifier);
-      }
+      name_size = ASN1_STRING_to_UTF8(&name, subject_alt_name->d.dNSName);
       if (name_size < 0) {
         gpr_log(GPR_ERROR, "Could not get utf8 from asn1 string.");
         result = TSI_INTERNAL_ERROR;
@@ -375,6 +369,7 @@ static tsi_result add_subject_alt_names_properties_to_peer(
     } else if (subject_alt_name->type == GEN_IPADD) {
       char ntop_buf[INET6_ADDRSTRLEN];
       int af;
+
       if (subject_alt_name->d.iPAddress->length == 4) {
         af = AF_INET;
       } else if (subject_alt_name->d.iPAddress->length == 16) {
@@ -391,6 +386,7 @@ static tsi_result add_subject_alt_names_properties_to_peer(
         result = TSI_INTERNAL_ERROR;
         break;
       }
+
       result = tsi_construct_string_peer_property_from_cstring(
           TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, name,
           &peer->properties[peer->property_count++]);