tests: use sig schemes as source of truth for valid hash+sig algs (#5129

)

crypto/s2n_evp_signing.c

@@ -75,29 +75,6 @@ S2N_RESULT s2n_evp_signing_set_pkey_overrides(struct s2n_pkey *pkey)
     return S2N_RESULT_OK;
 }
 
-static S2N_RESULT s2n_evp_signing_validate_hash_alg(s2n_signature_algorithm sig_alg, s2n_hash_algorithm hash_alg)
-{
-    switch (hash_alg) {
-        case S2N_HASH_NONE:
-        case S2N_HASH_MD5:
-            /* MD5 alone is never supported */
-            RESULT_BAIL(S2N_ERR_HASH_INVALID_ALGORITHM);
-            break;
-        case S2N_HASH_MD5_SHA1:
-            /* Only RSA supports MD5+SHA1.
-             * This should not be a problem, as we only allow MD5+SHA1 when
-             * falling back to TLS1.0 or 1.1, which only support RSA.
-             */
-            RESULT_ENSURE(sig_alg == S2N_SIGNATURE_RSA, S2N_ERR_HASH_INVALID_ALGORITHM);
-            break;
-        default:
-            break;
-    }
-    /* Hash algorithm must be recognized and supported by EVP_MD */
-    RESULT_ENSURE(s2n_hash_alg_to_evp_md(hash_alg) != NULL, S2N_ERR_HASH_INVALID_ALGORITHM);
-    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);
@@ -119,7 +96,6 @@ int s2n_evp_sign(const struct s2n_pkey *priv, s2n_signature_algorithm sig_alg,
     POSIX_ENSURE_REF(hash_state);
     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));
 
     DEFER_CLEANUP(EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(priv->pkey, NULL), EVP_PKEY_CTX_free_pointer);
     POSIX_ENSURE_REF(pctx);
@@ -150,7 +126,6 @@ int s2n_evp_verify(const struct s2n_pkey *pub, s2n_signature_algorithm sig_alg,
     POSIX_ENSURE_REF(hash_state);
     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);

tests/unit/s2n_ecdsa_test.c

@@ -55,17 +55,6 @@ int main(int argc, char **argv)
     char *cert_chain_pem = NULL;
     char *private_key_pem = NULL;
 
-    const int supported_hash_algorithms[] = {
-        S2N_HASH_NONE,
-        S2N_HASH_MD5,
-        S2N_HASH_SHA1,
-        S2N_HASH_SHA224,
-        S2N_HASH_SHA256,
-        S2N_HASH_SHA384,
-        S2N_HASH_SHA512,
-        S2N_HASH_MD5_SHA1
-    };
-
     BEGIN_TEST();
     EXPECT_SUCCESS(s2n_disable_tls13_in_test());
 
@@ -156,13 +145,25 @@ int main(int argc, char **argv)
     EXPECT_SUCCESS(s2n_hash_new(&hash_one));
     EXPECT_SUCCESS(s2n_hash_new(&hash_two));
 
