Skip to content

Commit 08d413a

Browse files
authored
refactor: replace memcmp to s2n_constant_time_equals (#4709)
1 parent 9964ee7 commit 08d413a

File tree

9 files changed

+16
-24
lines changed

9 files changed

+16
-24
lines changed

codebuild/bin/grep_simple_mistakes.sh

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,12 @@ done
3636
#############################################
3737
S2N_FILES_ASSERT_NOT_USING_MEMCMP=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/bindings/*")
3838
declare -A KNOWN_MEMCMP_USAGE
39-
KNOWN_MEMCMP_USAGE["$PWD/crypto/s2n_rsa.c"]=1
40-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_early_data.c"]=1
41-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_kem.c"]=1
42-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_cipher_suites.c"]=3
43-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_server_hello.c"]=3
44-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_security_policies.c"]=1
39+
KNOWN_MEMCMP_USAGE["$PWD/stuffer/s2n_stuffer_text.c"]=1
4540
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_psk.c"]=1
46-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_config.c"]=1
47-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_resume.c"]=2
48-
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_connection.c"]=1
4941
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_protocol_preferences.c"]=1
42+
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_cipher_suites.c"]=1
43+
KNOWN_MEMCMP_USAGE["$PWD/tls/s2n_config.c"]=1
5044
KNOWN_MEMCMP_USAGE["$PWD/utils/s2n_map.c"]=3
51-
KNOWN_MEMCMP_USAGE["$PWD/stuffer/s2n_stuffer_text.c"]=1
5245

5346
for file in $S2N_FILES_ASSERT_NOT_USING_MEMCMP; do
5447
# NOTE: this matches on 'memcmp', which will also match comments. However, there

crypto/s2n_rsa.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ static int s2n_rsa_keys_match(const struct s2n_pkey *pub, const struct s2n_pkey
168168
plain_out.size = sizeof(plain_outpad);
169169
POSIX_GUARD(s2n_rsa_decrypt(priv, &enc, &plain_out));
170170

171-
S2N_ERROR_IF(memcmp(plain_in.data, plain_out.data, plain_in.size), S2N_ERR_KEY_MISMATCH);
171+
POSIX_ENSURE(s2n_constant_time_equals(plain_in.data, plain_out.data, plain_in.size), S2N_ERR_KEY_MISMATCH);
172172

173173
return 0;
174174
}

tls/s2n_cipher_suites.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ int s2n_set_cipher_as_client(struct s2n_connection *conn, uint8_t wire[S2N_TLS_C
11511151
struct s2n_cipher_suite *cipher_suite = NULL;
11521152
for (size_t i = 0; i < security_policy->cipher_preferences->count; i++) {
11531153
const uint8_t *ours = security_policy->cipher_preferences->suites[i]->iana_value;
1154-
if (memcmp(wire, ours, S2N_TLS_CIPHER_SUITE_LEN) == 0) {
1154+
if (s2n_constant_time_equals(wire, ours, S2N_TLS_CIPHER_SUITE_LEN)) {
11551155
cipher_suite = security_policy->cipher_preferences->suites[i];
11561156
break;
11571157
}
@@ -1198,7 +1198,7 @@ static int s2n_wire_ciphers_contain(const uint8_t *match, const uint8_t *wire, u
11981198
for (size_t i = 0; i < count; i++) {
11991199
const uint8_t *theirs = wire + (i * cipher_suite_len) + (cipher_suite_len - S2N_TLS_CIPHER_SUITE_LEN);
12001200

1201-
if (!memcmp(match, theirs, S2N_TLS_CIPHER_SUITE_LEN)) {
1201+
if (s2n_constant_time_equals(match, theirs, S2N_TLS_CIPHER_SUITE_LEN)) {
12021202
return 1;
12031203
}
12041204
}

tls/s2n_connection.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -922,9 +922,8 @@ int s2n_connection_get_cipher_iana_value(struct s2n_connection *conn, uint8_t *f
922922
POSIX_ENSURE_MUT(second);
923923

924924
/* ensure we've negotiated a cipher suite */
925-
POSIX_ENSURE(memcmp(conn->secure->cipher_suite->iana_value,
926-
s2n_null_cipher_suite.iana_value, sizeof(s2n_null_cipher_suite.iana_value))
927-
!= 0,
925+
POSIX_ENSURE(!s2n_constant_time_equals(conn->secure->cipher_suite->iana_value,
926+
s2n_null_cipher_suite.iana_value, sizeof(s2n_null_cipher_suite.iana_value)),
928927
S2N_ERR_INVALID_STATE);
929928

930929
const uint8_t *iana_value = conn->secure->cipher_suite->iana_value;

tls/s2n_early_data.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ static S2N_RESULT s2n_early_data_validate(struct s2n_connection *conn)
9696
const size_t app_protocol_size = strlen(conn->application_protocol);
9797
if (app_protocol_size > 0 || config->application_protocol.size > 0) {
9898
RESULT_ENSURE_EQ(config->application_protocol.size, app_protocol_size + 1 /* null-terminating char */);
99-
RESULT_ENSURE_EQ(memcmp(config->application_protocol.data, conn->application_protocol, app_protocol_size), 0);
99+
RESULT_ENSURE(s2n_constant_time_equals(config->application_protocol.data, (uint8_t *) conn->application_protocol, app_protocol_size), S2N_ERR_SAFETY);
100100
}
101101

102102
return S2N_RESULT_OK;

tls/s2n_kem.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ int s2n_cipher_suite_to_kem(const uint8_t iana_value[S2N_TLS_CIPHER_SUITE_LEN],
296296
{
297297
for (size_t i = 0; i < s2n_array_len(kem_mapping); i++) {
298298
const struct s2n_iana_to_kem *candidate = &kem_mapping[i];
299-
if (memcmp(iana_value, candidate->iana_value, S2N_TLS_CIPHER_SUITE_LEN) == 0) {
299+
if (s2n_constant_time_equals(iana_value, candidate->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) {
300300
*compatible_params = candidate;
301301
return S2N_SUCCESS;
302302
}

tls/s2n_resume.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ static int s2n_tls12_deserialize_resumption_state(struct s2n_connection *conn, s
165165
S2N_ERROR_IF(protocol_version != conn->actual_protocol_version, S2N_ERR_INVALID_SERIALIZED_SESSION_STATE);
166166

167167
POSIX_GUARD(s2n_stuffer_read_bytes(from, cipher_suite, S2N_TLS_CIPHER_SUITE_LEN));
168-
S2N_ERROR_IF(memcmp(conn->secure->cipher_suite->iana_value, cipher_suite, S2N_TLS_CIPHER_SUITE_LEN), S2N_ERR_INVALID_SERIALIZED_SESSION_STATE);
168+
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);
169169

170170
uint64_t now = 0;
171171
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
767767
for (uint32_t i = 0; i < ticket_keys_len; i++) {
768768
PTR_GUARD_RESULT(s2n_set_get(config->ticket_keys, i, (void **) &ticket_key));
769769

770-
if (memcmp(ticket_key->key_name, name, S2N_TICKET_KEY_NAME_LEN) == 0) {
770+
if (s2n_constant_time_equals(ticket_key->key_name, name, S2N_TICKET_KEY_NAME_LEN)) {
771771
/* Check to see if the key has expired */
772772
if (now >= ticket_key->intro_timestamp
773773
+ config->encrypt_decrypt_key_lifetime_in_nanos

tls/s2n_security_policies.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1487,7 +1487,7 @@ int s2n_connection_is_valid_for_cipher_preferences(struct s2n_connection *conn,
14871487
struct s2n_cipher_suite *cipher = conn->secure->cipher_suite;
14881488
POSIX_ENSURE_REF(cipher);
14891489
for (int i = 0; i < security_policy->cipher_preferences->count; ++i) {
1490-
if (0 == memcmp(security_policy->cipher_preferences->suites[i]->iana_value, cipher->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) {
1490+
if (s2n_constant_time_equals(security_policy->cipher_preferences->suites[i]->iana_value, cipher->iana_value, S2N_TLS_CIPHER_SUITE_LEN)) {
14911491
return 1;
14921492
}
14931493
}

tls/s2n_server_hello.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ static int s2n_random_value_is_hello_retry(struct s2n_connection *conn)
5151
{
5252
POSIX_ENSURE_REF(conn);
5353

54-
POSIX_ENSURE(memcmp(hello_retry_req_random, conn->handshake_params.server_random, S2N_TLS_RANDOM_DATA_LEN) == 0,
54+
POSIX_ENSURE(s2n_constant_time_equals(hello_retry_req_random, conn->handshake_params.server_random, S2N_TLS_RANDOM_DATA_LEN),
5555
S2N_ERR_INVALID_HELLO_RETRY);
5656

5757
return S2N_SUCCESS;
@@ -157,7 +157,7 @@ static int s2n_server_hello_parse(struct s2n_connection *conn)
157157
S2N_ERROR_IF(compression_method != S2N_TLS_COMPRESSION_METHOD_NULL, S2N_ERR_BAD_MESSAGE);
158158

159159
bool session_ids_match = session_id_len != 0 && session_id_len == conn->session_id_len
160-
&& memcmp(session_id, conn->session_id, session_id_len) == 0;
160+
&& s2n_constant_time_equals(session_id, conn->session_id, session_id_len);
161161
if (!session_ids_match) {
162162
conn->ems_negotiated = false;
163163
}
@@ -234,7 +234,7 @@ static int s2n_server_hello_parse(struct s2n_connection *conn)
234234
if (session_ids_match) {
235235
/* check if the resumed session state is valid */
236236
POSIX_ENSURE(conn->resume_protocol_version == conn->actual_protocol_version, S2N_ERR_BAD_MESSAGE);
237-
POSIX_ENSURE(memcmp(conn->secure->cipher_suite->iana_value, cipher_suite_wire, S2N_TLS_CIPHER_SUITE_LEN) == 0,
237+
POSIX_ENSURE(s2n_constant_time_equals(conn->secure->cipher_suite->iana_value, cipher_suite_wire, S2N_TLS_CIPHER_SUITE_LEN),
238238
S2N_ERR_BAD_MESSAGE);
239239

240240
/* Session is resumed */

0 commit comments

Comments
 (0)