Skip to content

Commit

Permalink
Upstream merge 2024 11 18 (#2012)
Browse files Browse the repository at this point in the history
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
skmcgrail authored Dec 4, 2024
2 parents 066c700 + 899bd22 commit f6d3673
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 23 deletions.
9 changes: 9 additions & 0 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5034,6 +5034,15 @@ 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 Stat Counters.

Expand Down
32 changes: 28 additions & 4 deletions ssl/handshake_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1379,17 +1379,41 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) {
}
}

if (!ssl_has_certificate(hs)) {
if (!ssl_on_certificate_selected(hs)) {
return ssl_hs_error;
}

if (ssl_has_certificate(hs)) {
if (hs->config->check_client_certificate_type) {
// Check the certificate types advertised by the peer.
uint8_t cert_type;
switch (EVP_PKEY_id(hs->local_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 ssl_hs_error;
}
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 ssl_hs_error;
}
}
} else {
// Without a client certificate, the handshake buffer may be released.
hs->transcript.FreeBuffer();
}

if (!ssl_on_certificate_selected(hs) ||
!ssl_output_cert_chain(hs)) {
if (!ssl_output_cert_chain(hs)) {
return ssl_hs_error;
}


hs->state = state_send_client_key_exchange;
return ssl_hs_ok;
}
Expand Down
9 changes: 7 additions & 2 deletions ssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -747,13 +747,13 @@ bool ssl_cipher_requires_server_key_exchange(const SSL_CIPHER *cipher);
size_t ssl_cipher_get_record_split_len(const SSL_CIPHER *cipher);

// ssl_choose_tls13_cipher returns an |SSL_CIPHER| corresponding with the best
// available from |client_cipher_suites| compatible with |version|, |group_id| and
// available from |client_cipher_suites| compatible with |version| and
// configured |tls13_ciphers|. It returns NULL if there isn't a compatible
// cipher. |has_aes_hw| indicates if the choice should be made as if support for
// AES in hardware is available.
const SSL_CIPHER *ssl_choose_tls13_cipher(
const STACK_OF(SSL_CIPHER) *client_cipher_suites, bool has_aes_hw, uint16_t version,
uint16_t group_id, const STACK_OF(SSL_CIPHER) *tls13_ciphers);
const STACK_OF(SSL_CIPHER) *tls13_ciphers);


// Transcript layer.
Expand Down Expand Up @@ -3380,6 +3380,11 @@ 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;
};

