diff --git a/codebuild/bin/grep_simple_mistakes.sh b/codebuild/bin/grep_simple_mistakes.sh index 5425bcada3b..82efcbf5f53 100755 --- a/codebuild/bin/grep_simple_mistakes.sh +++ b/codebuild/bin/grep_simple_mistakes.sh @@ -36,19 +36,12 @@ done ############################################# S2N_FILES_ASSERT_NOT_USING_MEMCMP=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/bindings/*") declare -A KNOWN_MEMCMP_USAGE -KNOWN_MEMCMP_USAGE["$PWD/crypto/s2n_rsa.c"]=1 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_early_data.c"]=1 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_kem.c"]=1 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_cipher_suites.c"]=3 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_server_hello.c"]=3 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_security_policies.c"]=1 +KNOWN_MEMCMP_USAGE["$PWD/stuffer/s2n_stuffer_text.c"]=1 KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_psk.c"]=1 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_config.c"]=1 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_resume.c"]=2 -KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_connection.c"]=1 KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_protocol_preferences.c"]=1 +KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_cipher_suites.c"]=1 +KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_config.c"]=1 KNOWN_MEMCMP_USAGE["$PWD/utils/s2n_map.c"]=3 -KNOWN_MEMCMP_USAGE["$PWD/stuffer/s2n_stuffer_text.c"]=1 for file in $S2N_FILES_ASSERT_NOT_USING_MEMCMP; do # NOTE: this matches on 'memcmp', which will also match comments. However, there diff --git a/crypto/s2n_rsa.c b/crypto/s2n_rsa.c index 0274284a164..a89b59612b7 100644 --- a/crypto/s2n_rsa.c +++ b/crypto/s2n_rsa.c @@ -168,7 +168,7 @@ static int s2n_rsa_keys_match(const struct s2n_pkey *pub, const struct s2n_pkey plain_out.size = sizeof(plain_outpad); POSIX_GUARD(s2n_rsa_decrypt(priv, &enc, &plain_out)); - S2N_ERROR_IF(memcmp(plain_in.data, plain_out.data, plain_in.size), S2N_ERR_KEY_MISMATCH); + POSIX_ENSURE(s2n_constant_time_equals(plain_in.data, plain_out.data, plain_in.size), S2N_ERR_KEY_MISMATCH); return 0; } diff --git a/tls/s2n_cipher_suites.c b/tls/s2n_cipher_suites.c index 4c32e680147..abd0c8efa51 100644 --- a/tls/s2n_cipher_suites.c +++ b/tls/s2n_cipher_suites.c @@ -1151,7 +1151,7 @@ int s2n_set_cipher_as_client(struct s2n_connection *conn, uint8_t wire[S2N_TLS_C struct s2n_cipher_suite *cipher_suite = NULL; for (size_t i = 0; i < security_policy->cipher_preferences->count; i++) { const uint8_t *ours = security_policy->cipher_preferences->suites[i]->iana_value; - if (memcmp(wire, ours, S2N_TLS_CIPHER_SUITE_LEN) == 0) { + if (s2n_constant_time_equals(wire, ours, S2N_TLS_CIPHER_SUITE_LEN)) { cipher_suite = security_policy->cipher_preferences->suites[i]; break; } @@ -1198,7 +1198,7 @@ static int s2n_wire_ciphers_contain(const uint8_t *match, const uint8_t *wire, u for (size_t i = 0; i < count; i++) { const uint8_t *theirs = wire + (i * cipher_suite_len) + (cipher_suite_len - S2N_TLS_CIPHER_SUITE_LEN); - if (!memcmp(match, theirs, S2N_TLS_CIPHER_SUITE_LEN)) { + if (s2n_constant_time_equals(match, theirs, S2N_TLS_CIPHER_SUITE_LEN)) { return 1; } } diff --git a/tls/s2n_connection.c b/tls/s2n_connection.c index 3ab2ea53dc5..4fe13e573b4 100644 --- a/tls/s2n_connection.c +++ b/tls/s2n_connection.c @@ -922,9 +922,8 @@ int s2n_connection_get_cipher_iana_value(struct s2n_connection *conn, uint8_t *f POSIX_ENSURE_MUT(second); /* ensure we've negotiated a cipher suite */ - POSIX_ENSURE(memcmp(conn->secure->cipher_suite->iana_value, - s2n_null_cipher_suite.iana_value, sizeof(s2n_null_cipher_suite.iana_value)) - != 0, + POSIX_ENSURE(!s2n_constant_time_equals(conn->secure->cipher_suite->iana_value, + s2n_null_cipher_suite.iana_value, sizeof(s2n_null_cipher_suite.iana_value)), S2N_ERR_INVALID_STATE); const uint8_t *iana_value = conn->secure->cipher_suite->iana_value; diff --git a/tls/s2n_early_data.c b/tls/s2n_early_data.c index 6898721d9fe..bb61331e712 100644 --- a/tls/s2n_early_data.c +++ b/tls/s2n_early_data.c @@ -96,7 +96,7 @@ static S2N_RESULT s2n_early_data_validate(struct s2n_connection *conn) const size_t app_protocol_size = strlen(conn->application_protocol); if (app_protocol_size > 0 || config->application_protocol.size > 0) { RESULT_ENSURE_EQ(config->application_protocol.size, app_protocol_size + 1 /* null-terminating char */); - RESULT_ENSURE_EQ(memcmp(config->application_protocol.data, conn->application_protocol, app_protocol_size), 0); + RESULT_ENSURE(s2n_constant_time_equals(config->application_protocol.data, (uint8_t *) conn->application_protocol, app_protocol_size), S2N_ERR_SAFETY); } return S2N_RESULT_OK; diff --git a/tls/s2n_kem.c b/tls/s2n_kem.c index 175e2e62599..5d7c38363cc 100644 --- a/tls/s2n_kem.c +++ b/tls/s2n_kem.c @@ -296,7 +296,7 @@ int s2n_cipher_suite_to_kem(const uint8_t iana_value[S2N_TLS_CIPHER_SUITE_LEN], { for (size_t i = 0; i < s2n_array_len(kem_mapping); i++) { const struct s2n_iana_to_kem *candidate = &kem_mapping[i]; - if (memcmp(iana_value, candidate->iana_value, S2N_TLS_CIPHER_SUITE_LEN) == 0) { + if (s2n_constant_time_equals(iana_value, candidate->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) { *compatible_params = candidate; return S2N_SUCCESS; } diff --git a/tls/s2n_resume.c b/tls/s2n_resume.c index c6e0e32af4d..abca18fcc10 100644 --- a/tls/s2n_resume.c +++ b/tls/s2n_resume.c @@ -165,7 +165,7 @@ static int s2n_tls12_deserialize_resumption_state(struct s2n_connection *conn, s S2N_ERROR_IF(protocol_version != conn->actual_protocol_version, S2N_ERR_INVALID_SERIALIZED_SESSION_STATE); POSIX_GUARD(s2n_stuffer_read_bytes(from, cipher_suite, S2N_TLS_CIPHER_SUITE_LEN)); - S2N_ERROR_IF(memcmp(conn->secure->cipher_suite->iana_value, cipher_suite, S2N_TLS_CIPHER_SUITE_LEN), S2N_ERR_INVALID_SERIALIZED_SESSION_STATE); + POSIX_ENSURE(s2n_constant_time_equals(conn->secure->cipher_suite->iana_value, cipher_suite, S2N_TLS_CIPHER_SUITE_LEN), S2N_ERR_INVALID_SERIALIZED_SESSION_STATE); uint64_t now = 0; POSIX_GUARD_RESULT(s2n_config_wall_clock(conn->config, &now)); @@ -767,7 +767,7 @@ struct s2n_ticket_key *s2n_find_ticket_key(struct s2n_config *config, const uint for (uint32_t i = 0; i < ticket_keys_len; i++) { PTR_GUARD_RESULT(s2n_set_get(config->ticket_keys, i, (void **) &ticket_key)); - if (memcmp(ticket_key->key_name, name, S2N_TICKET_KEY_NAME_LEN) == 0) { + if (s2n_constant_time_equals(ticket_key->key_name, name, S2N_TICKET_KEY_NAME_LEN)) { /* Check to see if the key has expired */ if (now >= ticket_key->intro_timestamp + config->encrypt_decrypt_key_lifetime_in_nanos diff --git a/tls/s2n_security_policies.c b/tls/s2n_security_policies.c index d6a833ab2f8..12a801e00f1 100644 --- a/tls/s2n_security_policies.c +++ b/tls/s2n_security_policies.c @@ -1487,7 +1487,7 @@ int s2n_connection_is_valid_for_cipher_preferences(struct s2n_connection *conn, struct s2n_cipher_suite *cipher = conn->secure->cipher_suite; POSIX_ENSURE_REF(cipher); for (int i = 0; i < security_policy->cipher_preferences->count; ++i) { - if (0 == memcmp(security_policy->cipher_preferences->suites[i]->iana_value, cipher->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) { + if (s2n_constant_time_equals(security_policy->cipher_preferences->suites[i]->iana_value, cipher->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) { return 1; } } diff --git a/tls/s2n_server_hello.c b/tls/s2n_server_hello.c index 0524213f6e4..d08999e4bea 100644 --- a/tls/s2n_server_hello.c +++ b/tls/s2n_server_hello.c @@ -51,7 +51,7 @@ static int s2n_random_value_is_hello_retry(struct s2n_connection *conn) { POSIX_ENSURE_REF(conn); - POSIX_ENSURE(memcmp(hello_retry_req_random, conn->handshake_params.server_random, S2N_TLS_RANDOM_DATA_LEN) == 0, + POSIX_ENSURE(s2n_constant_time_equals(hello_retry_req_random, conn->handshake_params.server_random, S2N_TLS_RANDOM_DATA_LEN), S2N_ERR_INVALID_HELLO_RETRY); return S2N_SUCCESS; @@ -157,7 +157,7 @@ static int s2n_server_hello_parse(struct s2n_connection *conn) S2N_ERROR_IF(compression_method != S2N_TLS_COMPRESSION_METHOD_NULL, S2N_ERR_BAD_MESSAGE); bool session_ids_match = session_id_len != 0 && session_id_len == conn->session_id_len - && memcmp(session_id, conn->session_id, session_id_len) == 0; + && s2n_constant_time_equals(session_id, conn->session_id, session_id_len); if (!session_ids_match) { conn->ems_negotiated = false; } @@ -234,7 +234,7 @@ static int s2n_server_hello_parse(struct s2n_connection *conn) if (session_ids_match) { /* check if the resumed session state is valid */ POSIX_ENSURE(conn->resume_protocol_version == conn->actual_protocol_version, S2N_ERR_BAD_MESSAGE); - POSIX_ENSURE(memcmp(conn->secure->cipher_suite->iana_value, cipher_suite_wire, S2N_TLS_CIPHER_SUITE_LEN) == 0, + POSIX_ENSURE(s2n_constant_time_equals(conn->secure->cipher_suite->iana_value, cipher_suite_wire, S2N_TLS_CIPHER_SUITE_LEN), S2N_ERR_BAD_MESSAGE); /* Session is resumed */