cleanup detection logic

bindings/rust/s2n-tls/src/fingerprint.rs

@@ -394,7 +394,6 @@ mod tests {
 
     #[test]
     fn connection_fingerprint() -> Result<(), Box<dyn Error>> {
-        // TODO not protocol dependent
         let pair = simple_handshake()?;
         let client_hello = pair.server.client_hello()?;
 
@@ -663,7 +662,6 @@ mod tests {
     #[test]
     #[allow(deprecated)]
     fn legacy_connection_fingerprint() -> Result<(), Box<dyn Error>> {
-        // TODO not protocol dependent
         let pair = simple_handshake()?;
         let client_hello = pair.server.client_hello()?;
 

bindings/rust/s2n-tls/src/testing/s2n_tls.rs

@@ -16,7 +16,6 @@ mod tests {
 
     #[test]
     fn handshake_default() {
-        // TODO not protocol dependent
         let config = build_config(&security::DEFAULT).unwrap();
         assert!(TestPair::handshake_with_config(&config).is_ok());
     }
@@ -642,7 +641,6 @@ mod tests {
 
     #[test]
     fn no_application_protocol() -> Result<(), Error> {
-        // TODO not protocol dependent
         let config = config_builder(&security::DEFAULT)?.build()?;
         let mut pair = TestPair::from_config(&config);
         pair.handshake()?;
@@ -652,7 +650,6 @@ mod tests {
 
     #[test]
     fn application_protocol() -> Result<(), Error> {
-        // TODO not protocol dependent
         let config = config_builder(&security::DEFAULT)?.build()?;
         let mut pair = TestPair::from_config(&config);
         pair.server

codebuild/bin/grep_simple_mistakes.sh

@@ -225,16 +225,16 @@ if [[ -n $S2N_ENSURE_WITH_INVALID_ERROR_CODE ]]; then
 fi
 
 #############################################
-# Assert test don't specify the "default" security policy
+# Assert tests don't specify the "default" security policy.
 #
 # Since the "default" policies are subject to change, tests should instead specify
 # an immutable numbered policy to avoid unwanted testing behavior.
 #############################################
 S2N_DEFAULT_SECURITY_POLICY_USAGE=$(find "$PWD" -type f -name "s2n*.c" -not -path "*/bindings/*" -not -path "*/bin/*")
 declare -A KNOWN_DEFAULT_USAGE
-KNOWN_DEFAULT_USAGE["$PWD/tls/s2n_config.c"]=2
+KNOWN_DEFAULT_USAGE["$PWD/tls/s2n_config.c"]=1
 KNOWN_DEFAULT_USAGE["$PWD/tls/s2n_security_policies.c"]=5
-KNOWN_DEFAULT_USAGE["$PWD/tests/unit/s2n_security_policies_test.c"]=5
+KNOWN_DEFAULT_USAGE["$PWD/tests/unit/s2n_security_policies_test.c"]=1
 KNOWN_DEFAULT_USAGE["$PWD/tests/unit/s2n_client_hello_test.c"]=2
 KNOWN_DEFAULT_USAGE["$PWD/tests/unit/s2n_connection_preferences_test.c"]=1
 KNOWN_DEFAULT_USAGE["$PWD/tests/unit/s2n_config_test.c"]=1

tests/unit/s2n_client_hello_test.c

@@ -583,9 +583,7 @@ int main(int argc, char **argv)
             for (size_t i = 0; i < s2n_array_len(test_cases); i++) {
                 struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
                 EXPECT_NOT_NULL(conn);
-                dbg_bail = false;
                 EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(conn, test_cases[i].security_policy));
-                dbg_bail = true;
 
                 EXPECT_SUCCESS(s2n_client_hello_send(conn));
                 EXPECT_SUCCESS(s2n_parse_client_hello(conn));
@@ -1567,9 +1565,7 @@ int main(int argc, char **argv)
             DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
                     s2n_connection_ptr_free);
             EXPECT_SUCCESS(s2n_connection_set_config(client, config));
-            dbg_bail = false;
             EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(client, security_policy));
-            dbg_bail = true;
 
             EXPECT_SUCCESS(s2n_handshake_write_header(&client->handshake.io, TLS_CLIENT_HELLO));
             EXPECT_SUCCESS(s2n_client_hello_send(client));

tests/unit/s2n_config_test.c

@@ -72,9 +72,7 @@ int main(int argc, char **argv)
     const struct s2n_security_policy *default_security_policy = NULL, *tls13_security_policy = NULL, *fips_security_policy = NULL;
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default_tls13", &tls13_security_policy));
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default_fips", &fips_security_policy));
-    dbg_bail = false;
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default", &default_security_policy));
-    dbg_bail = true;
 
     char cert[S2N_MAX_TEST_PEM_SIZE] = { 0 };
     EXPECT_SUCCESS(s2n_read_test_pem(S2N_DEFAULT_TEST_CERT_CHAIN, cert, S2N_MAX_TEST_PEM_SIZE));
@@ -1094,7 +1092,7 @@ int main(int argc, char **argv)
         struct s2n_security_policy rfc9151_applied_locally = security_policy_rfc9151;
         rfc9151_applied_locally.certificate_preferences_apply_locally = true;
 
-        /* rfc9151 doesn't allow SHA256 signatures, but does allow SHA384 signatures, 
+        /* rfc9151 doesn't allow SHA256 signatures, but does allow SHA384 signatures,
          * so ecdsa_p384_sha256 is invalid and ecdsa_p384_sha384 is valid */
 
         /* valid certs are accepted */
@@ -1121,8 +1119,8 @@ int main(int argc, char **argv)
 
         /* certs in default_certs_by_type are validated */
         {
-            /* s2n_config_set_cert_chain_and_key_defaults populates default_certs_by_type 
-             * but doesn't populate domain_name_to_cert_map 
+            /* s2n_config_set_cert_chain_and_key_defaults populates default_certs_by_type
+             * but doesn't populate domain_name_to_cert_map
              */
             DEFER_CLEANUP(struct s2n_config *config = s2n_config_new_minimal(), s2n_config_ptr_free);
             EXPECT_SUCCESS(s2n_config_set_cert_chain_and_key_defaults(config, &invalid_cert, 1));

tests/unit/s2n_connection_preferences_test.c

@@ -31,9 +31,7 @@ int main(int argc, char **argv)
     const struct s2n_security_policy *default_security_policy = NULL, *tls13_security_policy = NULL, *fips_security_policy = NULL;
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default_tls13", &tls13_security_policy));
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default_fips", &fips_security_policy));
-    dbg_bail = false;
     EXPECT_SUCCESS(s2n_find_security_policy_from_version("default", &default_security_policy));
-    dbg_bail = true;
 
     /* Test default TLS1.2 */
     if (!s2n_is_in_fips_mode()) {
@@ -261,9 +259,7 @@ int main(int argc, char **argv)
 
         /* The static configs were mutated. Reset them to allow following unit tests to use them. */
         s2n_wipe_static_configs();
-        dbg_bail = false;
         EXPECT_SUCCESS(s2n_config_defaults_init());
-        dbg_bail = true;
     };
 
     /* s2n_connection_get_curve */

tests/unit/s2n_security_policies_test.c

@@ -507,9 +507,7 @@ int main(int argc, char **argv)
 
         for (size_t i = 0; i < s2n_array_len(tls12_only_security_policy_strings); i++) {
             security_policy = NULL;
-            dbg_bail = false;
             EXPECT_SUCCESS(s2n_find_security_policy_from_version(tls12_only_security_policy_strings[i], &security_policy));
-            dbg_bail = true;
             EXPECT_FALSE(s2n_security_policy_supports_tls13(security_policy));
         }
 
@@ -970,9 +968,7 @@ int main(int argc, char **argv)
     {
         /* 20230317 */
         {
-            dbg_bail = false;
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20230317, "default", rsa_chain_and_key));
-            dbg_bail = true;
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20230317, "default_tls13", rsa_chain_and_key));
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20230317, "default_fips", rsa_chain_and_key));
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20230317, "20230317", rsa_chain_and_key));
@@ -998,10 +994,8 @@ int main(int argc, char **argv)
 
         /* 20240331 */
         {
-            dbg_bail = false;
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240331,
                     "default", rsa_chain_and_key));
-            dbg_bail = true;
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240331,
                     "default_tls13", rsa_chain_and_key));
             EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240331,
