Skip to content

Commit

Permalink
more comprehensive sig alg check
Browse files Browse the repository at this point in the history
  • Loading branch information
goatgoose committed May 29, 2024
1 parent 3e56d26 commit 43d6d2b
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 101 deletions.
20 changes: 13 additions & 7 deletions crypto/s2n_evp_signing.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,19 @@ static S2N_RESULT s2n_evp_signing_validate_sig_alg(const struct s2n_pkey *key, s

/* Ensure that the signature algorithm type matches the key type. */
int pkey_type = EVP_PKEY_base_id(key->pkey);
if (pkey_type == EVP_PKEY_EC) {
RESULT_ENSURE(sig_alg == S2N_SIGNATURE_ECDSA, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
} else {
/* Any RSA signature algorithm can be used with any RSA key, so just ensure that the
* signature algorithm is any RSA signature algorithm.
*/
RESULT_ENSURE(sig_alg != S2N_SIGNATURE_ECDSA, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
switch (pkey_type) {
case EVP_PKEY_RSA:
RESULT_ENSURE(sig_alg == S2N_SIGNATURE_RSA || sig_alg == S2N_SIGNATURE_RSA_PSS_RSAE,
S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
break;
case EVP_PKEY_RSA_PSS:
RESULT_ENSURE(sig_alg == S2N_SIGNATURE_RSA_PSS_PSS, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
break;
case EVP_PKEY_EC:
RESULT_ENSURE(sig_alg == S2N_SIGNATURE_ECDSA, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
break;
default:
RESULT_BAIL(S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
}

return S2N_RESULT_OK;
Expand Down
42 changes: 28 additions & 14 deletions tests/unit/s2n_rsa_pss_rsae_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,35 +241,49 @@ int main(int argc, char **argv)
/* Test: If they share the same RSA key,
* an RSA cert and an RSA_PSS cert are equivalent for PSS signatures. */
{
struct s2n_pkey rsa_pss_public_key;
s2n_pkey_type rsa_pss_pkey_type;
EXPECT_OK(s2n_asn1der_to_public_key_and_type(&rsa_pss_public_key, &rsa_pss_pkey_type, &rsa_pss_cert_chain->cert_chain->head->raw));
EXPECT_EQUAL(rsa_pss_pkey_type, S2N_PKEY_TYPE_RSA_PSS);

/* Set the keys equal. */
RSA *rsa_key_copy = EVP_PKEY_get1_RSA(rsa_public_key.pkey);
POSIX_GUARD_OSSL(EVP_PKEY_set1_RSA(rsa_pss_public_key.pkey, rsa_key_copy), S2N_ERR_KEY_INIT);
DEFER_CLEANUP(struct s2n_pkey rsa_public_key_shared = { 0 }, s2n_pkey_free);
s2n_pkey_type rsa_public_key_shared_type = S2N_PKEY_TYPE_UNKNOWN;
EXPECT_OK(s2n_asn1der_to_public_key_and_type(&rsa_public_key_shared, &rsa_public_key_shared_type,
&rsa_cert_chain->cert_chain->head->raw));
EXPECT_EQUAL(rsa_public_key_shared_type, S2N_PKEY_TYPE_RSA);

DEFER_CLEANUP(struct s2n_pkey rsa_pss_public_key_shared = { 0 }, s2n_pkey_free);
s2n_pkey_type rsa_pss_pkey_type_shared = S2N_PKEY_TYPE_UNKNOWN;
EXPECT_OK(s2n_asn1der_to_public_key_and_type(&rsa_pss_public_key_shared, &rsa_pss_pkey_type_shared,
&rsa_pss_cert_chain->cert_chain->head->raw));
EXPECT_EQUAL(rsa_pss_pkey_type_shared, S2N_PKEY_TYPE_RSA_PSS);

/* Set the keys equal.
*
* EVP_PKEY_set1_RSA sets the EVP key type to RSA, even if the set key is RSA_PSS. To
* ensure that the RSA_PSS EVP key type remains set to RSA_PSS, the RSA key is
* overridden instead of the RSA_PSS key, since its key type is RSA anyway.
*/
RSA *rsa_key_copy = EVP_PKEY_get1_RSA(rsa_pss_public_key_shared.pkey);
POSIX_GUARD_OSSL(EVP_PKEY_set1_RSA(rsa_public_key_shared.pkey, rsa_key_copy), S2N_ERR_KEY_INIT);
RSA_free(rsa_key_copy);

/* RSA signed with PSS, RSA_PSS verified with PSS */
{
hash_state_new(sign_hash, random_msg);
hash_state_new(verify_hash, random_msg);

EXPECT_SUCCESS(rsa_public_key.sign(rsa_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_RSAE, &sign_hash, &result));
EXPECT_SUCCESS(rsa_pss_public_key.verify(&rsa_public_key, S2N_SIGNATURE_RSA_PSS_PSS, &verify_hash, &result));
EXPECT_SUCCESS(rsa_public_key_shared.sign(rsa_pss_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_RSAE,
&sign_hash, &result));
EXPECT_SUCCESS(rsa_pss_public_key_shared.verify(&rsa_pss_public_key_shared, S2N_SIGNATURE_RSA_PSS_PSS,
&verify_hash, &result));
};

/* RSA_PSS signed with PSS, RSA verified with PSS */
{
hash_state_new(sign_hash, random_msg);
hash_state_new(verify_hash, random_msg);

EXPECT_SUCCESS(rsa_pss_public_key.sign(rsa_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_PSS, &sign_hash, &result));
EXPECT_SUCCESS(rsa_public_key.verify(&rsa_public_key, S2N_SIGNATURE_RSA_PSS_RSAE, &verify_hash, &result));
EXPECT_SUCCESS(rsa_pss_public_key_shared.sign(rsa_pss_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_PSS,
&sign_hash, &result));
EXPECT_SUCCESS(rsa_public_key_shared.verify(&rsa_public_key_shared, S2N_SIGNATURE_RSA_PSS_RSAE,
&verify_hash, &result));
};

EXPECT_SUCCESS(s2n_pkey_free(&rsa_pss_public_key));
};

EXPECT_SUCCESS(s2n_pkey_free(&rsa_public_key));
Expand Down
196 changes: 116 additions & 80 deletions tests/unit/s2n_tls13_cert_verify_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,86 +221,6 @@ int run_tests(const struct s2n_tls13_cert_verify_test *test_case, s2n_mode verif
EXPECT_FAILURE_WITH_ERRNO(s2n_tls13_cert_verify_recv(verifying_conn), S2N_ERR_VERIFY_SIGNATURE);
};

/* Self-talk: Ensure that the signature algorithm used to sign the CertificateVerify message
* is validated against the certificate type
*/
if (verifier_mode == S2N_CLIENT) {
const struct s2n_signature_scheme *test_sig_schemes[] = {
test_case->sig_scheme,
test_case->with_wrong_sig_alg,
};

for (size_t sig_idx = 0; sig_idx < s2n_array_len(test_sig_schemes); sig_idx++) {
DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(server_conn);
EXPECT_SUCCESS(s2n_connection_set_blinding(server_conn, S2N_SELF_SERVICE_BLINDING));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));

DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client_conn);
EXPECT_SUCCESS(s2n_connection_set_blinding(client_conn, S2N_SELF_SERVICE_BLINDING));
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));

/* The server will sign the CertificateVerify message with the signature and hash
* algorithms corresponding with its certificate, regardless of the client's
* supported signature algorithms.
*
* The client only supports the single test signature scheme, which allows for the
* server to sign the CertificateVerify message with a signature algorithm that isn't
* supported by the client.
*/
const struct s2n_signature_scheme *client_advertised_sig_scheme = test_sig_schemes[sig_idx];
struct s2n_signature_preferences test_sig_preferences = {
.count = 1,
.signature_schemes = &client_advertised_sig_scheme,
};
struct s2n_security_policy client_policy = security_policy_test_all_tls13;
client_policy.signature_preferences = &test_sig_preferences;
client_conn->security_policy_override = &client_policy;

struct s2n_test_io_pair io_pair = { 0 };
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Send the CertificateVerify message. */
EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server_conn, client_conn, SERVER_CERT_VERIFY));
EXPECT_SUCCESS(s2n_stuffer_rewrite(&server_conn->handshake.io));
EXPECT_SUCCESS(s2n_tls13_cert_verify_send(server_conn));

/* Check that the expected signature algorithm was used by the server. */
EXPECT_EQUAL(server_conn->handshake_params.server_cert_sig_scheme, test_case->sig_scheme);

/* Overwrite the SignatureScheme field of the CertificateVerify message to lie to the
* client about which signature algorithm was used to sign the signature content. This
* will trick the client into always thinking its advertised signature algorithm was
* used.
*/
struct s2n_stuffer cert_verify_stuffer = server_conn->handshake.io;
EXPECT_SUCCESS(s2n_stuffer_rewrite(&cert_verify_stuffer));
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&cert_verify_stuffer, client_advertised_sig_scheme->iana_value));

EXPECT_SUCCESS(s2n_stuffer_wipe(&client_conn->handshake.io));
EXPECT_SUCCESS(s2n_stuffer_copy(&server_conn->handshake.io, &client_conn->handshake.io,
s2n_stuffer_data_available(&server_conn->handshake.io)));

int ret = s2n_tls13_cert_verify_recv(client_conn);

if (client_advertised_sig_scheme == test_case->sig_scheme) {
/* If the client's advertised signature scheme matches what the server actually
* used to sign the CertificateVerify message, validation should succeed.
*/
EXPECT_SUCCESS(ret);
} else {
/* Otherwise, the client should observe that the indicated signature algorithm from
* the server doesn't match the certificate type, and the connection should fail.
*/
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
}
}
}

