fix: avoid cert validation on connection_set_config

tests/pems/permutations/generate-certs.sh

@@ -170,6 +170,7 @@ then
     cert-gen   ec          ecdsa       384        SHA384      ec_ecdsa_p384_sha384
     cert-gen   ec          ecdsa       521        SHA384      ec_ecdsa_p521_sha384
     cert-gen   ec          ecdsa       521        SHA512      ec_ecdsa_p521_sha512
+    cert-gen   rsa        pkcsv1.5     512        SHA1        rsae_pkcs_512_sha1
     cert-gen   rsa        pkcsv1.5     2048       SHA1        rsae_pkcs_2048_sha1
     cert-gen   rsa        pkcsv1.5     2048       SHA224      rsae_pkcs_2048_sha224
     cert-gen   rsa        pkcsv1.5     2048       SHA256      rsae_pkcs_2048_sha256

tests/pems/permutations/rsae_pkcs_512_sha1/ca-cert.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBoTCCAUugAwIBAgIUOL0vNhopjHe1lGdOxUrjtr58fKwwDQYJKoZIhvcNAQEF
+BQAwHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwNjI4MjIyNjMx
+WhgPMjIwMzEyMDQyMjI2MzFaMBwxCzAJBgNVBAYTAlVTMQ0wCwYDVQQDDARyb290
+MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAM+TK0jfOm9OxYqWEJ+NWTBzdVQQAcrr
+RySqkex15wgKOMvQfbcIOvuRPOHA09Y9lRWxQzgoNXwrO/WrWJ0KcnECAwEAAaNj
+MGEwHQYDVR0OBBYEFGuQv3m+EZ+g83LuhPOGIq07GvcQMB8GA1UdIwQYMBaAFGuQ
+v3m+EZ+g83LuhPOGIq07GvcQMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQD
+AgIEMA0GCSqGSIb3DQEBBQUAA0EAe26pHHL4uRBNEqEfU8IXTMSoBMmGoHLDz7P7
+87i/Optxm7Hdb+SjtWUp+0NYIlJ4w52e/us8w3qT/S7mSaKrdQ==
+-----END CERTIFICATE-----

tests/pems/permutations/rsae_pkcs_512_sha1/client-cert.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBmDCCAUKgAwIBAgIULuW63lQamjlSAZ2F4CR5WdUahtAwDQYJKoZIhvcNAQEF
+BQAwHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwNjI4MjIyNjMx
+WhgPMjIwMzEyMDQyMjI2MzFaMB4xCzAJBgNVBAYTAlVTMQ8wDQYDVQQDDAZjbGll
+bnQwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAxpJOkCTsbF2qqp7r6hxJIkDsA4DX
+8BewqA6lClZYFIHuYdc3SmkNT0B94jpS5ernK+1SBILf2Xt8JFCliWhbJQIDAQAB
+o1gwVjAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFHZf7DM9xxxobjgJ
+JnqanzwDBJivMB8GA1UdIwQYMBaAFGuQv3m+EZ+g83LuhPOGIq07GvcQMA0GCSqG
+SIb3DQEBBQUAA0EAN13P1ieJk92ck/PStGP9rki4ZPcSU+c9KyTzpnHVYt6zXbDi
+JAYsrus9no+yej9TYMWakvFeIoItgrvNgHVZ7g==
+-----END CERTIFICATE-----

tests/pems/permutations/rsae_pkcs_512_sha1/client-key.pem

@@ -0,0 +1,10 @@
+-----BEGIN PRIVATE KEY-----
+MIIBVQIBADANBgkqhkiG9w0BAQEFAASCAT8wggE7AgEAAkEAxpJOkCTsbF2qqp7r
+6hxJIkDsA4DX8BewqA6lClZYFIHuYdc3SmkNT0B94jpS5ernK+1SBILf2Xt8JFCl
+iWhbJQIDAQABAkAKfhqmpTzU8RIel+0xXrNCmxmdicZfSnEsQDHaXPukga1Cbt3N
+QFgjmKqvQffi41K8/Zs8asmDMY9OIhTpfhqVAiEA7RNbOw4krT2TxcJbUu5ea2Sm
+bhF5TaYX8WjL9OXkCXsCIQDWbB5hNRXsAyA+Ac8+S7usxF5OxK9je2LEC+teGMJ7
+3wIhAJE32hpCf5TeszXf57DU8mE2NfwWGAfIRcJKPySz7QshAiEAxqYsDwrLYHgU
+6t1qTuCC4rCaXodBpfytp8sTJ33w0CkCIGJSf2hq2PgCESFXAD0NsESzQ+q5xSRU
+UtqYNy6B510+
+-----END PRIVATE KEY-----

