Skip to content

Commit

Permalink
[tls] Backport - Remove use of SSL_CTX_set_client_CA_list for TLS ser…
Browse files Browse the repository at this point in the history
…ver credentials. (#33583)

See #33558.
  • Loading branch information
matthewstevenson88 committed Jul 5, 2023
1 parent 45480dd commit 53e637c
Show file tree
Hide file tree
Showing 16 changed files with 129 additions and 35 deletions.
1 change: 1 addition & 0 deletions grpc.def

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions include/grpc/grpc_security.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

#include <grpc/support/port_platform.h>

#include <stdbool.h>

#include <grpc/grpc.h>
#include <grpc/grpc_security_constants.h>
#include <grpc/status.h>
Expand Down Expand Up @@ -896,6 +898,23 @@ GRPCAPI void grpc_tls_credentials_options_set_crl_directory(
GRPCAPI void grpc_tls_credentials_options_set_verify_server_cert(
grpc_tls_credentials_options* options, int verify_server_cert);

/**
* EXPERIMENTAL API - Subject to change
*
* Sets whether or not a TLS server should send a list of CA names in the
* ServerHello. This list of CA names is read from the server's trust bundle, so
* that the client can use this list as a hint to know which certificate it
* should send to the server.
*
* WARNING: This API is extremely dangerous and should not be used. If the
* server's trust bundle is too large, then the TLS server will be unable to
* form a ServerHello, and hence will be unusable. The definition of "too large"
* depends on the underlying SSL library being used and on the size of the CN
* fields of the certificates in the trust bundle.
*/
GRPCAPI void grpc_tls_credentials_options_set_send_client_ca_list(
grpc_tls_credentials_options* options, bool send_client_ca_list);

/**
* EXPERIMENTAL API - Subject to change
*
Expand Down
12 changes: 12 additions & 0 deletions include/grpcpp/security/tls_credentials_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ class TlsServerCredentialsOptions final : public TlsCredentialsOptions {
void set_cert_request_type(
grpc_ssl_client_certificate_request_type cert_request_type);

// Sets whether or not a TLS server should send a list of CA names in the
// ServerHello. This list of CA names is read from the server's trust bundle,
// so that the client can use this list as a hint to know which certificate it
// should send to the server.
//
// By default, this option is turned off.
//
// WARNING: This API is extremely dangerous and should not be used. If the
// server's trust bundle is too large, then the TLS server will be unable to
// form a ServerHello, and hence will be unusable.
void set_send_client_ca_list(bool send_client_ca_list);

private:
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,11 @@ void grpc_tls_credentials_options_set_tls_session_key_log_file_path(
}
options->set_tls_session_key_log_file_path(path != nullptr ? path : "");
}

void grpc_tls_credentials_options_set_send_client_ca_list(
grpc_tls_credentials_options* options, bool send_client_ca_list) {
if (options == nullptr) {
return;
}
options->set_send_client_ca_list(send_client_ca_list);
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion src/core/lib/security/security_connector/ssl_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init(
grpc_ssl_client_certificate_request_type client_certificate_request,
tsi_tls_version min_tls_version, tsi_tls_version max_tls_version,
tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger,
const char* crl_directory,
const char* crl_directory, bool send_client_ca_list,
tsi_ssl_server_handshaker_factory** handshaker_factory) {
size_t num_alpn_protocols = 0;
const char** alpn_protocol_strings =
Expand All @@ -483,6 +483,7 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init(
options.max_tls_version = max_tls_version;
options.key_logger = tls_session_key_logger;
options.crl_directory = crl_directory;
options.send_client_ca_list = send_client_ca_list;
const tsi_result result =
tsi_create_ssl_server_handshaker_factory_with_options(&options,
handshaker_factory);
Expand Down
2 changes: 1 addition & 1 deletion src/core/lib/security/security_connector/ssl_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init(
grpc_ssl_client_certificate_request_type client_certificate_request,
tsi_tls_version min_tls_version, tsi_tls_version max_tls_version,
tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger,
const char* crl_directory,
const char* crl_directory, bool send_client_ca_list,
tsi_ssl_server_handshaker_factory** handshaker_factory);

// Free the memory occupied by key cert pairs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ TlsServerSecurityConnector::UpdateHandshakerFactoryLocked() {
grpc_get_tsi_tls_version(options_->min_tls_version()),
grpc_get_tsi_tls_version(options_->max_tls_version()),
tls_session_key_logger_.get(), options_->crl_directory().c_str(),
&server_handshaker_factory_);
options_->send_client_ca_list(), &server_handshaker_factory_);
// Free memory.
grpc_tsi_ssl_pem_key_cert_pairs_destroy(pem_key_cert_pairs,
num_key_cert_pairs);
Expand Down
7 changes: 5 additions & 2 deletions src/core/tsi/ssl_transport_security.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2202,12 +2202,15 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options(
STACK_OF(X509_NAME)* root_names = nullptr;
result = ssl_ctx_load_verification_certs(
impl->ssl_contexts[i], options->pem_client_root_certs,
strlen(options->pem_client_root_certs), &root_names);
strlen(options->pem_client_root_certs),
options->send_client_ca_list ? &root_names : nullptr);
if (result != TSI_OK) {
gpr_log(GPR_ERROR, "Invalid verification certs.");
break;
}
SSL_CTX_set_client_CA_list(impl->ssl_contexts[i], root_names);
if (options->send_client_ca_list) {
SSL_CTX_set_client_CA_list(impl->ssl_contexts[i], root_names);
}
}
switch (options->client_certificate_request) {
case TSI_DONT_REQUEST_CLIENT_CERTIFICATE:
Expand Down
14 changes: 13 additions & 1 deletion src/core/tsi/ssl_transport_security.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,17 @@ struct tsi_ssl_server_handshaker_options {
// crl checking. Only OpenSSL version > 1.1 is supported for CRL checking
const char* crl_directory;

// If true, the SSL server sends a list of CA names to the client in the
// ServerHello. This list of CA names is extracted from the server's trust
// bundle, and the client may use this lint as a hint to decide which
// certificate it should send to the server.
//
// WARNING: This is an extremely dangerous option. If the server's trust
// bundle is sufficiently large, then setting this bit to true will result in
// the server being unable to generate a ServerHello, and hence the server
// will be unusable.
bool send_client_ca_list;

tsi_ssl_server_handshaker_options()
: pem_key_cert_pairs(nullptr),
num_key_cert_pairs(0),
Expand All @@ -338,7 +349,8 @@ struct tsi_ssl_server_handshaker_options {
min_tls_version(tsi_tls_version::TSI_TLS1_2),
max_tls_version(tsi_tls_version::TSI_TLS1_3),
key_logger(nullptr),
crl_directory(nullptr) {}
crl_directory(nullptr),
send_client_ca_list(true) {}
};

// Creates a server handshaker factory.
Expand Down
8 changes: 8 additions & 0 deletions src/cpp/common/tls_credentials_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,13 @@ void TlsServerCredentialsOptions::set_cert_request_type(
cert_request_type);
}

void TlsServerCredentialsOptions::set_send_client_ca_list(
bool send_client_ca_list) {
grpc_tls_credentials_options* options = c_credentials_options();
GPR_ASSERT(options != nullptr);
grpc_tls_credentials_options_set_send_client_ca_list(options,
send_client_ca_list);
}

} // namespace experimental
} // namespace grpc
2 changes: 2 additions & 0 deletions src/ruby/ext/grpc/rb_grpc_imports.generated.c

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/ruby/ext/grpc/rb_grpc_imports.generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions test/core/security/grpc_tls_credentials_options_comparator_test.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

57 changes: 30 additions & 27 deletions test/core/tsi/ssl_transport_security_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const size_t kSessionTicketEncryptionKeySize = 48;

// Indicates the TLS version used for the test.
static tsi_tls_version test_tls_version = tsi_tls_version::TSI_TLS1_3;
static bool test_send_client_ca_list = false;

typedef enum AlpnMode {
NO_ALPN,
Expand Down Expand Up @@ -1171,35 +1172,37 @@ TEST(SslTransportSecurityTest, MainTest) {
for (size_t i = 0; i < number_tls_versions; i++) {
// Set the TLS version to be used in the tests.
test_tls_version = tls_versions[i];
// Run all the tests using that TLS version for both the client and server.
ssl_tsi_test_do_handshake_tiny_handshake_buffer();
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_with_client_authentication();
ssl_tsi_test_do_handshake_with_client_authentication_and_root_store();
ssl_tsi_test_do_handshake_with_server_name_indication_exact_domain();
ssl_tsi_test_do_handshake_with_server_name_indication_wild_star_domain();
ssl_tsi_test_do_handshake_with_wrong_server_name_indication();
ssl_tsi_test_do_handshake_with_bad_server_cert();
ssl_tsi_test_do_handshake_with_bad_client_cert();
for (bool send_client_ca_list : {true, false}) {
test_send_client_ca_list = send_client_ca_list;
ssl_tsi_test_do_handshake_tiny_handshake_buffer();
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_with_client_authentication();
ssl_tsi_test_do_handshake_with_client_authentication_and_root_store();
ssl_tsi_test_do_handshake_with_server_name_indication_exact_domain();
ssl_tsi_test_do_handshake_with_server_name_indication_wild_star_domain();
ssl_tsi_test_do_handshake_with_wrong_server_name_indication();
ssl_tsi_test_do_handshake_with_bad_server_cert();
ssl_tsi_test_do_handshake_with_bad_client_cert();
#ifdef OPENSSL_IS_BORINGSSL
// BoringSSL and OpenSSL have different behaviors on mismatched ALPN.
ssl_tsi_test_do_handshake_alpn_client_no_server();
ssl_tsi_test_do_handshake_alpn_client_server_mismatch();
// BoringSSL and OpenSSL have different behaviors on mismatched ALPN.
ssl_tsi_test_do_handshake_alpn_client_no_server();
ssl_tsi_test_do_handshake_alpn_client_server_mismatch();
#endif
ssl_tsi_test_do_handshake_alpn_server_no_client();
ssl_tsi_test_do_handshake_alpn_client_server_ok();
ssl_tsi_test_do_handshake_session_cache();
ssl_tsi_test_do_round_trip_for_all_configs();
ssl_tsi_test_do_round_trip_with_error_on_stack();
ssl_tsi_test_do_round_trip_odd_buffer_size();
ssl_tsi_test_handshaker_factory_internals();
ssl_tsi_test_duplicate_root_certificates();
ssl_tsi_test_extract_x509_subject_names();
ssl_tsi_test_extract_cert_chain();
ssl_tsi_test_do_handshake_with_custom_bio_pair();
ssl_tsi_test_do_handshake_with_intermediate_ca();
ssl_tsi_test_do_handshake_alpn_server_no_client();
ssl_tsi_test_do_handshake_alpn_client_server_ok();
ssl_tsi_test_do_handshake_session_cache();
ssl_tsi_test_do_round_trip_for_all_configs();
ssl_tsi_test_do_round_trip_with_error_on_stack();
ssl_tsi_test_do_round_trip_odd_buffer_size();
ssl_tsi_test_handshaker_factory_internals();
ssl_tsi_test_duplicate_root_certificates();
ssl_tsi_test_extract_x509_subject_names();
ssl_tsi_test_extract_cert_chain();
ssl_tsi_test_do_handshake_with_custom_bio_pair();
ssl_tsi_test_do_handshake_with_intermediate_ca();
}
}
grpc_shutdown();
}
Expand Down
10 changes: 9 additions & 1 deletion tools/codegen/core/gen_grpc_tls_credentials_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,15 @@ class DataMember:
setter_move_semantics=True,
test_name="DifferentCrlDirectory",
test_value_1="\"crl_directory_1\"",
test_value_2="\"crl_directory_2\"")
test_value_2="\"crl_directory_2\""),
DataMember(
name="send_client_ca_list",
type="bool",
default_initializer="false",
test_name="DifferentSendClientCaListValues",
test_value_1="false",
test_value_2="true",
),
]


Expand Down

0 comments on commit 53e637c

Please sign in to comment.