address PR comments

pull/34861/head
Luwei Ge 1 year ago
parent ef88661b13
commit c2bc98c166
  1. 6
      include/grpc/grpc_security.h
  2. 6
      include/grpcpp/security/tls_credentials_options.h
  3. 14
      src/core/lib/security/credentials/tls/tls_credentials.cc
  4. 14
      test/cpp/client/credentials_test.cc
  5. 28
      test/cpp/server/credentials_test.cc

@ -819,8 +819,7 @@ GRPCAPI grpc_tls_credentials_options* grpc_tls_credentials_options_create(void);
* EXPERIMENTAL API - Subject to change
*
* Sets the minimum TLS version that will be negotiated during the TLS
* handshake. If not set, the underlying SSL library will pick the version
* automatically.
* handshake. If not set, the underlying SSL library will set it to TLS v1.2.
*/
GRPCAPI void grpc_tls_credentials_options_set_min_tls_version(
grpc_tls_credentials_options* options, grpc_tls_version min_tls_version);
@ -829,8 +828,7 @@ GRPCAPI void grpc_tls_credentials_options_set_min_tls_version(
* EXPERIMENTAL API - Subject to change
*
* Sets the maximum TLS version that will be negotiated during the TLS
* handshake. If not set, the underlying SSL library will pick the version
* automatically.
* handshake. If not set, the underlying SSL library will set it to TLS v1.3.
*/
GRPCAPI void grpc_tls_credentials_options_set_max_tls_version(
grpc_tls_credentials_options* options, grpc_tls_version max_tls_version);

@ -109,13 +109,11 @@ class TlsCredentialsOptions {
void set_crl_provider(std::shared_ptr<CrlProvider> crl_provider);
// Sets the minimum TLS version that will be negotiated during the TLS
// handshake. If not set, the underlying SSL library will pick the version
// automatically.
// handshake. If not set, the underlying SSL library will use TLS v1.2.
// @param tls_version: The minimum TLS version.
void set_min_tls_version(grpc_tls_version tls_version);
// Sets the maximum TLS version that will be negotiated during the TLS
// handshake. If not set, the underlying SSL library will pick the version
// automatically.
// handshake. If not set, the underlying SSL library will use TLS v1.3.
// @param tls_version: The maximum TLS version.
void set_max_tls_version(grpc_tls_version tls_version);

@ -47,9 +47,19 @@ bool CredentialOptionSanityCheck(grpc_tls_credentials_options* options,
return false;
}
// In this case, there will be non-retriable handshake errors.
if (options->min_tls_version() == grpc_tls_version::TLS1_3 &&
options->max_tls_version() == grpc_tls_version::TLS1_2) {
if (options->min_tls_version() > options->max_tls_version()) {
gpr_log(GPR_ERROR, "TLS min version must not be higher than max version.");
grpc_tls_credentials_options_destroy(options);
return false;
}
if (options->max_tls_version() > grpc_tls_version::TLS1_3) {
gpr_log(GPR_ERROR, "TLS max version must not be higher than v1.3.");
grpc_tls_credentials_options_destroy(options);
return false;
}
if (options->min_tls_version() < grpc_tls_version::TLS1_2) {
gpr_log(GPR_ERROR, "TLS min version must not be lower than v1.2.");
grpc_tls_credentials_options_destroy(options);
return false;
}
if (!options->crl_directory().empty() && options->crl_provider() != nullptr) {

@ -508,6 +508,20 @@ TEST(CredentialsTest, TlsChannelCredentialsWithGoodMinAndMaxTlsVersions) {
EXPECT_NE(channel_credentials, nullptr);
}
TEST(CredentialsTest, TlsChannelCredentialsWithMinTlsVersions) {
grpc::experimental::TlsChannelCredentialsOptions options;
options.set_min_tls_version(static_cast<grpc_tls_version>(-1));
auto channel_credentials = grpc::experimental::TlsCredentials(options);
EXPECT_EQ(channel_credentials, nullptr);
}
TEST(CredentialsTest, TlsChannelCredentialsWithMaxTlsVersions) {
grpc::experimental::TlsChannelCredentialsOptions options;
options.set_max_tls_version(static_cast<grpc_tls_version>(2));
auto channel_credentials = grpc::experimental::TlsCredentials(options);
EXPECT_EQ(channel_credentials, nullptr);
}
TEST(CredentialsTest, TlsChannelCredentialsWithBadMinAndMaxTlsVersions) {
grpc::experimental::TlsChannelCredentialsOptions options;
options.set_min_tls_version(grpc_tls_version::TLS1_3);

@ -294,6 +294,34 @@ TEST(
EXPECT_NE(server_credentials, nullptr);
}
TEST(CredentialsTest, TlsServerCredentialsWithMinTlsVersions) {
experimental::IdentityKeyCertPair key_cert_pair;
key_cert_pair.private_key = kIdentityCertPrivateKey;
key_cert_pair.certificate_chain = kIdentityCertContents;
std::vector<experimental::IdentityKeyCertPair> identity_key_cert_pairs;
identity_key_cert_pairs.emplace_back(key_cert_pair);
auto certificate_provider = std::make_shared<StaticDataCertificateProvider>(
kRootCertContents, identity_key_cert_pairs);
grpc::experimental::TlsServerCredentialsOptions options(certificate_provider);
options.set_min_tls_version(static_cast<grpc_tls_version>(-1));
auto server_credentials = grpc::experimental::TlsServerCredentials(options);
EXPECT_EQ(server_credentials, nullptr);
}
TEST(CredentialsTest, TlsServerCredentialsWithMaxTlsVersions) {
experimental::IdentityKeyCertPair key_cert_pair;
key_cert_pair.private_key = kIdentityCertPrivateKey;
key_cert_pair.certificate_chain = kIdentityCertContents;
std::vector<experimental::IdentityKeyCertPair> identity_key_cert_pairs;
identity_key_cert_pairs.emplace_back(key_cert_pair);
auto certificate_provider = std::make_shared<StaticDataCertificateProvider>(
kRootCertContents, identity_key_cert_pairs);
grpc::experimental::TlsServerCredentialsOptions options(certificate_provider);
options.set_max_tls_version(static_cast<grpc_tls_version>(2));
auto server_credentials = grpc::experimental::TlsServerCredentials(options);
EXPECT_EQ(server_credentials, nullptr);
}
TEST(
CredentialsTest,
TlsChannelCredentialsWithStaticDataCertificateProviderAndBadMinMaxTlsVersions) {

Loading…
Cancel
Save