-    for (size_t i = 0; i < s2n_array_len(supported_hash_algorithms); i++) {
-        int hash_alg = supported_hash_algorithms[i];
-
-        if (!s2n_hash_is_available(hash_alg) || hash_alg == S2N_HASH_NONE) {
-            /* Skip hash algorithms that are not available. */
+    /* Determining all possible valid combinations of hash algorithms and
+     * signature algorithms is actually surprisingly complicated.
+     *
+     * For example: awslc-fips will fail for MD5+ECDSA. However, that is not
+     * a real problem because there is no valid signature scheme that uses both
+     * MD5 and ECDSA.
+     *
+     * To avoid enumerating all the exceptions, just use the actual supported
+     * signature scheme list as the source of truth.
+     */
+    const struct s2n_signature_preferences *all_sig_schemes =
+            security_policy_test_all.signature_preferences;
+
+    for (size_t i = 0; i < all_sig_schemes->count; i++) {
+        const struct s2n_signature_scheme *scheme = all_sig_schemes->signature_schemes[i];
+        if (scheme->sig_alg != S2N_SIGNATURE_ECDSA) {
             continue;
         }
+        const s2n_hash_algorithm hash_alg = scheme->hash_alg;
 
         EXPECT_SUCCESS(s2n_hash_init(&hash_one, hash_alg));
         EXPECT_SUCCESS(s2n_hash_init(&hash_two, hash_alg));
@@ -173,25 +174,8 @@ int main(int argc, char **argv)
         /* Reset signature size when we compute a new signature */
         signature.size = maximum_signature_length;
 
-        /* Not all hash algorithms are supported for EVP ECDSA signing.
-         * See s2n_evp_signing_validate_hash_alg.
-         */
-        bool hash_is_md5 = (hash_alg == S2N_HASH_MD5 || hash_alg == S2N_HASH_MD5_SHA1);
-        bool hash_is_supported = !(hash_is_md5 && s2n_is_in_fips_mode());
-
-        int sign_result = s2n_pkey_sign(&priv_key, S2N_SIGNATURE_ECDSA, &hash_one, &signature);
-        if (hash_is_supported) {
-            EXPECT_SUCCESS(sign_result);
-        } else {
-            EXPECT_FAILURE_WITH_ERRNO(sign_result, S2N_ERR_HASH_INVALID_ALGORITHM);
-        }
-
-        int verify_result = s2n_pkey_verify(&pub_key, S2N_SIGNATURE_ECDSA, &hash_two, &signature);
-        if (hash_is_supported) {
-            EXPECT_SUCCESS(verify_result);
-        } else {
-            EXPECT_FAILURE_WITH_ERRNO(verify_result, S2N_ERR_HASH_INVALID_ALGORITHM);
-        }
+        EXPECT_SUCCESS(s2n_pkey_sign(&priv_key, S2N_SIGNATURE_ECDSA, &hash_one, &signature));
+        EXPECT_SUCCESS(s2n_pkey_verify(&pub_key, S2N_SIGNATURE_ECDSA, &hash_two, &signature));
 
         EXPECT_SUCCESS(s2n_hash_reset(&hash_one));
         EXPECT_SUCCESS(s2n_hash_reset(&hash_two));

tests/unit/s2n_evp_signing_test.c

@@ -36,12 +36,6 @@
 
 const uint8_t input_data[INPUT_DATA_SIZE] = "hello hash";
 
-static bool s2n_hash_alg_is_supported(s2n_signature_algorithm sig_alg, s2n_hash_algorithm hash_alg)
-{
-    return (hash_alg != S2N_HASH_NONE) && (hash_alg != S2N_HASH_MD5)
-            && (hash_alg != S2N_HASH_MD5_SHA1 || sig_alg == S2N_SIGNATURE_RSA);
-}
-
 static S2N_RESULT s2n_test_hash_init(struct s2n_hash_state *hash_state, s2n_hash_algorithm hash_alg)
 {
     RESULT_GUARD_POSIX(s2n_hash_init(hash_state, hash_alg));
@@ -112,26 +106,18 @@ int main(int argc, char **argv)
     EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&rsa_cert_chain,
             S2N_RSA_2048_PKCS1_CERT_CHAIN, S2N_RSA_2048_PKCS1_KEY));
 
-    /* Test that unsupported hash algs are treated as invalid.
-     * Later tests will ignore unsupported algs, so ensure they are actually invalid. */
-    {
-        /* This pkey should never actually be needed -- any pkey will do */
-        struct s2n_pkey *pkey = rsa_cert_chain->private_key;
-
-        for (s2n_signature_algorithm sig_alg = 0; sig_alg <= UINT8_MAX; sig_alg++) {
-            for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) {
-                if (s2n_hash_alg_is_supported(sig_alg, hash_alg)) {
-                    continue;
-                }
-
-                s2n_stack_blob(evp_signature, OUTPUT_DATA_SIZE, OUTPUT_DATA_SIZE);
-                EXPECT_ERROR_WITH_ERRNO(s2n_test_evp_sign(sig_alg, hash_alg, pkey, &evp_signature),
-                        S2N_ERR_HASH_INVALID_ALGORITHM);
-                EXPECT_ERROR_WITH_ERRNO(s2n_test_evp_verify(sig_alg, hash_alg, pkey, &evp_signature, &evp_signature),
-                        S2N_ERR_HASH_INVALID_ALGORITHM);
-            }
-        }
-    };
+    /* Determining all possible valid combinations of hash algorithms and
+     * signature algorithms is actually surprisingly complicated.
+     *
+     * For example: awslc-fips will fail for MD5+ECDSA. However, that is not
+     * a real problem because there is no valid signature scheme that uses both
+     * MD5 and ECDSA.
+     *
+     * To avoid enumerating all the exceptions, just use the actual supported
+     * signature scheme list as the source of truth.
+     */
+    const struct s2n_signature_preferences *all_sig_schemes =
+            security_policy_test_all.signature_preferences;
 
     /* EVP signing must match RSA signing */
     {
@@ -145,10 +131,12 @@ int main(int argc, char **argv)
         EXPECT_PKEY_USES_EVP_SIGNING(private_key);
         EXPECT_PKEY_USES_EVP_SIGNING(public_key);
 
-        for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) {
-            if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) {
+        for (size_t i = 0; i < all_sig_schemes->count; i++) {
+            const struct s2n_signature_scheme *scheme = all_sig_schemes->signature_schemes[i];
+            if (scheme->sig_alg != sig_alg) {
                 continue;
             }
+            const s2n_hash_algorithm hash_alg = scheme->hash_alg;
 
             /* Calculate the signature using EVP methods */
             s2n_stack_blob(evp_signature, OUTPUT_DATA_SIZE, OUTPUT_DATA_SIZE);
@@ -183,10 +171,12 @@ int main(int argc, char **argv)
         EXPECT_PKEY_USES_EVP_SIGNING(private_key);
         EXPECT_PKEY_USES_EVP_SIGNING(public_key);
 
-        for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) {
-            if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) {
+        for (size_t i = 0; i < all_sig_schemes->count; i++) {
+            const struct s2n_signature_scheme *scheme = all_sig_schemes->signature_schemes[i];
+            if (scheme->sig_alg != sig_alg) {
                 continue;
             }
+            const s2n_hash_algorithm hash_alg = scheme->hash_alg;
 
             /* Calculate the signature using EVP methods */
             s2n_stack_blob(evp_signature, OUTPUT_DATA_SIZE, OUTPUT_DATA_SIZE);
@@ -220,10 +210,12 @@ int main(int argc, char **argv)
         EXPECT_PKEY_USES_EVP_SIGNING(private_key);
         EXPECT_PKEY_USES_EVP_SIGNING(public_key);
 
-        for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) {
-            if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) {
+        for (size_t i = 0; i < all_sig_schemes->count; i++) {
+            const struct s2n_signature_scheme *scheme = all_sig_schemes->signature_schemes[i];
+            if (scheme->sig_alg != sig_alg) {
                 continue;
             }
+            const s2n_hash_algorithm hash_alg = scheme->hash_alg;
 
             /* Calculate the signature using EVP methods */
             s2n_stack_blob(evp_signature, OUTPUT_DATA_SIZE, OUTPUT_DATA_SIZE);
@@ -258,10 +250,12 @@ int main(int argc, char **argv)
         EXPECT_PKEY_USES_EVP_SIGNING(private_key);
         EXPECT_PKEY_USES_EVP_SIGNING(public_key);
 
-        for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) {
-            if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) {
+        for (size_t i = 0; i < all_sig_schemes->count; i++) {
+            const struct s2n_signature_scheme *scheme = all_sig_schemes->signature_schemes[i];
+            if (scheme->sig_alg != sig_alg) {
                 continue;
             }
+            const s2n_hash_algorithm hash_alg = scheme->hash_alg;
 
             /* Calculate the signature using EVP methods */
             s2n_stack_blob(evp_signature, OUTPUT_DATA_SIZE, OUTPUT_DATA_SIZE);

tests/unit/s2n_security_policies_test.c

@@ -651,7 +651,7 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(config->security_policy, &security_policy_test_all);
         EXPECT_EQUAL(config->security_policy->cipher_preferences, &cipher_preferences_test_all);
         EXPECT_EQUAL(config->security_policy->kem_preferences, &kem_preferences_all);
-        EXPECT_EQUAL(config->security_policy->signature_preferences, &s2n_signature_preferences_20201021);
+        EXPECT_EQUAL(config->security_policy->signature_preferences, &s2n_signature_preferences_all);
         EXPECT_EQUAL(config->security_policy->ecc_preferences, &s2n_ecc_preferences_test_all);
 
         EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "test_all_tls12"));
@@ -755,7 +755,7 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(security_policy, &security_policy_test_all);
         EXPECT_EQUAL(security_policy->cipher_preferences, &cipher_preferences_test_all);
         EXPECT_EQUAL(security_policy->kem_preferences, &kem_preferences_all);
-        EXPECT_EQUAL(security_policy->signature_preferences, &s2n_signature_preferences_20201021);
+        EXPECT_EQUAL(security_policy->signature_preferences, &s2n_signature_preferences_all);
         EXPECT_EQUAL(security_policy->ecc_preferences, &s2n_ecc_preferences_test_all);
 
         EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(conn, "test_all_tls12"));

tests/unit/s2n_signature_scheme_test.c

@@ -13,19 +13,24 @@
  * permissions and limitations under the License.
  */
 
-#include "tls/s2n_signature_scheme.c"
+#include "tls/s2n_signature_scheme.h"
 
 #include "s2n_test.h"
 
 int main(int argc, char **argv)
 {
     BEGIN_TEST();
 
+    const struct s2n_signature_preferences *all_prefs = &s2n_signature_preferences_all;
+
     /* Test all signature schemes */
     size_t policy_i = 0;
     while (security_policy_selection[policy_i].version != NULL) {
         const struct s2n_signature_preferences *sig_prefs =
                 security_policy_selection[policy_i].security_policy->signature_preferences;
+
+        bool s2n_rsa_pkcs1_md5_sha1_found = false;
+
         for (size_t sig_i = 0; sig_i < sig_prefs->count; sig_i++) {
             const struct s2n_signature_scheme *const sig_scheme = sig_prefs->signature_schemes[sig_i];
 
@@ -50,7 +55,36 @@ int main(int argc, char **argv)
                         sig_prefs->signature_schemes[dup_i];
                 EXPECT_NOT_EQUAL(sig_scheme->iana_value, potential_duplicate->iana_value);
             }
+
+            if (sig_scheme == &s2n_rsa_pkcs1_md5_sha1) {
+                s2n_rsa_pkcs1_md5_sha1_found = true;
+            }
+
+            /* s2n_null_sig_scheme is not a real signature scheme and is just a placeholder.
+             * It should not appear in any policy.
+             */
+            EXPECT_NOT_EQUAL(sig_scheme, &s2n_null_sig_scheme);
+
+            /* Must be included in s2n_signature_preferences_all */
+            bool in_all = false;
+            for (size_t all_i = 0; all_i < all_prefs->count; all_i++) {
+                if (sig_scheme == all_prefs->signature_schemes[all_i]) {
+                    in_all = true;
+                }
+            }
+            EXPECT_TRUE(in_all);
         }
+
+        /* Only s2n_signature_preferences_all should include s2n_rsa_pkcs1_md5_sha1
+         *
+         * s2n_rsa_pkcs1_md5_sha1 is the implicit default for pre-TLS1.2 when no signature
+         * schemes are provided. Any code that needs to handle "all signature schemes"
+         * also needs to handle s2n_rsa_pkcs1_md5_sha1. It is not explicitly included
+         * in any real signature preferences, but should still be tracked by
+         * s2n_signature_preferences_all.
+         */
+        EXPECT_EQUAL(s2n_rsa_pkcs1_md5_sha1_found, sig_prefs == all_prefs);
+
         policy_i++;
     }
 

tls/s2n_security_policies.c

@@ -1154,7 +1154,7 @@ const struct s2n_security_policy security_policy_test_all = {
     .minimum_protocol_version = S2N_SSLv3,
     .cipher_preferences = &cipher_preferences_test_all,
     .kem_preferences = &kem_preferences_all,
-    .signature_preferences = &s2n_signature_preferences_20201021,
+    .signature_preferences = &s2n_signature_preferences_all,
     .ecc_preferences = &s2n_ecc_preferences_test_all,
 };
 

tls/s2n_signature_scheme.c

@@ -204,6 +204,34 @@ const struct s2n_signature_scheme s2n_rsa_pss_pss_sha512 = {
     .minimum_protocol_version = S2N_TLS12,
 };
 
+/* ALL signature schemes, including the legacy default s2n_rsa_pkcs1_md5_sha1 scheme.
+ * New signature schemes must be added to this list.
+ */
+const struct s2n_signature_scheme* const s2n_sig_scheme_pref_list_all[] = {
+    &s2n_rsa_pkcs1_md5_sha1,
+    &s2n_rsa_pkcs1_sha1,
+    &s2n_rsa_pkcs1_sha224,
+    &s2n_rsa_pkcs1_sha256,
+    &s2n_rsa_pkcs1_sha384,
+    &s2n_rsa_pkcs1_sha512,
+    &s2n_ecdsa_sha1,
+    &s2n_ecdsa_sha224,
+    &s2n_ecdsa_sha256,
+    &s2n_ecdsa_sha384,
+    &s2n_ecdsa_sha512,
+    &s2n_rsa_pss_rsae_sha256,
+    &s2n_rsa_pss_rsae_sha384,
+    &s2n_rsa_pss_rsae_sha512,
+    &s2n_rsa_pss_pss_sha256,
+    &s2n_rsa_pss_pss_sha384,
+    &s2n_rsa_pss_pss_sha512,
+};
+
+const struct s2n_signature_preferences s2n_signature_preferences_all = {
+    .count = s2n_array_len(s2n_sig_scheme_pref_list_all),
+    .signature_schemes = s2n_sig_scheme_pref_list_all,
+};
+
 /* Chosen based on AWS server recommendations as of 05/24.
  *
  * The recommendations do not include PKCS1, but we must include it anyway for

tls/s2n_signature_scheme.h

@@ -82,5 +82,6 @@ extern const struct s2n_signature_preferences s2n_certificate_signature_preferen
 extern const struct s2n_signature_preferences s2n_signature_preferences_default_fips;
 extern const struct s2n_signature_preferences s2n_signature_preferences_null;
 extern const struct s2n_signature_preferences s2n_signature_preferences_test_all_fips;
+extern const struct s2n_signature_preferences s2n_signature_preferences_all;
 
 extern const struct s2n_signature_preferences s2n_certificate_signature_preferences_20201110;