Skip to content

Commit

Permalink
Apple assert fix (#121)
Browse files Browse the repository at this point in the history 
**ISSUE 1**: Debug builds on Apple were hitting asserts due to an `aws_byte_buf` where `capacity` wasn't set. Fixed by using `aws_byte_buf_from_array()`, instead of setting struct members directly.

**ISSUE 2**: Tests on 0-length input didn't actually DO anything. This was due to complicated loops in the tests. Fixed by making the loops even more complicated.
  • Loading branch information
@graebm
graebm authored Aug 10, 2022
1 parent f7bfcaa commit 1458c70
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 32 deletions.
12 changes: 6 additions & 6 deletions source/darwin/securityframework_ecc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,15 @@ static struct commoncrypto_ecc_key_pair *s_alloc_pair_and_init_buffers(
}

if (pub_x.ptr) {
cc_key_pair->key_pair.pub_x.buffer = cc_key_pair->key_pair.key_buf.buffer + 1;
cc_key_pair->key_pair.pub_x.len = s_key_coordinate_size;
cc_key_pair->key_pair.pub_x =
aws_byte_buf_from_array(cc_key_pair->key_pair.key_buf.buffer + 1, s_key_coordinate_size);

cc_key_pair->key_pair.pub_y.buffer = cc_key_pair->key_pair.pub_x.buffer + s_key_coordinate_size;
cc_key_pair->key_pair.pub_y.len = s_key_coordinate_size;
cc_key_pair->key_pair.pub_y =
aws_byte_buf_from_array(cc_key_pair->key_pair.pub_x.buffer + s_key_coordinate_size, s_key_coordinate_size);
}

cc_key_pair->key_pair.priv_d.buffer = cc_key_pair->key_pair.key_buf.buffer + 1 + (s_key_coordinate_size * 2);
cc_key_pair->key_pair.priv_d.len = s_key_coordinate_size;
cc_key_pair->key_pair.priv_d = aws_byte_buf_from_array(
cc_key_pair->key_pair.key_buf.buffer + 1 + (s_key_coordinate_size * 2), s_key_coordinate_size);
cc_key_pair->key_pair.vtable = &s_key_pair_vtable;
cc_key_pair->key_pair.curve_name = curve_name;

Expand Down
24 changes: 12 additions & 12 deletions source/windows/bcrypt_ecc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,14 @@ static struct aws_ecc_key_pair *s_alloc_pair_and_init_buffers(
aws_byte_buf_append(&key_impl->key_pair.key_buf, &priv_key);
}

key_impl->key_pair.pub_x.buffer = key_impl->key_pair.key_buf.buffer + sizeof(key_blob);
key_impl->key_pair.pub_x.len = key_impl->key_pair.pub_x.capacity = s_key_coordinate_size;
key_impl->key_pair.pub_x =
aws_byte_buf_from_array(key_impl->key_pair.key_buf.buffer + sizeof(key_blob), s_key_coordinate_size);

key_impl->key_pair.pub_y.buffer = key_impl->key_pair.pub_x.buffer + s_key_coordinate_size;
key_impl->key_pair.pub_y.len = key_impl->key_pair.pub_y.capacity = s_key_coordinate_size;
key_impl->key_pair.pub_y =
aws_byte_buf_from_array(key_impl->key_pair.pub_x.buffer + s_key_coordinate_size, s_key_coordinate_size);

key_impl->key_pair.priv_d.buffer = key_impl->key_pair.pub_y.buffer + s_key_coordinate_size;
key_impl->key_pair.priv_d.len = key_impl->key_pair.priv_d.capacity = s_key_coordinate_size;
key_impl->key_pair.priv_d =
aws_byte_buf_from_array(key_impl->key_pair.pub_y.buffer + s_key_coordinate_size, s_key_coordinate_size);

BCRYPT_ALG_HANDLE alg_handle = s_key_alg_handle_from_curve_name(curve_name);
NTSTATUS status = BCryptImportKeyPair(
Expand Down Expand Up @@ -434,14 +434,14 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_generate_random(

aws_byte_buf_secure_zero(&key_impl->key_pair.key_buf);

key_impl->key_pair.pub_x.buffer = key_impl->key_pair.key_buf.buffer + sizeof(BCRYPT_ECCKEY_BLOB);
key_impl->key_pair.pub_x.len = key_impl->key_pair.pub_x.capacity = key_coordinate_size;
key_impl->key_pair.pub_x =
aws_byte_buf_from_array(key_impl->key_pair.key_buf.buffer + sizeof(BCRYPT_ECCKEY_BLOB), key_coordinate_size);

key_impl->key_pair.pub_y.buffer = key_impl->key_pair.pub_x.buffer + key_coordinate_size;
key_impl->key_pair.pub_y.len = key_impl->key_pair.pub_y.capacity = key_coordinate_size;
key_impl->key_pair.pub_y =
aws_byte_buf_from_array(key_impl->key_pair.pub_x.buffer + key_coordinate_size, key_coordinate_size);

key_impl->key_pair.priv_d.buffer = key_impl->key_pair.pub_y.buffer + key_coordinate_size;
key_impl->key_pair.priv_d.len = key_impl->key_pair.priv_d.capacity = key_coordinate_size;
key_impl->key_pair.priv_d =
aws_byte_buf_from_array(key_impl->key_pair.pub_y.buffer + key_coordinate_size, key_coordinate_size);

if (s_derive_public_key(&key_impl->key_pair)) {
goto error;
Expand Down
28 changes: 14 additions & 14 deletions tests/test_case_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,31 @@ static inline int s_verify_hmac_test_case(
aws_cal_library_init(allocator);

/* test all possible segmentation lengths from 1 byte at a time to the entire
* input. */
for (size_t i = 1; i < input->len; ++i) {
* input. Using a do-while so that we still do 1 pass on 0-length input */
size_t advance_i = 1;
do {
uint8_t output[128] = {0};
struct aws_byte_buf output_buf = aws_byte_buf_from_array(output, expected->len);
output_buf.len = 0;
struct aws_byte_buf output_buf = aws_byte_buf_from_empty_array(output, AWS_ARRAY_SIZE(output));

struct aws_hmac *hmac = new_fn(allocator, secret);
ASSERT_NOT_NULL(hmac);

struct aws_byte_cursor input_cpy = *input;

while (input_cpy.len) {
size_t max_advance = input_cpy.len > i ? i : input_cpy.len;
size_t max_advance = aws_min_size(input_cpy.len, advance_i);
struct aws_byte_cursor segment = aws_byte_cursor_from_array(input_cpy.ptr, max_advance);
ASSERT_SUCCESS(aws_hmac_update(hmac, &segment));
aws_byte_cursor_advance(&input_cpy, max_advance);
}

size_t truncation_size = hmac->digest_size - expected->len;
size_t truncation_size = expected->len;

ASSERT_SUCCESS(aws_hmac_finalize(hmac, &output_buf, truncation_size));
ASSERT_BIN_ARRAYS_EQUALS(expected->ptr, expected->len, output_buf.buffer, output_buf.len);

aws_hmac_destroy(hmac);
}
} while (++advance_i <= input->len);

aws_cal_library_clean_up();

Expand All @@ -56,31 +56,31 @@ static inline int s_verify_hash_test_case(
aws_cal_library_init(allocator);

/* test all possible segmentation lengths from 1 byte at a time to the entire
* input. */
for (size_t i = 1; i < input->len; ++i) {
* input. Using a do-while so that we still do 1 pass on 0-length input */
size_t advance_i = 1;
do {
uint8_t output[128] = {0};
struct aws_byte_buf output_buf = aws_byte_buf_from_array(output, expected->len);
output_buf.len = 0;
struct aws_byte_buf output_buf = aws_byte_buf_from_empty_array(output, AWS_ARRAY_SIZE(output));

struct aws_hash *hash = new_fn(allocator);
ASSERT_NOT_NULL(hash);

struct aws_byte_cursor input_cpy = *input;

while (input_cpy.len) {
size_t max_advance = input_cpy.len > i ? i : input_cpy.len;
size_t max_advance = aws_min_size(input_cpy.len, advance_i);
struct aws_byte_cursor segment = aws_byte_cursor_from_array(input_cpy.ptr, max_advance);
ASSERT_SUCCESS(aws_hash_update(hash, &segment));
aws_byte_cursor_advance(&input_cpy, max_advance);
}

size_t truncation_size = hash->digest_size - expected->len;
size_t truncation_size = expected->len;

ASSERT_SUCCESS(aws_hash_finalize(hash, &output_buf, truncation_size));
ASSERT_BIN_ARRAYS_EQUALS(expected->ptr, expected->len, output_buf.buffer, output_buf.len);

aws_hash_destroy(hash);
}
} while (++advance_i <= input->len);

aws_cal_library_clean_up();

Expand Down

0 comments on commit 1458c70

Please sign in to comment.