From 51bccbdcf95334a98ad37e64ced50ea609895a53 Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Wed, 21 Feb 2024 13:38:05 -0800 Subject: [PATCH] [ssl] Support Windows system roots. (#34874) This PR is copied from #34276, since I did not have permissions to add commits to it. That PR has been verified to work (see the top-level description). This PR just makes the gRPC tests pass (e.g. adding includes, clang formatting). Closes #34874 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/34874 from matthewstevenson88:pull_34276 d5fb73e5b2d21dbdfc45d99d8252353393d7946b PiperOrigin-RevId: 609107146 --- BUILD | 1 + CMakeLists.txt | 3 + Makefile | 4 +- Package.swift | 1 + build_autogenerated.yaml | 3 + config.m4 | 1 + config.w32 | 1 + gRPC-Core.podspec | 1 + grpc.gemspec | 1 + grpc.gyp | 3 + package.xml | 1 + .../security_connector/load_system_roots.h | 3 + .../load_system_roots_fallback.cc | 5 +- .../load_system_roots_windows.cc | 87 +++++++++++++++++++ src/python/grpcio/grpc_core_dependencies.py | 1 + templates/Makefile.template | 2 +- test/core/security/system_roots_test.cc | 20 ++++- tools/doxygen/Doxyfile.c++.internal | 1 + tools/doxygen/Doxyfile.core.internal | 1 + 19 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 src/core/lib/security/security_connector/load_system_roots_windows.cc diff --git a/BUILD b/BUILD index d62fec7e444..96b6778cb0f 100644 --- a/BUILD +++ b/BUILD @@ -3723,6 +3723,7 @@ grpc_cc_library( "//src/core:lib/security/credentials/tls/tls_utils.cc", "//src/core:lib/security/security_connector/load_system_roots_fallback.cc", "//src/core:lib/security/security_connector/load_system_roots_supported.cc", + "//src/core:lib/security/security_connector/load_system_roots_windows.cc", "//src/core:lib/security/util/json_util.cc", ], hdrs = [ diff --git a/CMakeLists.txt b/CMakeLists.txt index 8774477efdc..f35a11e9bef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2431,6 +2431,7 @@ add_library(grpc src/core/lib/security/security_connector/insecure/insecure_security_connector.cc src/core/lib/security/security_connector/load_system_roots_fallback.cc src/core/lib/security/security_connector/load_system_roots_supported.cc + src/core/lib/security/security_connector/load_system_roots_windows.cc src/core/lib/security/security_connector/local/local_security_connector.cc src/core/lib/security/security_connector/security_connector.cc src/core/lib/security/security_connector/ssl/ssl_security_connector.cc @@ -3162,6 +3163,7 @@ add_library(grpc_unsecure src/core/lib/security/security_connector/insecure/insecure_security_connector.cc src/core/lib/security/security_connector/load_system_roots_fallback.cc src/core/lib/security/security_connector/load_system_roots_supported.cc + src/core/lib/security/security_connector/load_system_roots_windows.cc src/core/lib/security/security_connector/security_connector.cc src/core/lib/security/transport/client_auth_filter.cc src/core/lib/security/transport/legacy_server_auth_filter.cc @@ -5288,6 +5290,7 @@ add_library(grpc_authorization_provider src/core/lib/security/credentials/tls/tls_utils.cc src/core/lib/security/security_connector/load_system_roots_fallback.cc src/core/lib/security/security_connector/load_system_roots_supported.cc + src/core/lib/security/security_connector/load_system_roots_windows.cc src/core/lib/security/security_connector/security_connector.cc src/core/lib/security/transport/client_auth_filter.cc src/core/lib/security/transport/legacy_server_auth_filter.cc diff --git a/Makefile b/Makefile index 3b327e73096..b4dbdfea859 100644 --- a/Makefile +++ b/Makefile @@ -375,7 +375,7 @@ LDFLAGS += -pthread endif ifeq ($(SYSTEM),MINGW32) -LIBS = m pthread ws2_32 iphlpapi dbghelp bcrypt +LIBS = m pthread ws2_32 crypt32 iphlpapi dbghelp bcrypt LDFLAGS += -pthread endif @@ -1613,6 +1613,7 @@ LIBGRPC_SRC = \ src/core/lib/security/security_connector/insecure/insecure_security_connector.cc \ src/core/lib/security/security_connector/load_system_roots_fallback.cc \ src/core/lib/security/security_connector/load_system_roots_supported.cc \ + src/core/lib/security/security_connector/load_system_roots_windows.cc \ src/core/lib/security/security_connector/local/local_security_connector.cc \ src/core/lib/security/security_connector/security_connector.cc \ src/core/lib/security/security_connector/ssl/ssl_security_connector.cc \ @@ -2178,6 +2179,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/security/security_connector/insecure/insecure_security_connector.cc \ src/core/lib/security/security_connector/load_system_roots_fallback.cc \ src/core/lib/security/security_connector/load_system_roots_supported.cc \ + src/core/lib/security/security_connector/load_system_roots_windows.cc \ src/core/lib/security/security_connector/security_connector.cc \ src/core/lib/security/transport/client_auth_filter.cc \ src/core/lib/security/transport/legacy_server_auth_filter.cc \ diff --git a/Package.swift b/Package.swift index 2f056910c40..6156daefbb9 100644 --- a/Package.swift +++ b/Package.swift @@ -1720,6 +1720,7 @@ let package = Package( "src/core/lib/security/security_connector/load_system_roots_fallback.cc", "src/core/lib/security/security_connector/load_system_roots_supported.cc", "src/core/lib/security/security_connector/load_system_roots_supported.h", + "src/core/lib/security/security_connector/load_system_roots_windows.cc", "src/core/lib/security/security_connector/local/local_security_connector.cc", "src/core/lib/security/security_connector/local/local_security_connector.h", "src/core/lib/security/security_connector/security_connector.cc", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 3edb88fe556..ec54cf6353a 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1887,6 +1887,7 @@ libs: - src/core/lib/security/security_connector/insecure/insecure_security_connector.cc - src/core/lib/security/security_connector/load_system_roots_fallback.cc - src/core/lib/security/security_connector/load_system_roots_supported.cc + - src/core/lib/security/security_connector/load_system_roots_windows.cc - src/core/lib/security/security_connector/local/local_security_connector.cc - src/core/lib/security/security_connector/security_connector.cc - src/core/lib/security/security_connector/ssl/ssl_security_connector.cc @@ -2970,6 +2971,7 @@ libs: - src/core/lib/security/security_connector/insecure/insecure_security_connector.cc - src/core/lib/security/security_connector/load_system_roots_fallback.cc - src/core/lib/security/security_connector/load_system_roots_supported.cc + - src/core/lib/security/security_connector/load_system_roots_windows.cc - src/core/lib/security/security_connector/security_connector.cc - src/core/lib/security/transport/client_auth_filter.cc - src/core/lib/security/transport/legacy_server_auth_filter.cc @@ -4929,6 +4931,7 @@ libs: - src/core/lib/security/credentials/tls/tls_utils.cc - src/core/lib/security/security_connector/load_system_roots_fallback.cc - src/core/lib/security/security_connector/load_system_roots_supported.cc + - src/core/lib/security/security_connector/load_system_roots_windows.cc - src/core/lib/security/security_connector/security_connector.cc - src/core/lib/security/transport/client_auth_filter.cc - src/core/lib/security/transport/legacy_server_auth_filter.cc diff --git a/config.m4 b/config.m4 index 1dbc1b1e324..c823a980b8f 100644 --- a/config.m4 +++ b/config.m4 @@ -741,6 +741,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/security/security_connector/insecure/insecure_security_connector.cc \ src/core/lib/security/security_connector/load_system_roots_fallback.cc \ src/core/lib/security/security_connector/load_system_roots_supported.cc \ + src/core/lib/security/security_connector/load_system_roots_windows.cc \ src/core/lib/security/security_connector/local/local_security_connector.cc \ src/core/lib/security/security_connector/security_connector.cc \ src/core/lib/security/security_connector/ssl/ssl_security_connector.cc \ diff --git a/config.w32 b/config.w32 index 18f0c8e8725..2275f0669b5 100644 --- a/config.w32 +++ b/config.w32 @@ -706,6 +706,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\security\\security_connector\\insecure\\insecure_security_connector.cc " + "src\\core\\lib\\security\\security_connector\\load_system_roots_fallback.cc " + "src\\core\\lib\\security\\security_connector\\load_system_roots_supported.cc " + + "src\\core\\lib\\security\\security_connector\\load_system_roots_windows.cc " + "src\\core\\lib\\security\\security_connector\\local\\local_security_connector.cc " + "src\\core\\lib\\security\\security_connector\\security_connector.cc " + "src\\core\\lib\\security\\security_connector\\ssl\\ssl_security_connector.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index f55abe72e8e..eba3b22cdbf 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1829,6 +1829,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/security_connector/load_system_roots_fallback.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.h', + 'src/core/lib/security/security_connector/load_system_roots_windows.cc', 'src/core/lib/security/security_connector/local/local_security_connector.cc', 'src/core/lib/security/security_connector/local/local_security_connector.h', 'src/core/lib/security/security_connector/security_connector.cc', diff --git a/grpc.gemspec b/grpc.gemspec index 6ceb9e0e50a..c46ae2adcca 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1722,6 +1722,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/security/security_connector/load_system_roots_fallback.cc ) s.files += %w( src/core/lib/security/security_connector/load_system_roots_supported.cc ) s.files += %w( src/core/lib/security/security_connector/load_system_roots_supported.h ) + s.files += %w( src/core/lib/security/security_connector/load_system_roots_windows.cc ) s.files += %w( src/core/lib/security/security_connector/local/local_security_connector.cc ) s.files += %w( src/core/lib/security/security_connector/local/local_security_connector.h ) s.files += %w( src/core/lib/security/security_connector/security_connector.cc ) diff --git a/grpc.gyp b/grpc.gyp index 045162a458c..6448b135e88 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -927,6 +927,7 @@ 'src/core/lib/security/security_connector/insecure/insecure_security_connector.cc', 'src/core/lib/security/security_connector/load_system_roots_fallback.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.cc', + 'src/core/lib/security/security_connector/load_system_roots_windows.cc', 'src/core/lib/security/security_connector/local/local_security_connector.cc', 'src/core/lib/security/security_connector/security_connector.cc', 'src/core/lib/security/security_connector/ssl/ssl_security_connector.cc', @@ -1432,6 +1433,7 @@ 'src/core/lib/security/security_connector/insecure/insecure_security_connector.cc', 'src/core/lib/security/security_connector/load_system_roots_fallback.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.cc', + 'src/core/lib/security/security_connector/load_system_roots_windows.cc', 'src/core/lib/security/security_connector/security_connector.cc', 'src/core/lib/security/transport/client_auth_filter.cc', 'src/core/lib/security/transport/legacy_server_auth_filter.cc', @@ -2240,6 +2242,7 @@ 'src/core/lib/security/credentials/tls/tls_utils.cc', 'src/core/lib/security/security_connector/load_system_roots_fallback.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.cc', + 'src/core/lib/security/security_connector/load_system_roots_windows.cc', 'src/core/lib/security/security_connector/security_connector.cc', 'src/core/lib/security/transport/client_auth_filter.cc', 'src/core/lib/security/transport/legacy_server_auth_filter.cc', diff --git a/package.xml b/package.xml index 27eececb580..6720e2ebe1e 100644 --- a/package.xml +++ b/package.xml @@ -1704,6 +1704,7 @@ + diff --git a/src/core/lib/security/security_connector/load_system_roots.h b/src/core/lib/security/security_connector/load_system_roots.h index 153acf7f906..36303d979d1 100644 --- a/src/core/lib/security/security_connector/load_system_roots.h +++ b/src/core/lib/security/security_connector/load_system_roots.h @@ -25,6 +25,9 @@ namespace grpc_core { +// TODO(matthewstevenson88): Update LoadSystemRootCerts to use Slice +// instead of grpc_slice. + // Returns a slice containing roots from the OS trust store grpc_slice LoadSystemRootCerts(); diff --git a/src/core/lib/security/security_connector/load_system_roots_fallback.cc b/src/core/lib/security/security_connector/load_system_roots_fallback.cc index 45231c9aef9..90a16a8314e 100644 --- a/src/core/lib/security/security_connector/load_system_roots_fallback.cc +++ b/src/core/lib/security/security_connector/load_system_roots_fallback.cc @@ -19,7 +19,7 @@ #include #if !defined(GPR_LINUX) && !defined(GPR_ANDROID) && !defined(GPR_FREEBSD) && \ - !defined(GPR_APPLE) + !defined(GPR_APPLE) && !defined(GPR_WINDOWS) #include #include @@ -32,4 +32,5 @@ grpc_slice LoadSystemRootCerts() { return grpc_empty_slice(); } } // namespace grpc_core -#endif // !(GPR_LINUX || GPR_ANDROID || GPR_FREEBSD || GPR_APPLE) +#endif // !(GPR_LINUX || GPR_ANDROID || GPR_FREEBSD || GPR_APPLE || + // GPR_WINDOWS) diff --git a/src/core/lib/security/security_connector/load_system_roots_windows.cc b/src/core/lib/security/security_connector/load_system_roots_windows.cc new file mode 100644 index 00000000000..280d0900410 --- /dev/null +++ b/src/core/lib/security/security_connector/load_system_roots_windows.cc @@ -0,0 +1,87 @@ +// +// +// Copyright 2023 gRPC authors. +// +// Licensed 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. +// +// + +#include + +#if defined(GPR_WINDOWS) + +#pragma comment(lib, "crypt32") + +#include +#include + +#include + +#include +#include +#include +#include + +#include "src/core/lib/gpr/useful.h" +#include "src/core/lib/security/security_connector/load_system_roots.h" +#include "src/core/lib/slice/slice_internal.h" + +namespace grpc_core { +namespace { + +std::string Utf8Encode(const std::wstring& wstr) { + if (wstr.empty()) return ""; + + int size_needed = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), + NULL, 0, NULL, NULL); + std::string str_to(size_needed, 0); + WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &str_to[0], + size_needed, NULL, NULL); + return str_to; +} + +} // namespace + +grpc_slice LoadSystemRootCerts() { + std::string bundle_string; + + // Open root certificate store. + HANDLE root_cert_store = CertOpenSystemStoreW(NULL, L"ROOT"); + if (!root_cert_store) { + return grpc_empty_slice(); + } + + // Load all root certificates from certificate store. + PCCERT_CONTEXT cert = NULL; + while ((cert = CertEnumCertificatesInStore(root_cert_store, cert)) != NULL) { + // Append each certificate in PEM format. + DWORD size = 0; + CryptBinaryToStringW(cert->pbCertEncoded, cert->cbCertEncoded, + CRYPT_STRING_BASE64HEADER, NULL, &size); + std::vector pem(size); + CryptBinaryToStringW(cert->pbCertEncoded, cert->cbCertEncoded, + CRYPT_STRING_BASE64HEADER, pem.data(), &size); + bundle_string += Utf8Encode(pem.data()); + } + + CertCloseStore(root_cert_store, 0); + if (bundle_string.size() == 0) { + return grpc_empty_slice(); + } + + return grpc_slice_from_cpp_string(std::move(bundle_string)); +} + +} // namespace grpc_core + +#endif // GPR_WINDOWS diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 78472d72eb9..a171c782d35 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -715,6 +715,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/security/security_connector/insecure/insecure_security_connector.cc', 'src/core/lib/security/security_connector/load_system_roots_fallback.cc', 'src/core/lib/security/security_connector/load_system_roots_supported.cc', + 'src/core/lib/security/security_connector/load_system_roots_windows.cc', 'src/core/lib/security/security_connector/local/local_security_connector.cc', 'src/core/lib/security/security_connector/security_connector.cc', 'src/core/lib/security/security_connector/ssl/ssl_security_connector.cc', diff --git a/templates/Makefile.template b/templates/Makefile.template index 372259a38da..490b3d0fb53 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -403,7 +403,7 @@ endif ifeq ($(SYSTEM),MINGW32) - LIBS = m pthread ws2_32 iphlpapi dbghelp bcrypt + LIBS = m pthread ws2_32 crypt32 iphlpapi dbghelp bcrypt LDFLAGS += -pthread endif diff --git a/test/core/security/system_roots_test.cc b/test/core/security/system_roots_test.cc index 17cbaa59208..0bc6f251972 100644 --- a/test/core/security/system_roots_test.cc +++ b/test/core/security/system_roots_test.cc @@ -20,9 +20,12 @@ #include -#if defined(GPR_LINUX) || defined(GPR_FREEBSD) || defined(GPR_APPLE) +#if defined(GPR_LINUX) || defined(GPR_FREEBSD) || defined(GPR_APPLE) || \ + defined(GPR_WINDOWS) #include +#if defined(GPR_LINUX) || defined(GPR_FREEBSD) || defined(GPR_APPLE) #include +#endif // GPR_LINUX || GPR_FREEBSD || GPR_APPLE #include "gtest/gtest.h" @@ -48,6 +51,10 @@ namespace grpc { namespace { +// The GetAbsoluteFilePath and CreateRootCertsBundle helper functions are only +// defined on some platforms. On other platforms (e.g. Windows), we rely on +// built-in helper functions to play similar (but not exactly the same) roles. +#if defined(GPR_LINUX) || defined(GPR_FREEBSD) || defined(GPR_APPLE) TEST(AbsoluteFilePathTest, ConcatenatesCorrectly) { const char* directory = "nonexistent/test/directory"; const char* filename = "doesnotexist.txt"; @@ -80,6 +87,15 @@ TEST(CreateRootCertsBundleTest, BundlesCorrectly) { << "Expected: \"" << result_slice.as_string_view() << "\"\n" << "Actual: \"" << roots_bundle_str << "\""; } +#endif // GPR_LINUX || GPR_FREEBSD || GPR_APPLE + +#if defined(GPR_WINDOWS) +TEST(LoadSystemRootCertsTest, Success) { + grpc_slice roots_slice = grpc_core::LoadSystemRootCerts(); + EXPECT_FALSE(GRPC_SLICE_IS_EMPTY(roots_slice)); + grpc_slice_unref(roots_slice); +} +#endif // GPR_WINDOWS } // namespace } // namespace grpc @@ -96,4 +112,4 @@ int main() { "systems ***\n"); return 0; } -#endif // GPR_LINUX || GPR_FREEBSD || GPR_APPLE +#endif // GPR_LINUX || GPR_FREEBSD || GPR_APPLE || GPR_WINDOWS diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index c197f62a38a..91da0bd2666 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2721,6 +2721,7 @@ src/core/lib/security/security_connector/load_system_roots.h \ src/core/lib/security/security_connector/load_system_roots_fallback.cc \ src/core/lib/security/security_connector/load_system_roots_supported.cc \ src/core/lib/security/security_connector/load_system_roots_supported.h \ +src/core/lib/security/security_connector/load_system_roots_windows.cc \ src/core/lib/security/security_connector/local/local_security_connector.cc \ src/core/lib/security/security_connector/local/local_security_connector.h \ src/core/lib/security/security_connector/security_connector.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 8b61684a971..d328e17185f 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2496,6 +2496,7 @@ src/core/lib/security/security_connector/load_system_roots.h \ src/core/lib/security/security_connector/load_system_roots_fallback.cc \ src/core/lib/security/security_connector/load_system_roots_supported.cc \ src/core/lib/security/security_connector/load_system_roots_supported.h \ +src/core/lib/security/security_connector/load_system_roots_windows.cc \ src/core/lib/security/security_connector/local/local_security_connector.cc \ src/core/lib/security/security_connector/local/local_security_connector.h \ src/core/lib/security/security_connector/security_connector.cc \