Skip to content

Commit

Permalink
enforce result checking for blob and mem (#4389)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmayclin authored Jan 31, 2024
1 parent c74f442 commit c128140
Show file tree
Hide file tree
Showing 10 changed files with 19 additions and 19 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ if (BUILD_TESTING)
find . -name '${test_case_name}.c.o' -exec objcopy --redefine-syms libcrypto.symbols {} \\\;
)
endif()
target_compile_options(${test_case_name} PRIVATE -Wno-implicit-function-declaration -Wno-deprecated -D_POSIX_C_SOURCE=200809L -std=gnu99)
target_compile_options(${test_case_name} PRIVATE -Wno-implicit-function-declaration -Wno-deprecated -Wunused-result -D_POSIX_C_SOURCE=200809L -std=gnu99)
if (S2N_LTO)
target_compile_options(${test_case_name} PRIVATE -flto)
endif()
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_kem_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ int main(int argc, char **argv)
struct s2n_stuffer io_stuffer = { 0 };
EXPECT_SUCCESS(s2n_stuffer_init(&io_stuffer, &io_blob));

s2n_alloc(&(kem_params.private_key), TEST_PRIVATE_KEY_LENGTH);
EXPECT_SUCCESS(s2n_alloc(&(kem_params.private_key), TEST_PRIVATE_KEY_LENGTH));
POSIX_CHECKED_MEMCPY(kem_params.private_key.data, TEST_PRIVATE_KEY, TEST_PRIVATE_KEY_LENGTH);

/* {0, 5} = length of ciphertext to follow
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_self_talk_nonblocking_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ int test_send(int use_tls13, int use_iov, int prefer_throughput)
struct iovec *iov = NULL;
if (!use_iov) {
data_size = 10000000;
s2n_alloc(&blob, data_size);
EXPECT_SUCCESS(s2n_alloc(&blob, data_size));
EXPECT_OK(s2n_get_public_random_data(&blob));
} else {
iov = malloc(sizeof(*iov) * iov_size);
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/s2n_server_extensions_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ int main(int argc, char **argv)

struct s2n_cert_chain_and_key fake_chain_and_key = { 0 };
static uint8_t sct_list[] = { 0xff, 0xff, 0xff };
s2n_blob_init(&fake_chain_and_key.sct_list, sct_list, sizeof(sct_list));
EXPECT_SUCCESS(s2n_blob_init(&fake_chain_and_key.sct_list, sct_list, sizeof(sct_list)));

conn->ct_level_requested = S2N_CT_SUPPORT_REQUEST;
conn->handshake_params.our_chain_and_key = &fake_chain_and_key;
Expand Down Expand Up @@ -221,7 +221,7 @@ int main(int argc, char **argv)

struct s2n_cert_chain_and_key fake_chain_and_key = { 0 };
static uint8_t fake_ocsp[] = { 0xff, 0xff, 0xff };
s2n_blob_init(&fake_chain_and_key.ocsp_status, fake_ocsp, sizeof(fake_ocsp));
EXPECT_SUCCESS(s2n_blob_init(&fake_chain_and_key.ocsp_status, fake_ocsp, sizeof(fake_ocsp)));

conn->status_type = S2N_STATUS_REQUEST_OCSP;
conn->handshake_params.our_chain_and_key = &fake_chain_and_key;
Expand Down Expand Up @@ -549,7 +549,7 @@ int main(int argc, char **argv)

struct s2n_cert_chain_and_key fake_chain_and_key = { 0 };
static uint8_t fake_ocsp[] = { 0xff, 0xff, 0xff };
s2n_blob_init(&fake_chain_and_key.ocsp_status, fake_ocsp, sizeof(fake_ocsp));
EXPECT_SUCCESS(s2n_blob_init(&fake_chain_and_key.ocsp_status, fake_ocsp, sizeof(fake_ocsp)));

/* For our test status_request extension */
server_conn->status_type = S2N_STATUS_REQUEST_OCSP;
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_client_finished.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ int s2n_tls13_client_finished_recv(struct s2n_connection *conn)

/* read finished mac from handshake */
struct s2n_blob wire_finished_mac = { 0 };
s2n_blob_init(&wire_finished_mac, s2n_stuffer_raw_read(&conn->handshake.io, length), length);
POSIX_GUARD(s2n_blob_init(&wire_finished_mac, s2n_stuffer_raw_read(&conn->handshake.io, length), length));

