ARROW-10206: [C++][Python][FlightRPC] Allow disabling server validation

ci/conda_env_cpp.yml

@@ -25,7 +25,7 @@ cmake
 gflags
 glog
 gmock>=1.8.1
-grpc-cpp>=1.21.4
+grpc-cpp>=1.27.3
 gtest=1.8.1
 libprotobuf
 libutf8proc

cpp/cmake_modules/Findzstd.cmake

@@ -23,18 +23,24 @@ set(ZSTD_LIB_NAME_BASE "${ZSTD_MSVC_LIB_PREFIX}zstd")
 if(ARROW_ZSTD_USE_SHARED)
   set(ZSTD_LIB_NAMES)
   if(CMAKE_IMPORT_LIBRARY_SUFFIX)
-    list(APPEND ZSTD_LIB_NAMES
-                "${CMAKE_IMPORT_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${CMAKE_IMPORT_LIBRARY_SUFFIX}")
+    list(
+      APPEND
+        ZSTD_LIB_NAMES
+        "${CMAKE_IMPORT_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${CMAKE_IMPORT_LIBRARY_SUFFIX}"
+      )
   endif()
-  list(APPEND ZSTD_LIB_NAMES
-              "${CMAKE_SHARED_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${CMAKE_SHARED_LIBRARY_SUFFIX}")
+  list(
+    APPEND
+      ZSTD_LIB_NAMES
+      "${CMAKE_SHARED_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${CMAKE_SHARED_LIBRARY_SUFFIX}")
 else()
   if(MSVC AND NOT DEFINED ZSTD_MSVC_STATIC_LIB_SUFFIX)
     set(ZSTD_MSVC_STATIC_LIB_SUFFIX "_static")
   endif()
   set(ZSTD_STATIC_LIB_SUFFIX
       "${ZSTD_MSVC_STATIC_LIB_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX}")
-  set(ZSTD_LIB_NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${ZSTD_STATIC_LIB_SUFFIX}")
+  set(ZSTD_LIB_NAMES
+      "${CMAKE_STATIC_LIBRARY_PREFIX}${ZSTD_LIB_NAME_BASE}${ZSTD_STATIC_LIB_SUFFIX}")
 endif()
 
 # First, find via if specified ZTD_ROOT
@@ -66,7 +72,9 @@ else()
                  PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES})
   else()
     # Third, check all other CMake paths
-    find_library(ZSTD_LIB NAMES ${ZSTD_LIB_NAMES} PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES})
+    find_library(ZSTD_LIB
+                 NAMES ${ZSTD_LIB_NAMES}
+                 PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES})
     find_path(ZSTD_INCLUDE_DIR NAMES zstd.h PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES})
   endif()
 endif()

cpp/src/arrow/flight/CMakeLists.txt

