Skip to content

Commit

Permalink
Merge pull request from GHSA-97r4-p6c4-5gv3
Browse files Browse the repository at this point in the history 
Co-authored-by: Lindsay Stewart <[email protected]>
  • Loading branch information
@dougch @lrstewart
dougch and lrstewart authored Oct 5, 2023
1 parent 2a6ead7 commit 4654fec
Show file tree
Hide file tree
Showing 4 changed files with 234 additions and 78 deletions.
59 changes: 0 additions & 59 deletions tests/integrationv2/test_signature_algorithms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,62 +190,3 @@ def test_s2n_client_signature_algorithms(managed_process, cipher, provider, othe
Provider.ServerMode, expected_signature(protocol, signature)) in results.stdout or not server_sigalg_used
assert (signature_marker(Provider.ClientMode, expected_signature(protocol, signature))
in results.stdout) == client_auth


@pytest.mark.uncollect_if(func=skip_ciphers)
@pytest.mark.parametrize("cipher", [Ciphers.SECURITY_POLICY_20210816], ids=get_parameter_name)
@pytest.mark.parametrize("provider", [OpenSSL])
@pytest.mark.parametrize("protocol", [Protocols.TLS12], ids=get_parameter_name)
@pytest.mark.parametrize("certificate", ALL_TEST_CERTS, ids=get_parameter_name)
# RSA_SHA224, ECDSA_SHA224 signature algorithms are not included in security_policy_20210816[].
@pytest.mark.parametrize("signature", [Signatures.RSA_SHA224, Signatures.ECDSA_SHA224], ids=get_parameter_name)
def test_s2n_server_tls12_signature_algorithm_fallback(managed_process, cipher, provider, protocol, certificate,
signature):
port = next(available_ports)

random_bytes = data_bytes(64)
client_options = ProviderOptions(
mode=Provider.ClientMode,
port=port,
data_to_send=random_bytes,
insecure=False,
signature_algorithm=signature,
protocol=protocol
)

server_options = copy.copy(client_options)
server_options.extra_flags = None
server_options.data_to_send = None
server_options.mode = Provider.ServerMode
server_options.key = certificate.key
server_options.cert = certificate.cert
server_options.cipher = cipher

server = managed_process(S2N, server_options, timeout=5)
client = managed_process(provider, client_options, timeout=5)

for results in client.get_results():
results.assert_success()

expected_version = get_expected_s2n_version(protocol, provider)

# Due to signature algorithm mismatch created, if the negotiated key exchange algorithm is :
# 1. (RSA,DHE_RSA, DH_RSA, RSA_PSK, ECDH_RSA,ECDHE_RSA), server is expected to behave as if client had
# sent value { sha1, rsa}.
# 2. (ECDH_ECDSA, ECDHE_ECDSA), server is expected to behave as if client had sent value {sha1,ecdsa}.
# This is the default tls1.2 fallback behavior and is the fallback behavior for missing signature_algorithms
# extension.
#
# This is inferred from the rfc- https://www.rfc-editor.org/rfc/rfc5246#section-7.4.1.4.1
if signature == Signatures.RSA_SHA224:
expected_sig_alg_tls12 = Signatures.RSA_SHA1
else:
expected_sig_alg_tls12 = Signatures.ECDSA_SHA1

for results in server.get_results():
results.assert_success()
assert to_bytes("Actual protocol version: {}".format(
expected_version)) in results.stdout
assert signature_marker(Provider.ServerMode,
expected_sig_alg_tls12) in results.stdout
assert random_bytes in results.stdout
7 changes: 7 additions & 0 deletions tests/unit/s2n_client_hello_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,13 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_connection_set_io_pair(client_conn, &io_pair));
EXPECT_SUCCESS(s2n_connection_set_io_pair(server_conn, &io_pair));

/* We have to update the client's security policy after it sends the ClientHello.
* The client sends all signature algorithms in its security policy, and
* won't accept any signature algorithm it receives that's not in its security policy.
* So we need to change the security policy between sending and receiving.
*/
EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server_conn, client_conn, SERVER_HELLO));
client_config->security_policy = &security_policy_20190214;
EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));