/* get tls13 keys */
s2n_tls13_connection_keys(keys, conn);
Expand Down
4 changes: 2 additions & 2 deletions tls/s2n_record_write.c
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ int s2n_record_writev(struct s2n_connection *conn, uint8_t content_type, const s
/* If we're AEAD, write the sequence number as an IV, and generate the AAD */
if (cipher_suite->record_alg->cipher->type == S2N_AEAD) {
struct s2n_stuffer iv_stuffer = { 0 };
s2n_blob_init(&iv, aad_iv, sizeof(aad_iv));
POSIX_GUARD(s2n_blob_init(&iv, aad_iv, sizeof(aad_iv)));
POSIX_GUARD(s2n_stuffer_init(&iv_stuffer, &iv));

if (cipher_suite->record_alg->flags & S2N_TLS12_AES_GCM_AEAD_NONCE) {
Expand Down Expand Up @@ -429,7 +429,7 @@ int s2n_record_writev(struct s2n_connection *conn, uint8_t content_type, const s
POSIX_GUARD_RESULT(s2n_aead_aad_init(conn, sequence_number, content_type, data_bytes_to_take, &aad));
}
} else if (cipher_suite->record_alg->cipher->type == S2N_CBC || cipher_suite->record_alg->cipher->type == S2N_COMPOSITE) {
s2n_blob_init(&iv, implicit_iv, block_size);
POSIX_GUARD(s2n_blob_init(&iv, implicit_iv, block_size));

/* For TLS1.1/1.2; write the IV with random data */
if (conn->actual_protocol_version > S2N_TLS10) {
Expand Down
4 changes: 2 additions & 2 deletions tls/s2n_resume.c
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ int s2n_decrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *

POSIX_GUARD(s2n_stuffer_read(from, &iv));

s2n_blob_init(&aes_key_blob, key->aes_key, S2N_AES256_KEY_LEN);
POSIX_GUARD(s2n_blob_init(&aes_key_blob, key->aes_key, S2N_AES256_KEY_LEN));
POSIX_GUARD(s2n_session_key_alloc(&aes_ticket_key));
POSIX_GUARD(s2n_aes256_gcm.init(&aes_ticket_key));
POSIX_GUARD(s2n_aes256_gcm.set_decryption_key(&aes_ticket_key, &aes_key_blob));
Expand Down Expand Up @@ -925,7 +925,7 @@ int s2n_decrypt_session_cache(struct s2n_connection *conn, struct s2n_stuffer *f

POSIX_GUARD(s2n_stuffer_read(from, &iv));

s2n_blob_init(&aes_key_blob, key->aes_key, S2N_AES256_KEY_LEN);
POSIX_GUARD(s2n_blob_init(&aes_key_blob, key->aes_key, S2N_AES256_KEY_LEN));
POSIX_GUARD(s2n_session_key_alloc(&aes_ticket_key));
POSIX_GUARD(s2n_aes256_gcm.init(&aes_ticket_key));
POSIX_GUARD(s2n_aes256_gcm.set_decryption_key(&aes_ticket_key, &aes_key_blob));
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_server_finished.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ int s2n_tls13_server_finished_recv(struct s2n_connection *conn)

/* read finished mac from handshake */
struct s2n_blob wire_finished_mac = { 0 };
s2n_blob_init(&wire_finished_mac, s2n_stuffer_raw_read(&conn->handshake.io, length), length);
POSIX_GUARD(s2n_blob_init(&wire_finished_mac, s2n_stuffer_raw_read(&conn->handshake.io, length), length));

/* get tls13 keys */
s2n_tls13_connection_keys(keys, conn);
Expand Down
8 changes: 4 additions & 4 deletions utils/s2n_blob.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ struct s2n_blob {

bool s2n_blob_is_growable(const struct s2n_blob *b);
S2N_RESULT s2n_blob_validate(const struct s2n_blob *b);
int s2n_blob_init(struct s2n_blob *b, uint8_t *data, uint32_t size);
int S2N_RESULT_MUST_USE s2n_blob_init(struct s2n_blob *b, uint8_t *data, uint32_t size);
int s2n_blob_zero(struct s2n_blob *b);
int s2n_blob_char_to_lower(struct s2n_blob *b);
int s2n_hex_string_to_bytes(const uint8_t *str, struct s2n_blob *blob);
int s2n_blob_slice(const struct s2n_blob *b, struct s2n_blob *slice, uint32_t offset, uint32_t size);
int S2N_RESULT_MUST_USE s2n_blob_char_to_lower(struct s2n_blob *b);
int S2N_RESULT_MUST_USE s2n_hex_string_to_bytes(const uint8_t *str, struct s2n_blob *blob);
int S2N_RESULT_MUST_USE s2n_blob_slice(const struct s2n_blob *b, struct s2n_blob *slice, uint32_t offset, uint32_t size);

#define s2n_stack_blob(name, requested_size, maximum) \
size_t name##_requested_size = (requested_size); \
Expand Down
6 changes: 3 additions & 3 deletions utils/s2n_mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ int s2n_mem_cleanup(void);
* Generally, s2n_realloc is preferred over s2n_alloc. This is because calling
* s2n_alloc on a blob that already has memory allocated will leak memory.
*/
int s2n_alloc(struct s2n_blob *b, uint32_t size);
int s2n_realloc(struct s2n_blob *b, uint32_t size);
int S2N_RESULT_MUST_USE s2n_alloc(struct s2n_blob *b, uint32_t size);
int S2N_RESULT_MUST_USE s2n_realloc(struct s2n_blob *b, uint32_t size);
int s2n_free(struct s2n_blob *b);
int s2n_free_without_wipe(struct s2n_blob *b);
int s2n_free_object(uint8_t **p_data, uint32_t size);
int s2n_dup(struct s2n_blob *from, struct s2n_blob *to);
int S2N_RESULT_MUST_USE s2n_dup(struct s2n_blob *from, struct s2n_blob *to);

/* Unlike free, s2n_free_or_wipe accepts static blobs.
* It frees allocated blobs and wipes static blobs.
Expand Down

0 comments on commit c128140

Please sign in to comment.