Skip to content

Commit

Permalink
Clean up selecting a signature algorithm (#4285)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lrstewart
lrstewart authored Nov 29, 2023
1 parent 8b10ecf commit ea337a4
Show file tree
Hide file tree
Showing 12 changed files with 944 additions and 739 deletions.
1 change: 1 addition & 0 deletions error/s2n_errno.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_DECODE_PRIVATE_KEY, "error decoding private key") \
ERR_ENTRY(S2N_ERR_INVALID_SIGNATURE_ALGORITHM, "Invalid signature algorithm") \
ERR_ENTRY(S2N_ERR_INVALID_SIGNATURE_SCHEME, "Invalid signature scheme") \
ERR_ENTRY(S2N_ERR_NO_VALID_SIGNATURE_SCHEME, "Unable to negotiate a supported signature scheme") \
ERR_ENTRY(S2N_ERR_CBC_VERIFY, "Failed CBC verification") \
ERR_ENTRY(S2N_ERR_DH_COPYING_PUBLIC_KEY, "error copying Diffie-Hellman public key") \
ERR_ENTRY(S2N_ERR_SIGN, "error signing data") \
Expand Down
1 change: 1 addition & 0 deletions error/s2n_errno.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ typedef enum {
S2N_ERR_INVALID_HELLO_RETRY,
S2N_ERR_INVALID_SIGNATURE_ALGORITHM,
S2N_ERR_INVALID_SIGNATURE_SCHEME,
S2N_ERR_NO_VALID_SIGNATURE_SCHEME,
S2N_ERR_CBC_VERIFY,
S2N_ERR_DH_COPYING_PUBLIC_KEY,
S2N_ERR_SIGN,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_client_hello_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ int main(int argc, char **argv)

EXPECT_SUCCESS(s2n_client_hello_send(conn));
EXPECT_TRUE(s2n_stuffer_data_available(&conn->handshake.io) > 0);
EXPECT_FAILURE_WITH_ERRNO(s2n_client_hello_recv(conn), S2N_ERR_INVALID_SIGNATURE_SCHEME);
EXPECT_FAILURE_WITH_ERRNO(s2n_client_hello_recv(conn), S2N_ERR_NO_VALID_SIGNATURE_SCHEME);

EXPECT_SUCCESS(s2n_connection_free(conn));
};
Expand Down
30 changes: 1 addition & 29 deletions tests/unit/s2n_client_signature_algorithms_extension_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,33 +71,6 @@ int main(int argc, char **argv)
s2n_connection_free(server_conn);
};

/* Test that unknown TLS_EXTENSION_SIGNATURE_ALGORITHMS values are ignored and negotiation fails */
{
struct s2n_sig_scheme_list sig_hash_algs = {
.iana_list = { 0xFF01, 0xFFFF },
.len = 2,
};
struct s2n_connection *conn;
EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));

struct s2n_stuffer signature_algorithms_extension = { 0 };
EXPECT_SUCCESS(s2n_stuffer_alloc(&signature_algorithms_extension, 2 + (sig_hash_algs.len * 2)));
POSIX_GUARD(s2n_stuffer_write_uint16(&signature_algorithms_extension, sig_hash_algs.len * 2));
for (size_t i = 0; i < sig_hash_algs.len; i++) {
POSIX_GUARD(s2n_stuffer_write_uint16(&signature_algorithms_extension, sig_hash_algs.iana_list[i]));
}

/* If only unknown algorithms are offered, expect choosing a scheme to fail for TLS1.3 */
conn->actual_protocol_version = S2N_TLS13;
EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(conn, &signature_algorithms_extension));
EXPECT_EQUAL(conn->handshake_params.client_sig_hash_algs.len, sig_hash_algs.len);
EXPECT_FAILURE(s2n_choose_sig_scheme_from_peer_preference_list(conn, &conn->handshake_params.client_sig_hash_algs,
&conn->handshake_params.server_cert_sig_scheme));

EXPECT_SUCCESS(s2n_stuffer_free(&signature_algorithms_extension));
EXPECT_SUCCESS(s2n_connection_free(conn));
};

/* Test that a valid algorithm is chosen when it is offered among unknown algorithms */
{
struct s2n_sig_scheme_list sig_hash_algs = {
Expand All @@ -119,8 +92,7 @@ int main(int argc, char **argv)
/* If a valid algorithm is offered among unknown algorithms, the valid one should be chosen */
EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(conn, &signature_algorithms_extension));
EXPECT_EQUAL(conn->handshake_params.client_sig_hash_algs.len, sig_hash_algs.len);
EXPECT_SUCCESS(s2n_choose_sig_scheme_from_peer_preference_list(conn, &conn->handshake_params.client_sig_hash_algs,
&conn->handshake_params.server_cert_sig_scheme));
EXPECT_OK(s2n_signature_algorithm_select(conn));
EXPECT_EQUAL(conn->handshake_params.server_cert_sig_scheme->iana_value, TLS_SIGNATURE_SCHEME_RSA_PKCS1_SHA384);

EXPECT_SUCCESS(s2n_stuffer_free(&signature_algorithms_extension));
Expand Down
120 changes: 0 additions & 120 deletions tests/unit/s2n_security_policies_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,126 +106,6 @@ int main(int argc, char **argv)
}

