Complete the migration to s2n_pq_is_enabled() (#2510)

* Migrate ad-hoc PQ checks to consolidated s2n_pq_is_enabled function

* Fix typo in comment

* Better parameterization for TLS1.3 PQ handshake test

Author: bbutch

error/s2n_errno.c

@@ -215,7 +215,6 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_SESSION_TICKET_NOT_SUPPORTED, "Session ticket not supported for this connection") \
     ERR_ENTRY(S2N_ERR_OCSP_NOT_SUPPORTED, "OCSP stapling was requested, but is not supported") \
     ERR_ENTRY(S2N_ERR_INVALID_SIGNATURE_ALGORITHMS_PREFERENCES, "Invalid signature algorithms preferences version") \
-    ERR_ENTRY(S2N_ERR_PQ_KEMS_DISALLOWED_IN_FIPS, "PQ KEMs are disallowed while in FIPS mode") \
     ERR_ENTRY(S2N_RSA_PSS_NOT_SUPPORTED, "RSA-PSS signing not supported by underlying libcrypto implementation") \
     ERR_ENTRY(S2N_ERR_MAX_INNER_PLAINTEXT_SIZE, "Inner plaintext size exceeds limit") \
     ERR_ENTRY(S2N_ERR_INVALID_ECC_PREFERENCES, "Invalid ecc curves preferences version") \

error/s2n_errno.h

