Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl(common+storage): remove OpenSSL dependency on Windows #13785

Merged
merged 56 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
394ff96
move OpenSSL header to implementation file
teo-tsirpanis Mar 15, 2024
5e668e1
port google_cloud_cpp_common to use Windows crypto APIs
teo-tsirpanis Mar 15, 2024
cb1ec7c
port google_cloud_cpp_rest_internal to use Windows crypto APIs
teo-tsirpanis Mar 16, 2024
0af9bb6
forward `google::cloud::storage::internal::SignStringWithPem` to `goo…
teo-tsirpanis Mar 16, 2024
d9c3e96
define `WIN32_LEAN_AND_MEAN` from build system and add it in CMake as…
teo-tsirpanis Mar 16, 2024
eedb3e5
fix copy-paste typo
teo-tsirpanis Mar 16, 2024
900660a
port google_cloud_cpp_storage to use Windows crypto APIs
teo-tsirpanis Mar 16, 2024
6a14b35
explicitly list OpenSSL as a non-Windows dependency in vcpkg.json
teo-tsirpanis Mar 16, 2024
aa21ced
fix calls to `find_dependency(OpenSSL)`
teo-tsirpanis Mar 16, 2024
0e9634b
fix typo
teo-tsirpanis Mar 16, 2024
1f05827
format Bazel files
teo-tsirpanis Mar 16, 2024
bb6d7ff
fix typos
teo-tsirpanis Mar 16, 2024
d6ba802
remove unnecessary references to BoringSSL on Bazel
teo-tsirpanis Mar 16, 2024
966a85a
change the scope of some conditionals
teo-tsirpanis Mar 16, 2024
f451ca2
Merge branch 'main' into win32-crypto
teo-tsirpanis Mar 22, 2024
49cc636
reorder headers
teo-tsirpanis Mar 22, 2024
f284e3d
improve reporting errors
teo-tsirpanis Mar 22, 2024
3f62ebc
refactor `hash_function_impl` to avoid static casts to handle types.
teo-tsirpanis Mar 22, 2024
12367c2
rename `internal/openssl_utils` to `pem_signing` and split the platfo…
teo-tsirpanis Mar 22, 2024
c527adf
split the platform-specific code of `oauth2_service_account_credentia…
teo-tsirpanis Mar 22, 2024
3a8fbad
move `FormatWin32Errors` to a new header and use it in more places
teo-tsirpanis Mar 22, 2024
0b364f4
Merge branch 'main' into win32-crypto
teo-tsirpanis Mar 22, 2024
54ef0bd
add back Win32 MD5 implementation after the rebase
teo-tsirpanis Mar 22, 2024
16e691f
change the scope for some conditionals, similar to what was done in MD5
teo-tsirpanis Mar 22, 2024
4cb351b
support specifying libs and platform-specific requirements in pkg-con…
teo-tsirpanis Mar 22, 2024
e881a45
Merge branch 'main' into win32-crypto
teo-tsirpanis Mar 22, 2024
70bc2e7
ignore `CMakeUserPresets.json`
teo-tsirpanis Mar 22, 2024
26f32ba
split the platform-specific code of `hash_function_impl.cc`
teo-tsirpanis Mar 22, 2024
c65856d
add missing header
teo-tsirpanis Mar 22, 2024
29a4206
fix copyright years
teo-tsirpanis Mar 23, 2024
9050343
simplify PEM encoding
teo-tsirpanis Mar 23, 2024
34c0a9d
change `kP12PrivateKeyIdMarker` to an inline function
teo-tsirpanis Mar 24, 2024
d14ca28
add header
teo-tsirpanis Mar 24, 2024
08391a5
move the bulk of `FormatWin32Errors` to an implementation source file
teo-tsirpanis Mar 24, 2024
27da91f
rename `pem_signing` to `sign_using_sha256`
teo-tsirpanis Mar 24, 2024
adb4b20
refactor `ParseServiceAccountP12File` to use multiple functions and d…
teo-tsirpanis Mar 24, 2024
abcf520
move `ParseServiceAccountP12File` to its own header and fix bugs
teo-tsirpanis Mar 24, 2024
3e0085a
split `MD5HashFunction` into two subclasses for BCrypt/OpenSSL implem…
teo-tsirpanis Mar 24, 2024
4f354d5
break down `ExportCertificatePrivateKey` into three functions
teo-tsirpanis Mar 24, 2024
38b7203
add missing header
teo-tsirpanis Mar 24, 2024
2df8d6e
update more usages of `MD5HashFunction`
teo-tsirpanis Mar 24, 2024
986b64a
fix inconsistent missing override warning
teo-tsirpanis Mar 24, 2024
a0ca7a5
fix formatting
teo-tsirpanis Mar 28, 2024
bad3c28
fix copyright headers of renamed files
teo-tsirpanis Mar 28, 2024
baf1cb4
avoid some reinterpret casts
teo-tsirpanis Mar 28, 2024
a8a68e5
fix clang-tidy warning
teo-tsirpanis Mar 28, 2024
60ce35b
change `UniquePtrReinterpretProxy` to have the unique pointer as a pr…
teo-tsirpanis Mar 28, 2024
46b413b
fail if the p12 file has the bad bit after reading
teo-tsirpanis Mar 28, 2024
dca0bf4
clang-format
teo-tsirpanis Mar 28, 2024
e2554fb
add new directories with installed headers
teo-tsirpanis Mar 28, 2024
aeeb6f6
define `_WIN32_WINNT` to Windows 10
teo-tsirpanis Mar 28, 2024
0ad1dc9
add comment on casting the size of p12 files to 32-bit unsigned integer
teo-tsirpanis Mar 28, 2024
cdd4cfe
clang-format
teo-tsirpanis Mar 28, 2024
9b928f7
Revert "clang-format"
teo-tsirpanis Mar 28, 2024
919fa1e
format and cleanup
teo-tsirpanis Mar 28, 2024
d2444bd
Merge branch 'main' into win32-crypto
teo-tsirpanis Mar 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ cmake-build-*/
# `google-cloud-cpp` developers use this file to configure the development
# workflow build.
.cloudcxxrc

