fix: Validate received signature algorithm in EVP verify (#4574)

crypto/s2n_evp_signing.c

@@ -19,6 +19,7 @@
 #include "crypto/s2n_pkey.h"
 #include "crypto/s2n_rsa_pss.h"
 #include "error/s2n_errno.h"
+#include "tls/s2n_signature_algorithms.h"
 #include "utils/s2n_safety.h"
 
 DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY_CTX *, EVP_PKEY_CTX_free);
@@ -97,6 +98,20 @@ static S2N_RESULT s2n_evp_signing_validate_hash_alg(s2n_signature_algorithm sig_
     return S2N_RESULT_OK;
 }
 
+static S2N_RESULT s2n_evp_signing_validate_sig_alg(const struct s2n_pkey *key, s2n_signature_algorithm sig_alg)
+{
+    RESULT_ENSURE_REF(key);
+
+    /* Ensure that the signature algorithm type matches the key type. */
+    s2n_pkey_type pkey_type = S2N_PKEY_TYPE_UNKNOWN;
+    RESULT_GUARD(s2n_pkey_get_type(key->pkey, &pkey_type));
+    s2n_pkey_type sig_alg_type = S2N_PKEY_TYPE_UNKNOWN;
+    RESULT_GUARD(s2n_signature_algorithm_get_pkey_type(sig_alg, &sig_alg_type));
+    RESULT_ENSURE(pkey_type == sig_alg_type, S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
+
+    return S2N_RESULT_OK;
+}
+
 int s2n_evp_sign(const struct s2n_pkey *priv, s2n_signature_algorithm sig_alg,
         struct s2n_hash_state *hash_state, struct s2n_blob *signature)
 {
@@ -136,6 +151,7 @@ int s2n_evp_verify(const struct s2n_pkey *pub, s2n_signature_algorithm sig_alg,
     POSIX_ENSURE_REF(signature);
     POSIX_ENSURE(s2n_evp_signing_supported(), S2N_ERR_HASH_NOT_READY);
     POSIX_GUARD_RESULT(s2n_evp_signing_validate_hash_alg(sig_alg, hash_state->alg));
+    POSIX_GUARD_RESULT(s2n_evp_signing_validate_sig_alg(pub, sig_alg));
 
     DEFER_CLEANUP(EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(pub->pkey, NULL), EVP_PKEY_CTX_free_pointer);
     POSIX_ENSURE_REF(pctx);

crypto/s2n_pkey.c

@@ -194,6 +194,30 @@ S2N_RESULT s2n_asn1der_to_public_key_and_type(struct s2n_pkey *pub_key,
     return S2N_RESULT_OK;
 }
 
+S2N_RESULT s2n_pkey_get_type(EVP_PKEY *evp_pkey, s2n_pkey_type *pkey_type)
+{
+    RESULT_ENSURE_REF(evp_pkey);
+    RESULT_ENSURE_REF(pkey_type);
+    *pkey_type = S2N_PKEY_TYPE_UNKNOWN;
+
+    int type = EVP_PKEY_base_id(evp_pkey);
+    switch (type) {
+        case EVP_PKEY_RSA:
+            *pkey_type = S2N_PKEY_TYPE_RSA;
+            break;
+        case EVP_PKEY_RSA_PSS:
+            *pkey_type = S2N_PKEY_TYPE_RSA_PSS;
+            break;
+        case EVP_PKEY_EC:
+            *pkey_type = S2N_PKEY_TYPE_ECDSA;
+            break;
+        default:
+            RESULT_BAIL(S2N_ERR_DECODE_CERTIFICATE);
+    }
+
+    return S2N_RESULT_OK;
+}
+
 S2N_RESULT s2n_pkey_from_x509(X509 *cert, struct s2n_pkey *pub_key_out,
         s2n_pkey_type *pkey_type_out)
 {
@@ -204,23 +228,19 @@ S2N_RESULT s2n_pkey_from_x509(X509 *cert, struct s2n_pkey *pub_key_out,
     DEFER_CLEANUP(EVP_PKEY *evp_public_key = X509_get_pubkey(cert), EVP_PKEY_free_pointer);
     RESULT_ENSURE(evp_public_key != NULL, S2N_ERR_DECODE_CERTIFICATE);
 
-    /* Check for success in decoding certificate according to type */
-    int type = EVP_PKEY_base_id(evp_public_key);
-    switch (type) {
-        case EVP_PKEY_RSA:
+    RESULT_GUARD(s2n_pkey_get_type(evp_public_key, pkey_type_out));
+    switch (*pkey_type_out) {
+        case S2N_PKEY_TYPE_RSA:
             RESULT_GUARD(s2n_rsa_pkey_init(pub_key_out));
             RESULT_GUARD(s2n_evp_pkey_to_rsa_public_key(&pub_key_out->key.rsa_key, evp_public_key));
-            *pkey_type_out = S2N_PKEY_TYPE_RSA;
             break;
-        case EVP_PKEY_RSA_PSS:
+        case S2N_PKEY_TYPE_RSA_PSS:
             RESULT_GUARD(s2n_rsa_pss_pkey_init(pub_key_out));
             RESULT_GUARD(s2n_evp_pkey_to_rsa_pss_public_key(&pub_key_out->key.rsa_key, evp_public_key));
-            *pkey_type_out = S2N_PKEY_TYPE_RSA_PSS;
             break;
-        case EVP_PKEY_EC:
+        case S2N_PKEY_TYPE_ECDSA:
             RESULT_GUARD(s2n_ecdsa_pkey_init(pub_key_out));
             RESULT_GUARD(s2n_evp_pkey_to_ecdsa_public_key(&pub_key_out->key.ecdsa_key, evp_public_key));
-            *pkey_type_out = S2N_PKEY_TYPE_ECDSA;
             break;
         default:
             RESULT_BAIL(S2N_ERR_DECODE_CERTIFICATE);

crypto/s2n_pkey.h

@@ -71,5 +71,6 @@ int s2n_pkey_free(struct s2n_pkey *pkey);
 
 S2N_RESULT s2n_asn1der_to_private_key(struct s2n_pkey *priv_key, struct s2n_blob *asn1der, int type_hint);
 S2N_RESULT s2n_asn1der_to_public_key_and_type(struct s2n_pkey *pub_key, s2n_pkey_type *pkey_type, struct s2n_blob *asn1der);
+S2N_RESULT s2n_pkey_get_type(EVP_PKEY *evp_pkey, s2n_pkey_type *pkey_type);
 S2N_RESULT s2n_pkey_from_x509(X509 *cert, struct s2n_pkey *pub_key_out,
         s2n_pkey_type *pkey_type_out);

tests/unit/s2n_rsa_pss_rsae_test.c

@@ -245,35 +245,53 @@ 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_as_pss = { 0 }, s2n_pkey_free);
+            s2n_pkey_type rsa_public_key_as_pss_type = S2N_PKEY_TYPE_UNKNOWN;
+            EXPECT_OK(s2n_asn1der_to_public_key_and_type(&rsa_public_key_as_pss, &rsa_public_key_as_pss_type,
+                    &rsa_cert_chain->cert_chain->head->raw));
+            EXPECT_EQUAL(rsa_public_key_as_pss_type, S2N_PKEY_TYPE_RSA);
+
+            DEFER_CLEANUP(struct s2n_pkey rsa_pss_public_key = { 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, &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.
+             *
+             * When EVP signing APIs are enabled, s2n-tls validates the signature algorithm type
+             * against the EVP pkey type, so the EVP pkey type must be RSA_PSS in order to use the
+             * RSA_PSS signature algorithm. However, EVP_PKEY_set1_RSA sets the EVP pkey type to
+             * RSA, even if the EVP pkey type was RSA_PSS (there is no EVP_PKEY_set1_RSA_PSS API).
+             *
+             * To ensure that the RSA_PSS EVP pkey type remains set to RSA_PSS, the RSA key is
+             * overridden instead of the RSA_PSS key, since its pkey type is RSA anyway.
+             */
+            RSA *rsa_key_copy = EVP_PKEY_get1_RSA(rsa_pss_public_key.pkey);
+            POSIX_GUARD_OSSL(EVP_PKEY_set1_RSA(rsa_public_key_as_pss.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_as_pss.sign(rsa_pss_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_RSAE,
+                        &sign_hash, &result));
+                EXPECT_SUCCESS(rsa_pss_public_key.verify(&rsa_pss_public_key, 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.sign(rsa_pss_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_PSS,
+                        &sign_hash, &result));
+                EXPECT_SUCCESS(rsa_public_key_as_pss.verify(&rsa_public_key_as_pss, 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));

tests/unit/s2n_tls13_cert_verify_test.c

@@ -234,5 +234,123 @@ 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 test_server_parameters[] = {
+            {
+                    .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_client_sig_schemes[] = {
+            &s2n_rsa_pss_rsae_sha256,
+            &s2n_rsa_pss_pss_sha256,
+            &s2n_ecdsa_sha256,
+        };
+
+        for (size_t param_idx = 0; param_idx < s2n_array_len(test_server_parameters); param_idx++) {
+            struct s2n_tls13_cert_verify_test server_parameters = test_server_parameters[param_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, server_parameters.cert_file,
+                    server_parameters.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. Due to the fallback logic in
+             * the s2n-tls server, the signature algorithm corresponding with the test certificate
+             * will always be used 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_idx = 0; sig_idx < s2n_array_len(test_client_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 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_client_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, server_parameters.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 == server_parameters.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();
 }

tls/s2n_auth_selection.c

@@ -20,6 +20,7 @@
 #include "crypto/s2n_signature.h"
 #include "tls/s2n_cipher_suites.h"
 #include "tls/s2n_kex.h"
+#include "tls/s2n_signature_algorithms.h"
 #include "utils/s2n_safety.h"
 
 /* This module should contain any logic related to choosing a valid combination of
@@ -56,31 +57,12 @@ int s2n_get_auth_method_for_cert_type(s2n_pkey_type cert_type, s2n_authenticatio
     POSIX_BAIL(S2N_ERR_CERT_TYPE_UNSUPPORTED);
 }
 
-static int s2n_get_cert_type_for_sig_alg(s2n_signature_algorithm sig_alg, s2n_pkey_type *cert_type)
-{
-    switch (sig_alg) {
-        case S2N_SIGNATURE_RSA_PSS_RSAE:
-        case S2N_SIGNATURE_RSA:
-            *cert_type = S2N_PKEY_TYPE_RSA;
-            return S2N_SUCCESS;
-        case S2N_SIGNATURE_ECDSA:
-            *cert_type = S2N_PKEY_TYPE_ECDSA;
-            return S2N_SUCCESS;
-        case S2N_SIGNATURE_RSA_PSS_PSS:
-            *cert_type = S2N_PKEY_TYPE_RSA_PSS;
-            return S2N_SUCCESS;
-        case S2N_SIGNATURE_ANONYMOUS:
-            POSIX_BAIL(S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
-    }
-    POSIX_BAIL(S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
-}
-
 static int s2n_is_sig_alg_valid_for_cipher_suite(s2n_signature_algorithm sig_alg, struct s2n_cipher_suite *cipher_suite)
 {
     POSIX_ENSURE_REF(cipher_suite);
 
-    s2n_pkey_type cert_type_for_sig_alg;
-    POSIX_GUARD(s2n_get_cert_type_for_sig_alg(sig_alg, &cert_type_for_sig_alg));
+    s2n_pkey_type cert_type_for_sig_alg = S2N_PKEY_TYPE_UNKNOWN;
+    POSIX_GUARD_RESULT(s2n_signature_algorithm_get_pkey_type(sig_alg, &cert_type_for_sig_alg));
 
     /* Non-ephemeral key exchange methods require encryption, and RSA-PSS certificates
      * do not support encryption.
@@ -110,8 +92,8 @@ static int s2n_certs_exist_for_sig_scheme(struct s2n_connection *conn, const str
 {
     POSIX_ENSURE_REF(sig_scheme);
 
-    s2n_pkey_type cert_type;
-    POSIX_GUARD(s2n_get_cert_type_for_sig_alg(sig_scheme->sig_alg, &cert_type));
+    s2n_pkey_type cert_type = S2N_PKEY_TYPE_UNKNOWN;
+    POSIX_GUARD_RESULT(s2n_signature_algorithm_get_pkey_type(sig_scheme->sig_alg, &cert_type));
 
     /* A valid cert must exist for the authentication method. */
     struct s2n_cert_chain_and_key *cert = s2n_get_compatible_cert_chain_and_key(conn, cert_type);
@@ -226,8 +208,8 @@ int s2n_select_certs_for_server_auth(struct s2n_connection *conn, struct s2n_cer
     POSIX_ENSURE_REF(conn->handshake_params.server_cert_sig_scheme);
     s2n_signature_algorithm sig_alg = conn->handshake_params.server_cert_sig_scheme->sig_alg;
 
-    s2n_pkey_type cert_type = 0;
-    POSIX_GUARD(s2n_get_cert_type_for_sig_alg(sig_alg, &cert_type));
+    s2n_pkey_type cert_type = S2N_PKEY_TYPE_UNKNOWN;
+    POSIX_GUARD_RESULT(s2n_signature_algorithm_get_pkey_type(sig_alg, &cert_type));
 
     *chosen_certs = s2n_get_compatible_cert_chain_and_key(conn, cert_type);
     S2N_ERROR_IF(*chosen_certs == NULL, S2N_ERR_CERT_TYPE_UNSUPPORTED);

tls/s2n_signature_algorithms.c

@@ -373,3 +373,26 @@ int s2n_recv_supported_sig_scheme_list(struct s2n_stuffer *in, struct s2n_sig_sc
 
     return 0;
 }
+
+S2N_RESULT s2n_signature_algorithm_get_pkey_type(s2n_signature_algorithm sig_alg, s2n_pkey_type *pkey_type)
+{
+    RESULT_ENSURE_REF(pkey_type);
+    *pkey_type = S2N_PKEY_TYPE_UNKNOWN;
+
+    switch (sig_alg) {
+        case S2N_SIGNATURE_RSA:
+        case S2N_SIGNATURE_RSA_PSS_RSAE:
+            *pkey_type = S2N_PKEY_TYPE_RSA;
+            break;
+        case S2N_SIGNATURE_RSA_PSS_PSS:
+            *pkey_type = S2N_PKEY_TYPE_RSA_PSS;
+            break;
+        case S2N_SIGNATURE_ECDSA:
+            *pkey_type = S2N_PKEY_TYPE_ECDSA;
+            break;
+        default:
+            RESULT_BAIL(S2N_ERR_INVALID_SIGNATURE_ALGORITHM);
+    }
+
+    return S2N_RESULT_OK;
+}