EXPECT_EQUAL(server_conn->secure->cipher_suite, &s2n_ecdhe_ecdsa_with_aes_128_cbc_sha);
Expand Down
198 changes: 196 additions & 2 deletions tests/unit/s2n_signature_algorithms_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "tls/s2n_security_policies.h"
#include "tls/s2n_signature_scheme.h"

#define LENGTH 3
#define LENGTH (s2n_array_len(test_signature_schemes))
#define STUFFER_SIZE (LENGTH * TLS_SIGNATURE_SCHEME_LEN + 10)

#define RSA_CIPHER_SUITE &s2n_ecdhe_rsa_with_aes_128_cbc_sha
Expand All @@ -37,6 +37,8 @@ const struct s2n_signature_scheme *const test_signature_schemes[] = {
&s2n_ecdsa_secp384r1_sha384,
&s2n_rsa_pkcs1_sha256,
&s2n_rsa_pkcs1_sha224,
&s2n_rsa_pkcs1_sha1,
&s2n_ecdsa_sha1,
};

const struct s2n_signature_preferences test_preferences = {
Expand All @@ -57,6 +59,8 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&ecdsa_cert_chain,
S2N_ECDSA_P384_PKCS1_CERT_CHAIN, S2N_ECDSA_P384_PKCS1_KEY));

struct s2n_cert_chain_and_key *certs[] = { ecdsa_cert_chain, rsa_cert_chain };

/* s2n_supported_sig_schemes_count & s2n_supported_sig_scheme_list_size */
{
struct s2n_config *config = s2n_config_new();
Expand Down Expand Up @@ -485,6 +489,45 @@ int main(int argc, char **argv)
EXPECT_EQUAL(result.iana_value, s2n_ecdsa_sha1.iana_value);
};
};

/* Do not fall back to a default if not allowed by security policy */
{
const struct s2n_signature_scheme *const no_defaults[] = {
&s2n_ecdsa_secp384r1_sha384,
&s2n_rsa_pkcs1_sha256,
&s2n_rsa_pkcs1_sha224,
};

const struct s2n_signature_preferences no_defaults_preferences = {
.count = s2n_array_len(no_defaults),
.signature_schemes = test_signature_schemes,
};

struct s2n_security_policy no_defaults_security_policy = {
.minimum_protocol_version = security_policy->minimum_protocol_version,
.cipher_preferences = security_policy->cipher_preferences,
.kem_preferences = security_policy->kem_preferences,
.signature_preferences = &no_defaults_preferences,
.ecc_preferences = security_policy->ecc_preferences,
};
conn->security_policy_override = &no_defaults_security_policy;

/* Client / RSA */
{
struct s2n_signature_scheme actual = { 0 };
conn->secure->cipher_suite = RSA_CIPHER_SUITE;
EXPECT_SUCCESS(s2n_choose_default_sig_scheme(conn, &actual, S2N_SERVER));
EXPECT_EQUAL(actual.iana_value, 0);
};

/* Server / ECDSA */
{
struct s2n_signature_scheme actual = { 0 };
conn->handshake_params.client_cert_pkey_type = S2N_PKEY_TYPE_ECDSA;
EXPECT_SUCCESS(s2n_choose_default_sig_scheme(conn, &actual, S2N_CLIENT));
EXPECT_EQUAL(actual.iana_value, 0);
};
};
};

s2n_connection_free(conn);
Expand Down Expand Up @@ -567,7 +610,7 @@ int main(int argc, char **argv)
/* Peer list contains no signature schemes that we support */
struct s2n_sig_scheme_list peer_list = {
.len = 1,
.iana_list = { 0 },
.iana_list = { 1 },
};

EXPECT_SUCCESS(s2n_choose_sig_scheme_from_peer_preference_list(conn, &peer_list, &result));
Expand Down Expand Up @@ -835,6 +878,157 @@ int main(int argc, char **argv)
s2n_config_free(config);
}

