Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream merge 2024 11 18 #2012

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Upstream merge 2024 11 18 #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75d31b4
Check client certificate types in TLS <= 1.2
davidben Mar 1, 2024
899bd22
Remove unused group_id parameter in TLS 1.3 cipher suite selection
davidben Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions include/openssl/ssl.h
View file
Open in desktop
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
View file
Open in desktop
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();
}
skmcgrail marked this conversation as resolved.
Show resolved Hide resolved

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
View file
Open in desktop
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.
skmcgrail marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading