Skip to content

Commit

Permalink
Remove SSL_set_check_client_certificate_type and SSL_set_check_ecdsa_…
Browse files Browse the repository at this point in the history
…curve

These were temporary flags in case of compatibility issues with some
older changes, set to be removed after June 2024. It is now well past
June 2024 and no one ever had to use these APIs. Remove them.

Update-Note: Removed some unused APIs.
Change-Id: I5fe34e0ebcb30f81281e413017d5a6a968a96a97
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76127
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: Bob Beck <[email protected]>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Feb 11, 2025
1 parent 0147d7f commit bef0b8b
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 97 deletions.
18 changes: 0 additions & 18 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4897,24 +4897,6 @@ OPENSSL_EXPORT int SSL_used_hello_retry_request(const SSL *ssl);
// https://bugs.openjdk.java.net/browse/JDK-8213202
OPENSSL_EXPORT void SSL_set_jdk11_workaround(SSL *ssl, int enable);

// SSL_set_check_client_certificate_type configures whether the client, in
// TLS 1.2 and below, will check its certificate against the server's requested
// certificate types.
//
// By default, this option is enabled. If disabled, certificate selection within
// the library may not function correctly. This flag is provided temporarily in
// case of compatibility issues. It will be removed sometime after June 2024.
OPENSSL_EXPORT void SSL_set_check_client_certificate_type(SSL *ssl, int enable);

// SSL_set_check_ecdsa_curve configures whether the server, in TLS 1.2 and
// below, will check its certificate against the client's supported ECDSA
// curves.
//
// By default, this option is enabled. If disabled, certificate selection within
// the library may not function correctly. This flag is provided temporarily in
// case of compatibility issues. It will be removed sometime after June 2024.
OPENSSL_EXPORT void SSL_set_check_ecdsa_curve(SSL *ssl, int enable);


// Deprecated functions.

Expand Down
34 changes: 16 additions & 18 deletions ssl/handshake_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1246,26 +1246,24 @@ static bool check_credential(SSL_HANDSHAKE *hs, const SSL_CREDENTIAL *cred,
return false;
}

if (hs->config->check_client_certificate_type) {
// Check the certificate types advertised by the peer.
uint8_t cert_type;
switch (EVP_PKEY_id(cred->pubkey.get())) {
case EVP_PKEY_RSA:
cert_type = SSL3_CT_RSA_SIGN;
break;
case EVP_PKEY_EC:
case EVP_PKEY_ED25519:
cert_type = TLS_CT_ECDSA_SIGN;
break;
default:
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
return false;
}
if (std::find(hs->certificate_types.begin(), hs->certificate_types.end(),
cert_type) == hs->certificate_types.end()) {
// Check the certificate types advertised by the peer.
uint8_t cert_type;
switch (EVP_PKEY_id(cred->pubkey.get())) {
case EVP_PKEY_RSA:
cert_type = SSL3_CT_RSA_SIGN;
break;
case EVP_PKEY_EC:
case EVP_PKEY_ED25519:
cert_type = TLS_CT_ECDSA_SIGN;
break;
default:
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
return false;
}
}
if (std::find(hs->certificate_types.begin(), hs->certificate_types.end(),
cert_type) == hs->certificate_types.end()) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
return false;
}