tests/pems/permutations/rsae_pkcs_512_sha1/server-chain.pem

@@ -0,0 +1,33 @@
+-----BEGIN CERTIFICATE-----
+MIIBmDCCAUKgAwIBAgIUdVOeN8EEdVAEaCYb3SsqasYAclAwDQYJKoZIhvcNAQEF
+BQAwHjELMAkGA1UEBhMCVVMxDzANBgNVBAMMBmJyYW5jaDAgFw0yNDA2MjgyMjI2
+MzFaGA8yMjAzMTIwNDIyMjYzMVowHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBGxl
+YWYwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAzw9mmvGMx4KTf0g5pbKAEwTY3g1P
+N4wIY3egdN6Jn0fOEp7C+m1bPMe7vfVOJXwf/sDtAIk1cu83641TZipLcQIDAQAB
+o1gwVjAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFM+ZjHL6FWpQf/rz
+BTp47pxvzKBKMB8GA1UdIwQYMBaAFBsz11YK9dErkBvLRLnK4LnnTKbuMA0GCSqG
+SIb3DQEBBQUAA0EAnltE4ACfhG1t0IqiKNh8hvpxqOOkUvvVViMV3tUP/V1Kn7nE
+vdjl4sADQSBEKDMczOFJew+z/4gumtKt2vQObA==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBozCCAU2gAwIBAgIULuW63lQamjlSAZ2F4CR5WdUahs8wDQYJKoZIhvcNAQEF
+BQAwHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwNjI4MjIyNjMx
+WhgPMjIwMzEyMDQyMjI2MzFaMB4xCzAJBgNVBAYTAlVTMQ8wDQYDVQQDDAZicmFu
+Y2gwXDANBgkqhkiG9w0BAQEFAANLADBIAkEAxGirjO1UgCfKgbGq9LqmBbKL8FIg
+GLcRHOAXdkovX633ZAt9JtI8TYOM/fAzdAE++bGIdj3z2TyHo/BRppLLbwIDAQAB
+o2MwYTAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwICBDAdBgNVHQ4EFgQU
+GzPXVgr10SuQG8tEucrguedMpu4wHwYDVR0jBBgwFoAUa5C/eb4Rn6Dzcu6E84Yi
+rTsa9xAwDQYJKoZIhvcNAQEFBQADQQCNLdvQISyt0SmwqfQlSxLyFaCHNs2+sA6/
+PGtZERzUpzqXmAYVrFzWlTeA4NnBC9DEbmWAHDaSMXBNslnVKhAF
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBoTCCAUugAwIBAgIUOL0vNhopjHe1lGdOxUrjtr58fKwwDQYJKoZIhvcNAQEF
+BQAwHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwNjI4MjIyNjMx
+WhgPMjIwMzEyMDQyMjI2MzFaMBwxCzAJBgNVBAYTAlVTMQ0wCwYDVQQDDARyb290
+MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBAM+TK0jfOm9OxYqWEJ+NWTBzdVQQAcrr
+RySqkex15wgKOMvQfbcIOvuRPOHA09Y9lRWxQzgoNXwrO/WrWJ0KcnECAwEAAaNj
+MGEwHQYDVR0OBBYEFGuQv3m+EZ+g83LuhPOGIq07GvcQMB8GA1UdIwQYMBaAFGuQ
+v3m+EZ+g83LuhPOGIq07GvcQMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQD
+AgIEMA0GCSqGSIb3DQEBBQUAA0EAe26pHHL4uRBNEqEfU8IXTMSoBMmGoHLDz7P7
+87i/Optxm7Hdb+SjtWUp+0NYIlJ4w52e/us8w3qT/S7mSaKrdQ==
+-----END CERTIFICATE-----

tests/pems/permutations/rsae_pkcs_512_sha1/server-key.pem

