[tls] Allow skipping server cert verification when no default roots are present. (#34859)

This PR fixes a bug identified in #29667, where the TLS channel
credentials still require a trust bundle even if the user has explicitly
opted to not verify the server certificate. This PR is based on #29810.
pull/34876/head
Matthew Stevenson 1 year ago committed by GitHub
parent 0b1e381d56
commit 52f9e011f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 51
      CMakeLists.txt
  2. 16
      build_autogenerated.yaml
  3. 3
      src/core/lib/security/security_connector/ssl_utils.cc
  4. 7
      src/core/tsi/ssl_transport_security.cc
  5. 37
      test/core/tsi/ssl_transport_security_test.cc
  6. 26
      test/cpp/end2end/BUILD
  7. 161
      test/cpp/end2end/tls_credentials_test.cc
  8. 24
      tools/run_tests/generated/tests.json

51
CMakeLists.txt generated

@ -1412,6 +1412,7 @@ if(gRPC_BUILD_TESTS)
add_dependencies(buildtests_cxx timer_manager_test)
add_dependencies(buildtests_cxx timer_test)
add_dependencies(buildtests_cxx tls_certificate_verifier_test)
add_dependencies(buildtests_cxx tls_credentials_test)
add_dependencies(buildtests_cxx tls_key_export_test)
add_dependencies(buildtests_cxx tls_security_connector_test)
add_dependencies(buildtests_cxx too_many_pings_test)
@ -25097,6 +25098,56 @@ target_link_libraries(tls_certificate_verifier_test
)
endif()
if(gRPC_BUILD_TESTS)
add_executable(tls_credentials_test
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.cc
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.pb.h
${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.h
test/cpp/end2end/test_service_impl.cc
test/cpp/end2end/tls_credentials_test.cc
)
target_compile_features(tls_credentials_test PUBLIC cxx_std_14)
target_include_directories(tls_credentials_test
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/include
${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
${_gRPC_RE2_INCLUDE_DIR}
${_gRPC_SSL_INCLUDE_DIR}
${_gRPC_UPB_GENERATED_DIR}
${_gRPC_UPB_GRPC_GENERATED_DIR}
${_gRPC_UPB_INCLUDE_DIR}
${_gRPC_XXHASH_INCLUDE_DIR}
${_gRPC_ZLIB_INCLUDE_DIR}
third_party/googletest/googletest/include
third_party/googletest/googletest
third_party/googletest/googlemock/include
third_party/googletest/googlemock
${_gRPC_PROTO_GENS_DIR}
)
target_link_libraries(tls_credentials_test
${_gRPC_ALLTARGETS_LIBRARIES}
gtest
grpc++_test_util
)
endif()
if(gRPC_BUILD_TESTS)

@ -16616,6 +16616,22 @@ targets:
- gtest
- grpc++
- grpc_test_util
- name: tls_credentials_test
gtest: true
build: test
language: c++
headers:
- test/cpp/end2end/test_service_impl.h
src:
- src/proto/grpc/testing/echo.proto
- src/proto/grpc/testing/echo_messages.proto
- src/proto/grpc/testing/simple_messages.proto
- src/proto/grpc/testing/xds/v3/orca_load_report.proto
- test/cpp/end2end/test_service_impl.cc
- test/cpp/end2end/tls_credentials_test.cc
deps:
- gtest
- grpc++_test_util
- name: tls_key_export_test
gtest: true
build: test

@ -417,7 +417,7 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init(
tsi_ssl_client_handshaker_factory** handshaker_factory) {
const char* root_certs;
const tsi_ssl_root_certs_store* root_store;
if (pem_root_certs == nullptr) {
if (pem_root_certs == nullptr && !skip_server_certificate_verification) {
gpr_log(GPR_INFO,
"No root certificates specified; use ones stored in system default "
"locations instead");
@ -436,7 +436,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init(
pem_key_cert_pair->private_key != nullptr &&
pem_key_cert_pair->cert_chain != nullptr;
tsi_ssl_client_handshaker_options options;
GPR_DEBUG_ASSERT(root_certs != nullptr);
options.pem_root_certs = root_certs;
options.root_store = root_store;
options.alpn_protocols =

@ -2061,7 +2061,8 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
if (factory == nullptr) return TSI_INVALID_ARGUMENT;
*factory = nullptr;
if (options->pem_root_certs == nullptr && options->root_store == nullptr) {
if (options->pem_root_certs == nullptr && options->root_store == nullptr &&
!options->skip_server_certificate_verification) {
return TSI_INVALID_ARGUMENT;
}
@ -2123,7 +2124,9 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
SSL_CTX_set_cert_store(ssl_context, options->root_store->store);
}
#endif
if (OPENSSL_VERSION_NUMBER < 0x10100000 || options->root_store == nullptr) {
if (OPENSSL_VERSION_NUMBER < 0x10100000 ||
(options->root_store == nullptr &&
options->pem_root_certs != nullptr)) {
result = ssl_ctx_load_verification_certs(
ssl_context, options->pem_root_certs, strlen(options->pem_root_certs),
nullptr);

@ -86,7 +86,9 @@ typedef struct ssl_key_cert_lib {
bool use_bad_server_cert;
bool use_bad_client_cert;
bool use_root_store;
bool use_pem_root_certs;
bool use_cert_signed_by_intermediate_ca;
bool skip_server_certificate_verification;
char* root_cert;
tsi_ssl_root_certs_store* root_store;
tsi_ssl_pem_key_cert_pair* server_pem_key_cert_pairs;
@ -111,6 +113,7 @@ typedef struct ssl_tsi_test_fixture {
size_t session_ticket_key_size;
size_t network_bio_buf_size;
size_t ssl_bio_buf_size;
bool verify_root_cert_subject;
tsi_ssl_server_handshaker_factory* server_handshaker_factory;
tsi_ssl_client_handshaker_factory* client_handshaker_factory;
} ssl_tsi_test_fixture;
@ -125,7 +128,9 @@ static void ssl_test_setup_handshakers(tsi_test_fixture* fixture) {
ssl_alpn_lib* alpn_lib = ssl_fixture->alpn_lib;
// Create client handshaker factory.
tsi_ssl_client_handshaker_options client_options;
client_options.pem_root_certs = key_cert_lib->root_cert;
if (key_cert_lib->use_pem_root_certs) {
client_options.pem_root_certs = key_cert_lib->root_cert;
}
if (ssl_fixture->force_client_auth) {
client_options.pem_key_cert_pair =
key_cert_lib->use_bad_client_cert
@ -145,6 +150,8 @@ static void ssl_test_setup_handshakers(tsi_test_fixture* fixture) {
}
client_options.min_tls_version = test_tls_version;
client_options.max_tls_version = test_tls_version;
client_options.skip_server_certificate_verification =
key_cert_lib->skip_server_certificate_verification;
ASSERT_EQ(tsi_create_ssl_client_handshaker_factory_with_options(
&client_options, &ssl_fixture->client_handshaker_factory),
TSI_OK);
@ -396,10 +403,12 @@ static void ssl_test_check_handshaker_peers(tsi_test_fixture* fixture) {
check_session_reusage(ssl_fixture, &peer);
check_alpn(ssl_fixture, &peer);
check_security_level(&peer);
if (!ssl_fixture->session_reused) {
check_verified_root_cert_subject(ssl_fixture, &peer);
} else {
check_verified_root_cert_subject_unset(ssl_fixture, &peer);
if (ssl_fixture->verify_root_cert_subject) {
if (!ssl_fixture->session_reused) {
check_verified_root_cert_subject(ssl_fixture, &peer);
} else {
check_verified_root_cert_subject_unset(ssl_fixture, &peer);
}
}
if (ssl_fixture->server_name_indication == nullptr ||
strcmp(ssl_fixture->server_name_indication, SSL_TSI_TEST_WRONG_SNI) ==
@ -534,6 +543,7 @@ static std::string GenerateTrustBundle() {
static tsi_test_fixture* ssl_tsi_test_fixture_create() {
ssl_tsi_test_fixture* ssl_fixture = grpc_core::Zalloc<ssl_tsi_test_fixture>();
tsi_test_fixture_init(&ssl_fixture->base);
ssl_fixture->verify_root_cert_subject = true;
ssl_fixture->base.test_unused_bytes = true;
ssl_fixture->base.vtable = &vtable;
// Create ssl_key_cert_lib.
@ -541,6 +551,8 @@ static tsi_test_fixture* ssl_tsi_test_fixture_create() {
key_cert_lib->use_bad_server_cert = false;
key_cert_lib->use_bad_client_cert = false;
key_cert_lib->use_root_store = false;
key_cert_lib->use_pem_root_certs = true;
key_cert_lib->skip_server_certificate_verification = false;
key_cert_lib->server_num_key_cert_pairs =
SSL_TSI_TEST_SERVER_KEY_CERT_PAIRS_NUM;
key_cert_lib->bad_server_num_key_cert_pairs =
@ -651,6 +663,20 @@ void ssl_tsi_test_do_handshake_with_root_store() {
tsi_test_fixture_destroy(fixture);
}
void ssl_tsi_test_do_handshake_skipping_server_certificate_verification() {
gpr_log(GPR_INFO,
"ssl_tsi_test_do_handshake_skipping_server_certificate_verification");
tsi_test_fixture* fixture = ssl_tsi_test_fixture_create();
ssl_tsi_test_fixture* ssl_fixture =
reinterpret_cast<ssl_tsi_test_fixture*>(fixture);
ssl_fixture->verify_root_cert_subject = false;
ssl_fixture->key_cert_lib->use_root_store = false;
ssl_fixture->key_cert_lib->use_pem_root_certs = false;
ssl_fixture->key_cert_lib->skip_server_certificate_verification = true;
tsi_test_do_handshake(fixture);
tsi_test_fixture_destroy(fixture);
}
void ssl_tsi_test_do_handshake_with_large_server_handshake_messages(
const std::string& trust_bundle) {
gpr_log(GPR_INFO,
@ -1236,6 +1262,7 @@ TEST(SslTransportSecurityTest, MainTest) {
ssl_tsi_test_do_handshake_small_handshake_buffer();
ssl_tsi_test_do_handshake();
ssl_tsi_test_do_handshake_with_root_store();
ssl_tsi_test_do_handshake_skipping_server_certificate_verification();
ssl_tsi_test_do_handshake_with_large_server_handshake_messages(
trust_bundle);
ssl_tsi_test_do_handshake_with_client_authentication();

@ -1037,6 +1037,32 @@ grpc_cc_test(
],
)
grpc_cc_test(
name = "tls_credentials_test",
srcs = ["tls_credentials_test.cc"],
data = [
"//src/core/tsi/test_creds:ca.pem",
"//src/core/tsi/test_creds:client.key",
"//src/core/tsi/test_creds:client.pem",
"//src/core/tsi/test_creds:server1.key",
"//src/core/tsi/test_creds:server1.pem",
],
external_deps = [
"gtest",
],
tags = ["tls_credentials_test"],
deps = [
":test_service_impl",
"//:gpr",
"//:grpc",
"//:grpc++",
"//src/proto/grpc/testing:echo_messages_proto",
"//src/proto/grpc/testing:echo_proto",
"//test/core/util:grpc_test_util",
"//test/cpp/util:test_util",
],
)
grpc_cc_test(
name = "crl_provider_test",
srcs = ["crl_provider_test.cc"],

@ -0,0 +1,161 @@
//
//
// 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 <memory>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "absl/synchronization/notification.h"
#include <grpc/grpc_security.h>
#include <grpcpp/channel.h>
#include <grpcpp/client_context.h>
#include <grpcpp/create_channel.h>
#include <grpcpp/security/tls_certificate_verifier.h>
#include <grpcpp/security/tls_credentials_options.h>
#include <grpcpp/server.h>
#include <grpcpp/server_builder.h>
#include "src/core/lib/iomgr/load_file.h"
#include "test/core/util/port.h"
#include "test/core/util/test_config.h"
#include "test/cpp/end2end/test_service_impl.h"
namespace grpc {
namespace testing {
namespace {
using ::grpc::experimental::ExternalCertificateVerifier;
using ::grpc::experimental::TlsChannelCredentialsOptions;
constexpr char kCaCertPath[] = "src/core/tsi/test_creds/ca.pem";
constexpr char kServerCertPath[] = "src/core/tsi/test_creds/server1.pem";
constexpr char kServerKeyPath[] = "src/core/tsi/test_creds/server1.key";
constexpr char kMessage[] = "Hello";
std::string ReadFile(const std::string& file_path) {
grpc_slice slice;
GPR_ASSERT(GRPC_LOG_IF_ERROR("load_file",
grpc_load_file(file_path.c_str(), 0, &slice)));
std::string file_contents(grpc_core::StringViewFromSlice(slice));
grpc_slice_unref(slice);
return file_contents;
}
class NoOpCertificateVerifier : public ExternalCertificateVerifier {
public:
~NoOpCertificateVerifier() override = default;
bool Verify(grpc::experimental::TlsCustomVerificationCheckRequest*,
std::function<void(grpc::Status)>,
grpc::Status* sync_status) override {
*sync_status = grpc::Status(grpc::StatusCode::OK, "");
return true;
}
void Cancel(grpc::experimental::TlsCustomVerificationCheckRequest*) override {
}
};
class TlsCredentialsTest : public ::testing::Test {
protected:
void RunServer(absl::Notification* notification) {
std::string root_cert = ReadFile(kCaCertPath);
grpc::SslServerCredentialsOptions::PemKeyCertPair key_cert_pair = {
ReadFile(kServerKeyPath), ReadFile(kServerCertPath)};
grpc::SslServerCredentialsOptions ssl_options;
ssl_options.pem_key_cert_pairs.push_back(key_cert_pair);
ssl_options.pem_root_certs = root_cert;
grpc::ServerBuilder builder;
TestServiceImpl service_;
builder
.AddListeningPort(server_addr_, grpc::SslServerCredentials(ssl_options))
.RegisterService(&service_);
server_ = builder.BuildAndStart();
notification->Notify();
server_->Wait();
}
void TearDown() override {
if (server_ != nullptr) {
server_->Shutdown();
server_thread_->join();
delete server_thread_;
}
}
TestServiceImpl service_;
std::unique_ptr<Server> server_ = nullptr;
std::thread* server_thread_ = nullptr;
std::string server_addr_;
};
void DoRpc(const std::string& server_addr,
const TlsChannelCredentialsOptions& tls_options) {
std::shared_ptr<Channel> channel =
grpc::CreateChannel(server_addr, TlsCredentials(tls_options));
auto stub = grpc::testing::EchoTestService::NewStub(channel);
grpc::testing::EchoRequest request;
grpc::testing::EchoResponse response;
request.set_message(kMessage);
ClientContext context;
context.set_deadline(grpc_timeout_seconds_to_deadline(/*time_s=*/10));
grpc::Status result = stub->Echo(&context, request, &response);
EXPECT_TRUE(result.ok());
if (!result.ok()) {
gpr_log(GPR_ERROR, "Echo failed: %d, %s, %s",
static_cast<int>(result.error_code()),
result.error_message().c_str(), result.error_details().c_str());
}
EXPECT_EQ(response.message(), kMessage);
}
// How do we test that skipping server certificate verification works as
// expected? Give the server credentials that chain up to a custom CA (that does
// not belong to the default or OS trust store), do not configure the client to
// have this CA in its trust store, and attempt to establish a connection
// between the client and server.
TEST_F(TlsCredentialsTest, SkipServerCertificateVerification) {
server_addr_ = absl::StrCat("localhost:",
std::to_string(grpc_pick_unused_port_or_die()));
absl::Notification notification;
server_thread_ = new std::thread([&]() { RunServer(&notification); });
notification.WaitForNotification();
TlsChannelCredentialsOptions tls_options;
tls_options.set_certificate_verifier(
ExternalCertificateVerifier::Create<NoOpCertificateVerifier>());
tls_options.set_check_call_host(/*check_call_host=*/false);
tls_options.set_verify_server_certs(/*verify_server_certs=*/false);
DoRpc(server_addr_, tls_options);
}
} // namespace
} // namespace testing
} // namespace grpc
int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(&argc, argv);
::testing::InitGoogleTest(&argc, argv);
int ret = RUN_ALL_TESTS();
return ret;
}

@ -10661,6 +10661,30 @@
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux",
"mac",
"posix",
"windows"
],
"cpu_cost": 1.0,
"exclude_configs": [],
"exclude_iomgrs": [],
"flaky": false,
"gtest": true,
"language": "c++",
"name": "tls_credentials_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,

Loading…
Cancel
Save