// All currently supported credentials require a signature. Note this does not
Expand Down
2 changes: 1 addition & 1 deletion ssl/handshake_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ static TLS12ServerParams choose_params(SSL_HANDSHAKE *hs,
// ECDSA keys must additionally be checked against the peer's supported
// curve list.
int key_type = EVP_PKEY_id(cred->pubkey.get());
if (hs->config->check_ecdsa_curve && key_type == EVP_PKEY_EC) {
if (key_type == EVP_PKEY_EC) {
EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(cred->pubkey.get());
uint16_t group_id;
if (!ssl_nid_to_group_id(
Expand Down
9 changes: 0 additions & 9 deletions ssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -3711,15 +3711,6 @@ struct SSL_CONFIG {
// alps_use_new_codepoint if set indicates we use new ALPS extension codepoint
// to negotiate and convey application settings.
bool alps_use_new_codepoint : 1;

// check_client_certificate_type indicates whether the client, in TLS 1.2 and
// below, will check its certificate against the server's requested
// certificate types.
bool check_client_certificate_type : 1;

// check_ecdsa_curve indicates whether the server, in TLS 1.2 and below, will
// check its certificate against the client's supported ECDSA curves.
bool check_ecdsa_curve : 1;
};

// From RFC 8446, used in determining PSK modes.
Expand Down
18 changes: 1 addition & 17 deletions ssl/ssl_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,7 @@ SSL_CONFIG::SSL_CONFIG(SSL *ssl_arg)
jdk11_workaround(false),
quic_use_legacy_codepoint(false),
permute_extensions(false),
alps_use_new_codepoint(false),
check_client_certificate_type(true),
check_ecdsa_curve(true) {
alps_use_new_codepoint(false) {
assert(ssl);
}

Expand Down Expand Up @@ -2924,20 +2922,6 @@ void SSL_set_jdk11_workaround(SSL *ssl, int enable) {
ssl->config->jdk11_workaround = !!enable;
}

void SSL_set_check_client_certificate_type(SSL *ssl, int enable) {
if (!ssl->config) {
return;
}
ssl->config->check_client_certificate_type = !!enable;
}

void SSL_set_check_ecdsa_curve(SSL *ssl, int enable) {
if (!ssl->config) {
return;
}
ssl->config->check_ecdsa_curve = !!enable;
}

void SSL_set_quic_use_legacy_codepoint(SSL *ssl, int use_legacy) {
if (!ssl->config) {
return;
Expand Down
23 changes: 0 additions & 23 deletions ssl/test/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2129,16 +2129,6 @@ func addBasicTests() {
shouldFail: true,
expectedError: ":UNKNOWN_CERTIFICATE_TYPE:",
},
{
name: "NoCheckClientCertificateTypes",
config: Config{
MaxVersion: VersionTLS12,
ClientAuth: RequestClientCert,
ClientCertificateTypes: []byte{CertTypeECDSASign},
},
shimCertificate: &rsaCertificate,
flags: []string{"-no-check-client-certificate-type"},
},
{
name: "UnauthenticatedECDH",
config: Config{
Expand Down Expand Up @@ -13914,19 +13904,6 @@ func addCurveTests() {
expectedError: ":WRONG_CURVE:",
})

// This behavior may, temporarily, be disabled with a flag.
testCases = append(testCases, testCase{
testType: serverTest,
name: "NoCheckECDSACurve-TLS12",
config: Config{
MinVersion: VersionTLS12,
MaxVersion: VersionTLS12,
CurvePreferences: []CurveID{CurveP384},
},
shimCertificate: &ecdsaP256Certificate,
flags: []string{"-no-check-ecdsa-curve"},
})

// If the ECDSA certificate is ineligible due to a curve mismatch, the
// server may still consider a PSK cipher suite.
testCases = append(testCases, testCase{
Expand Down
9 changes: 0 additions & 9 deletions ssl/test/test_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,6 @@ const Flag<TestConfig> *FindFlag(const char *name) {
BoolFlag("-fips-202205", &TestConfig::fips_202205),
BoolFlag("-wpa-202304", &TestConfig::wpa_202304),
BoolFlag("-cnsa-202407", &TestConfig::cnsa_202407),
BoolFlag("-no-check-client-certificate-type",
&TestConfig::no_check_client_certificate_type),
BoolFlag("-no-check-ecdsa-curve", &TestConfig::no_check_ecdsa_curve),
OptionalIntFlag("-expect-selected-credential",
&TestConfig::expect_selected_credential),
// Credential flags are stateful. First, use one of the
Expand Down Expand Up @@ -2221,12 +2218,6 @@ bssl::UniquePtr<SSL> TestConfig::NewSSL(
if (ignore_rsa_key_usage) {
SSL_set_enforce_rsa_key_usage(ssl.get(), 0);
}
if (no_check_client_certificate_type) {
SSL_set_check_client_certificate_type(ssl.get(), 0);
}
if (no_check_ecdsa_curve) {
SSL_set_check_ecdsa_curve(ssl.get(), 0);
}
if (no_tls13) {
SSL_set_options(ssl.get(), SSL_OP_NO_TLSv1_3);
}
Expand Down
2 changes: 0 additions & 2 deletions ssl/test/test_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ struct TestConfig {
bool fips_202205 = false;
bool wpa_202304 = false;
bool cnsa_202407 = false;
bool no_check_client_certificate_type = false;
bool no_check_ecdsa_curve = false;
std::optional<int> expect_selected_credential;
std::vector<CredentialConfig> credentials;
int private_key_delay_ms = 0;
Expand Down

0 comments on commit bef0b8b

Please sign in to comment.