@@ -61,6 +61,48 @@ set_source_files_properties(${FLIGHT_GENERATED_PROTO_FILES} PROPERTIES GENERATED
 
 add_custom_target(flight_grpc_gen ALL DEPENDS ${FLIGHT_GENERATED_PROTO_FILES})
 
+# <KLUDGE> -Werror / /WX cause try_compile to fail because there seems to be no
+# way to pass -isystem $GRPC_INCLUDE_DIR instead of -I$GRPC_INCLUDE_DIR
+set(CMAKE_CXX_FLAGS_BACKUP "${CMAKE_CXX_FLAGS}")
+string(REPLACE "/WX" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+string(REPLACE "-Werror" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+# Probe the version of gRPC being used to see if it supports disabling server
+# verification when using TLS.
+message(STATUS "Checking support for TlsCredentialsOptions...")
+get_property(CURRENT_INCLUDE_DIRECTORIES
+             DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+             PROPERTY INCLUDE_DIRECTORIES)
+try_compile(HAS_GRPC_132 ${CMAKE_CURRENT_BINARY_DIR}/try_compile SOURCES
+            "${CMAKE_CURRENT_SOURCE_DIR}/try_compile/check_tls_opts_132.cc"
+            CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${CURRENT_INCLUDE_DIRECTORIES}"
+            LINK_LIBRARIES gRPC::grpc
+            OUTPUT_VARIABLE TSL_CREDENTIALS_OPTIONS_CHECK_OUTPUT CXX_STANDARD 11)
+
+if(HAS_GRPC_132)
+  message(STATUS "TlsCredentialsOptions found in grpc::experimental.")
+  add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc::experimental)
+else()
+  message(STATUS "TlsCredentialsOptions not found in grpc::experimental.")
+  message(STATUS "Build output: ${TSL_CREDENTIALS_OPTIONS_CHECK_OUTPUT}")
+
+  try_compile(HAS_GRPC_127 ${CMAKE_CURRENT_BINARY_DIR}/try_compile SOURCES
+              "${CMAKE_CURRENT_SOURCE_DIR}/try_compile/check_tls_opts_127.cc"
+              CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${CURRENT_INCLUDE_DIRECTORIES}"
+              OUTPUT_VARIABLE TSL_CREDENTIALS_OPTIONS_CHECK_OUTPUT CXX_STANDARD 11)
+
+  if(HAS_GRPC_127)
+    message(STATUS "TlsCredentialsOptions found in grpc_impl::experimental.")
+    add_definitions(-DGRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS=grpc_impl::experimental)
+  else()
+    message(STATUS "TlsCredentialsOptions not found in grpc_impl::experimental.")
+    message(STATUS "Build output: ${TSL_CREDENTIALS_OPTIONS_CHECK_OUTPUT}")
+  endif()
+endif()
+
+# </KLUDGE> Restore the CXXFLAGS that were modified above
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS_BACKUP}")
+
 # Note, we do not compile the generated Protobuf sources directly, instead
 # compiling then via protocol_internal.cc which contains some gRPC template
 # overrides to enable Flight-specific optimizations. See comments in

cpp/src/arrow/flight/client.cc

@@ -30,10 +30,15 @@
 
 #ifdef GRPCPP_PP_INCLUDE
 #include <grpcpp/grpcpp.h>
+#if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)
+#include <grpcpp/security/tls_credentials_options.h>
+#endif
 #else
 #include <grpc++/grpc++.h>
 #endif
 
+#include <grpc/grpc_security_constants.h>
+
 #include "arrow/buffer.h"
 #include "arrow/ipc/reader.h"
 #include "arrow/ipc/writer.h"
@@ -84,6 +89,9 @@ std::shared_ptr<FlightWriteSizeStatusDetail> FlightWriteSizeStatusDetail::Unwrap
   return std::dynamic_pointer_cast<FlightWriteSizeStatusDetail>(status.detail());
 }
 