@@ -0,0 +1,10 @@
+-----BEGIN PRIVATE KEY-----
+MIIBUwIBADANBgkqhkiG9w0BAQEFAASCAT0wggE5AgEAAkEAzw9mmvGMx4KTf0g5
+pbKAEwTY3g1PN4wIY3egdN6Jn0fOEp7C+m1bPMe7vfVOJXwf/sDtAIk1cu83641T
+ZipLcQIDAQABAkBEjoHXfXCyQh6Z/wzvOtnC8lDnvJpk9t10KZCcAW6pqKBw+M8D
+63YyvzM4vsyBsDAbCvtMyVvospVvjdNeU1VdAiEA/kP42+tIh+2Sd795+UHsccNV
+8FwS7SMA6lgLk19xcwsCIQDQeP5EoWs7sZXFlN4omylxh6Fq9FNCKjgn0lm7or1I
+8wIgM7GGCtAO8vOt74KSPcbVV1urQS62+lc/fGViFRg2bHkCID6gmoIzm+tK5ht9
+JWA9fK3GeQ+QZpKx7DzKTHq54PNRAiA2RnWEH3f1z4IRmpXU0dRZPtsGTDSmFova
+GZRjCeq5UA==
+-----END PRIVATE KEY-----

tests/unit/s2n_config_test.c

@@ -255,23 +255,6 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_connection_free(conn));
             EXPECT_SUCCESS(s2n_config_free(config));
         };
-
-        /* Test that security policy validation is enforced on the config */
-        {
-            DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
-            EXPECT_NOT_NULL(config);
-            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free);
-            EXPECT_NOT_NULL(conn);
-
-            DEFER_CLEANUP(struct s2n_cert_chain_and_key *invalid_cert = NULL, s2n_cert_chain_and_key_ptr_free);
-            EXPECT_SUCCESS(s2n_test_cert_permutation_load_server_chain(&invalid_cert, "rsae", "pss", "4096", "sha384"));
-            EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, invalid_cert));
-            struct s2n_security_policy rfc9151_applied_locally = security_policy_rfc9151;
-            rfc9151_applied_locally.certificate_preferences_apply_locally = true;
-            config->security_policy = &rfc9151_applied_locally;
-
-            EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_config(conn, config), S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT);
-        };
     };
 
     /* s2n_config_set_session_tickets_onoff */

tests/unit/s2n_security_policy_cert_preferences_test.c

@@ -18,6 +18,7 @@
 #include "tls/s2n_certificate_keys.h"
 #include "tls/s2n_security_policies.h"
 #include "tls/s2n_signature_scheme.h"
+#include "utils/s2n_map.h"
 
 #define CHAIN_LENGTH 3
 
@@ -265,6 +266,58 @@ int main(int argc, char **argv)
         }
     };
 
+    /* s2n_config invariant: always respects config->security_policy cert preferences */
+    {
+        DEFER_CLEANUP(struct s2n_cert_chain_and_key *cert = NULL, s2n_cert_chain_and_key_ptr_free);
+        EXPECT_SUCCESS(
+                s2n_test_cert_permutation_load_server_chain(&cert, "ec", "ecdsa", "p384", "sha256"));
+        /* configure security policy then load an invalid cert */
+        {
+            DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
+            EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "rfc9151"));
+
+            EXPECT_FAILURE(s2n_config_add_cert_chain_and_key_to_store(config, cert));
+
+            /* assert that no certs were loaded */
+            uint32_t domain_certs = 0;
+            EXPECT_EQUAL(s2n_config_get_num_default_certs(config), 0);
+            EXPECT_OK(s2n_map_size(config->domain_name_to_cert_map, &domain_certs));
+            EXPECT_EQUAL(domain_certs, 0);
+            EXPECT_EQUAL(s2n_config_get_num_default_certs(config), 0);
+        };
+
+        /* load a cert then configure an invalid security policy */
+        {
+            DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
+            EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, cert));
+            const struct s2n_security_policy *default_sp = config->security_policy;
+            EXPECT_FAILURE(s2n_config_set_cipher_preferences(config, "rfc9151"));
+
+            /* assert that the security policy was not changed */
+            EXPECT_EQUAL(config->security_policy, default_sp);
+        };
+    };
+
+    /* default policy check: ensure that the default security policy doesn't 
+     * enforce certificate preferences.
+     * 
+     * Adding certificate preferences to the default security policy would be a
+     * breaking change, because it would prevent customers from adding 
+     * non-compliant certs unless they first set the security policy.
+     * 
+     * This test ensures that such a breaking change would be visible and
+     * deliberate.
+     */
+    {
+        DEFER_CLEANUP(struct s2n_cert_chain_and_key *cert = NULL, s2n_cert_chain_and_key_ptr_free);
+        /* use a very insecure cert that would not be included in any reasonable cert preferences */
+        EXPECT_SUCCESS(s2n_test_cert_permutation_load_server_chain(&cert, "rsae", "pkcs", "512", "sha1"));
+
+        DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, cert));
+        EXPECT_EQUAL(s2n_config_get_num_default_certs(config), 1);
+    };
+
     END_TEST();
     return S2N_SUCCESS;
 }