return 0;
}

Expand All @@ -318,5 +238,121 @@ int main(int argc, char **argv)
run_tests(&test_cases[i], S2N_SERVER);
}

/* Self-talk: Ensure that the signature algorithm used to sign the CertificateVerify message
* is validated against the certificate type
*/
if (s2n_is_tls13_fully_supported()) {
struct s2n_tls13_cert_verify_test sha256_test_cases[] = {
{
.cert_file = S2N_RSA_2048_PKCS1_CERT_CHAIN,
.key_file = S2N_RSA_2048_PKCS1_KEY,
.sig_scheme = &s2n_rsa_pss_rsae_sha256,
},
{
.cert_file = S2N_RSA_PSS_2048_SHA256_LEAF_CERT,
.key_file = S2N_RSA_PSS_2048_SHA256_LEAF_KEY,
.sig_scheme = &s2n_rsa_pss_pss_sha256,
},
{
.cert_file = S2N_ECDSA_P256_PKCS1_CERT_CHAIN,
.key_file = S2N_ECDSA_P256_PKCS1_KEY,
.sig_scheme = &s2n_ecdsa_sha256,
}
};

const struct s2n_signature_scheme *test_sha256_sig_algs[] = {
&s2n_rsa_pss_rsae_sha256,
&s2n_rsa_pss_pss_sha256,
&s2n_ecdsa_sha256,
};

for (size_t test_idx = 0; test_idx < s2n_array_len(sha256_test_cases); test_idx++) {
struct s2n_tls13_cert_verify_test test_case = sha256_test_cases[test_idx];

DEFER_CLEANUP(struct s2n_cert_chain_and_key *cert_chain = NULL, s2n_cert_chain_and_key_ptr_free);
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&cert_chain, test_case.cert_file, test_case.key_file));

DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "test_all_tls13"));
EXPECT_SUCCESS(s2n_config_disable_x509_verification(config));

/* The server only supports the single test certificate, and will only use the
* corresponding signature algorithm to sign the CertificateVerify message, regardless
* of the client's advertised signature schemes.
*/
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, cert_chain));

for (size_t sig_alg_idx = 0; sig_alg_idx < s2n_array_len(test_sha256_sig_algs); sig_alg_idx++) {
DEFER_CLEANUP(struct s2n_connection *server_conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(server_conn);
EXPECT_SUCCESS(s2n_connection_set_blinding(server_conn, S2N_SELF_SERVICE_BLINDING));
EXPECT_SUCCESS(s2n_connection_set_config(server_conn, config));

DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client_conn);
EXPECT_SUCCESS(s2n_connection_set_blinding(client_conn, S2N_SELF_SERVICE_BLINDING));
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));

/* The client only supports the single test signature scheme, which allows for the
* server to sign the CertificateVerify message with a signature algorithm that
* isn't supported by the client.
*/
const struct s2n_signature_scheme *client_advertised_sig_scheme = test_sha256_sig_algs[sig_alg_idx];
struct s2n_signature_preferences test_sig_preferences = {
.count = 1,
.signature_schemes = &client_advertised_sig_scheme,
};
struct s2n_security_policy client_policy = security_policy_test_all_tls13;
client_policy.signature_preferences = &test_sig_preferences;
client_conn->security_policy_override = &client_policy;

struct s2n_test_io_pair io_pair = { 0 };
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Send the CertificateVerify message. */
EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server_conn, client_conn,
SERVER_CERT_VERIFY));
EXPECT_SUCCESS(s2n_stuffer_rewrite(&server_conn->handshake.io));
EXPECT_SUCCESS(s2n_tls13_cert_verify_send(server_conn));

/* Check that the expected signature algorithm was used by the server. */
EXPECT_EQUAL(server_conn->handshake_params.server_cert_sig_scheme, test_case.sig_scheme);

/* Overwrite the SignatureScheme field of the CertificateVerify message to lie to the
* client about which signature algorithm was used to sign the signature content. This
* will trick the client into always thinking its advertised signature algorithm was
* used.
*/
struct s2n_stuffer cert_verify_stuffer = server_conn->handshake.io;
EXPECT_SUCCESS(s2n_stuffer_rewrite(&cert_verify_stuffer));
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&cert_verify_stuffer,
client_advertised_sig_scheme->iana_value));

EXPECT_SUCCESS(s2n_stuffer_wipe(&client_conn->handshake.io));
EXPECT_SUCCESS(s2n_stuffer_copy(&server_conn->handshake.io, &client_conn->handshake.io,
s2n_stuffer_data_available(&server_conn->handshake.io)));

int ret = s2n_tls13_cert_verify_recv(client_conn);

if (client_advertised_sig_scheme == test_case.sig_scheme) {
/* If the client's advertised signature scheme matches what the server actually
* used to sign the CertificateVerify message, validation should succeed.
*/
EXPECT_SUCCESS(ret);
} else {
/* Otherwise, the client should observe that the indicated signature algorithm
* from the server doesn't match the certificate type, and the connection
* should fail.
*/
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
}
}
}
}

END_TEST();
}

0 comments on commit 43d6d2b

Please sign in to comment.