# Ignore the file with user-defined CMake presets
CMakeUserPresets.json
1 change: 1 addition & 0 deletions ci/cloudbuild/builds/cmake-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ expected_dirs+=(
./include/google/cloud/gkehub/v1/multiclusteringress
./include/google/cloud/grpc_utils
./include/google/cloud/internal
./include/google/cloud/internal/win32
# no RPC services in google/cloud/metastore/logging
./include/google/cloud/metastore/logging
./include/google/cloud/metastore/logging/v1
Expand Down
13 changes: 12 additions & 1 deletion cmake/AddPkgConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ endmacro ()
# * ARGN: the names of any pkgconfig modules the generated module depends on
#
function (google_cloud_cpp_add_pkgconfig library name description)
cmake_parse_arguments(_opt "WITH_SHORT_TARGET" "" "" ${ARGN})
cmake_parse_arguments(_opt "WITH_SHORT_TARGET" "" "LIBS;WIN32_LIBS;NON_WIN32_LIBS;WIN32_REQUIRES;NON_WIN32_REQUIRES" ${ARGN})
if (_opt_WITH_SHORT_TARGET)
set(target "${library}")
else ()
Expand All @@ -60,6 +60,17 @@ function (google_cloud_cpp_add_pkgconfig library name description)
else ()
set(GOOGLE_CLOUD_CPP_PC_LIBS "-lgoogle_cloud_cpp_${library}")
endif ()
list(TRANSFORM _opt_LIBS PREPEND "-l" OUTPUT_VARIABLE _opt_LIBS)
teo-tsirpanis marked this conversation as resolved.
Show resolved Hide resolved
string(JOIN " " GOOGLE_CLOUD_CPP_PC_LIBS "${GOOGLE_CLOUD_CPP_PC_LIBS}" ${_opt_LIBS})
if (WIN32)
list(TRANSFORM _opt_WIN32_LIBS PREPEND "-l" OUTPUT_VARIABLE _opt_WIN32_LIBS)
string(JOIN " " GOOGLE_CLOUD_CPP_PC_LIBS "${GOOGLE_CLOUD_CPP_PC_LIBS}" ${_opt_WIN32_LIBS})
string(JOIN " " GOOGLE_CLOUD_CPP_PC_REQUIRES "${GOOGLE_CLOUD_CPP_PC_REQUIRES}" ${_opt_WIN32_REQUIRES})
else ()
list(TRANSFORM _opt_NON_WIN32_LIBS PREPEND "-l" OUTPUT_VARIABLE _opt_NON_WIN32_LIBS)
string(JOIN " " GOOGLE_CLOUD_CPP_PC_LIBS "${GOOGLE_CLOUD_CPP_PC_LIBS}" ${_opt_NON_WIN32_LIBS})
string(JOIN " " GOOGLE_CLOUD_CPP_PC_REQUIRES "${GOOGLE_CLOUD_CPP_PC_REQUIRES}" ${_opt_NON_WIN32_REQUIRES})
endif ()
get_target_property(target_defs ${target} INTERFACE_COMPILE_DEFINITIONS)
if (target_defs)
foreach (def ${target_defs})
Expand Down
41 changes: 34 additions & 7 deletions google/cloud/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,19 @@ cc_library(
],
"//conditions:default": [],
}),
linkopts = select({
"@platforms//os:windows": [
"-DEFAULTLIB:bcrypt.lib",
],
"//conditions:default": [],
}),
local_defines = select({
"@platforms//os:windows": [
"WIN32_LEAN_AND_MEAN",
"_WIN32_WINNT=0x0A00",
],
"//conditions:default": [],
}),
target_compatible_with = select(
{
":enable_opentelemetry_valid": [],
Expand All @@ -99,7 +112,6 @@ to your build command, or set this value in your `.bazelrc` file(s).
"//:__pkg__",
],
deps = [
"@boringssl//:crypto",
teo-tsirpanis marked this conversation as resolved.
Show resolved Hide resolved
"@com_google_absl//absl/base",
"@com_google_absl//absl/functional:function_ref",
"@com_google_absl//absl/strings",
Expand All @@ -113,6 +125,11 @@ to your build command, or set this value in your `.bazelrc` file(s).
"@io_opentelemetry_cpp//api",
],
"//conditions:default": [],
}) + select({
"@platforms//os:windows": [],
"//conditions:default": [
"@boringssl//:crypto",
],
}),
)