@@ -111,7 +111,6 @@ typedef enum {
     S2N_ERR_BAD_KEY_SHARE,
     S2N_ERR_CANCELLED,
     S2N_ERR_PROTOCOL_DOWNGRADE_DETECTED,
-    S2N_ERR_PQ_KEMS_DISALLOWED_IN_FIPS,
     S2N_ERR_MAX_INNER_PLAINTEXT_SIZE,
     S2N_ERR_RECORD_STUFFER_SIZE,
     S2N_ERR_FRAGMENT_LENGTH_TOO_LARGE,

tests/integrationv2/common.py

@@ -5,6 +5,7 @@
 import threading
 
 from constants import TEST_CERT_DIRECTORY
+from global_flags import get_flag, S2N_NO_PQ, S2N_FIPS_MODE
 
 
 def data_bytes(n_bytes):
@@ -27,6 +28,13 @@ def data_bytes(n_bytes):
     return bytes(byte_array)
 
 
+def pq_enabled():
+    """
+    Returns true or false to indicate whether PQ crypto is enabled in s2n
+    """
+    return not (get_flag(S2N_NO_PQ, False) or get_flag(S2N_FIPS_MODE, False))
+
+
 class AvailablePorts(object):
     """
     This iterator will atomically return the next number.

tests/integrationv2/test_pq_handshake.py

@@ -2,7 +2,7 @@
 import os
 
 from configuration import available_ports, PROVIDERS, PROTOCOLS
-from common import Ciphers, ProviderOptions, Protocols, data_bytes, KemGroups, Certificates
+from common import Ciphers, ProviderOptions, Protocols, data_bytes, KemGroups, Certificates, pq_enabled
 from fixtures import managed_process
 from providers import Provider, S2N, OpenSSL
 from utils import invalid_test_parameters, get_parameter_name
@@ -163,7 +163,13 @@ def test_s2nc_to_s2nd_pq_handshake(managed_process, protocol, client_cipher, ser
     server = managed_process(S2N, server_options, timeout=5)
     client = managed_process(S2N, client_options, timeout=5)
 
-    expected_result = EXPECTED_RESULTS.get((client_cipher, server_cipher), None)
+    if pq_enabled():
+        expected_result = EXPECTED_RESULTS.get((client_cipher, server_cipher), None)
+    else:
+        # If PQ is not enabled in s2n, we expect classic handshakes to be negotiated.
+        # Leave the expected cipher blank, as there are multiple possibilities - the
+        # important thing is that kem and kem_group are NONE.
+        expected_result = {"cipher": "", "kem": "NONE", "kem_group": "NONE"}
 
     # Client and server are both s2n; can make meaningful assertions about negotiation for both
     for results in client.get_results():
@@ -181,6 +187,10 @@ def test_s2nc_to_s2nd_pq_handshake(managed_process, protocol, client_cipher, ser
 @pytest.mark.parametrize("cipher", [Ciphers.PQ_TLS_1_0_2020_12], ids=get_parameter_name)
 @pytest.mark.parametrize("kem_group", KEM_GROUPS, ids=get_parameter_name)
 def test_s2nc_to_oqs_openssl_pq_handshake(managed_process, protocol, cipher, kem_group):
+    # If PQ is not enabled in s2n, there is no reason to test against oqs_openssl
+    if not pq_enabled():
+        return
+
     host = "localhost"
     port = next(available_ports)
 
@@ -223,6 +233,10 @@ def test_s2nc_to_oqs_openssl_pq_handshake(managed_process, protocol, cipher, kem
 @pytest.mark.parametrize("cipher", [Ciphers.PQ_TLS_1_0_2020_12], ids=get_parameter_name)
 @pytest.mark.parametrize("kem_group", KEM_GROUPS, ids=get_parameter_name)
 def test_oqs_openssl_to_s2nd_pq_handshake(managed_process, protocol, cipher, kem_group):
+    # If PQ is not enabled in s2n, there is no reason to test against oqs_openssl
+    if not pq_enabled():
+        return
+
     host = "localhost"
     port = next(available_ports)
 

tests/integrationv2/test_well_known_endpoints.py

@@ -2,7 +2,7 @@
 
 from constants import TRUST_STORE_BUNDLE
 from configuration import available_ports, PROTOCOLS
-from common import ProviderOptions, Protocols, Ciphers
+from common import ProviderOptions, Protocols, Ciphers, pq_enabled
 from fixtures import managed_process
 from global_flags import get_flag, S2N_FIPS_MODE
 from providers import Provider, S2N
@@ -30,13 +30,34 @@
     Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02
 ]
 
-EXPECTED_RESULTS = {
-    ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2019_06): {"cipher": "ECDHE-BIKE-RSA-AES256-GCM-SHA384", "kem": "BIKE1r1-Level1"},
-    ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11): {"cipher": "ECDHE-SIKE-RSA-AES256-GCM-SHA384", "kem": "SIKEp503r1-KEM"},
-    ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_07): {"cipher": "ECDHE-KYBER-RSA-AES256-GCM-SHA384", "kem": "kyber512r2"},
-    ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_02): {"cipher": "ECDHE-BIKE-RSA-AES256-GCM-SHA384", "kem": "BIKE1r2-Level1"},
-    ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02): {"cipher": "ECDHE-SIKE-RSA-AES256-GCM-SHA384", "kem": "SIKEp434r2-KEM"},
-}
+
+if pq_enabled():
+    EXPECTED_RESULTS = {
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2019_06):
+            {"cipher": "ECDHE-BIKE-RSA-AES256-GCM-SHA384", "kem": "BIKE1r1-Level1"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11):
+            {"cipher": "ECDHE-SIKE-RSA-AES256-GCM-SHA384", "kem": "SIKEp503r1-KEM"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_07):
+            {"cipher": "ECDHE-KYBER-RSA-AES256-GCM-SHA384", "kem": "kyber512r2"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_02):
+            {"cipher": "ECDHE-BIKE-RSA-AES256-GCM-SHA384", "kem": "BIKE1r2-Level1"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02):
+            {"cipher": "ECDHE-SIKE-RSA-AES256-GCM-SHA384", "kem": "SIKEp434r2-KEM"},
+    }
+else:
+    EXPECTED_RESULTS = {
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2019_06):
+            {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": "NONE"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11):
+            {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": "NONE"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_07):
+            {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": "NONE"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_02):
+            {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": "NONE"},
+        ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02):
+            {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": "NONE"},
+    }
+
 
 @pytest.mark.uncollect_if(func=invalid_test_parameters)
 @pytest.mark.parametrize("protocol", PROTOCOLS, ids=get_parameter_name)

tests/integrationv2/utils.py

@@ -1,6 +1,5 @@
 from common import Protocols, Curves, Ciphers
 from providers import S2N, OpenSSL
-from global_flags import get_flag, S2N_NO_PQ, S2N_FIPS_MODE
 
 
 def get_expected_s2n_version(protocol, provider):
@@ -63,12 +62,6 @@ def invalid_test_parameters(*args, **kwargs):
         return True
 
     if cipher is not None:
-        # TODO Remove this check once the pq-enabled update is complete; once complete,
-        # s2n will ignore PQ ciphers if PQ is not enabled, so we won't have to perform
-        # this check in the tests (See discussion in PR #2426).
-        if cipher.pq and (get_flag(S2N_NO_PQ, False) or get_flag(S2N_FIPS_MODE, False)):
-            return True
-
         # If the selected protocol doesn't allow the cipher, don't test
         if protocol is not None:
             if cipher.min_version > protocol:

tests/testlib/s2n_pq_hybrid_test_utils.c

@@ -20,7 +20,7 @@
 #include "utils/s2n_safety.h"
 #include "crypto/s2n_drbg.h"
 #include "crypto/s2n_openssl.h"
-#include "crypto/s2n_fips.h"
+#include "pq-crypto/s2n_pq.h"
 #include "stuffer/s2n_stuffer.h"
 #include "tests/testlib/s2n_testlib.h"
 #include "tls/s2n_kex.h"
@@ -55,7 +55,6 @@ int s2n_hybrid_pq_entropy(void *ptr, uint32_t size) {
 
 static int setup_connection(struct s2n_connection *conn, const struct s2n_kem *kem, struct s2n_cipher_suite *cipher_suite,
         const char *cipher_pref_version) {
-    S2N_ERROR_IF(s2n_is_in_fips_mode(), S2N_ERR_PQ_KEMS_DISALLOWED_IN_FIPS);
     conn->actual_protocol_version = S2N_TLS12;
 
     const struct s2n_ecc_preferences *ecc_preferences = NULL;
@@ -73,7 +72,7 @@ static int setup_connection(struct s2n_connection *conn, const struct s2n_kem *k
 int s2n_test_hybrid_ecdhe_kem_with_kat(const struct s2n_kem *kem, struct s2n_cipher_suite *cipher_suite,
         const char *cipher_pref_version, const char * kat_file_name, uint32_t server_key_message_length,
         uint32_t client_key_message_length) {
-    S2N_ERROR_IF(s2n_is_in_fips_mode(), S2N_ERR_PQ_KEMS_DISALLOWED_IN_FIPS);
+    ENSURE_POSIX(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
 
     /* Part 1 setup a client and server connection with everything they need for a key exchange */
     struct s2n_connection *client_conn = NULL, *server_conn = NULL;

tests/testlib/s2n_pq_kat_test_utils.c

@@ -19,7 +19,7 @@
 #include "tests/testlib/s2n_nist_kats.h"
 #include "utils/s2n_mem.h"
 #include "utils/s2n_safety.h"
-#include "crypto/s2n_fips.h"
+#include "pq-crypto/s2n_pq.h"
 #include "pq-crypto/s2n_pq_random.h"
 
 /* We include s2n_drbg.c directly in order to access the static functions in our entropy callbacks. */
@@ -103,7 +103,7 @@ S2N_RESULT s2n_get_random_bytes_for_pq_kat_tests(uint8_t *buffer, uint32_t num_b
 }
 
 int s2n_test_kem_with_kat(const struct s2n_kem *kem, const char *kat_file_name) {
-    S2N_ERROR_IF(s2n_is_in_fips_mode(), S2N_ERR_PQ_KEMS_DISALLOWED_IN_FIPS);
+    ENSURE_POSIX(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED);
     ENSURE_POSIX(s2n_in_unit_test(), S2N_ERR_NOT_IN_UNIT_TEST);
 
     notnull_check(kem);

tests/unit/s2n_choose_supported_group_test.c

@@ -138,7 +138,6 @@ int main() {
         }
     }
 
-#if !defined(S2N_NO_PQ)
     /* Test for PQ */
     {
         const struct s2n_kem_group *test_kem_groups[] = {
@@ -313,7 +312,6 @@ int main() {
             EXPECT_SUCCESS(s2n_connection_free(server_conn));
         }
     }
-#endif
 
     END_TEST();
     return 0;

tests/unit/s2n_cipher_suite_match_test.c

@@ -19,7 +19,7 @@
 #include <string.h>
 
 #include "crypto/s2n_ecc_evp.h"
-#include "crypto/s2n_fips.h"
+#include "pq-crypto/s2n_pq.h"
 #include "tls/s2n_cipher_suites.h"
 #include "tls/s2n_connection.h"
 #include "tls/s2n_security_policies.h"
@@ -226,7 +226,7 @@ int main(int argc, char **argv)
             TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
             TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
             TLS_ECDHE_BIKE_RSA_WITH_AES_256_GCM_SHA384,
-            TLS_ECDHE_BIKE_RSA_WITH_AES_256_GCM_SHA384,
+            TLS_ECDHE_SIKE_RSA_WITH_AES_256_GCM_SHA384,
             TLS_ECDHE_KYBER_RSA_WITH_AES_256_GCM_SHA384,
         };
         const uint8_t cipher_count = sizeof(wire_ciphers) / S2N_TLS_CIPHER_SUITE_LEN;
@@ -359,11 +359,27 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(conn->secure.cipher_suite, s2n_cipher_suite_from_wire(expected_rsa_wire_choice));
         EXPECT_SUCCESS(s2n_connection_wipe(conn));
 
-#if !defined(S2N_NO_PQ)
-        if (!s2n_is_in_fips_mode()) {
-            /* There is no support for PQ KEMs while in FIPS mode */
-            /* Test that clients that support PQ ciphers can negotiate them. */
-            const uint8_t expected_pq_wire_choice[] = {TLS_ECDHE_BIKE_RSA_WITH_AES_256_GCM_SHA384};
+        /* Test that PQ cipher suites are marked available/unavailable appropriately in s2n_cipher_suites_init() */
+        {
+            const struct s2n_cipher_suite *pq_suites[] = {
+                    &s2n_ecdhe_sike_rsa_with_aes_256_gcm_sha384,
+                    &s2n_ecdhe_bike_rsa_with_aes_256_gcm_sha384,
+                    &s2n_ecdhe_kyber_rsa_with_aes_256_gcm_sha384,
+            };
+
+            for (size_t i = 0; i < s2n_array_len(pq_suites); i++) {
+                if (s2n_pq_is_enabled()) {
+                    EXPECT_EQUAL(pq_suites[i]->available, 1);
+                    EXPECT_NOT_NULL(pq_suites[i]->record_alg);
+                } else {
+                    EXPECT_EQUAL(pq_suites[i]->available, 0);
+                    EXPECT_NULL(pq_suites[i]->record_alg);
+                }
+            }
+        }
+
+        /* Test that clients that support PQ ciphers can negotiate them. */
+        {
             uint8_t client_extensions_data[] = {
                     0xFE, 0x01, /* PQ KEM extension ID */
                     0x00, 0x04, /* Total extension length in bytes */
@@ -377,7 +393,14 @@ int main(int argc, char **argv)
             conn->secure.client_pq_kem_extension.data = client_extensions_data;
             conn->secure.client_pq_kem_extension.size = client_extensions_len;
             EXPECT_SUCCESS(s2n_set_cipher_as_tls_server(conn, wire_ciphers, cipher_count));
-            EXPECT_EQUAL(conn->secure.cipher_suite, s2n_cipher_suite_from_wire(expected_pq_wire_choice));
+            const uint8_t bike_cipher[] = {TLS_ECDHE_BIKE_RSA_WITH_AES_256_GCM_SHA384};
+            const uint8_t ecc_cipher[] = {TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384};
+            if (s2n_pq_is_enabled()) {
+                EXPECT_EQUAL(conn->secure.cipher_suite, s2n_cipher_suite_from_wire(bike_cipher));
+            } else {
+                EXPECT_EQUAL(conn->secure.cipher_suite, s2n_cipher_suite_from_wire(ecc_cipher));
+            }
+
             EXPECT_SUCCESS(s2n_connection_wipe(conn));
 
             /* Test cipher preferences that use PQ cipher suites that require TLS 1.2 fall back to classic ciphers if a client
@@ -395,7 +418,6 @@ int main(int argc, char **argv)
                 EXPECT_SUCCESS(s2n_connection_wipe(conn));
             }
         }
-#endif
 
         /* Clean+free to setup for ECDSA tests */
         EXPECT_SUCCESS(s2n_config_free(server_config));