/* Self-Talk tests: default signature schemes */
{
const struct s2n_signature_scheme *const default_schemes[] = {
&s2n_rsa_pkcs1_sha1,
&s2n_ecdsa_sha1
};

const struct s2n_signature_scheme *const sha256_schemes[] = {
&s2n_rsa_pkcs1_sha256,
&s2n_ecdsa_sha256
};

const struct s2n_signature_scheme *const sha384_schemes[] = {
&s2n_rsa_pkcs1_sha384,
&s2n_ecdsa_sha384
};

const struct s2n_signature_preferences defaults_preferences = {
.count = s2n_array_len(default_schemes),
.signature_schemes = default_schemes,
};

const struct s2n_signature_preferences sha256_preferences = {
.count = s2n_array_len(sha256_schemes),
.signature_schemes = sha256_schemes,
};
for(size_t i = 0; i < sha256_preferences.count; i++) {
for(size_t j = 0; j < defaults_preferences.count; j++) {
EXPECT_NOT_EQUAL(sha256_preferences.signature_schemes[i]->iana_value,
defaults_preferences.signature_schemes[j]->iana_value);
}
}

const struct s2n_signature_preferences sha384_preferences = {
.count = s2n_array_len(sha384_schemes),
.signature_schemes = sha384_schemes,
};
for(size_t i = 0; i < sha384_preferences.count; i++) {
for(size_t j = 0; j < defaults_preferences.count; j++) {
EXPECT_NOT_EQUAL(sha384_preferences.signature_schemes[i]->iana_value,
defaults_preferences.signature_schemes[j]->iana_value);
}
}

/* The policy needs to negotiate TLS1.2 and forward secret kex */
struct s2n_security_policy defaults_policy = security_policy_20190214;
defaults_policy.signature_preferences = &defaults_preferences;
struct s2n_security_policy sha256_policy = security_policy_20190214;
sha256_policy.signature_preferences = &sha256_preferences;
struct s2n_security_policy sha384_policy = security_policy_20190214;
sha384_policy.signature_preferences = &sha384_preferences;

/* Self-Talk test: client and server can negotiate without any defaults */
for (size_t cert_i = 0; cert_i < s2n_array_len(certs); cert_i++) {
/* Setup config */
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config));
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, certs[cert_i]));

/* Setup connections */
DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));
EXPECT_SUCCESS(s2n_connection_set_blinding(client_conn, S2N_SELF_SERVICE_BLINDING));

/* Create nonblocking pipes */
DEFER_CLEANUP(struct s2n_test_io_pair io_pair, s2n_io_pair_close);
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Client and server security policies should match but include no defaults */
client_conn->security_policy_override = &sha256_policy;
server_conn->security_policy_override = &sha256_policy;

EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));
EXPECT_EQUAL(client_conn->actual_protocol_version, S2N_TLS12);
EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);
};

/* Self-Talk test: server does not fallback to unsupported defaults */
for (size_t cert_i = 0; cert_i < s2n_array_len(certs); cert_i++) {
/* Setup config */
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config));
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, certs[cert_i]));

/* Setup connections */
DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));
EXPECT_SUCCESS(s2n_connection_set_blinding(server_conn, S2N_SELF_SERVICE_BLINDING));

/* Create nonblocking pipes */
DEFER_CLEANUP(struct s2n_test_io_pair io_pair, s2n_io_pair_close);
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Client and server security policies should have no signature schemes
* in common, and not include the default signature schemes.
*/
client_conn->security_policy_override = &sha256_policy;
server_conn->security_policy_override = &sha384_policy;

EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate_test_server_and_client(server_conn, client_conn),
S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
EXPECT_EQUAL(client_conn->actual_protocol_version, S2N_TLS12);
EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);
};

/* Self-Talk test: client does not accept unsupported defaults */
for (size_t cert_i = 0; cert_i < s2n_array_len(certs); cert_i++) {
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config));
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, certs[cert_i]));

/* Setup connections */
DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));
EXPECT_SUCCESS(s2n_connection_set_blinding(client_conn, S2N_SELF_SERVICE_BLINDING));

/* Create nonblocking pipes */
DEFER_CLEANUP(struct s2n_test_io_pair io_pair, s2n_io_pair_close);
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Client and server security policies should have no signature schemes in common.
* Server should include the default policies.
*/
client_conn->security_policy_override = &sha256_policy;
server_conn->security_policy_override = &defaults_policy;

EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate_test_server_and_client(server_conn, client_conn),
S2N_ERR_INVALID_SIGNATURE_SCHEME);
EXPECT_EQUAL(client_conn->actual_protocol_version, S2N_TLS12);
EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);
};
};

EXPECT_SUCCESS(s2n_cert_chain_and_key_free(rsa_cert_chain));
EXPECT_SUCCESS(s2n_cert_chain_and_key_free(ecdsa_cert_chain));

Expand Down
48 changes: 31 additions & 17 deletions tls/s2n_signature_algorithms.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,19 +165,6 @@ int s2n_get_and_validate_negotiated_signature_scheme(struct s2n_connection *conn
}
}

/* We require an exact match in TLS 1.3, but all previous versions can fall back to the default SignatureScheme.
* This means that an s2n client will accept the default SignatureScheme from a TLS server, even if the client did
* not send it in it's ClientHello. This pre-TLS1.3 behavior is an intentional choice to maximize support. */
struct s2n_signature_scheme default_scheme = { 0 };
POSIX_GUARD(s2n_choose_default_sig_scheme(conn, &default_scheme, S2N_PEER_MODE(conn->mode)));

if ((conn->actual_protocol_version <= S2N_TLS12)
&& (s2n_signature_scheme_valid_to_accept(conn, &default_scheme) == S2N_SUCCESS)
&& (actual_iana_val == default_scheme.iana_value)) {
*chosen_sig_scheme = default_scheme;
return S2N_SUCCESS;
}

POSIX_BAIL(S2N_ERR_INVALID_SIGNATURE_SCHEME);
}

Expand All @@ -198,15 +185,42 @@ int s2n_choose_default_sig_scheme(struct s2n_connection *conn, struct s2n_signat
/* Default our signature digest algorithms.
* For >=TLS 1.2 this default may be overridden by the signature_algorithms extension.
*/
const struct s2n_signature_scheme *default_sig_scheme = &s2n_rsa_pkcs1_md5_sha1;
if (auth_method == S2N_AUTHENTICATION_ECDSA) {
*sig_scheme_out = s2n_ecdsa_sha1;
default_sig_scheme = &s2n_ecdsa_sha1;
} else if (conn->actual_protocol_version >= S2N_TLS12) {
*sig_scheme_out = s2n_rsa_pkcs1_sha1;
default_sig_scheme = &s2n_rsa_pkcs1_sha1;
}

if (conn->actual_protocol_version < S2N_TLS12) {
/* Before TLS1.2, signature algorithms were fixed, not chosen / negotiated. */
*sig_scheme_out = *default_sig_scheme;
return S2N_SUCCESS;
} else {
*sig_scheme_out = s2n_rsa_pkcs1_md5_sha1;
/* If we attempt to negotiate a default in TLS1.2, we should ensure that
* default is allowed by the local security policy.
*/
const struct s2n_signature_preferences *signature_preferences = NULL;
POSIX_GUARD(s2n_connection_get_signature_preferences(conn, &signature_preferences));
POSIX_ENSURE_REF(signature_preferences);
for (size_t i = 0; i < signature_preferences->count; i++) {
if (signature_preferences->signature_schemes[i]->iana_value == default_sig_scheme->iana_value) {
*sig_scheme_out = *default_sig_scheme;
return S2N_SUCCESS;
}
}
/* We cannot bail with an error here because existing logic assumes
* that this method should always succeed and calls it even when no default
* is actually necessary.
* If no valid default exists, set an unusable, invalid empty scheme.
*/
*sig_scheme_out = (struct s2n_signature_scheme){
.hash_alg = S2N_HASH_NONE,
.sig_alg = S2N_SIGNATURE_ANONYMOUS,
};
return S2N_SUCCESS;
}

return S2N_SUCCESS;
}

int s2n_choose_sig_scheme_from_peer_preference_list(struct s2n_connection *conn, struct s2n_sig_scheme_list *peer_wire_prefs,
Expand Down

0 comments on commit 4654fec

Please sign in to comment.