Expand Down Expand Up @@ -265,24 +282,34 @@ cc_library(
name = "google_cloud_cpp_rest_internal",
srcs = google_cloud_cpp_rest_internal_srcs,
hdrs = google_cloud_cpp_rest_internal_hdrs,
# These are needed to use the BoringSSL libraries, and should be set in
# any downstream dependency too.
defines = select({
linkopts = select({
"@platforms//os:windows": [
"-DEFAULTLIB:bcrypt.lib",
"-DEFAULTLIB:crypt32.lib",
],
"//conditions:default": [],
}),
local_defines = select({
"@platforms//os:windows": [
"WIN32_LEAN_AND_MEAN",
"_WIN32_WINNT=0x0A00",
],
"//conditions:default": [],
}),
visibility = ["//:__subpackages__"],
deps = [
":google_cloud_cpp_common",
"@boringssl//:crypto",
"@boringssl//:ssl",
"@com_github_curl_curl//:curl",
"@com_github_nlohmann_json//:nlohmann_json",
"@com_google_absl//absl/functional:function_ref",
"@com_google_absl//absl/types:span",
],
] + select({
"@platforms//os:windows": [],
"//conditions:default": [
"@boringssl//:crypto",
"@boringssl//:ssl",
],
}),
)

[cc_test(
Expand Down
4 changes: 3 additions & 1 deletion google/cloud/config-rest.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ find_dependency(google_cloud_cpp_common)
find_dependency(absl)
find_dependency(CURL)
find_dependency(nlohmann_json)
find_dependency(OpenSSL)
if (NOT WIN32)
find_dependency(OpenSSL)
endif ()

include("${CMAKE_CURRENT_LIST_DIR}/google_cloud_cpp_rest_internal-targets.cmake")
18 changes: 13 additions & 5 deletions google/cloud/google_cloud_cpp_common.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
# limitations under the License.
# ~~~

find_package(OpenSSL REQUIRED)
if (NOT WIN32)
find_package(OpenSSL REQUIRED)
endif ()

# Generate the version information from the CMake values.
configure_file(internal/version_info.h.in
Expand Down Expand Up @@ -179,8 +181,13 @@ target_link_libraries(
absl::str_format
absl::time
absl::variant
Threads::Threads
OpenSSL::Crypto)
Threads::Threads)
if (WIN32)
target_compile_definitions(google_cloud_cpp_common PRIVATE WIN32_LEAN_AND_MEAN _WIN32_WINNT=0x0A00)
coryan marked this conversation as resolved.
Show resolved Hide resolved
target_link_libraries(google_cloud_cpp_common PUBLIC bcrypt)
else ()
target_link_libraries(google_cloud_cpp_common PUBLIC OpenSSL::Crypto)
endif ()

if (opentelemetry IN_LIST GOOGLE_CLOUD_CPP_ENABLE)
find_package(opentelemetry-cpp CONFIG)
Expand Down Expand Up @@ -249,8 +256,9 @@ google_cloud_cpp_add_pkgconfig(
"absl_time"
"absl_time_zone"
"absl_variant"
"openssl"
"${GOOGLE_CLOUD_CPP_OPENTELEMETRY_API}")
"${GOOGLE_CLOUD_CPP_OPENTELEMETRY_API}"
NON_WIN32_REQUIRES openssl
WIN32_LIBS bcrypt)

# Create and install the CMake configuration files.
configure_file("config.cmake.in" "google_cloud_cpp_common-config.cmake" @ONLY)
Expand Down
10 changes: 8 additions & 2 deletions google/cloud/google_cloud_cpp_rest_internal.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ google_cloud_cpp_rest_internal_hdrs = [
"internal/oauth2_refreshing_credentials_wrapper.h",
"internal/oauth2_service_account_credentials.h",
"internal/oauth2_universe_domain.h",
"internal/openssl_util.h",
"internal/parse_service_account_p12_file.h",
"internal/populate_rest_options.h",
"internal/rest_carrier.h",
"internal/rest_client.h",
Expand All @@ -65,10 +65,12 @@ google_cloud_cpp_rest_internal_hdrs = [
"internal/rest_request.h",
"internal/rest_response.h",
"internal/rest_retry_loop.h",
"internal/sign_using_sha256.h",
"internal/tracing_http_payload.h",
"internal/tracing_rest_client.h",
"internal/tracing_rest_response.h",
"internal/unified_rest_credentials.h",
"internal/win32/win32_helpers.h",
"rest_options.h",
]

Expand Down Expand Up @@ -105,7 +107,8 @@ google_cloud_cpp_rest_internal_srcs = [
"internal/oauth2_refreshing_credentials_wrapper.cc",
"internal/oauth2_service_account_credentials.cc",
"internal/oauth2_universe_domain.cc",
"internal/openssl_util.cc",
"internal/openssl/parse_service_account_p12_file.cc",
"internal/openssl/sign_using_sha256.cc",
"internal/populate_rest_options.cc",
"internal/rest_carrier.cc",
"internal/rest_context.cc",
Expand All @@ -117,4 +120,7 @@ google_cloud_cpp_rest_internal_srcs = [
"internal/tracing_rest_client.cc",
"internal/tracing_rest_response.cc",
"internal/unified_rest_credentials.cc",
"internal/win32/parse_service_account_p12_file.cc",
"internal/win32/sign_using_sha256.cc",
"internal/win32/win32_helpers.cc",
]
26 changes: 20 additions & 6 deletions google/cloud/google_cloud_cpp_rest_internal.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

include(IncludeNlohmannJson)
find_package(CURL REQUIRED)
find_package(OpenSSL REQUIRED)
if (NOT WIN32)
find_package(OpenSSL REQUIRED)
endif ()

# the library
add_library(
Expand Down Expand Up @@ -90,8 +92,9 @@ add_library(
internal/oauth2_service_account_credentials.h
internal/oauth2_universe_domain.cc
internal/oauth2_universe_domain.h
internal/openssl_util.cc
internal/openssl_util.h
internal/openssl/parse_service_account_p12_file.cc
internal/openssl/sign_using_sha256.cc
internal/parse_service_account_p12_file.h
internal/populate_rest_options.cc
internal/populate_rest_options.h
internal/rest_carrier.cc
Expand All @@ -109,6 +112,7 @@ add_library(
internal/rest_response.cc
internal/rest_response.h
internal/rest_retry_loop.h
internal/sign_using_sha256.h
internal/tracing_http_payload.cc
internal/tracing_http_payload.h
internal/tracing_rest_client.cc
Expand All @@ -117,15 +121,22 @@ add_library(
internal/tracing_rest_response.h
internal/unified_rest_credentials.cc
internal/unified_rest_credentials.h
internal/win32/parse_service_account_p12_file.cc
internal/win32/sign_using_sha256.cc
internal/win32/win32_helpers.cc
internal/win32/win32_helpers.h
rest_options.h)
target_link_libraries(
google_cloud_cpp_rest_internal
PUBLIC absl::span google-cloud-cpp::common CURL::libcurl
nlohmann_json::nlohmann_json OpenSSL::SSL OpenSSL::Crypto)
nlohmann_json::nlohmann_json)
if (WIN32)
target_compile_definitions(google_cloud_cpp_rest_internal PRIVATE WIN32_LEAN_AND_MEAN _WIN32_WINNT=0x0A00)
teo-tsirpanis marked this conversation as resolved.
Show resolved Hide resolved
# We use `setsockopt()` directly, which requires the ws2_32 (Winsock2 for
# Windows32?) library on Windows.
target_link_libraries(google_cloud_cpp_rest_internal PUBLIC ws2_32)
target_link_libraries(google_cloud_cpp_rest_internal PUBLIC ws2_32 bcrypt crypt32)
else ()
target_link_libraries(google_cloud_cpp_rest_internal PUBLIC OpenSSL::SSL OpenSSL::Crypto)
endif ()
google_cloud_cpp_add_common_options(google_cloud_cpp_rest_internal)
target_include_directories(
Expand Down Expand Up @@ -167,7 +178,10 @@ google_cloud_cpp_install_headers(google_cloud_cpp_rest_internal
google_cloud_cpp_add_pkgconfig(
rest_internal "REST library for the Google Cloud C++ Client Library"
"Provides REST Transport for the Google Cloud C++ Client Library."
"google_cloud_cpp_common" "libcurl" "openssl")
"google_cloud_cpp_common"
"libcurl"
NON_WIN32_REQUIRES openssl
WIN32_LIBS ws2_32 bcrypt crypt32)

# Create and install the CMake configuration files.
include(CMakePackageConfigHelpers)
Expand Down
5 changes: 5 additions & 0 deletions google/cloud/internal/curl_wrappers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
#include "google/cloud/log.h"
#include "absl/strings/match.h"
#include "absl/strings/str_split.h"
#ifndef _WIN32
#include <openssl/crypto.h>
#include <openssl/opensslv.h>
#endif
#include <algorithm>
#include <cctype>
#include <csignal>
Expand All @@ -40,6 +42,9 @@ namespace {
// LibreSSL calls itself OpenSSL > 2.0, but it really is based on SSL 1.0.2
// and requires locks.
#define GOOGLE_CLOUD_CPP_SSL_REQUIRES_LOCKS 1
#elif defined(_WIN32)
// We don't use OpenSSL on Windows.
#define GOOGLE_CLOUD_CPP_SSL_REQUIRES_LOCKS 0
#elif OPENSSL_VERSION_NUMBER < 0x10100000L // Older than version 1.1.0
// Before 1.1.0 OpenSSL requires locks to be used by multiple threads.
#define GOOGLE_CLOUD_CPP_SSL_REQUIRES_LOCKS 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#ifndef _WIN32
#include "google/cloud/internal/curl_options.h"
#include "google/cloud/internal/curl_wrappers.h"
#include <gmock/gmock.h>
Expand Down Expand Up @@ -42,3 +43,4 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace rest_internal
} // namespace cloud
} // namespace google
#endif // _WIN32
2 changes: 1 addition & 1 deletion google/cloud/internal/make_jwt_assertion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#include "google/cloud/internal/make_jwt_assertion.h"
#include "google/cloud/internal/base64_transforms.h"
#include "google/cloud/internal/openssl_util.h"
#include "google/cloud/internal/sign_using_sha256.h"

namespace google {
namespace cloud {
Expand Down
1 change: 1 addition & 0 deletions google/cloud/internal/oauth2_google_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "google/cloud/internal/oauth2_google_application_default_credentials_file.h"
#include "google/cloud/internal/oauth2_http_client_factory.h"
#include "google/cloud/internal/oauth2_service_account_credentials.h"
#include "google/cloud/internal/parse_service_account_p12_file.h"
#include "google/cloud/internal/throw_delegate.h"
#include <nlohmann/json.hpp>
#include <fstream>
Expand Down
Loading
Loading