From ead40c5378475ffe6bee735d5abc005a57413972 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Wed, 2 Oct 2024 23:09:50 -0700 Subject: [PATCH] refactor: make s2n_array_len constant (#4801) --- codebuild/bin/grep_simple_mistakes.sh | 5 ++-- tests/unit/s2n_certificate_parsing_test.c | 2 +- tests/unit/s2n_safety_test.c | 12 ++++++++++ tests/unit/s2n_server_alpn_extension_test.c | 26 +++++++++++++++++++++ tls/extensions/s2n_server_alpn.c | 1 - tls/s2n_cipher_suites.c | 2 +- utils/s2n_safety.h | 9 ++++++- 7 files changed, 50 insertions(+), 7 deletions(-) diff --git a/codebuild/bin/grep_simple_mistakes.sh b/codebuild/bin/grep_simple_mistakes.sh index af1d5bb334..835b5e0305 100755 --- a/codebuild/bin/grep_simple_mistakes.sh +++ b/codebuild/bin/grep_simple_mistakes.sh @@ -93,11 +93,10 @@ done ############################################# S2N_FILES_ARRAY_SIZING_RETURN=$(find "$PWD" -type f -name "s2n*.c" -path "*") for file in $S2N_FILES_ARRAY_SIZING_RETURN; do - RESULT_ARR_DIV=`grep -Ern 'sizeof\((.*)\) \/ sizeof\(\1\[0\]\)' $file` - + RESULT_ARR_DIV=`grep -Ern 'sizeof\(.*?\) / sizeof\(' $file` if [ "${#RESULT_ARR_DIV}" != "0" ]; then FAILED=1 - printf "\e[1;34mUsage of 'sizeof(array) / sizeof(array[0])' check failed. Use s2n_array_len(array) instead in $file:\e[0m\n$RESULT_ARR_DIV\n\n" + printf "\e[1;34mUsage of 'sizeof(array) / sizeof(T)' check failed. Use s2n_array_len instead in $file:\e[0m\n$RESULT_ARR_DIV\n\n" fi done diff --git a/tests/unit/s2n_certificate_parsing_test.c b/tests/unit/s2n_certificate_parsing_test.c index 2b0b0e5bc5..260f763fc5 100644 --- a/tests/unit/s2n_certificate_parsing_test.c +++ b/tests/unit/s2n_certificate_parsing_test.c @@ -77,7 +77,7 @@ int main(int argc, char **argv) CERTIFICATE_2, CERTIFICATE_3, }; - struct s2n_blob expected_certs[sizeof(expected_cert_strs) / sizeof(char *)] = { 0 }; + struct s2n_blob expected_certs[s2n_array_len(expected_cert_strs)] = { 0 }; EXPECT_EQUAL(s2n_array_len(expected_certs), s2n_array_len(expected_cert_strs)); for (size_t i = 0; i < s2n_array_len(expected_certs); i++) { diff --git a/tests/unit/s2n_safety_test.c b/tests/unit/s2n_safety_test.c index 267124e09b..400a20181e 100644 --- a/tests/unit/s2n_safety_test.c +++ b/tests/unit/s2n_safety_test.c @@ -445,6 +445,18 @@ int main(int argc, char **argv) } } + /* Test: s2n_array_len */ + { + /* Must return correct length */ + uint16_t test_data[10] = { 0 }; + EXPECT_EQUAL(s2n_array_len(test_data), 10); + EXPECT_NOT_EQUAL(s2n_array_len(test_data), sizeof(test_data)); + + /* Must be usable as an array size / constant expression */ + uint16_t test_data_dup[s2n_array_len(test_data)] = { 0 }; + EXPECT_EQUAL(sizeof(test_data), sizeof(test_data_dup)); + } + END_TEST(); return 0; } diff --git a/tests/unit/s2n_server_alpn_extension_test.c b/tests/unit/s2n_server_alpn_extension_test.c index 56dfe98f1b..3a23f2d6a6 100644 --- a/tests/unit/s2n_server_alpn_extension_test.c +++ b/tests/unit/s2n_server_alpn_extension_test.c @@ -120,6 +120,32 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_free(server_conn)); }; + /* Send and receive maximum-sized alpn */ + { + const uint8_t max_size = UINT8_MAX; + + DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT), + s2n_connection_ptr_free); + EXPECT_NOT_NULL(client); + EXPECT_NULL(s2n_get_application_protocol(client)); + + DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER), + s2n_connection_ptr_free); + EXPECT_NOT_NULL(server); + memset(server->application_protocol, 'a', sizeof(server->application_protocol) - 1); + + DEFER_CLEANUP(struct s2n_stuffer stuffer = { 0 }, s2n_stuffer_free); + EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0)); + EXPECT_SUCCESS(s2n_server_alpn_extension.send(server, &stuffer)); + EXPECT_SUCCESS(s2n_server_alpn_extension.recv(client, &stuffer)); + EXPECT_EQUAL(s2n_stuffer_data_available(&stuffer), 0); + + const char *client_alpn = s2n_get_application_protocol(client); + EXPECT_NOT_NULL(client_alpn); + EXPECT_EQUAL(strlen(client_alpn), max_size); + EXPECT_STRING_EQUAL(client_alpn, server->application_protocol); + }; + END_TEST(); return 0; } diff --git a/tls/extensions/s2n_server_alpn.c b/tls/extensions/s2n_server_alpn.c index 54f9e5e856..6681ed5d63 100644 --- a/tls/extensions/s2n_server_alpn.c +++ b/tls/extensions/s2n_server_alpn.c @@ -66,7 +66,6 @@ static int s2n_alpn_recv(struct s2n_connection *conn, struct s2n_stuffer *extens uint8_t protocol_len = 0; POSIX_GUARD(s2n_stuffer_read_uint8(extension, &protocol_len)); - POSIX_ENSURE_LT(protocol_len, s2n_array_len(conn->application_protocol)); uint8_t *protocol = s2n_stuffer_raw_read(extension, protocol_len); POSIX_ENSURE_REF(protocol); diff --git a/tls/s2n_cipher_suites.c b/tls/s2n_cipher_suites.c index abd0c8efa5..d7d563dca9 100644 --- a/tls/s2n_cipher_suites.c +++ b/tls/s2n_cipher_suites.c @@ -1068,7 +1068,7 @@ int s2n_cipher_suites_init(void) /* Reset any selected record algorithms */ S2N_RESULT s2n_cipher_suites_cleanup(void) { - const int num_cipher_suites = sizeof(s2n_all_cipher_suites) / sizeof(struct s2n_cipher_suite *); + const int num_cipher_suites = s2n_array_len(s2n_all_cipher_suites); for (int i = 0; i < num_cipher_suites; i++) { struct s2n_cipher_suite *cur_suite = s2n_all_cipher_suites[i]; cur_suite->available = 0; diff --git a/utils/s2n_safety.h b/utils/s2n_safety.h index d0ac5f2fc1..7a6c3af0e2 100644 --- a/utils/s2n_safety.h +++ b/utils/s2n_safety.h @@ -97,7 +97,14 @@ S2N_CLEANUP_RESULT s2n_connection_apply_error_blinding(struct s2n_connection** c } \ struct __useless_struct_to_allow_trailing_semicolon__ -#define s2n_array_len(array) ((array != NULL) ? (sizeof(array) / sizeof(array[0])) : 0) +/* This method works for ARRAYS, not for POINTERS. + * Calling sizeof on an array declared in the current function correctly returns + * the total size of the array. But once the array is passed to another function, + * it behaves like a pointer. Calling sizeof on a pointer only returns the size + * of the pointer address itself (so usually 8). + * Newer compilers (gcc >= 8.1, clang >= 8.0) will warn if the argument is a pointer. + */ +#define s2n_array_len(array) (sizeof(array) / sizeof(array[0])) int s2n_mul_overflow(uint32_t a, uint32_t b, uint32_t* out);