// From RFC 8446, used in determining PSK modes.
Expand Down
2 changes: 1 addition & 1 deletion ssl/s3_both.cc
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ class CipherScorer {

const SSL_CIPHER *ssl_choose_tls13_cipher(
const STACK_OF(SSL_CIPHER) *client_cipher_suites, bool has_aes_hw, uint16_t version,
uint16_t group_id, const STACK_OF(SSL_CIPHER) *tls13_ciphers) {
const STACK_OF(SSL_CIPHER) *tls13_ciphers) {

const SSL_CIPHER *best = nullptr;
CipherScorer scorer(has_aes_hw);
Expand Down
10 changes: 9 additions & 1 deletion ssl/ssl_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,8 @@ SSL_CONFIG::SSL_CONFIG(SSL *ssl_arg)
permute_extensions(false),
conf_max_version_use_default(true),
conf_min_version_use_default(true),
alps_use_new_codepoint(false) {
alps_use_new_codepoint(false),
check_client_certificate_type(true) {
assert(ssl);
}

Expand Down Expand Up @@ -3224,6 +3225,13 @@ 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_quic_use_legacy_codepoint(SSL *ssl, int use_legacy) {
if (!ssl->config) {
return;
Expand Down
21 changes: 21 additions & 0 deletions ssl/test/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,27 @@ func addBasicTests() {
}),
},
},
{
name: "CheckClientCertificateTypes",
config: Config{
MaxVersion: VersionTLS12,
ClientAuth: RequestClientCert,
ClientCertificateTypes: []byte{CertTypeECDSASign},
},
shimCertificate: &rsaCertificate,
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
8 changes: 6 additions & 2 deletions ssl/test/test_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,7 @@ std::vector<Flag> SortedFlags() {
&TestConfig::application_settings),
OptionalStringFlag("-expect-peer-application-settings",
&TestConfig::expect_peer_application_settings),
BoolFlag("-alps-use-new-codepoint",
&TestConfig::alps_use_new_codepoint),
BoolFlag("-alps-use-new-codepoint", &TestConfig::alps_use_new_codepoint),
Base64Flag("-quic-transport-params", &TestConfig::quic_transport_params),
Base64Flag("-expect-quic-transport-params",
&TestConfig::expect_quic_transport_params),
Expand Down Expand Up @@ -429,6 +428,8 @@ std::vector<Flag> SortedFlags() {
StringFlag("-ssl-fuzz-seed-path-prefix", &TestConfig::ssl_fuzz_seed_path_prefix),
StringFlag("-tls13-ciphersuites", &TestConfig::tls13_ciphersuites),
StringPairVectorFlag("-multiple-certs-slot", &TestConfig::multiple_certs_slot),
BoolFlag("-no-check-client-certificate-type",
&TestConfig::no_check_client_certificate_type),
};
std::sort(flags.begin(), flags.end(), [](const Flag &a, const Flag &b) {
return strcmp(a.name, b.name) < 0;
Expand Down Expand Up @@ -1907,6 +1908,9 @@ bssl::UniquePtr<SSL> TestConfig::NewSSL(
if (enforce_rsa_key_usage) {
SSL_set_enforce_rsa_key_usage(ssl.get(), 1);
}
if (no_check_client_certificate_type) {
SSL_set_check_client_certificate_type(ssl.get(), 0);
}
if (no_tls13) {
SSL_set_options(ssl.get(), SSL_OP_NO_TLSv1_3);
}
Expand Down
1 change: 1 addition & 0 deletions ssl/test/test_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ struct TestConfig {
// When |multiple_certs_slot| is defined, the certificates defined are
// prioritized over certs defined with |cert_file| and |key_file|.
std::vector<std::pair<std::string, std::string>> multiple_certs_slot;
bool no_check_client_certificate_type = false;

std::vector<const char*> handshaker_args;

Expand Down
17 changes: 4 additions & 13 deletions ssl/tls13_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ static int ssl_ext_supported_versions_add_serverhello(SSL_HANDSHAKE *hs,
return 1;
}

static const SSL_CIPHER *choose_tls13_cipher(const SSL *ssl, uint16_t group_id) {
static const SSL_CIPHER *choose_tls13_cipher(const SSL *ssl) {
STACK_OF(SSL_CIPHER) *tls13_ciphers = nullptr;
if (ssl->ctx->tls13_cipher_list &&
ssl->ctx->tls13_cipher_list.get()->ciphers &&
Expand All @@ -122,7 +122,7 @@ static const SSL_CIPHER *choose_tls13_cipher(const SSL *ssl, uint16_t group_id)
ssl->config->aes_hw_override
? ssl->config->aes_hw_override_value
: EVP_has_aes_hardware(),
ssl_protocol_version(ssl), group_id, tls13_ciphers);
ssl_protocol_version(ssl), tls13_ciphers);
}

static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) {
Expand Down Expand Up @@ -227,21 +227,14 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
client_hello.session_id_len);
hs->session_id_len = client_hello.session_id_len;

uint16_t group_id;
if (!tls1_get_shared_group(hs, &group_id)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_GROUP);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
return ssl_hs_error;
}

if (!ssl_parse_client_cipher_list(ssl, &client_hello, &ssl->client_cipher_suites)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
return ssl_hs_error;
}

// Negotiate the cipher suite.
hs->new_cipher = choose_tls13_cipher(ssl, group_id);
hs->new_cipher = choose_tls13_cipher(ssl);
if (hs->new_cipher == NULL) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
Expand Down Expand Up @@ -581,22 +574,20 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) {

ScopedCBB cbb;
CBB body, session_id, extensions;
uint16_t group_id;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) ||
!CBB_add_u16(&body, TLS1_2_VERSION) ||
!CBB_add_bytes(&body, kHelloRetryRequest, SSL3_RANDOM_SIZE) ||
!CBB_add_u8_length_prefixed(&body, &session_id) ||
!CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
!CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
!CBB_add_u8(&body, 0 /* no compression */) ||
!tls1_get_shared_group(hs, &group_id) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
!CBB_add_u16(&extensions, TLSEXT_TYPE_supported_versions) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, ssl->version) ||
!CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, group_id)) {
!CBB_add_u16(&extensions, hs->new_session->group_id)) {
return ssl_hs_error;
}
if (hs->ech_is_inner) {
Expand Down

0 comments on commit f6d3673

Please sign in to comment.