if (has_tls_13_cipher) {
/* Validate that s2n_tls13_default_sig_scheme() is successful on all TLS 1.3 Security Policies for all
* TLS 1.3 Ciphers */
{
struct s2n_cipher_suite tls_13_ciphers[] = {
s2n_tls13_aes_128_gcm_sha256,
s2n_tls13_aes_256_gcm_sha384,
s2n_tls13_chacha20_poly1305_sha256
};

for (size_t i = 0; i < s2n_array_len(tls_13_ciphers); i++) {
struct s2n_config *config = s2n_config_new();
EXPECT_NOT_NULL(config);

if (security_policy_selection[policy_index].security_policy->minimum_protocol_version > s2n_get_highest_fully_supported_tls_version()) {
/* We purposefully do not allow users to configure Security Policies with a minimum allowed TLS
* versions that are greater than what libcrypto supports. */
EXPECT_FAILURE(s2n_config_set_cipher_preferences(config, security_policy_selection[policy_index].version));
EXPECT_SUCCESS(s2n_config_free(config));
continue;
}

struct s2n_cert_chain_and_key *default_cert;
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&default_cert, S2N_DEFAULT_TEST_CERT_CHAIN, S2N_DEFAULT_TEST_PRIVATE_KEY));

EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, default_cert));
EXPECT_TRUE(config->is_rsa_cert_configured);

struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT);
struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER);
EXPECT_NOT_NULL(client_conn);
EXPECT_NOT_NULL(server_conn);

EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, security_policy_selection[policy_index].version));
EXPECT_SUCCESS(s2n_config_set_verification_ca_location(config, S2N_DEFAULT_TEST_CERT_CHAIN, NULL));
EXPECT_NOT_NULL(config->default_certs_by_type.certs[S2N_PKEY_TYPE_RSA]);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));

client_conn->actual_protocol_version = S2N_TLS13;
server_conn->actual_protocol_version = S2N_TLS13;
client_conn->secure->cipher_suite = &tls_13_ciphers[i];
server_conn->secure->cipher_suite = &tls_13_ciphers[i];

const struct s2n_signature_scheme *chosen_scheme = NULL;

if (s2n_is_rsa_pss_signing_supported()) {
/* If RSA PSS signing is supported, then we should always be able to select a default Signature
* Scheme for RSA Certs for TLS 1.3 */
EXPECT_SUCCESS(s2n_tls13_default_sig_scheme(client_conn, &chosen_scheme));
EXPECT_SUCCESS(s2n_tls13_default_sig_scheme(server_conn, &chosen_scheme));
} else {
/* We can't pick a default TLS 1.3 signature scheme when configured with an RSA Cert when we
* do not support RSA PSS signing since RSA PSS signing is required for TLS 1.3 */
EXPECT_FAILURE(s2n_tls13_default_sig_scheme(client_conn, &chosen_scheme));
EXPECT_FAILURE(s2n_tls13_default_sig_scheme(server_conn, &chosen_scheme));
}

EXPECT_SUCCESS(s2n_connection_free(client_conn));
EXPECT_SUCCESS(s2n_connection_free(server_conn));
EXPECT_SUCCESS(s2n_config_free(config));
EXPECT_SUCCESS(s2n_cert_chain_and_key_free(default_cert));
}
}

/* Same as above test, but with ECDSA Certificates */
{
struct s2n_cipher_suite tls_13_ciphers[] = {
s2n_tls13_aes_128_gcm_sha256,
s2n_tls13_aes_256_gcm_sha384,
s2n_tls13_chacha20_poly1305_sha256
};

for (size_t i = 0; i < s2n_array_len(tls_13_ciphers); i++) {
struct s2n_config *config = s2n_config_new();
EXPECT_NOT_NULL(config);

if (security_policy_selection[policy_index].security_policy->minimum_protocol_version > s2n_get_highest_fully_supported_tls_version()) {
/* We purposefully do not allow users to configure Security Policies with a minimum allowed TLS
* Version of TLS 1.3, if TLS 1.3 algorithms aren't fully supported by the libcrypto we're using */
EXPECT_FAILURE(s2n_config_set_cipher_preferences(config, security_policy_selection[policy_index].version));
EXPECT_SUCCESS(s2n_config_free(config));
continue;
}

struct s2n_cert_chain_and_key *default_cert;
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&default_cert, S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY));

EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, default_cert));
EXPECT_FALSE(config->is_rsa_cert_configured);

struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT);
struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER);
EXPECT_NOT_NULL(client_conn);
EXPECT_NOT_NULL(server_conn);

EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, security_policy_selection[policy_index].version));
EXPECT_SUCCESS(s2n_config_set_verification_ca_location(config, S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, NULL));
EXPECT_NOT_NULL(config->default_certs_by_type.certs[S2N_PKEY_TYPE_ECDSA]);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));

client_conn->actual_protocol_version = S2N_TLS13;
server_conn->actual_protocol_version = S2N_TLS13;
client_conn->secure->cipher_suite = &tls_13_ciphers[i];
server_conn->secure->cipher_suite = &tls_13_ciphers[i];

const struct s2n_signature_scheme *chosen_scheme = NULL;

/* If an ECDSA Certificate is configured, then we should always be able to pick a default Signature
* Scheme (even if RSA PSS is not supported by the libcrypto) */
EXPECT_SUCCESS(s2n_tls13_default_sig_scheme(client_conn, &chosen_scheme));
EXPECT_SUCCESS(s2n_tls13_default_sig_scheme(server_conn, &chosen_scheme));

EXPECT_SUCCESS(s2n_connection_free(client_conn));
EXPECT_SUCCESS(s2n_connection_free(server_conn));
EXPECT_SUCCESS(s2n_config_free(config));
EXPECT_SUCCESS(s2n_cert_chain_and_key_free(default_cert));
}
}

bool has_tls_13_sig_alg = false;
bool has_rsa_pss = false;

Expand Down
Loading

0 comments on commit ea337a4

Please sign in to comment.