+FlightClientOptions::FlightClientOptions()
+    : write_size_limit_bytes(0), disable_server_verification(false) {}
+
 FlightClientOptions FlightClientOptions::Defaults() { return FlightClientOptions(); }
 
 struct ClientRpc {
@@ -835,6 +843,31 @@ class GrpcMetadataReader : public FlightMetadataReader {
   std::shared_ptr<std::mutex> read_mutex_;
 };
 
+namespace {
+// Dummy self-signed certificate to be used because TlsCredentials
+// requires root CA certs, even if you are skipping server
+// verification.
+#if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)
+constexpr char BLANK_ROOT_PEM[] =
+    "-----BEGIN CERTIFICATE-----\n"
+    "MIICwzCCAaugAwIBAgIJAM12DOkcaqrhMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNV\n"
+    "BAMTCWxvY2FsaG9zdDAeFw0yMDEwMDcwODIyNDFaFw0zMDEwMDUwODIyNDFaMBQx\n"
+    "EjAQBgNVBAMTCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n"
+    "ggEBALjJ8KPEpF0P4GjMPrJhjIBHUL0AX9E4oWdgJRCSFkPKKEWzQabTQBikMOhI\n"
+    "W4VvBMaHEBuECE5OEyrzDRiAO354I4F4JbBfxMOY8NIW0uWD6THWm2KkCzoZRIPW\n"
+    "yZL6dN+mK6cEH+YvbNuy5ZQGNjGG43tyiXdOCAc4AI9POeTtjdMpbbpR2VY4Ad/E\n"
+    "oTEiS3gNnN7WIAdgMhCJxjzvPwKszV3f7pwuTHzFMsuHLKr6JeaVUYfbi4DxxC8Z\n"
+    "k6PF6dLlLf3ngTSLBJyaXP1BhKMvz0TaMK3F0y2OGwHM9J8np2zWjTlNVEzffQZx\n"
+    "SWMOQManlJGs60xYx9KCPJMZZsMCAwEAAaMYMBYwFAYDVR0RBA0wC4IJbG9jYWxo\n"
+    "b3N0MA0GCSqGSIb3DQEBBQUAA4IBAQC0LrmbcNKgO+D50d/wOc+vhi9K04EZh8bg\n"
+    "WYAK1kLOT4eShbzqWGV/1EggY4muQ6ypSELCLuSsg88kVtFQIeRilA6bHFqQSj6t\n"
+    "sqgh2cWsMwyllCtmX6Maf3CLb2ZdoJlqUwdiBdrbIbuyeAZj3QweCtLKGSQzGDyI\n"
+    "KH7G8nC5d0IoRPiCMB6RnMMKsrhviuCdWbAFHop7Ff36JaOJ8iRa2sSf2OXE8j/5\n"
+    "obCXCUvYHf4Zw27JcM2AnnQI9VJLnYxis83TysC5s2Z7t0OYNS9kFmtXQbUNlmpS\n"
+    "doQ/Eu47vWX7S0TXeGziGtbAOKxbHE0BGGPDOAB/jGW/JVbeTiXY\n"
+    "-----END CERTIFICATE-----\n";
+#endif
+}  // namespace
 class FlightClient::FlightClientImpl {
  public:
   Status Connect(const Location& location, const FlightClientOptions& options) {
@@ -845,18 +878,49 @@ class FlightClient::FlightClientImpl {
     if (scheme == kSchemeGrpc || scheme == kSchemeGrpcTcp || scheme == kSchemeGrpcTls) {
       grpc_uri << location.uri_->host() << ":" << location.uri_->port_text();
 
-      if (scheme == "grpc+tls") {
-        grpc::SslCredentialsOptions ssl_options;
-        if (!options.tls_root_certs.empty()) {
-          ssl_options.pem_root_certs = options.tls_root_certs;
-        }
-        if (!options.cert_chain.empty()) {
-          ssl_options.pem_cert_chain = options.cert_chain;
-        }
-        if (!options.private_key.empty()) {
-          ssl_options.pem_private_key = options.private_key;
+      if (scheme == kSchemeGrpcTls) {
+        if (options.disable_server_verification) {
+#if !defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)
+          return Status::NotImplemented(
+              "Using encryption with server verification disabled is unsupported. "
+              "Please use a release of Arrow Flight built with gRPC 1.27 or higher.");
+#else
+          namespace ge = GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS;
+
+          // A callback to supply to TlsCredentialsOptions that accepts any server
+          // arguments.
+          struct NoOpTlsAuthorizationCheck
+              : public ge::TlsServerAuthorizationCheckInterface {
+            int Schedule(ge::TlsServerAuthorizationCheckArg* arg) override {
+              arg->set_success(1);
+              arg->set_status(GRPC_STATUS_OK);
+              return 0;
+            }
+          };
+
+          noop_auth_check_ = std::make_shared<ge::TlsServerAuthorizationCheckConfig>(
+              std::make_shared<NoOpTlsAuthorizationCheck>());
+          auto materials_config = std::make_shared<ge::TlsKeyMaterialsConfig>();
+          materials_config->set_pem_root_certs(BLANK_ROOT_PEM);
+          ge::TlsCredentialsOptions tls_options(
+              GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE,
+              GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION, materials_config,
+              std::shared_ptr<ge::TlsCredentialReloadConfig>(), noop_auth_check_);
+          creds = ge::TlsCredentials(tls_options);
+#endif
+        } else {
+          grpc::SslCredentialsOptions ssl_options;
+          if (!options.tls_root_certs.empty()) {
+            ssl_options.pem_root_certs = options.tls_root_certs;
+          }
+          if (!options.cert_chain.empty()) {
+            ssl_options.pem_cert_chain = options.cert_chain;
+          }
+          if (!options.private_key.empty()) {
+            ssl_options.pem_private_key = options.private_key;
+          }
+          creds = grpc::SslCredentials(ssl_options);
         }
-        creds = grpc::SslCredentials(ssl_options);
       } else {
         creds = grpc::InsecureChannelCredentials();
       }
@@ -1101,6 +1165,15 @@ class FlightClient::FlightClientImpl {
  private:
   std::unique_ptr<pb::FlightService::Stub> stub_;
   std::shared_ptr<ClientAuthHandler> auth_handler_;
+#if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)
+  // Scope the TlsServerAuthorizationCheckConfig to be at the class instance level, since
+  // it gets created during Connect() and needs to persist to DoAction() calls. gRPC does
+  // not correctly increase the reference count of this object:
+  // https://github.com/grpc/grpc/issues/22287
+  std::shared_ptr<
+      GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS::TlsServerAuthorizationCheckConfig>
+      noop_auth_check_;
+#endif
   int64_t write_size_limit_bytes_;
 };
 

cpp/src/arrow/flight/client.h

@@ -90,6 +90,8 @@ class ARROW_FLIGHT_EXPORT FlightWriteSizeStatusDetail : public arrow::StatusDeta
 
 class ARROW_FLIGHT_EXPORT FlightClientOptions {
  public:
+  FlightClientOptions();
+
   /// \brief Root certificates to use for validating server
   /// certificates.
   std::string tls_root_certs;
@@ -108,10 +110,14 @@ class ARROW_FLIGHT_EXPORT FlightClientOptions {
   /// positive. When enabled, FlightStreamWriter.Write* may yield a
   /// IOError with error detail FlightWriteSizeStatusDetail.
   int64_t write_size_limit_bytes;
+
   /// \brief Generic connection options, passed to the underlying
   ///     transport; interpretation is implementation-dependent.
   std::vector<std::pair<std::string, util::variant<int, std::string>>> generic_options;
 
+  /// \brief Use TLS without validating the server certificate. Use with caution.
+  bool disable_server_verification;
+
   /// \brief Get default options.
   static FlightClientOptions Defaults();
 };

cpp/src/arrow/flight/flight_test.cc

@@ -1800,6 +1800,32 @@ TEST_F(TestTls, DoAction) {
   ASSERT_EQ(result->body->ToString(), "Hello, world!");
 }
 
+#if defined(GRPC_NAMESPACE_FOR_TLS_CREDENTIALS_OPTIONS)
+TEST_F(TestTls, DisableServerVerification) {
+  std::unique_ptr<FlightClient> client;
+  auto client_options = FlightClientOptions();
+  // For security reasons, if encryption is being used,
+  // the client should be configured to verify the server by default.
+  ASSERT_EQ(client_options.disable_server_verification, false);
+  client_options.disable_server_verification = true;
+  ASSERT_OK(FlightClient::Connect(location_, client_options, &client));
+
+  FlightCallOptions options;
+  options.timeout = TimeoutDuration{5.0};
+  Action action;
+  action.type = "test";
+  action.body = Buffer::FromString("");
+  std::unique_ptr<ResultStream> results;
+  ASSERT_OK(client->DoAction(options, action, &results));
+  ASSERT_NE(results, nullptr);
+
+  std::unique_ptr<Result> result;
+  ASSERT_OK(results->Next(&result));
+  ASSERT_NE(result, nullptr);
+  ASSERT_EQ(result->body->ToString(), "Hello, world!");
+}
+#endif
+
 TEST_F(TestTls, OverrideHostname) {
   std::unique_ptr<FlightClient> client;
   auto client_options = FlightClientOptions();

cpp/src/arrow/flight/try_compile/check_tls_opts_127.cc

@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you 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.
+
+// Dummy file for checking if TlsCredentialsOptions exists in
+// the grpc_impl::experimental namespace. gRPC versions 1.27-1.31
+// put it here. This is for supporting disabling server
+// validation when using TLS.
+
+#include <grpc/grpc_security_constants.h>
+#include <grpcpp/grpcpp.h>
+#include <grpcpp/security/tls_credentials_options.h>
+
+static grpc_tls_server_verification_option check(
+    const grpc_impl::experimental::TlsCredentialsOptions* options) {
+  grpc_tls_server_verification_option server_opt = options->server_verification_option();
+  return server_opt;
+}
+
+int main(int argc, const char** argv) {
+  grpc_tls_server_verification_option opt = check(nullptr);
+  return 0;
+}

cpp/src/arrow/flight/try_compile/check_tls_opts_132.cc

@@ -0,0 +1,36 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you 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.
+
+// Dummy file for checking if TlsCredentialsOptions exists in
+// the grpc::experimental namespace. gRPC versions 1.32 and higher
+// put it here. This is for supporting disabling server
+// validation when using TLS.
+
+#include <grpc/grpc_security_constants.h>
+#include <grpcpp/grpcpp.h>
+#include <grpcpp/security/tls_credentials_options.h>
+
+static grpc_tls_server_verification_option check(
+    const grpc::experimental::TlsCredentialsOptions* options) {
+  grpc_tls_server_verification_option server_opt = options->server_verification_option();
+  return server_opt;
+}
+
+int main(int argc, const char** argv) {
+  grpc_tls_server_verification_option opt = check(nullptr);
+  return 0;
+}