@@ -1055,11 +1049,9 @@ int main(int argc, char **argv)
                 { .cert = ecdsa_chain_and_key, .start_index = 1 },
             };
 
-            dbg_bail = false;
             EXPECT_OK(s2n_test_default_backwards_compatible("default",
                     versioned_policies, s2n_array_len(versioned_policies),
                     supported_certs, s2n_array_len(supported_certs)));
-            dbg_bail = true;
         };
 
         /* "default_tls13" */

tls/s2n_config.c

@@ -51,10 +51,6 @@ static int monotonic_clock(void *data, uint64_t *nanoseconds)
     return 0;
 }
 
-/* Used to add exception when creating a new config */
-bool dbg_config_init = true;
-/* Control exception to the "default" policy usage */
-bool dbg_bail = true;
 static int wall_clock(void *data, uint64_t *nanoseconds)
 {
     struct timespec current_time = { 0 };
@@ -104,11 +100,7 @@ static int s2n_config_init(struct s2n_config *config)
 
     config->client_hello_cb_mode = S2N_CLIENT_HELLO_CB_BLOCKING;
 
-    /* TODO remove */
-    /* avoid bailing when creating a new config `s2n_config_new()` */
-    dbg_config_init = false;
     POSIX_GUARD(s2n_config_setup_default(config));
-    dbg_config_init = true;
     if (s2n_use_default_tls13_config()) {
         POSIX_GUARD(s2n_config_setup_tls13(config));
     } else if (s2n_is_in_fips_mode()) {

tls/s2n_config.h

@@ -31,10 +31,6 @@
 #include "utils/s2n_blob.h"
 #include "utils/s2n_set.h"
 
-/* TODO remove */
-extern bool dbg_config_init;
-extern bool dbg_bail;
-
 #define S2N_MAX_TICKET_KEYS       48
 #define S2N_MAX_TICKET_KEY_HASHES 500 /* 10KB */
 

tls/s2n_security_policies.c

@@ -18,7 +18,6 @@
 #include "api/s2n.h"
 #include "tls/s2n_certificate_keys.h"
 #include "tls/s2n_connection.h"
-#include "utils/s2n_init.h"
 #include "utils/s2n_safety.h"
 
 /* TLS1.2 default as of 05/24 */
@@ -1271,22 +1270,6 @@ int s2n_find_security_policy_from_version(const char *version, const struct s2n_
     POSIX_ENSURE_REF(version);
     POSIX_ENSURE_REF(security_policy);
 
-    bool matches_default = strcmp(version, "default") == 0;
-    bool should_bail =
-            /* allow for exception for tests which actually want to test the "default" policy */
-            dbg_bail &&
-            /* allow for s2n_config_new object creation */
-            dbg_config_init &&
-            /* s2n_init() creates a "default" static config so only bail after initialization is complete; */
-            s2n_is_initialized() &&
-            /* attempting to use the "default" policy */
-            matches_default;
-
-    if (should_bail) {
-        printf("\nBail------- s2n_find_from_version: config_init: %d", dbg_config_init);
-        POSIX_BAIL(S2N_ERR_INVALID_SECURITY_POLICY);
-    }
-
     for (int i = 0; security_policy_selection[i].version != NULL; i++) {
         if (!strcasecmp(version, security_policy_selection[i].version)) {
             *security_policy = security_policy_selection[i].security_policy;