tls/s2n_config.c

@@ -532,6 +532,8 @@ static int s2n_config_add_cert_chain_and_key_impl(struct s2n_config *config, str
     POSIX_ENSURE_REF(config->domain_name_to_cert_map);
     POSIX_ENSURE_REF(cert_key_pair);
 
+    POSIX_GUARD_RESULT(s2n_security_policy_validate_certificate_chain(config->security_policy, cert_key_pair));
+
     s2n_pkey_type cert_type = s2n_cert_chain_and_key_get_pkey_type(cert_key_pair);
     config->is_rsa_cert_configured |= (cert_type == S2N_PKEY_TYPE_RSA);
 
@@ -567,6 +569,11 @@ S2N_RESULT s2n_config_validate_loaded_certificates(const struct s2n_config *conf
     RESULT_ENSURE_REF(config);
     RESULT_ENSURE_REF(security_policy);
 
+    if (security_policy->certificate_key_preferences == NULL
+            && security_policy->certificate_signature_preferences == NULL) {
+        return S2N_RESULT_OK;
+    }
+
     /* validate the default certs */
     for (int i = 0; i < S2N_CERT_TYPE_COUNT; i++) {
         struct s2n_cert_chain_and_key *cert = config->default_certs_by_type.certs[i];
@@ -577,6 +584,10 @@ S2N_RESULT s2n_config_validate_loaded_certificates(const struct s2n_config *conf
     }
 
     /* validate the certs in the domain map */
+    if (config->domain_name_to_cert_map == NULL) {
+        return S2N_RESULT_OK;
+    }
+
     struct s2n_map_iterator iter = { 0 };
     RESULT_GUARD(s2n_map_iterator_init(&iter, config->domain_name_to_cert_map));
 

tls/s2n_connection.c

@@ -287,11 +287,14 @@ int s2n_connection_set_config(struct s2n_connection *conn, struct s2n_config *co
         return 0;
     }
 
-    const struct s2n_security_policy *security_policy = conn->security_policy_override;
-    if (!security_policy) {
-        security_policy = config->security_policy;
+    /* s2n_config invariant: any s2n_config is always in a state that respects the
+     * config->security_policy certificate preferences. Therefore we only need to
+     * validate certificates here if the connection is using a security policy override.
+     */
+    const struct s2n_security_policy *security_policy_override = conn->security_policy_override;
+    if (security_policy_override) {
+        POSIX_GUARD_RESULT(s2n_config_validate_loaded_certificates(config, security_policy_override));
     }
-    POSIX_GUARD_RESULT(s2n_config_validate_loaded_certificates(config, security_policy));
 
     /* We only support one client certificate */
     if (s2n_config_get_num_default_certs(config) > 1 && conn->mode == S2N_CLIENT) {

tls/s2n_security_policies.c

@@ -1278,6 +1278,8 @@ int s2n_config_set_cipher_preferences(struct s2n_config *config, const char *ver
     /* If the security policy's minimum version is higher than what libcrypto supports, return an error. */
     POSIX_ENSURE((security_policy->minimum_protocol_version <= s2n_get_highest_fully_supported_tls_version()), S2N_ERR_PROTOCOL_VERSION_UNSUPPORTED);
 
+    /* If the config contains certificates violating the security policy cert preferences, return an error. */
+    POSIX_GUARD_RESULT(s2n_config_validate_loaded_certificates(config, security_policy));
     config->security_policy = security_policy;
     return 0;
 }