From f424086f4b596e504f695e762802002a861e4a81 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Fri, 10 Oct 2025 15:56:11 -0700 Subject: [PATCH 01/28] cleanup ec loading --- include/aws/cal/private/der.h | 6 + include/aws/cal/private/ecc.h | 10 +- source/der.c | 4 + source/ecc.c | 377 ++++++++++++++++++++++++++++------ tests/ecc_test.c | 4 +- 5 files changed, 339 insertions(+), 62 deletions(-) diff --git a/include/aws/cal/private/der.h b/include/aws/cal/private/der.h index cbec136b..6c5ad7be 100644 --- a/include/aws/cal/private/der.h +++ b/include/aws/cal/private/der.h @@ -177,6 +177,12 @@ AWS_CAL_API void aws_der_decoder_destroy(struct aws_der_decoder *decoder); */ AWS_CAL_API bool aws_der_decoder_next(struct aws_der_decoder *decoder); +/** + * Resets der decoder to the start. + * @param decoder The decoder to reset + */ +AWS_CAL_API void aws_der_decoder_reset(struct aws_der_decoder *decoder); + /** * The type of the current TLV * @param decoder The decoder to inspect diff --git a/include/aws/cal/private/ecc.h b/include/aws/cal/private/ecc.h index ec349251..9d489088 100644 --- a/include/aws/cal/private/ecc.h +++ b/include/aws/cal/private/ecc.h @@ -13,10 +13,16 @@ struct aws_der_decoder; AWS_EXTERN_C_BEGIN +/* + * Helper to load keypair from various ASN1 format. + * Note: there are several formats in the wild: Sec1 and PKCS8 for private key and X509 for public key. + * This function attempts to automatically recognize the format and load from it. + * Depending on data available in the asn, either private or public key might remain uninitialized. + */ AWS_CAL_API int aws_der_decoder_load_ecc_key_pair( struct aws_der_decoder *decoder, - struct aws_byte_cursor *out_public_x_coor, - struct aws_byte_cursor *out_public_y_coor, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name); diff --git a/source/der.c b/source/der.c index f900a29f..89db8412 100644 --- a/source/der.c +++ b/source/der.c @@ -461,6 +461,10 @@ bool aws_der_decoder_next(struct aws_der_decoder *decoder) { return (++decoder->tlv_idx < (int)decoder->tlvs.length); } +void aws_der_decoder_reset(struct aws_der_decoder *decoder) { + decoder->tlv_idx = 0; +} + static struct der_tlv s_decoder_tlv(struct aws_der_decoder *decoder) { AWS_FATAL_ASSERT(decoder->tlv_idx < (int)decoder->tlvs.length); struct der_tlv tlv = {0}; diff --git a/source/ecc.c b/source/ecc.c index 07bc22e5..53e24882 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -189,97 +189,358 @@ size_t aws_ecc_key_coordinate_byte_size_from_curve_name(enum aws_ecc_curve_name } } -int aws_der_decoder_load_ecc_key_pair( +static void s_parse_public_key( + struct aws_byte_cursor public_key, + size_t key_coordinate_size, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord) { + + aws_byte_cursor_advance(&public_key, 1); + *out_public_x_coord = public_key; + out_public_x_coord->len = key_coordinate_size; + out_public_y_coord->ptr = public_key.ptr + key_coordinate_size; + out_public_y_coord->len = key_coordinate_size; +} + +/* + * ECPrivateKey ::= SEQUENCE { + * version INTEGER { ecPrivkeyVer1(1) }, + * privateKey OCTET STRING, + * parameters [0] ECParameters {{ NamedCurve }} OPTIONAL, + * publicKey [1] BIT STRING OPTIONAL + * } + */ +static int s_der_decoder_load_ecc_private_key_pair_from_sec1( struct aws_der_decoder *decoder, - struct aws_byte_cursor *out_public_x_coor, - struct aws_byte_cursor *out_public_y_coor, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - - AWS_ZERO_STRUCT(*out_public_x_coor); - AWS_ZERO_STRUCT(*out_public_y_coor); + AWS_PRECONDITION(decoder); + AWS_PRECONDITION(out_public_x_coord); + AWS_PRECONDITION(out_public_y_coord); + AWS_PRECONDITION(out_private_d); + AWS_PRECONDITION(out_curve_name); + + AWS_ZERO_STRUCT(*out_public_x_coord); + AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); - /* we could have private key or a public key, or a full pair. */ - struct aws_byte_cursor pair_part_1; - AWS_ZERO_STRUCT(pair_part_1); - struct aws_byte_cursor pair_part_2; - AWS_ZERO_STRUCT(pair_part_2); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } - bool curve_name_recognized = false; + struct aws_byte_cursor version_cur; + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } - /* work with this pointer and move it to the next after using it. We need - * to know which curve we're dealing with before we can figure out which is which. */ - struct aws_byte_cursor *current_part = &pair_part_1; + if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + } - while (aws_der_decoder_next(decoder)) { - enum aws_der_type type = aws_der_decoder_tlv_type(decoder); + struct aws_byte_cursor private_key_cur; + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } - if (type == AWS_DER_OBJECT_IDENTIFIER) { - struct aws_byte_cursor oid; - AWS_ZERO_STRUCT(oid); - aws_der_decoder_tlv_blob(decoder, &oid); - /* There can be other OID's so just look for one that is the curve. */ - if (!aws_ecc_curve_name_from_oid(&oid, out_curve_name)) { - curve_name_recognized = true; - } - continue; - } + struct aws_byte_cursor oid; + AWS_ZERO_STRUCT(oid); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || + aws_der_decoder_tlv_blob(decoder, &oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + enum aws_ecc_curve_name curve_name; + if (!aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } - /* you'd think we'd get some type hints on which key this is, but it's not consistent - * as far as I can tell. */ - if (type == AWS_DER_BIT_STRING || type == AWS_DER_OCTET_STRING) { - aws_der_decoder_tlv_string(decoder, current_part); - current_part = &pair_part_2; + struct aws_byte_cursor public_key_cur; + if (aws_der_decoder_next(decoder)) { + if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } } - if (!curve_name_recognized) { + size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); + size_t public_key_blob_size = key_coordinate_size * 2 + 1; + + if (private_key_cur.len != key_coordinate_size || + (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + *out_private_d = private_key_cur; + + if (public_key_cur.len > 0) { + s_parse_public_key(public_key_cur, key_coordinate_size, out_public_x_coord, out_public_y_coord); + } + + *out_curve_name = curve_name; + + return AWS_OP_SUCCESS; +} + +static uint8_t s_ec_private_key_oid[] = { + 0x2A, + 0x86, + 0x48, + 0xCE, + 0x3D, + 0x02, + 0x01, +}; +STATIC_INIT_BYTE_CURSOR(s_ec_private_key_oid, ec_private_key_oid_cursor) + +static uint8_t s_ec_public_key_oid[] = { + 0x2A, + 0x86, + 0x48, + 0xCE, + 0x3D, + 0x02, + 0x01, +}; +STATIC_INIT_BYTE_CURSOR(s_ec_public_key_oid, ec_public_key_oid_cursor) + +/* + * PrivateKeyInfo ::= SEQUENCE { + * version Integer, + * privateKeyAlgorithm PrivateKeyAlgorithmIdentifier, + * privateKey PrivateKey + * } + * PrivateKeyAlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters ANY DEFINED BY algorithm OPTIONAL + * } + * ECPrivateKey ::= SEQUENCE { + * version INTEGER { ecPrivkeyVer1(1) }, + * privateKey OCTET STRING, + * parameters [0] ECParameters {{ NamedCurve }} OPTIONAL, + * publicKey [1] BIT STRING OPTIONAL + * } + */ +static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( + struct aws_der_decoder *decoder, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, + struct aws_byte_cursor *out_private_d, + enum aws_ecc_curve_name *out_curve_name) { + AWS_PRECONDITION(decoder); + AWS_PRECONDITION(out_public_x_coord); + AWS_PRECONDITION(out_public_y_coord); + AWS_PRECONDITION(out_private_d); + AWS_PRECONDITION(out_curve_name); + + AWS_ZERO_STRUCT(*out_public_x_coord); + AWS_ZERO_STRUCT(*out_public_y_coord); + AWS_ZERO_STRUCT(*out_private_d); + + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + /* version */ + struct aws_byte_cursor version_cur; + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + } + + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + struct aws_byte_cursor algo_oid; + AWS_ZERO_STRUCT(algo_oid); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || + aws_der_decoder_tlv_blob(decoder, &algo_oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + /* + * Why check both public and private? Cause in real world standards are mostly a suggestion. + * A lot of private keys in the wild use public also oid and its defacto standard for a lot of libs. + */ + if (!aws_byte_cursor_eq(&algo_oid, &s_ec_public_key_oid_cursor) && + !aws_byte_cursor_eq(&algo_oid, &s_ec_private_key_oid_cursor)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } + + struct aws_byte_cursor curve_oid; + AWS_ZERO_STRUCT(curve_oid); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || + aws_der_decoder_tlv_blob(decoder, &curve_oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + enum aws_ecc_curve_name curve_name; + if (!aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } - size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(*out_curve_name); + /* private key sequence */ + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } - struct aws_byte_cursor *private_key = NULL; - struct aws_byte_cursor *public_key = NULL; + struct aws_byte_cursor inner_version_cur; + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &inner_version_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } - size_t public_key_blob_size = key_coordinate_size * 2 + 1; + if (inner_version_cur.len != 1 || inner_version_cur.ptr[0] != 0) { + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + } - if (pair_part_1.ptr && pair_part_1.len) { - if (pair_part_1.len == key_coordinate_size) { - private_key = &pair_part_1; - } else if (pair_part_1.len == public_key_blob_size) { - public_key = &pair_part_1; - } + struct aws_byte_cursor private_key_cur; + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (pair_part_2.ptr && pair_part_2.len) { - if (pair_part_2.len == key_coordinate_size) { - private_key = &pair_part_2; - } else if (pair_part_2.len == public_key_blob_size) { - public_key = &pair_part_2; + struct aws_byte_cursor inner_oid; + AWS_ZERO_STRUCT(inner_oid); + struct aws_byte_cursor public_key_cur; + AWS_ZERO_STRUCT(public_key_cur); + if (aws_der_decoder_next(decoder)) { + if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { + if (aws_der_decoder_tlv_blob(decoder, &inner_oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + enum aws_ecc_curve_name inner_curve_name; + if (!aws_ecc_curve_name_from_oid(&inner_oid, &inner_curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } + if (inner_curve_name != curve_name) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + } else if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } } - if (!private_key && !public_key) { - return aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); + size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); + size_t public_key_blob_size = key_coordinate_size * 2 + 1; + + if (private_key_cur.len != key_coordinate_size || + (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + *out_private_d = private_key_cur; + + if (public_key_cur.len > 0) { + s_parse_public_key(public_key_cur, key_coordinate_size, out_public_x_coord, out_public_y_coord); + } + + *out_curve_name = curve_name; + + return AWS_OP_SUCCESS; +} + +/* + * SubjectPublicKeyInfo ::= SEQUENCE { + * algorithm AlgorithmIdentifier, + * subjectPublicKey BIT STRING + * } + * AlgorithmIdentifier ::= SEQUENCE { + * algorithm OBJECT IDENTIFIER, + * parameters ANY DEFINED BY algorithm OPTIONAL + * } + */ +static int s_der_decoder_load_ecc_public_key_pair_from_asn1( + struct aws_der_decoder *decoder, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, + struct aws_byte_cursor *out_private_d, + enum aws_ecc_curve_name *out_curve_name) { + AWS_PRECONDITION(decoder); + AWS_PRECONDITION(out_public_x_coord); + AWS_PRECONDITION(out_public_y_coord); + AWS_PRECONDITION(out_private_d); + AWS_PRECONDITION(out_curve_name); + + AWS_ZERO_STRUCT(*out_public_x_coord); + AWS_ZERO_STRUCT(*out_public_y_coord); + AWS_ZERO_STRUCT(*out_private_d); + + /* sequence */ + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + /* algo identifier sequence */ + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + struct aws_byte_cursor algo_oid; + AWS_ZERO_STRUCT(algo_oid); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || + aws_der_decoder_tlv_blob(decoder, &algo_oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (private_key) { - *out_private_d = *private_key; + if (!aws_byte_cursor_eq(&algo_oid, &s_ec_public_key_oid_cursor)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } + + struct aws_byte_cursor curve_oid; + AWS_ZERO_STRUCT(curve_oid); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || + aws_der_decoder_tlv_blob(decoder, &curve_oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (public_key) { - aws_byte_cursor_advance(public_key, 1); - *out_public_x_coor = *public_key; - out_public_x_coor->len = key_coordinate_size; - out_public_y_coor->ptr = public_key->ptr + key_coordinate_size; - out_public_y_coor->len = key_coordinate_size; + enum aws_ecc_curve_name curve_name; + if (!aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } + struct aws_byte_cursor public_key_cur; + if (!aws_der_decoder_next(decoder) && aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); + if (public_key_cur.len > 0) { + s_parse_public_key(public_key_cur, key_coordinate_size, out_public_x_coord, out_public_y_coord); + } + + *out_curve_name = curve_name; + return AWS_OP_SUCCESS; } +int aws_der_decoder_load_ecc_key_pair( + struct aws_der_decoder *decoder, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, + struct aws_byte_cursor *out_private_d, + enum aws_ecc_curve_name *out_curve_name) { + + if (s_der_decoder_load_ecc_public_key_pair_from_asn1( + decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + return AWS_OP_SUCCESS; + } + + if (s_der_decoder_load_ecc_private_key_pair_from_pkcs8( + decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + return AWS_OP_SUCCESS; + } + + if (s_der_decoder_load_ecc_private_key_pair_from_sec1( + decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + return AWS_OP_SUCCESS; + } + + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); +} + void aws_ecc_key_pair_acquire(struct aws_ecc_key_pair *key_pair) { aws_atomic_fetch_add(&key_pair->ref_count, 1); } diff --git a/tests/ecc_test.c b/tests/ecc_test.c index a724d730..148505e8 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -654,9 +654,9 @@ AWS_TEST_CASE(ecdsa_test_import_asn1_key_pair_invalid_fails, s_ecdsa_test_import /* this test exists because we have to manually handle signature encoding/decoding on windows. this takes an encoded signature and makes sure we decode and verify it properly. How do we know we encode properly b.t.w? Well we have tests that verify signatures we generated, so we already know - that anything we signed can be decoded. What we don't have proven is that we're not just symetrically + that anything we signed can be decoded. What we don't have proven is that we're not just symmetrically wrong. So, let's take the format we know signatures must be in ASN.1 DER encoded, and make sure we can - verify it. Since we KNOW the signing and verifying code is symetric, verifying the verification side should + verify it. Since we KNOW the signing and verifying code is symmetric, verifying the verification side should prove our encoding/decoding code is correct to the spec. */ static int s_ecdsa_test_signature_format_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From d9ad56ae4db0acd8ff7ce32c007f803fec1c4a3e Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 01:29:57 -0700 Subject: [PATCH 02/28] fix --- source/ecc.c | 25 ++++++++++++++----------- tests/ecc_test.c | 4 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 53e24882..6c6247b0 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -226,7 +226,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -235,7 +235,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + if (version_cur.len != 1 || version_cur.ptr[0] != 1) { return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } @@ -246,18 +246,18 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( struct aws_byte_cursor oid; AWS_ZERO_STRUCT(oid); - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || - aws_der_decoder_tlv_blob(decoder, &oid)) { + if (!aws_der_decoder_next(decoder) || !aws_der_decoder_next(decoder) /* skip context specific structure */ + || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &oid)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name curve_name; - if (!aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } struct aws_byte_cursor public_key_cur; - if (aws_der_decoder_next(decoder)) { + if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -347,7 +347,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + if (version_cur.len != 1 || version_cur.ptr[0] != 1) { return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } @@ -379,7 +379,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( } enum aws_ecc_curve_name curve_name; - if (!aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { + if (aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } @@ -412,7 +412,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name inner_curve_name; - if (!aws_ecc_curve_name_from_oid(&inner_oid, &inner_curve_name)) { + if (aws_ecc_curve_name_from_oid(&inner_oid, &inner_curve_name)) { return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } if (inner_curve_name != curve_name) { @@ -497,12 +497,13 @@ static int s_der_decoder_load_ecc_public_key_pair_from_asn1( } enum aws_ecc_curve_name curve_name; - if (!aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { + if (aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } struct aws_byte_cursor public_key_cur; - if (!aws_der_decoder_next(decoder) && aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + AWS_ZERO_STRUCT(public_key_cur); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -528,11 +529,13 @@ int aws_der_decoder_load_ecc_key_pair( return AWS_OP_SUCCESS; } + aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_pkcs8( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { return AWS_OP_SUCCESS; } + aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_sec1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { return AWS_OP_SUCCESS; diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 148505e8..10e09288 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -636,14 +636,14 @@ static int s_ecdsa_test_import_asn1_key_pair_invalid_fails_fn(struct aws_allocat struct aws_ecc_key_pair *signing_key = aws_ecc_key_pair_new_from_asn1(allocator, &bad_full_key_asn1_1); ASSERT_NULL(signing_key); - ASSERT_INT_EQUALS(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER, aws_last_error()); + ASSERT_INT_EQUALS(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT, aws_last_error()); struct aws_byte_cursor bad_full_key_asn1_2 = aws_byte_cursor_from_array(bad_asn1_encoded_full_key_raw_2, sizeof(bad_asn1_encoded_full_key_raw_2)); signing_key = aws_ecc_key_pair_new_from_asn1(allocator, &bad_full_key_asn1_2); ASSERT_NULL(signing_key); - ASSERT_INT_EQUALS(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER, aws_last_error()); + ASSERT_INT_EQUALS(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT, aws_last_error()); aws_cal_library_clean_up(); return AWS_OP_SUCCESS; From 46b8cf80b53bdfd0a338d567d543a7263d5088cc Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:03:23 -0700 Subject: [PATCH 03/28] test and win build fix --- source/ecc.c | 15 +++++++++++++-- tests/CMakeLists.txt | 1 + tests/ecc_test.c | 29 ++++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 6c6247b0..9cf8a8aa 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -240,6 +240,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } struct aws_byte_cursor private_key_cur; + AWS_ZERO_STRUCT(private_key_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -257,7 +258,8 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } struct aws_byte_cursor public_key_cur; - if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ + AWS_ZERO_STRUCT(private_key_cur); + if (aws_der_decoder_next(decoder) || aws_der_decoder_next(decoder)) { /* skip context wrapper */ if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -508,13 +510,17 @@ static int s_der_decoder_load_ecc_public_key_pair_from_asn1( } size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); + if ((public_key_cur.len != 0 && public_key_cur.len != (key_coordinate_size * 2 + 1))) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + if (public_key_cur.len > 0) { s_parse_public_key(public_key_cur, key_coordinate_size, out_public_x_coord, out_public_y_coord); } *out_curve_name = curve_name; - return AWS_OP_SUCCESS; + return AWS_OP_SUCCESS; } int aws_der_decoder_load_ecc_key_pair( @@ -524,6 +530,11 @@ int aws_der_decoder_load_ecc_key_pair( struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { + /** + * Since this is a generic api to parse from ans1, we can encounter several key structures. + * Just go through them one by one and see if any match the expected type. + * Note: We should at least get some id in the structure or unique enough layout so ordering does not matter. + */ if (s_der_decoder_load_ecc_public_key_pair_from_asn1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { return AWS_OP_SUCCESS; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 134743f1..e34d38ee 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -78,6 +78,7 @@ add_test_case(ecdsa_p384_test_key_gen) add_test_case(ecdsa_p256_test_key_gen_export) add_test_case(ecdsa_p384_test_key_gen_export) add_test_case(ecdsa_p256_test_import_asn1_key_pair) +add_test_case(ecdsa_p256_test_import_asn1_sec1_key_pair) add_test_case(ecdsa_p384_test_import_asn1_key_pair) add_test_case(ecdsa_test_import_asn1_key_pair_public_only) add_test_case(ecdsa_test_import_asn1_key_pair_invalid_fails) diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 10e09288..a634015c 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -506,9 +506,36 @@ static int s_ecdsa_p256_test_import_asn1_key_pair_fn(struct aws_allocator *alloc return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); } - AWS_TEST_CASE(ecdsa_p256_test_import_asn1_key_pair, s_ecdsa_p256_test_import_asn1_key_pair_fn) +static int s_ecdsa_p256_test_import_asn1_sec_key_pair_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + uint8_t asn1_encoded_key_raw[] = { +0x30, 0x81, 0x87, 0x02, 0x01, 0x01, 0x04, 0x20, +0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, +0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, +0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, +0x01, 0x23, 0x45, 0x67, 0xA0, 0x0A, 0x06, 0x08, +0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, +0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, +0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD + }; + + struct aws_byte_cursor asn1_encoded_key = + aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); + + return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); +} +AWS_TEST_CASE(ecdsa_p256_test_import_asn1_sec1_key_pair, s_ecdsa_p256_test_import_asn1_sec_key_pair_fn) + static int s_ecdsa_p384_test_import_asn1_key_pair_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From 932907296401a39b115629cabbdd6dd740417dea Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:05:19 -0700 Subject: [PATCH 04/28] lint --- source/ecc.c | 2 +- tests/ecc_test.c | 23 +++++++---------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 9cf8a8aa..2f610152 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -520,7 +520,7 @@ static int s_der_decoder_load_ecc_public_key_pair_from_asn1( *out_curve_name = curve_name; - return AWS_OP_SUCCESS; + return AWS_OP_SUCCESS; } int aws_der_decoder_load_ecc_key_pair( diff --git a/tests/ecc_test.c b/tests/ecc_test.c index a634015c..9025693b 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -512,22 +512,13 @@ static int s_ecdsa_p256_test_import_asn1_sec_key_pair_fn(struct aws_allocator *a (void)ctx; uint8_t asn1_encoded_key_raw[] = { -0x30, 0x81, 0x87, 0x02, 0x01, 0x01, 0x04, 0x20, -0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, -0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, -0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, -0x01, 0x23, 0x45, 0x67, 0xA0, 0x0A, 0x06, 0x08, -0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, -0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, -0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD - }; + 0x30, 0x81, 0x87, 0x02, 0x01, 0x01, 0x04, 0x20, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, + 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, + 0xA0, 0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, + 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, + 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, + 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, + 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD}; struct aws_byte_cursor asn1_encoded_key = aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); From 04de02cebb5f1acc5ab3ad7910afe408b15c3ea7 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:12:19 -0700 Subject: [PATCH 05/28] test fix --- source/ecc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 2f610152..306730ce 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -258,8 +258,8 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } struct aws_byte_cursor public_key_cur; - AWS_ZERO_STRUCT(private_key_cur); - if (aws_der_decoder_next(decoder) || aws_der_decoder_next(decoder)) { /* skip context wrapper */ + AWS_ZERO_STRUCT(public_key_cur); + if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } From e1022e4d2a64aa8789f56c0d87d456b627603e0a Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:33:25 -0700 Subject: [PATCH 06/28] fix --- source/darwin/securityframework_ecc.c | 4 ++++ source/ecc.c | 17 +++++++++++++++++ tests/CMakeLists.txt | 1 - tests/ecc_test.c | 21 ++------------------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/source/darwin/securityframework_ecc.c b/source/darwin/securityframework_ecc.c index 64648444..ef7ed767 100644 --- a/source/darwin/securityframework_ecc.c +++ b/source/darwin/securityframework_ecc.c @@ -549,10 +549,14 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( CFMutableDictionaryRef key_attributes = NULL; CFDataRef key_data = NULL; + AWS_LOGF_DEBUG(0, "ccc"); + if (!decoder) { return NULL; } + AWS_LOGF_DEBUG(0, "ddd"); + /* we could have private key or a public key, or a full pair. */ struct aws_byte_cursor pub_x; AWS_ZERO_STRUCT(pub_x); diff --git a/source/ecc.c b/source/ecc.c index 306730ce..ed4e8ad3 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -226,22 +226,28 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); + AWS_LOGF_DEBUG(0, "foo0"); + if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + AWS_LOGF_DEBUG(0, "foo0"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_byte_cursor version_cur; if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { + AWS_LOGF_DEBUG(0, "foo1"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } if (version_cur.len != 1 || version_cur.ptr[0] != 1) { + AWS_LOGF_DEBUG(0, "foo2"); return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } struct aws_byte_cursor private_key_cur; AWS_ZERO_STRUCT(private_key_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { + AWS_LOGF_DEBUG(0, "foo3"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -249,11 +255,13 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(oid); if (!aws_der_decoder_next(decoder) || !aws_der_decoder_next(decoder) /* skip context specific structure */ || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &oid)) { + AWS_LOGF_DEBUG(0, "foo4"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name curve_name; if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + AWS_LOGF_DEBUG(0, "foo5"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } @@ -261,6 +269,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(public_key_cur); if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + AWS_LOGF_DEBUG(0, "foo6"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } } @@ -270,6 +279,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { + AWS_LOGF_DEBUG(0, "foo7"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -280,6 +290,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } *out_curve_name = curve_name; + AWS_LOGF_DEBUG(0, "foo8"); return AWS_OP_SUCCESS; } @@ -530,6 +541,8 @@ int aws_der_decoder_load_ecc_key_pair( struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { + AWS_LOGF_DEBUG(0, "aaa"); + /** * Since this is a generic api to parse from ans1, we can encounter several key structures. * Just go through them one by one and see if any match the expected type. @@ -537,21 +550,25 @@ int aws_der_decoder_load_ecc_key_pair( */ if (s_der_decoder_load_ecc_public_key_pair_from_asn1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + AWS_LOGF_DEBUG(0, "1"); return AWS_OP_SUCCESS; } aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_pkcs8( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + AWS_LOGF_DEBUG(0, "2"); return AWS_OP_SUCCESS; } aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_sec1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { + AWS_LOGF_DEBUG(0, "3"); return AWS_OP_SUCCESS; } + AWS_LOGF_DEBUG(0, "4"); return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e34d38ee..134743f1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -78,7 +78,6 @@ add_test_case(ecdsa_p384_test_key_gen) add_test_case(ecdsa_p256_test_key_gen_export) add_test_case(ecdsa_p384_test_key_gen_export) add_test_case(ecdsa_p256_test_import_asn1_key_pair) -add_test_case(ecdsa_p256_test_import_asn1_sec1_key_pair) add_test_case(ecdsa_p384_test_import_asn1_key_pair) add_test_case(ecdsa_test_import_asn1_key_pair_public_only) add_test_case(ecdsa_test_import_asn1_key_pair_invalid_fails) diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 9025693b..5c7827bd 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -447,6 +447,8 @@ static int s_ecdsa_test_import_asn1_key_pair( aws_cal_library_test_init(allocator); + AWS_LOGF_DEBUG(0, "bbb"); + struct aws_ecc_key_pair *imported_key = aws_ecc_key_pair_new_from_asn1(allocator, &asn1_cur); ASSERT_NOT_NULL(imported_key); @@ -508,25 +510,6 @@ static int s_ecdsa_p256_test_import_asn1_key_pair_fn(struct aws_allocator *alloc } AWS_TEST_CASE(ecdsa_p256_test_import_asn1_key_pair, s_ecdsa_p256_test_import_asn1_key_pair_fn) -static int s_ecdsa_p256_test_import_asn1_sec_key_pair_fn(struct aws_allocator *allocator, void *ctx) { - (void)ctx; - - uint8_t asn1_encoded_key_raw[] = { - 0x30, 0x81, 0x87, 0x02, 0x01, 0x01, 0x04, 0x20, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, - 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, - 0xA0, 0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, - 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, - 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, - 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, - 0x67, 0x89, 0xAB, 0xCD, 0xEF, 0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD}; - - struct aws_byte_cursor asn1_encoded_key = - aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); - - return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); -} -AWS_TEST_CASE(ecdsa_p256_test_import_asn1_sec1_key_pair, s_ecdsa_p256_test_import_asn1_sec_key_pair_fn) - static int s_ecdsa_p384_test_import_asn1_key_pair_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From dfb363ad3c0b9deb873b9c9f2b4d2e1a24838730 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:36:24 -0700 Subject: [PATCH 07/28] Add codecov --- .github/workflows/codecov.yml | 32 +++++++++++++++++++++++++++ source/darwin/securityframework_ecc.c | 4 ---- source/ecc.c | 19 +--------------- tests/ecc_test.c | 2 -- 4 files changed, 33 insertions(+), 24 deletions(-) create mode 100644 .github/workflows/codecov.yml diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml new file mode 100644 index 00000000..6ad094b8 --- /dev/null +++ b/.github/workflows/codecov.yml @@ -0,0 +1,32 @@ +name: Code coverage check + +on: + push: + +env: + BUILDER_VERSION: v0.9.74 + BUILDER_SOURCE: releases + BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net + PACKAGE_NAME: aws-c-s3 + RUN: ${{ github.run_id }}-${{ github.run_number }} + CRT_CI_ROLE: ${{ secrets.CRT_CI_ROLE_ARN }} + AWS_DEFAULT_REGION: us-east-1 + +permissions: + id-token: write # This is required for requesting the JWT + +jobs: + codecov-linux: + runs-on: ubuntu-24.04 + steps: + - uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ env.CRT_CI_ROLE }} + aws-region: ${{ env.AWS_DEFAULT_REGION }} + - name: Checkout Sources + uses: actions/checkout@v4 + - name: Build ${{ env.PACKAGE_NAME }} + consumers + run: | + python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')" + chmod a+x builder + ./builder build -p ${{ env.PACKAGE_NAME }} --compiler=gcc --cmake-extra=-DASSERT_LOCK_HELD=ON --coverage diff --git a/source/darwin/securityframework_ecc.c b/source/darwin/securityframework_ecc.c index ef7ed767..64648444 100644 --- a/source/darwin/securityframework_ecc.c +++ b/source/darwin/securityframework_ecc.c @@ -549,14 +549,10 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( CFMutableDictionaryRef key_attributes = NULL; CFDataRef key_data = NULL; - AWS_LOGF_DEBUG(0, "ccc"); - if (!decoder) { return NULL; } - AWS_LOGF_DEBUG(0, "ddd"); - /* we could have private key or a public key, or a full pair. */ struct aws_byte_cursor pub_x; AWS_ZERO_STRUCT(pub_x); diff --git a/source/ecc.c b/source/ecc.c index ed4e8ad3..280d61e8 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -226,28 +226,22 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); - AWS_LOGF_DEBUG(0, "foo0"); - if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { - AWS_LOGF_DEBUG(0, "foo0"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_byte_cursor version_cur; if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { - AWS_LOGF_DEBUG(0, "foo1"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } if (version_cur.len != 1 || version_cur.ptr[0] != 1) { - AWS_LOGF_DEBUG(0, "foo2"); return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } struct aws_byte_cursor private_key_cur; AWS_ZERO_STRUCT(private_key_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { - AWS_LOGF_DEBUG(0, "foo3"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -255,13 +249,11 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(oid); if (!aws_der_decoder_next(decoder) || !aws_der_decoder_next(decoder) /* skip context specific structure */ || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &oid)) { - AWS_LOGF_DEBUG(0, "foo4"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name curve_name; if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { - AWS_LOGF_DEBUG(0, "foo5"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } @@ -269,7 +261,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( AWS_ZERO_STRUCT(public_key_cur); if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { - AWS_LOGF_DEBUG(0, "foo6"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } } @@ -279,7 +270,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { - AWS_LOGF_DEBUG(0, "foo7"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -290,7 +280,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } *out_curve_name = curve_name; - AWS_LOGF_DEBUG(0, "foo8"); return AWS_OP_SUCCESS; } @@ -541,8 +530,6 @@ int aws_der_decoder_load_ecc_key_pair( struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - AWS_LOGF_DEBUG(0, "aaa"); - /** * Since this is a generic api to parse from ans1, we can encounter several key structures. * Just go through them one by one and see if any match the expected type. @@ -550,25 +537,21 @@ int aws_der_decoder_load_ecc_key_pair( */ if (s_der_decoder_load_ecc_public_key_pair_from_asn1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { - AWS_LOGF_DEBUG(0, "1"); return AWS_OP_SUCCESS; } aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_pkcs8( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { - AWS_LOGF_DEBUG(0, "2"); return AWS_OP_SUCCESS; } aws_der_decoder_reset(decoder); if (s_der_decoder_load_ecc_private_key_pair_from_sec1( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { - AWS_LOGF_DEBUG(0, "3"); return AWS_OP_SUCCESS; } - - AWS_LOGF_DEBUG(0, "4"); + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 5c7827bd..b9b6a8f6 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -447,8 +447,6 @@ static int s_ecdsa_test_import_asn1_key_pair( aws_cal_library_test_init(allocator); - AWS_LOGF_DEBUG(0, "bbb"); - struct aws_ecc_key_pair *imported_key = aws_ecc_key_pair_new_from_asn1(allocator, &asn1_cur); ASSERT_NOT_NULL(imported_key); From d55902cb7e10277db79f935ab5b04f6964cb7dba Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:40:16 -0700 Subject: [PATCH 08/28] wrong name --- .github/workflows/codecov.yml | 2 +- source/ecc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index 6ad094b8..2d6ce4b6 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -7,7 +7,7 @@ env: BUILDER_VERSION: v0.9.74 BUILDER_SOURCE: releases BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net - PACKAGE_NAME: aws-c-s3 + PACKAGE_NAME: aws-c-cal RUN: ${{ github.run_id }}-${{ github.run_number }} CRT_CI_ROLE: ${{ secrets.CRT_CI_ROLE_ARN }} AWS_DEFAULT_REGION: us-east-1 diff --git a/source/ecc.c b/source/ecc.c index 280d61e8..306730ce 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -551,7 +551,7 @@ int aws_der_decoder_load_ecc_key_pair( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { return AWS_OP_SUCCESS; } - + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } From 33df8921cc9386a2f08bf18f9c8ba654381804f8 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 11:48:27 -0700 Subject: [PATCH 09/28] docs --- source/ecc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/ecc.c b/source/ecc.c index 306730ce..d4a302ba 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -203,6 +203,7 @@ static void s_parse_public_key( } /* + * Load key from sec1 container. Aka "EC PRIVATE KEY" in pem * ECPrivateKey ::= SEQUENCE { * version INTEGER { ecPrivkeyVer1(1) }, * privateKey OCTET STRING, @@ -307,6 +308,7 @@ static uint8_t s_ec_public_key_oid[] = { STATIC_INIT_BYTE_CURSOR(s_ec_public_key_oid, ec_public_key_oid_cursor) /* + * Load key from PKCS8 container with the following format and "PRIVATE KEY" in pem * PrivateKeyInfo ::= SEQUENCE { * version Integer, * privateKeyAlgorithm PrivateKeyAlgorithmIdentifier, @@ -445,6 +447,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( } /* + * Load public key from x509 ec key structure, "EC PUBLIC KEY" or "PUBLIC KEY" in pem * SubjectPublicKeyInfo ::= SEQUENCE { * algorithm AlgorithmIdentifier, * subjectPublicKey BIT STRING From 0b6a6d36d56fc1c2b8752bac724fee6fefefb1b8 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Mon, 13 Oct 2025 12:56:49 -0700 Subject: [PATCH 10/28] add pkcs8 parsing test --- tests/CMakeLists.txt | 1 + tests/ecc_test.c | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 134743f1..368a7cc3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -78,6 +78,7 @@ add_test_case(ecdsa_p384_test_key_gen) add_test_case(ecdsa_p256_test_key_gen_export) add_test_case(ecdsa_p384_test_key_gen_export) add_test_case(ecdsa_p256_test_import_asn1_key_pair) +add_test_case(ecdsa_p256_test_import_asn1_pkcs8_key_pair) add_test_case(ecdsa_p384_test_import_asn1_key_pair) add_test_case(ecdsa_test_import_asn1_key_pair_public_only) add_test_case(ecdsa_test_import_asn1_key_pair_invalid_fails) diff --git a/tests/ecc_test.c b/tests/ecc_test.c index b9b6a8f6..c128cdd8 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -508,6 +508,26 @@ static int s_ecdsa_p256_test_import_asn1_key_pair_fn(struct aws_allocator *alloc } AWS_TEST_CASE(ecdsa_p256_test_import_asn1_key_pair, s_ecdsa_p256_test_import_asn1_key_pair_fn) +static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + uint8_t asn1_encoded_key_raw[] = { + 0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x78, 0xed, 0xed, 0xcf, 0x95, 0x9e, 0x42, 0x24, 0x37, 0xa4, 0x56, + 0xed, 0x08, 0x19, 0x3c, 0x53, 0x4b, 0x6f, 0xff, 0x40, 0x64, 0x48, 0x6a, 0x49, 0x86, 0x0c, 0xb7, 0x0a, 0xe5, + 0x2d, 0xbd, 0xd6, 0xa0, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0xa1, 0x44, 0x03, + 0x42, 0x00, 0x04, 0xbf, 0x61, 0x63, 0x46, 0x93, 0x2d, 0x00, 0x33, 0x19, 0xe3, 0x3a, 0x19, 0xc6, 0xc8, 0x55, + 0xf5, 0xc8, 0x44, 0x91, 0xe9, 0x9b, 0x83, 0x36, 0x67, 0x5d, 0x25, 0x0d, 0x7b, 0xe0, 0xc0, 0xf1, 0xd2, 0xaa, + 0x5c, 0xdf, 0xfb, 0xa9, 0x37, 0x19, 0x8d, 0x82, 0x47, 0x28, 0x88, 0xbe, 0x46, 0x7f, 0x3c, 0xcd, 0x41, 0xaa, + 0x08, 0x9a, 0x37, 0x0d, 0x61, 0x7f, 0x5f, 0xeb, 0x9f, 0x55, 0xf7, 0x54, 0xda, 0x0a, + }; + + struct aws_byte_cursor asn1_encoded_key = + aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); + + return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); +} +AWS_TEST_CASE(ecdsa_p256_test_import_asn1_pkcs8_key_pair, s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn) + static int s_ecdsa_p384_test_import_asn1_key_pair_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From 21deb54b53183ceee553966e026e39957d5e0b3c Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 01:30:01 -0700 Subject: [PATCH 11/28] address comments --- include/aws/cal/private/der.h | 8 +++ include/aws/cal/private/ecc.h | 2 +- source/der.c | 11 +++- source/ecc.c | 95 +++++++++++++++++++++-------------- tests/ecc_test.c | 16 +++--- 5 files changed, 85 insertions(+), 47 deletions(-) diff --git a/include/aws/cal/private/der.h b/include/aws/cal/private/der.h index 6c5ad7be..c531491d 100644 --- a/include/aws/cal/private/der.h +++ b/include/aws/cal/private/der.h @@ -164,6 +164,14 @@ AWS_CAL_API int aws_der_encoder_get_contents(struct aws_der_encoder *encoder, st */ AWS_CAL_API struct aws_der_decoder *aws_der_decoder_new(struct aws_allocator *allocator, struct aws_byte_cursor input); +/** + * Initializes new decoder from string at the current location. + * Useful for cases where asn1 structure is nested inside another one, ex. ec pkcs8. + * @param decoder Current decoder + * @return Initialized decoder, or NULL + */ +AWS_CAL_API struct aws_der_decoder *aws_der_decoder_nested_tlv_decoder(struct aws_der_decoder *decoder); + /** * Cleans up a DER encoder * @param decoder The encoder to clean up diff --git a/include/aws/cal/private/ecc.h b/include/aws/cal/private/ecc.h index 9d489088..b1608349 100644 --- a/include/aws/cal/private/ecc.h +++ b/include/aws/cal/private/ecc.h @@ -17,7 +17,7 @@ AWS_EXTERN_C_BEGIN * Helper to load keypair from various ASN1 format. * Note: there are several formats in the wild: Sec1 and PKCS8 for private key and X509 for public key. * This function attempts to automatically recognize the format and load from it. - * Depending on data available in the asn, either private or public key might remain uninitialized. + * Depending on data available in the asn, either private or public key might be empty (zeroed out). */ AWS_CAL_API int aws_der_decoder_load_ecc_key_pair( struct aws_der_decoder *decoder, diff --git a/source/der.c b/source/der.c index 882afb91..195c7eab 100644 --- a/source/der.c +++ b/source/der.c @@ -400,6 +400,15 @@ struct aws_der_decoder *aws_der_decoder_new(struct aws_allocator *allocator, str return NULL; } +struct aws_der_decoder *aws_der_decoder_nested_tlv_decoder(struct aws_der_decoder *decoder) { + struct aws_byte_cursor cursor; + AWS_ZERO_STRUCT(cursor); + if (aws_der_decoder_tlv_string(decoder, &cursor)) { + return NULL; + } + return aws_der_decoder_new(decoder->allocator, cursor); +} + void aws_der_decoder_destroy(struct aws_der_decoder *decoder) { if (!decoder) { return; @@ -468,7 +477,7 @@ bool aws_der_decoder_next(struct aws_der_decoder *decoder) { } void aws_der_decoder_reset(struct aws_der_decoder *decoder) { - decoder->tlv_idx = 0; + decoder->tlv_idx = -1; } static struct der_tlv s_decoder_tlv(struct aws_der_decoder *decoder) { diff --git a/source/ecc.c b/source/ecc.c index d4a302ba..c6cc1978 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -196,10 +196,8 @@ static void s_parse_public_key( struct aws_byte_cursor *out_public_y_coord) { aws_byte_cursor_advance(&public_key, 1); - *out_public_x_coord = public_key; - out_public_x_coord->len = key_coordinate_size; - out_public_y_coord->ptr = public_key.ptr + key_coordinate_size; - out_public_y_coord->len = key_coordinate_size; + *out_public_x_coord = aws_byte_cursor_advance(&public_key, key_coordinate_size); + *out_public_y_coord = public_key; } /* @@ -217,21 +215,17 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( struct aws_byte_cursor *out_public_y_coord, struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - AWS_PRECONDITION(decoder); - AWS_PRECONDITION(out_public_x_coord); - AWS_PRECONDITION(out_public_y_coord); - AWS_PRECONDITION(out_private_d); - AWS_PRECONDITION(out_curve_name); AWS_ZERO_STRUCT(*out_public_x_coord); AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); - if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_byte_cursor version_cur; + AWS_ZERO_STRUCT(version_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -331,11 +325,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( struct aws_byte_cursor *out_public_y_coord, struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - AWS_PRECONDITION(decoder); - AWS_PRECONDITION(out_public_x_coord); - AWS_PRECONDITION(out_public_y_coord); - AWS_PRECONDITION(out_private_d); - AWS_PRECONDITION(out_curve_name); AWS_ZERO_STRUCT(*out_public_x_coord); AWS_ZERO_STRUCT(*out_public_y_coord); @@ -347,11 +336,12 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( /* version */ struct aws_byte_cursor version_cur; + AWS_ZERO_STRUCT(version_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (version_cur.len != 1 || version_cur.ptr[0] != 1) { + if (version_cur.len != 1 || version_cur.ptr[0] != 0) { return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } @@ -387,43 +377,64 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } - /* private key sequence */ - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + /* private key string */ + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OCTET_STRING) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - struct aws_byte_cursor inner_version_cur; - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &inner_version_cur)) { + struct aws_der_decoder *nested_decoder = aws_der_decoder_nested_tlv_decoder(decoder); + + if (!nested_decoder) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (inner_version_cur.len != 1 || inner_version_cur.ptr[0] != 0) { - return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_type(nested_decoder) != AWS_DER_SEQUENCE) { + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; + } + + struct aws_byte_cursor inner_version_cur; + AWS_ZERO_STRUCT(inner_version_cur); + if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_unsigned_integer(nested_decoder, &inner_version_cur)) { + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; + } + + + if (inner_version_cur.len != 1 || inner_version_cur.ptr[0] != 1) { + aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + goto on_nested_failure; } struct aws_byte_cursor private_key_cur; - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_string(nested_decoder, &private_key_cur)) { + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; } struct aws_byte_cursor inner_oid; AWS_ZERO_STRUCT(inner_oid); struct aws_byte_cursor public_key_cur; AWS_ZERO_STRUCT(public_key_cur); - if (aws_der_decoder_next(decoder)) { - if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { - if (aws_der_decoder_tlv_blob(decoder, &inner_oid)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + if (aws_der_decoder_next(nested_decoder)) { + if (aws_der_decoder_tlv_type(nested_decoder) == AWS_DER_OBJECT_IDENTIFIER) { + if (aws_der_decoder_tlv_blob(nested_decoder, &inner_oid)) { + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; } enum aws_ecc_curve_name inner_curve_name; if (aws_ecc_curve_name_from_oid(&inner_oid, &inner_curve_name)) { - return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + goto on_nested_failure; } if (inner_curve_name != curve_name) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; } - } else if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } else if (!aws_der_decoder_next(nested_decoder) /* skip context specific */ || + aws_der_decoder_tlv_string(nested_decoder, &public_key_cur)) { + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; } } @@ -432,7 +443,8 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + goto on_nested_failure; } *out_private_d = private_key_cur; @@ -442,8 +454,13 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( } *out_curve_name = curve_name; + aws_der_decoder_destroy(nested_decoder); return AWS_OP_SUCCESS; + +on_nested_failure: + aws_der_decoder_destroy(nested_decoder); + return AWS_OP_ERR; } /* @@ -463,11 +480,6 @@ static int s_der_decoder_load_ecc_public_key_pair_from_asn1( struct aws_byte_cursor *out_public_y_coord, struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - AWS_PRECONDITION(decoder); - AWS_PRECONDITION(out_public_x_coord); - AWS_PRECONDITION(out_public_y_coord); - AWS_PRECONDITION(out_private_d); - AWS_PRECONDITION(out_curve_name); AWS_ZERO_STRUCT(*out_public_x_coord); AWS_ZERO_STRUCT(*out_public_y_coord); @@ -533,6 +545,12 @@ int aws_der_decoder_load_ecc_key_pair( struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { + AWS_PRECONDITION(decoder); + AWS_PRECONDITION(out_public_x_coord); + AWS_PRECONDITION(out_public_y_coord); + AWS_PRECONDITION(out_private_d); + AWS_PRECONDITION(out_curve_name); + /** * Since this is a generic api to parse from ans1, we can encounter several key structures. * Just go through them one by one and see if any match the expected type. @@ -544,6 +562,7 @@ int aws_der_decoder_load_ecc_key_pair( } aws_der_decoder_reset(decoder); + if (s_der_decoder_load_ecc_private_key_pair_from_pkcs8( decoder, out_public_x_coord, out_public_y_coord, out_private_d, out_curve_name) == AWS_OP_SUCCESS) { return AWS_OP_SUCCESS; diff --git a/tests/ecc_test.c b/tests/ecc_test.c index c128cdd8..48b58c32 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -512,13 +512,15 @@ static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator (void)ctx; uint8_t asn1_encoded_key_raw[] = { - 0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0x78, 0xed, 0xed, 0xcf, 0x95, 0x9e, 0x42, 0x24, 0x37, 0xa4, 0x56, - 0xed, 0x08, 0x19, 0x3c, 0x53, 0x4b, 0x6f, 0xff, 0x40, 0x64, 0x48, 0x6a, 0x49, 0x86, 0x0c, 0xb7, 0x0a, 0xe5, - 0x2d, 0xbd, 0xd6, 0xa0, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0xa1, 0x44, 0x03, - 0x42, 0x00, 0x04, 0xbf, 0x61, 0x63, 0x46, 0x93, 0x2d, 0x00, 0x33, 0x19, 0xe3, 0x3a, 0x19, 0xc6, 0xc8, 0x55, - 0xf5, 0xc8, 0x44, 0x91, 0xe9, 0x9b, 0x83, 0x36, 0x67, 0x5d, 0x25, 0x0d, 0x7b, 0xe0, 0xc0, 0xf1, 0xd2, 0xaa, - 0x5c, 0xdf, 0xfb, 0xa9, 0x37, 0x19, 0x8d, 0x82, 0x47, 0x28, 0x88, 0xbe, 0x46, 0x7f, 0x3c, 0xcd, 0x41, 0xaa, - 0x08, 0x9a, 0x37, 0x0d, 0x61, 0x7f, 0x5f, 0xeb, 0x9f, 0x55, 0xf7, 0x54, 0xda, 0x0a, + 0x30, 0x81, 0x87, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02, + 0x01, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0x04, 0x6D, 0x30, 0x6B, 0x02, + 0x01, 0x01, 0x04, 0x20, 0x18, 0xCD, 0x6B, 0x61, 0x25, 0xB5, 0x37, 0x61, 0xB8, 0xF4, 0x6A, 0xBA, + 0x42, 0xAF, 0xA9, 0x70, 0x14, 0x9D, 0x72, 0x40, 0x6D, 0x84, 0x00, 0xA8, 0x40, 0x02, 0x13, 0x86, + 0x8C, 0xA9, 0x2D, 0x63, 0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0x95, 0x4E, 0x22, 0xE2, 0xDB, 0x79, + 0xCC, 0xA7, 0x56, 0x6F, 0xC2, 0x29, 0xA1, 0x0B, 0x05, 0x96, 0x22, 0x66, 0x06, 0xC3, 0x6D, 0x0A, + 0xD1, 0xD8, 0x57, 0x82, 0x02, 0x19, 0x74, 0xCD, 0x52, 0x4D, 0x5A, 0x57, 0x18, 0xCA, 0xDF, 0xF5, + 0x81, 0x9B, 0x9D, 0x0C, 0xBF, 0x2E, 0xB9, 0x91, 0xD2, 0x1A, 0xBB, 0x76, 0x8A, 0x06, 0x66, 0xB3, + 0xBF, 0x7C, 0x6C, 0x30, 0xF6, 0xBF, 0x8C, 0x84, 0x73, 0x96 }; struct aws_byte_cursor asn1_encoded_key = From 0e9cbd5cdb10334f68e2f64215aa7690dbe2f5c3 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 11:53:38 -0700 Subject: [PATCH 12/28] small refactor --- source/ecc.c | 176 ++++++++++++++++++++++++----------------------- tests/ecc_test.c | 18 +++-- 2 files changed, 98 insertions(+), 96 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index c6cc1978..78556e59 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -201,24 +201,18 @@ static void s_parse_public_key( } /* - * Load key from sec1 container. Aka "EC PRIVATE KEY" in pem - * ECPrivateKey ::= SEQUENCE { - * version INTEGER { ecPrivkeyVer1(1) }, - * privateKey OCTET STRING, - * parameters [0] ECParameters {{ NamedCurve }} OPTIONAL, - * publicKey [1] BIT STRING OPTIONAL - * } + * Both pkcs8 and sec1 have a shared overlapped structure. + * This helper extracts common fields and then validation can differ in the caller. */ -static int s_der_decoder_load_ecc_private_key_pair_from_sec1( +static int s_der_decoder_sec1_private_key_helper( struct aws_der_decoder *decoder, - struct aws_byte_cursor *out_public_x_coord, - struct aws_byte_cursor *out_public_y_coord, - struct aws_byte_cursor *out_private_d, - enum aws_ecc_curve_name *out_curve_name) { + struct aws_byte_cursor *out_private_cursor, + struct aws_byte_cursor *out_public_cursor, + enum aws_ecc_curve_name *out_curve_name, + bool *curve_name_set) { - AWS_ZERO_STRUCT(*out_public_x_coord); - AWS_ZERO_STRUCT(*out_public_y_coord); - AWS_ZERO_STRUCT(*out_private_d); + AWS_ZERO_STRUCT(*out_private_cursor); + AWS_ZERO_STRUCT(*out_public_cursor); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); @@ -235,29 +229,74 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( } struct aws_byte_cursor private_key_cur; - AWS_ZERO_STRUCT(private_key_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_byte_cursor oid; AWS_ZERO_STRUCT(oid); - if (!aws_der_decoder_next(decoder) || !aws_der_decoder_next(decoder) /* skip context specific structure */ - || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &oid)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + struct aws_byte_cursor public_key_cur; + AWS_ZERO_STRUCT(public_key_cur); + enum aws_ecc_curve_name curve_name = {0}; + + *curve_name_set = false; + if (aws_der_decoder_next(decoder)) { + if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { + if (aws_der_decoder_tlv_blob(decoder, &oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } + *curve_name_set = true; + aws_der_decoder_next(decoder); /* skip to field after */ + } + + if (aws_der_decoder_next(decoder) /* skip context specific */) { + if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + } } + *out_public_cursor = public_key_cur; + *out_private_cursor = private_key_cur; + *out_curve_name = curve_name; + + return AWS_OP_SUCCESS; +} + +/* + * Load key from sec1 container. Aka "EC PRIVATE KEY" in pem + * ECPrivateKey ::= SEQUENCE { + * version INTEGER { ecPrivkeyVer1(1) }, + * privateKey OCTET STRING, + * parameters [0] ECParameters {{ NamedCurve }} OPTIONAL, + * publicKey [1] BIT STRING OPTIONAL + * } + */ +static int s_der_decoder_load_ecc_private_key_pair_from_sec1( + struct aws_der_decoder *decoder, + struct aws_byte_cursor *out_public_x_coord, + struct aws_byte_cursor *out_public_y_coord, + struct aws_byte_cursor *out_private_d, + enum aws_ecc_curve_name *out_curve_name) { + + AWS_ZERO_STRUCT(*out_public_x_coord); + AWS_ZERO_STRUCT(*out_public_y_coord); + AWS_ZERO_STRUCT(*out_private_d); + + struct aws_byte_cursor private_key_cur; + struct aws_byte_cursor public_key_cur; enum aws_ecc_curve_name curve_name; - if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { - return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + bool curve_name_set = false; + if (s_der_decoder_sec1_private_key_helper( + decoder, &private_key_cur, &public_key_cur, &curve_name, &curve_name_set)) { + return AWS_OP_ERR; } - struct aws_byte_cursor public_key_cur; - AWS_ZERO_STRUCT(public_key_cur); - if (aws_der_decoder_next(decoder) && aws_der_decoder_next(decoder)) { /* skip context wrapper */ - if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - } + if (!curve_name_set) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); @@ -334,18 +373,23 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - /* version */ - struct aws_byte_cursor version_cur; - AWS_ZERO_STRUCT(version_cur); - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur)) { + if (!aws_der_decoder_next(decoder)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (version_cur.len != 1 || version_cur.ptr[0] != 0) { - return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + /* version */ + struct aws_byte_cursor version_cur; + AWS_ZERO_STRUCT(version_cur); + /* Note: this is not really spec compliant, but some implementations ignore the top level version. + * So be lax here and treat version as optional. */ + if (aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur) == AWS_OP_SUCCESS) { + if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); + } + aws_der_decoder_next(decoder); } - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -388,54 +432,20 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_type(nested_decoder) != AWS_DER_SEQUENCE) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; - } - - struct aws_byte_cursor inner_version_cur; - AWS_ZERO_STRUCT(inner_version_cur); - if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_unsigned_integer(nested_decoder, &inner_version_cur)) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; - } - - - if (inner_version_cur.len != 1 || inner_version_cur.ptr[0] != 1) { - aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); - goto on_nested_failure; - } - struct aws_byte_cursor private_key_cur; - if (!aws_der_decoder_next(nested_decoder) || aws_der_decoder_tlv_string(nested_decoder, &private_key_cur)) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; + struct aws_byte_cursor public_key_cur; + enum aws_ecc_curve_name inner_curve_name; + bool curve_name_set = false; + if (s_der_decoder_sec1_private_key_helper( + nested_decoder, &private_key_cur, &public_key_cur, &inner_curve_name, &curve_name_set)) { + aws_der_decoder_destroy(nested_decoder); + return AWS_OP_ERR; } - struct aws_byte_cursor inner_oid; - AWS_ZERO_STRUCT(inner_oid); - struct aws_byte_cursor public_key_cur; - AWS_ZERO_STRUCT(public_key_cur); - if (aws_der_decoder_next(nested_decoder)) { - if (aws_der_decoder_tlv_type(nested_decoder) == AWS_DER_OBJECT_IDENTIFIER) { - if (aws_der_decoder_tlv_blob(nested_decoder, &inner_oid)) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; - } - enum aws_ecc_curve_name inner_curve_name; - if (aws_ecc_curve_name_from_oid(&inner_oid, &inner_curve_name)) { - aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); - goto on_nested_failure; - } - if (inner_curve_name != curve_name) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; - } - } else if (!aws_der_decoder_next(nested_decoder) /* skip context specific */ || - aws_der_decoder_tlv_string(nested_decoder, &public_key_cur)) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; - } + aws_der_decoder_destroy(nested_decoder); + + if (curve_name_set && inner_curve_name != curve_name) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } size_t key_coordinate_size = aws_ecc_key_coordinate_byte_size_from_curve_name(curve_name); @@ -443,8 +453,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { - aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - goto on_nested_failure; + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } *out_private_d = private_key_cur; @@ -454,13 +463,8 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( } *out_curve_name = curve_name; - aws_der_decoder_destroy(nested_decoder); return AWS_OP_SUCCESS; - -on_nested_failure: - aws_der_decoder_destroy(nested_decoder); - return AWS_OP_ERR; } /* diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 48b58c32..96228d56 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -512,16 +512,14 @@ static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator (void)ctx; uint8_t asn1_encoded_key_raw[] = { - 0x30, 0x81, 0x87, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02, - 0x01, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0x04, 0x6D, 0x30, 0x6B, 0x02, - 0x01, 0x01, 0x04, 0x20, 0x18, 0xCD, 0x6B, 0x61, 0x25, 0xB5, 0x37, 0x61, 0xB8, 0xF4, 0x6A, 0xBA, - 0x42, 0xAF, 0xA9, 0x70, 0x14, 0x9D, 0x72, 0x40, 0x6D, 0x84, 0x00, 0xA8, 0x40, 0x02, 0x13, 0x86, - 0x8C, 0xA9, 0x2D, 0x63, 0xA1, 0x44, 0x03, 0x42, 0x00, 0x04, 0x95, 0x4E, 0x22, 0xE2, 0xDB, 0x79, - 0xCC, 0xA7, 0x56, 0x6F, 0xC2, 0x29, 0xA1, 0x0B, 0x05, 0x96, 0x22, 0x66, 0x06, 0xC3, 0x6D, 0x0A, - 0xD1, 0xD8, 0x57, 0x82, 0x02, 0x19, 0x74, 0xCD, 0x52, 0x4D, 0x5A, 0x57, 0x18, 0xCA, 0xDF, 0xF5, - 0x81, 0x9B, 0x9D, 0x0C, 0xBF, 0x2E, 0xB9, 0x91, 0xD2, 0x1A, 0xBB, 0x76, 0x8A, 0x06, 0x66, 0xB3, - 0xBF, 0x7C, 0x6C, 0x30, 0xF6, 0xBF, 0x8C, 0x84, 0x73, 0x96 - }; + 0x30, 0x81, 0x87, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02, 0x01, 0x06, + 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07, 0x04, 0x6D, 0x30, 0x6B, 0x02, 0x01, 0x01, 0x04, 0x20, + 0x18, 0xCD, 0x6B, 0x61, 0x25, 0xB5, 0x37, 0x61, 0xB8, 0xF4, 0x6A, 0xBA, 0x42, 0xAF, 0xA9, 0x70, 0x14, 0x9D, + 0x72, 0x40, 0x6D, 0x84, 0x00, 0xA8, 0x40, 0x02, 0x13, 0x86, 0x8C, 0xA9, 0x2D, 0x63, 0xA1, 0x44, 0x03, 0x42, + 0x00, 0x04, 0x95, 0x4E, 0x22, 0xE2, 0xDB, 0x79, 0xCC, 0xA7, 0x56, 0x6F, 0xC2, 0x29, 0xA1, 0x0B, 0x05, 0x96, + 0x22, 0x66, 0x06, 0xC3, 0x6D, 0x0A, 0xD1, 0xD8, 0x57, 0x82, 0x02, 0x19, 0x74, 0xCD, 0x52, 0x4D, 0x5A, 0x57, + 0x18, 0xCA, 0xDF, 0xF5, 0x81, 0x9B, 0x9D, 0x0C, 0xBF, 0x2E, 0xB9, 0x91, 0xD2, 0x1A, 0xBB, 0x76, 0x8A, 0x06, + 0x66, 0xB3, 0xBF, 0x7C, 0x6C, 0x30, 0xF6, 0xBF, 0x8C, 0x84, 0x73, 0x96}; struct aws_byte_cursor asn1_encoded_key = aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); From 03300005bfd1cbc19d90ea4e880f5837ac39d15a Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 14:18:31 -0700 Subject: [PATCH 13/28] test fix --- include/aws/cal/private/der.h | 5 +++++ source/ecc.c | 28 +++++++++++++++++++--------- tests/ecc_test.c | 3 ++- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/include/aws/cal/private/der.h b/include/aws/cal/private/der.h index c531491d..8a904d8f 100644 --- a/include/aws/cal/private/der.h +++ b/include/aws/cal/private/der.h @@ -60,6 +60,11 @@ enum aws_der_type { /* forms */ AWS_DER_FORM_CONSTRUCTED = 0x20, AWS_DER_FORM_PRIMITIVE = 0x00, + + /* context specific */ + /* TODO: we should probably handle tags more generically, but for now first 2 tags cover all cases. */ + AWS_DER_CONTEXT_SPECIFIC_TAG0 = 0xa0, + AWS_DER_CONTEXT_SPECIFIC_TAG1 = 0xa1, }; AWS_EXTERN_C_BEGIN diff --git a/source/ecc.c b/source/ecc.c index 78556e59..fa9ee4da 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -241,18 +241,27 @@ static int s_der_decoder_sec1_private_key_helper( *curve_name_set = false; if (aws_der_decoder_next(decoder)) { - if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { - if (aws_der_decoder_tlv_blob(decoder, &oid)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - } - if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { - return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + + /* tag 0 is optional params */ + if (aws_der_decoder_tlv_type(decoder) == AWS_DER_CONTEXT_SPECIFIC_TAG0) { + aws_der_decoder_next(decoder); + + if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { + if (aws_der_decoder_tlv_blob(decoder, &oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); + } + *curve_name_set = true; + aws_der_decoder_next(decoder); /* skip to field after */ } - *curve_name_set = true; - aws_der_decoder_next(decoder); /* skip to field after */ } - if (aws_der_decoder_next(decoder) /* skip context specific */) { + /* tag 1 is optional public key */ + if (aws_der_decoder_tlv_type(decoder) == AWS_DER_CONTEXT_SPECIFIC_TAG1) { + aws_der_decoder_next(decoder); + if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -290,6 +299,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_sec1( struct aws_byte_cursor public_key_cur; enum aws_ecc_curve_name curve_name; bool curve_name_set = false; + if (s_der_decoder_sec1_private_key_helper( decoder, &private_key_cur, &public_key_cur, &curve_name, &curve_name_set)) { return AWS_OP_ERR; diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 96228d56..5ac34314 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -519,7 +519,8 @@ static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator 0x00, 0x04, 0x95, 0x4E, 0x22, 0xE2, 0xDB, 0x79, 0xCC, 0xA7, 0x56, 0x6F, 0xC2, 0x29, 0xA1, 0x0B, 0x05, 0x96, 0x22, 0x66, 0x06, 0xC3, 0x6D, 0x0A, 0xD1, 0xD8, 0x57, 0x82, 0x02, 0x19, 0x74, 0xCD, 0x52, 0x4D, 0x5A, 0x57, 0x18, 0xCA, 0xDF, 0xF5, 0x81, 0x9B, 0x9D, 0x0C, 0xBF, 0x2E, 0xB9, 0x91, 0xD2, 0x1A, 0xBB, 0x76, 0x8A, 0x06, - 0x66, 0xB3, 0xBF, 0x7C, 0x6C, 0x30, 0xF6, 0xBF, 0x8C, 0x84, 0x73, 0x96}; + 0x66, 0xB3, 0xBF, 0x7C, 0x6C, 0x30, 0xF6, 0xBF, 0x8C, 0x84, 0x73, 0x96, + }; struct aws_byte_cursor asn1_encoded_key = aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); From aa69808501038528f4b85505570a48df383042da Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 14:32:54 -0700 Subject: [PATCH 14/28] zero --- source/ecc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/ecc.c b/source/ecc.c index fa9ee4da..d8be6cd8 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -229,6 +229,7 @@ static int s_der_decoder_sec1_private_key_helper( } struct aws_byte_cursor private_key_cur; + AWS_ZERO_STRUCT(private_key_cur); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_string(decoder, &private_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } From 04c33c949bac4a434ea5e4da8f7d14f053b58dc1 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 14:40:45 -0700 Subject: [PATCH 15/28] loggin: --- source/ecc.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/source/ecc.c b/source/ecc.c index d8be6cd8..4e9fc8c2 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -380,11 +380,15 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); + AWS_LOGF_DEBUG(0, "here 1"); + if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + AWS_LOGF_DEBUG(0, "here 2"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } if (!aws_der_decoder_next(decoder)) { + AWS_LOGF_DEBUG(0, "here 3"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -395,12 +399,14 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( * So be lax here and treat version as optional. */ if (aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur) == AWS_OP_SUCCESS) { if (version_cur.len != 1 || version_cur.ptr[0] != 0) { + AWS_LOGF_DEBUG(0, "here 4"); return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } aws_der_decoder_next(decoder); } if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { + AWS_LOGF_DEBUG(0, "here 5"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -408,6 +414,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(algo_oid); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &algo_oid)) { + AWS_LOGF_DEBUG(0, "here 6"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -417,6 +424,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( */ if (!aws_byte_cursor_eq(&algo_oid, &s_ec_public_key_oid_cursor) && !aws_byte_cursor_eq(&algo_oid, &s_ec_private_key_oid_cursor)) { + AWS_LOGF_DEBUG(0, "here 7"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } @@ -424,22 +432,26 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(curve_oid); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &curve_oid)) { + AWS_LOGF_DEBUG(0, "here 8"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name curve_name; if (aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { + AWS_LOGF_DEBUG(0, "here 9"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } /* private key string */ if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OCTET_STRING) { + AWS_LOGF_DEBUG(0, "here 10"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_der_decoder *nested_decoder = aws_der_decoder_nested_tlv_decoder(decoder); if (!nested_decoder) { + AWS_LOGF_DEBUG(0, "here 11"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -449,6 +461,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( bool curve_name_set = false; if (s_der_decoder_sec1_private_key_helper( nested_decoder, &private_key_cur, &public_key_cur, &inner_curve_name, &curve_name_set)) { + AWS_LOGF_DEBUG(0, "here 12"); aws_der_decoder_destroy(nested_decoder); return AWS_OP_ERR; } @@ -456,6 +469,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( aws_der_decoder_destroy(nested_decoder); if (curve_name_set && inner_curve_name != curve_name) { + AWS_LOGF_DEBUG(0, "here 13"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -464,6 +478,7 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { + AWS_LOGF_DEBUG(0, "here 14"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -475,6 +490,8 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( *out_curve_name = curve_name; + AWS_LOGF_DEBUG(0, "here 15"); + return AWS_OP_SUCCESS; } From 398dca07fedcda163729adfbbfc7f448d9aae7c0 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 14:47:37 -0700 Subject: [PATCH 16/28] revert --- source/ecc.c | 6 ++++-- source/unix/opensslcrypto_ecc.c | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 4e9fc8c2..faac8659 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -196,8 +196,10 @@ static void s_parse_public_key( struct aws_byte_cursor *out_public_y_coord) { aws_byte_cursor_advance(&public_key, 1); - *out_public_x_coord = aws_byte_cursor_advance(&public_key, key_coordinate_size); - *out_public_y_coord = public_key; + *out_public_x_coord = public_key; + out_public_x_coord->len = key_coordinate_size; + out_public_y_coord->ptr = public_key.ptr + key_coordinate_size; + out_public_y_coord->len = key_coordinate_size; } /* diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index f8d33316..3202d6f6 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -314,6 +314,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( enum aws_ecc_curve_name curve_name; if (aws_der_decoder_load_ecc_key_pair(decoder, &pub_x, &pub_y, &priv_d, &curve_name)) { + AWS_LOGF_DEBUG(0, "foo1"); goto error; } @@ -325,6 +326,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (!d2i_ECPrivateKey(&key_impl->ec_key, (const unsigned char **)&encoded_keys->ptr, encoded_keys->len)) { aws_mem_release(allocator, key_impl); aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); + AWS_LOGF_DEBUG(0, "foo2"); goto error; } @@ -340,6 +342,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (pub_x.ptr) { temp_buf = aws_byte_buf_from_array(pub_x.ptr, pub_x.len); if (aws_byte_buf_init_copy(&key->pub_x, allocator, &temp_buf)) { + AWS_LOGF_DEBUG(0, "foo3"); goto error; } } @@ -347,6 +350,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (pub_y.ptr) { temp_buf = aws_byte_buf_from_array(pub_y.ptr, pub_y.len); if (aws_byte_buf_init_copy(&key->pub_y, allocator, &temp_buf)) { + AWS_LOGF_DEBUG(0, "foo4"); goto error; } } @@ -354,6 +358,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (priv_d.ptr) { temp_buf = aws_byte_buf_from_array(priv_d.ptr, priv_d.len); if (aws_byte_buf_init_copy(&key->priv_d, allocator, &temp_buf)) { + AWS_LOGF_DEBUG(0, "foo5"); goto error; } } @@ -362,6 +367,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( key = aws_ecc_key_pair_new_from_public_key(allocator, curve_name, &pub_x, &pub_y); if (!key) { + AWS_LOGF_DEBUG(0, "foo6"); goto error; } } From 853480d12b72520666862f1dc0f2c4bb663633a1 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 15:28:47 -0700 Subject: [PATCH 17/28] fix lc --- source/unix/opensslcrypto_ecc.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 3202d6f6..5fd26d98 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -228,6 +228,21 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_generate_random( return NULL; } +static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { + BIGNUM *priv_n = BN_bin2bn(priv.ptr, priv.len, NULL); + if (!priv_n) { + return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; + } + + if (!EC_KEY_set_private_key(key, priv_n)) { + BN_free(priv_n); + return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; + } + + BN_free(priv_n); + return AWS_OP_SUCCESS; +} + struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( struct aws_allocator *allocator, enum aws_ecc_curve_name curve_name, @@ -321,9 +336,10 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (priv_d.ptr) { struct libcrypto_ecc_key *key_impl = aws_mem_calloc(allocator, 1, sizeof(struct libcrypto_ecc_key)); key_impl->key_pair.curve_name = curve_name; - /* as awkward as it seems, there's not a great way to manually set the public key, so let openssl just parse - * the der document manually now that we know what parts are what. */ - if (!d2i_ECPrivateKey(&key_impl->ec_key, (const unsigned char **)&encoded_keys->ptr, encoded_keys->len)) { + + key_impl->ec_key = EC_KEY_new_by_curve_name(s_curve_name_to_nid(curve_name)); + + if (s_ec_key_set_private_key(key_impl->ec_key, priv_d)) { aws_mem_release(allocator, key_impl); aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); AWS_LOGF_DEBUG(0, "foo2"); From d1d299066582ba98da84c46d67183fee404fe87c Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 15:36:24 -0700 Subject: [PATCH 18/28] gen pub if missing --- source/unix/opensslcrypto_ecc.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 5fd26d98..76a9192d 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -239,6 +239,21 @@ static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; } + BN_free(priv_n); + return AWS_OP_SUCCESS; +} + +static int s_ec_key_set_public_key(EC_KEY* key, const struct aws_byte_cursor *public_key_x, const struct aws_byte_cursor *public_key_y) { + BIGNUM *priv_n = BN_bin2bn(priv.ptr, priv.len, NULL); + if (!priv_n) { + return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; + } + + if (!EC_KEY_set_private_key(key, priv_n)) { + BN_free(priv_n); + return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; + } + BN_free(priv_n); return AWS_OP_SUCCESS; } @@ -346,6 +361,15 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( goto error; } + if (!pub_x.ptr || !pub_y.ptr ) { + if (!EC_KEY_generate_key(key_impl->ec_key)) { + aws_mem_release(allocator, key_impl); + aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); + AWS_LOGF_DEBUG(0, "foo2.5"); + goto error; + } + } + key_impl->key_pair.allocator = allocator; key_impl->key_pair.vtable = &vtable; key_impl->key_pair.impl = key_impl; From 622aa13c0f2760b590e1373e336b5ecc4851374b Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 15:38:28 -0700 Subject: [PATCH 19/28] remove for now --- source/unix/opensslcrypto_ecc.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 76a9192d..da007ecc 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -243,21 +243,6 @@ static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { return AWS_OP_SUCCESS; } -static int s_ec_key_set_public_key(EC_KEY* key, const struct aws_byte_cursor *public_key_x, const struct aws_byte_cursor *public_key_y) { - BIGNUM *priv_n = BN_bin2bn(priv.ptr, priv.len, NULL); - if (!priv_n) { - return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; - } - - if (!EC_KEY_set_private_key(key, priv_n)) { - BN_free(priv_n); - return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; - } - - BN_free(priv_n); - return AWS_OP_SUCCESS; -} - struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( struct aws_allocator *allocator, enum aws_ecc_curve_name curve_name, From 07cc6a80bf8f97095527e2cad7ed90852f30d4b0 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 15:44:08 -0700 Subject: [PATCH 20/28] logs --- source/unix/opensslcrypto_ecc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index da007ecc..2c95fbb9 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -347,12 +347,15 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( } if (!pub_x.ptr || !pub_y.ptr ) { + AWS_LOGF_DEBUG(0, "we are here"); if (!EC_KEY_generate_key(key_impl->ec_key)) { aws_mem_release(allocator, key_impl); aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); AWS_LOGF_DEBUG(0, "foo2.5"); goto error; } + } else { + AWS_LOGF_DEBUG(0, "or there"); } key_impl->key_pair.allocator = allocator; From b9515f58b0adcab98f05bc06e34e18c03dd4ebee Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 15:53:34 -0700 Subject: [PATCH 21/28] set pub --- source/unix/opensslcrypto_ecc.c | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 2c95fbb9..623de51d 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -243,6 +243,35 @@ static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { return AWS_OP_SUCCESS; } +static int s_ec_key_set_public_key(EC_KEY* key, struct aws_byte_cursor pub_x, struct aws_byte_cursor pub_y) { + BIGNUM *pub_x_num = BN_bin2bn(pub_x.ptr, pub_x.len, NULL); + BIGNUM *pub_y_num = BN_bin2bn(pub_y.ptr, pub_x.len, NULL); + + const EC_GROUP *group = EC_KEY_get0_group(key); + EC_POINT *point = EC_POINT_new(group); + + if (EC_POINT_set_affine_coordinates_GFp(group, point, pub_x_num, pub_y_num, NULL) != 1) { + EC_POINT_free(point); + BN_free(pub_x_num); + BN_free(pub_y_num); + return AWS_OP_ERR; + } + + if (EC_KEY_set_public_key(key, point) != 1) { + EC_POINT_free(point); + BN_free(pub_x_num); + BN_free(pub_y_num); + return AWS_OP_ERR; + } + + EC_POINT_free(point); + BN_free(pub_x_num); + BN_free(pub_y_num); + + return AWS_OP_SUCCESS; + +} + struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( struct aws_allocator *allocator, enum aws_ecc_curve_name curve_name, @@ -355,7 +384,12 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( goto error; } } else { - AWS_LOGF_DEBUG(0, "or there"); + if (s_ec_key_set_public_key(key_impl->ec_key, pub_x, pub_y)) { + aws_mem_release(allocator, key_impl); + aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); + AWS_LOGF_DEBUG(0, "foo2"); + goto error; + } } key_impl->key_pair.allocator = allocator; From c35d64a37e5c8436945fc7d0a0a4f8cef02365db Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 16:06:11 -0700 Subject: [PATCH 22/28] fix and lint --- source/ecc.c | 19 +----- source/unix/opensslcrypto_ecc.c | 103 +++++++------------------------- 2 files changed, 23 insertions(+), 99 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index faac8659..73d2f51b 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -196,7 +196,7 @@ static void s_parse_public_key( struct aws_byte_cursor *out_public_y_coord) { aws_byte_cursor_advance(&public_key, 1); - *out_public_x_coord = public_key; + *out_public_x_coord = public_key; out_public_x_coord->len = key_coordinate_size; out_public_y_coord->ptr = public_key.ptr + key_coordinate_size; out_public_y_coord->len = key_coordinate_size; @@ -382,15 +382,11 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(*out_public_y_coord); AWS_ZERO_STRUCT(*out_private_d); - AWS_LOGF_DEBUG(0, "here 1"); - if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { - AWS_LOGF_DEBUG(0, "here 2"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } if (!aws_der_decoder_next(decoder)) { - AWS_LOGF_DEBUG(0, "here 3"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -401,14 +397,12 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( * So be lax here and treat version as optional. */ if (aws_der_decoder_tlv_unsigned_integer(decoder, &version_cur) == AWS_OP_SUCCESS) { if (version_cur.len != 1 || version_cur.ptr[0] != 0) { - AWS_LOGF_DEBUG(0, "here 4"); return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } aws_der_decoder_next(decoder); } if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { - AWS_LOGF_DEBUG(0, "here 5"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -416,7 +410,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(algo_oid); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &algo_oid)) { - AWS_LOGF_DEBUG(0, "here 6"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -426,7 +419,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( */ if (!aws_byte_cursor_eq(&algo_oid, &s_ec_public_key_oid_cursor) && !aws_byte_cursor_eq(&algo_oid, &s_ec_private_key_oid_cursor)) { - AWS_LOGF_DEBUG(0, "here 7"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } @@ -434,26 +426,22 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( AWS_ZERO_STRUCT(curve_oid); if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER || aws_der_decoder_tlv_blob(decoder, &curve_oid)) { - AWS_LOGF_DEBUG(0, "here 8"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } enum aws_ecc_curve_name curve_name; if (aws_ecc_curve_name_from_oid(&curve_oid, &curve_name)) { - AWS_LOGF_DEBUG(0, "here 9"); return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } /* private key string */ if (!aws_der_decoder_next(decoder) || aws_der_decoder_tlv_type(decoder) != AWS_DER_OCTET_STRING) { - AWS_LOGF_DEBUG(0, "here 10"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } struct aws_der_decoder *nested_decoder = aws_der_decoder_nested_tlv_decoder(decoder); if (!nested_decoder) { - AWS_LOGF_DEBUG(0, "here 11"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -463,7 +451,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( bool curve_name_set = false; if (s_der_decoder_sec1_private_key_helper( nested_decoder, &private_key_cur, &public_key_cur, &inner_curve_name, &curve_name_set)) { - AWS_LOGF_DEBUG(0, "here 12"); aws_der_decoder_destroy(nested_decoder); return AWS_OP_ERR; } @@ -471,7 +458,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( aws_der_decoder_destroy(nested_decoder); if (curve_name_set && inner_curve_name != curve_name) { - AWS_LOGF_DEBUG(0, "here 13"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -480,7 +466,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( if (private_key_cur.len != key_coordinate_size || (public_key_cur.len != 0 && public_key_cur.len != public_key_blob_size)) { - AWS_LOGF_DEBUG(0, "here 14"); return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } @@ -492,8 +477,6 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( *out_curve_name = curve_name; - AWS_LOGF_DEBUG(0, "here 15"); - return AWS_OP_SUCCESS; } diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 623de51d..1b9d3556 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -228,7 +228,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_generate_random( return NULL; } -static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { +static int s_ec_key_set_private_key(EC_KEY *key, struct aws_byte_cursor priv) { BIGNUM *priv_n = BN_bin2bn(priv.ptr, priv.len, NULL); if (!priv_n) { return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; @@ -243,7 +243,7 @@ static int s_ec_key_set_private_key(EC_KEY* key, struct aws_byte_cursor priv) { return AWS_OP_SUCCESS; } -static int s_ec_key_set_public_key(EC_KEY* key, struct aws_byte_cursor pub_x, struct aws_byte_cursor pub_y) { +static int s_ec_key_set_public_key(EC_KEY *key, struct aws_byte_cursor pub_x, struct aws_byte_cursor pub_y) { BIGNUM *pub_x_num = BN_bin2bn(pub_x.ptr, pub_x.len, NULL); BIGNUM *pub_y_num = BN_bin2bn(pub_y.ptr, pub_x.len, NULL); @@ -251,17 +251,11 @@ static int s_ec_key_set_public_key(EC_KEY* key, struct aws_byte_cursor pub_x, st EC_POINT *point = EC_POINT_new(group); if (EC_POINT_set_affine_coordinates_GFp(group, point, pub_x_num, pub_y_num, NULL) != 1) { - EC_POINT_free(point); - BN_free(pub_x_num); - BN_free(pub_y_num); - return AWS_OP_ERR; + goto on_error; } if (EC_KEY_set_public_key(key, point) != 1) { - EC_POINT_free(point); - BN_free(pub_x_num); - BN_free(pub_y_num); - return AWS_OP_ERR; + goto on_error; } EC_POINT_free(point); @@ -270,6 +264,11 @@ static int s_ec_key_set_public_key(EC_KEY* key, struct aws_byte_cursor pub_x, st return AWS_OP_SUCCESS; +on_error: + EC_POINT_free(point); + BN_free(pub_x_num); + BN_free(pub_y_num); + return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); } struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( @@ -278,9 +277,6 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( const struct aws_byte_cursor *public_key_x, const struct aws_byte_cursor *public_key_y) { struct libcrypto_ecc_key *key_impl = aws_mem_calloc(allocator, 1, sizeof(struct libcrypto_ecc_key)); - BIGNUM *pub_x_num = NULL; - BIGNUM *pub_y_num = NULL; - EC_POINT *point = NULL; if (!key_impl) { return NULL; @@ -293,49 +289,16 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_public_key_impl( key_impl->key_pair.impl = key_impl; aws_atomic_init_int(&key_impl->key_pair.ref_count, 1); - if (aws_byte_buf_init_copy_from_cursor(&key_impl->key_pair.pub_x, allocator, *public_key_x)) { - s_key_pair_destroy(&key_impl->key_pair); - return NULL; - } + aws_byte_buf_init_copy_from_cursor(&key_impl->key_pair.pub_x, allocator, *public_key_x); + aws_byte_buf_init_copy_from_cursor(&key_impl->key_pair.pub_y, allocator, *public_key_y); - if (aws_byte_buf_init_copy_from_cursor(&key_impl->key_pair.pub_y, allocator, *public_key_y)) { - s_key_pair_destroy(&key_impl->key_pair); - return NULL; + if (s_ec_key_set_public_key(key_impl->ec_key, *public_key_x, *public_key_y)) { + goto on_error; } - pub_x_num = BN_bin2bn(public_key_x->ptr, public_key_x->len, NULL); - pub_y_num = BN_bin2bn(public_key_y->ptr, public_key_y->len, NULL); - - const EC_GROUP *group = EC_KEY_get0_group(key_impl->ec_key); - point = EC_POINT_new(group); - - if (EC_POINT_set_affine_coordinates_GFp(group, point, pub_x_num, pub_y_num, NULL) != 1) { - goto error; - } - - if (EC_KEY_set_public_key(key_impl->ec_key, point) != 1) { - goto error; - } - - EC_POINT_free(point); - BN_free(pub_x_num); - BN_free(pub_y_num); - return &key_impl->key_pair; -error: - if (point) { - EC_POINT_free(point); - } - - if (pub_x_num) { - BN_free(pub_x_num); - } - - if (pub_y_num) { - BN_free(pub_y_num); - } - +on_error: s_key_pair_destroy(&key_impl->key_pair); return NULL; @@ -358,7 +321,6 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( enum aws_ecc_curve_name curve_name; if (aws_der_decoder_load_ecc_key_pair(decoder, &pub_x, &pub_y, &priv_d, &curve_name)) { - AWS_LOGF_DEBUG(0, "foo1"); goto error; } @@ -370,24 +332,21 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( if (s_ec_key_set_private_key(key_impl->ec_key, priv_d)) { aws_mem_release(allocator, key_impl); - aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); - AWS_LOGF_DEBUG(0, "foo2"); + aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); goto error; } - if (!pub_x.ptr || !pub_y.ptr ) { - AWS_LOGF_DEBUG(0, "we are here"); + /* if pub is missing, generate it */ + if (!pub_x.ptr || !pub_y.ptr) { if (!EC_KEY_generate_key(key_impl->ec_key)) { aws_mem_release(allocator, key_impl); - aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); - AWS_LOGF_DEBUG(0, "foo2.5"); + aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); goto error; } } else { if (s_ec_key_set_public_key(key_impl->ec_key, pub_x, pub_y)) { aws_mem_release(allocator, key_impl); - aws_raise_error(AWS_ERROR_CAL_MISSING_REQUIRED_KEY_COMPONENT); - AWS_LOGF_DEBUG(0, "foo2"); + aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); goto error; } } @@ -398,38 +357,20 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_from_asn1( aws_atomic_init_int(&key_impl->key_pair.ref_count, 1); key = &key_impl->key_pair; - struct aws_byte_buf temp_buf; - AWS_ZERO_STRUCT(temp_buf); - if (pub_x.ptr) { - temp_buf = aws_byte_buf_from_array(pub_x.ptr, pub_x.len); - if (aws_byte_buf_init_copy(&key->pub_x, allocator, &temp_buf)) { - AWS_LOGF_DEBUG(0, "foo3"); - goto error; - } + aws_byte_buf_init_copy_from_cursor(&key->pub_x, allocator, pub_x); } if (pub_y.ptr) { - temp_buf = aws_byte_buf_from_array(pub_y.ptr, pub_y.len); - if (aws_byte_buf_init_copy(&key->pub_y, allocator, &temp_buf)) { - AWS_LOGF_DEBUG(0, "foo4"); - goto error; - } + aws_byte_buf_init_copy_from_cursor(&key->pub_y, allocator, pub_y); } - if (priv_d.ptr) { - temp_buf = aws_byte_buf_from_array(priv_d.ptr, priv_d.len); - if (aws_byte_buf_init_copy(&key->priv_d, allocator, &temp_buf)) { - AWS_LOGF_DEBUG(0, "foo5"); - goto error; - } - } + aws_byte_buf_init_copy_from_cursor(&key->priv_d, allocator, priv_d); } else { key = aws_ecc_key_pair_new_from_public_key(allocator, curve_name, &pub_x, &pub_y); if (!key) { - AWS_LOGF_DEBUG(0, "foo6"); goto error; } } From 7b9a83d6ad0b4364d721d8d9c72325bc204f5276 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Tue, 14 Oct 2025 16:24:27 -0700 Subject: [PATCH 23/28] typo --- source/unix/opensslcrypto_ecc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 1b9d3556..3e64f3fe 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -245,7 +245,7 @@ static int s_ec_key_set_private_key(EC_KEY *key, struct aws_byte_cursor priv) { static int s_ec_key_set_public_key(EC_KEY *key, struct aws_byte_cursor pub_x, struct aws_byte_cursor pub_y) { BIGNUM *pub_x_num = BN_bin2bn(pub_x.ptr, pub_x.len, NULL); - BIGNUM *pub_y_num = BN_bin2bn(pub_y.ptr, pub_x.len, NULL); + BIGNUM *pub_y_num = BN_bin2bn(pub_y.ptr, pub_y.len, NULL); const EC_GROUP *group = EC_KEY_get0_group(key); EC_POINT *point = EC_POINT_new(group); From ac70d69c472e8b0f50802aba3b92f79226bffd5a Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 15 Oct 2025 10:58:54 -0700 Subject: [PATCH 24/28] restore advance fix --- source/ecc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 73d2f51b..d8be6cd8 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -196,10 +196,8 @@ static void s_parse_public_key( struct aws_byte_cursor *out_public_y_coord) { aws_byte_cursor_advance(&public_key, 1); - *out_public_x_coord = public_key; - out_public_x_coord->len = key_coordinate_size; - out_public_y_coord->ptr = public_key.ptr + key_coordinate_size; - out_public_y_coord->len = key_coordinate_size; + *out_public_x_coord = aws_byte_cursor_advance(&public_key, key_coordinate_size); + *out_public_y_coord = public_key; } /* From efbb69ec45428dfd6ddc1f91f5d94b62fbb82f9a Mon Sep 17 00:00:00 2001 From: Dmitriy Musatkin <63878209+DmitriyMusatkin@users.noreply.github.com> Date: Wed, 15 Oct 2025 21:29:19 -0700 Subject: [PATCH 25/28] Update source/unix/opensslcrypto_ecc.c Co-authored-by: Dengke Tang --- source/unix/opensslcrypto_ecc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 3e64f3fe..4eb89a96 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -231,7 +231,7 @@ struct aws_ecc_key_pair *aws_ecc_key_pair_new_generate_random( static int s_ec_key_set_private_key(EC_KEY *key, struct aws_byte_cursor priv) { BIGNUM *priv_n = BN_bin2bn(priv.ptr, priv.len, NULL); if (!priv_n) { - return AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED; + return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); } if (!EC_KEY_set_private_key(key, priv_n)) { From cbd51fb6f1294268db5d6e4df12e29168cd393a1 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 15 Oct 2025 23:03:46 -0700 Subject: [PATCH 26/28] addressing comments --- source/ecc.c | 20 +++++++++++--------- source/unix/opensslcrypto_ecc.c | 15 ++++++++++++--- tests/CMakeLists.txt | 1 + tests/ecc_test.c | 19 +++++++++++++++++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index d8be6cd8..eec435a0 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -247,16 +247,18 @@ static int s_der_decoder_sec1_private_key_helper( if (aws_der_decoder_tlv_type(decoder) == AWS_DER_CONTEXT_SPECIFIC_TAG0) { aws_der_decoder_next(decoder); - if (aws_der_decoder_tlv_type(decoder) == AWS_DER_OBJECT_IDENTIFIER) { - if (aws_der_decoder_tlv_blob(decoder, &oid)) { - return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); - } - if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { - return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); - } - *curve_name_set = true; - aws_der_decoder_next(decoder); /* skip to field after */ + if (aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + + if (aws_der_decoder_tlv_blob(decoder, &oid)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } + if (aws_ecc_curve_name_from_oid(&oid, &curve_name)) { + return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } + *curve_name_set = true; + aws_der_decoder_next(decoder); /* skip to field after */ } /* tag 1 is optional public key */ diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 4eb89a96..12c6fdb7 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -265,9 +265,18 @@ static int s_ec_key_set_public_key(EC_KEY *key, struct aws_byte_cursor pub_x, st return AWS_OP_SUCCESS; on_error: - EC_POINT_free(point); - BN_free(pub_x_num); - BN_free(pub_y_num); + if (point){ + EC_POINT_free(point); + } + + if (pub_x_num) { + BN_free(pub_x_num); + } + + if (pub_x_num) { + BN_free(pub_y_num); + } + return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 368a7cc3..8f17cc9f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -79,6 +79,7 @@ add_test_case(ecdsa_p256_test_key_gen_export) add_test_case(ecdsa_p384_test_key_gen_export) add_test_case(ecdsa_p256_test_import_asn1_key_pair) add_test_case(ecdsa_p256_test_import_asn1_pkcs8_key_pair) +add_test_case(ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair) add_test_case(ecdsa_p384_test_import_asn1_key_pair) add_test_case(ecdsa_test_import_asn1_key_pair_public_only) add_test_case(ecdsa_test_import_asn1_key_pair_invalid_fails) diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 5ac34314..2e9f21b4 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -529,6 +529,25 @@ static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator } AWS_TEST_CASE(ecdsa_p256_test_import_asn1_pkcs8_key_pair, s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn) +static int s_ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair_fn(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + uint8_t asn1_encoded_key_raw[] = { + 0x30, 0x41, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, + 0xCE, 0x3D, 0x02, 0x01, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, + 0x01, 0x07, 0x04, 0x24, 0x30, 0x22, 0x02, 0x01, 0x01, 0x04, 0x20, 0x18, + 0xCD, 0x6B, 0x61, 0x25, 0xB5, 0x37, 0x61, 0xB8, 0xF4, 0x6A, 0xBA, 0x42, + 0xAF, 0xA9, 0x70, 0x14, 0x9D, 0x72, 0x40, 0x6D, 0x84, 0x00, 0xA8, 0x40, + 0x02, 0x13, 0x86, 0x8C, 0xA9, 0x2D, 0x63 + }; + + struct aws_byte_cursor asn1_encoded_key = + aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); + + return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); +} +AWS_TEST_CASE(ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair, s_ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair_fn) + static int s_ecdsa_p384_test_import_asn1_key_pair_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From 255dfd73ed95485e997f5c56539e5cda2fb659a0 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Thu, 16 Oct 2025 01:25:16 -0700 Subject: [PATCH 27/28] address comments --- source/ecc.c | 24 +++++++++++++++++++----- source/unix/opensslcrypto_ecc.c | 6 +++--- tests/CMakeLists.txt | 1 - tests/ecc_test.c | 19 ------------------- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index eec435a0..97cc523f 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -243,9 +243,12 @@ static int s_der_decoder_sec1_private_key_helper( *curve_name_set = false; if (aws_der_decoder_next(decoder)) { + bool version_found = false; /* tag 0 is optional params */ if (aws_der_decoder_tlv_type(decoder) == AWS_DER_CONTEXT_SPECIFIC_TAG0) { - aws_der_decoder_next(decoder); + if (!aws_der_decoder_next(decoder)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } if (aws_der_decoder_tlv_type(decoder) != AWS_DER_OBJECT_IDENTIFIER) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); @@ -258,12 +261,21 @@ static int s_der_decoder_sec1_private_key_helper( return aws_raise_error(AWS_ERROR_CAL_UNKNOWN_OBJECT_IDENTIFIER); } *curve_name_set = true; - aws_der_decoder_next(decoder); /* skip to field after */ + version_found = true; + } + + /* tag 1 is optional public key + * Note: even though its optional, I could not get any modern tools to generate a key without one. + * So from practical perspective it almost always is present. + * */ + if (version_found && !aws_der_decoder_next(decoder)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); } - /* tag 1 is optional public key */ if (aws_der_decoder_tlv_type(decoder) == AWS_DER_CONTEXT_SPECIFIC_TAG1) { - aws_der_decoder_next(decoder); + if (!aws_der_decoder_next(decoder)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } if (aws_der_decoder_tlv_string(decoder, &public_key_cur)) { return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); @@ -399,7 +411,9 @@ static int s_der_decoder_load_ecc_private_key_pair_from_pkcs8( if (version_cur.len != 1 || version_cur.ptr[0] != 0) { return aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_KEY_FORMAT); } - aws_der_decoder_next(decoder); + if (!aws_der_decoder_next(decoder)) { + return aws_raise_error(AWS_ERROR_CAL_MALFORMED_ASN1_ENCOUNTERED); + } } if (aws_der_decoder_tlv_type(decoder) != AWS_DER_SEQUENCE) { diff --git a/source/unix/opensslcrypto_ecc.c b/source/unix/opensslcrypto_ecc.c index 12c6fdb7..e771f93e 100644 --- a/source/unix/opensslcrypto_ecc.c +++ b/source/unix/opensslcrypto_ecc.c @@ -265,10 +265,10 @@ static int s_ec_key_set_public_key(EC_KEY *key, struct aws_byte_cursor pub_x, st return AWS_OP_SUCCESS; on_error: - if (point){ + if (point) { EC_POINT_free(point); } - + if (pub_x_num) { BN_free(pub_x_num); } @@ -276,7 +276,7 @@ static int s_ec_key_set_public_key(EC_KEY *key, struct aws_byte_cursor pub_x, st if (pub_x_num) { BN_free(pub_y_num); } - + return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8f17cc9f..368a7cc3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -79,7 +79,6 @@ add_test_case(ecdsa_p256_test_key_gen_export) add_test_case(ecdsa_p384_test_key_gen_export) add_test_case(ecdsa_p256_test_import_asn1_key_pair) add_test_case(ecdsa_p256_test_import_asn1_pkcs8_key_pair) -add_test_case(ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair) add_test_case(ecdsa_p384_test_import_asn1_key_pair) add_test_case(ecdsa_test_import_asn1_key_pair_public_only) add_test_case(ecdsa_test_import_asn1_key_pair_invalid_fails) diff --git a/tests/ecc_test.c b/tests/ecc_test.c index 2e9f21b4..5ac34314 100644 --- a/tests/ecc_test.c +++ b/tests/ecc_test.c @@ -529,25 +529,6 @@ static int s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn(struct aws_allocator } AWS_TEST_CASE(ecdsa_p256_test_import_asn1_pkcs8_key_pair, s_ecdsa_p256_test_import_asn1_pkcs8_key_pair_fn) -static int s_ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair_fn(struct aws_allocator *allocator, void *ctx) { - (void)ctx; - - uint8_t asn1_encoded_key_raw[] = { - 0x30, 0x41, 0x02, 0x01, 0x00, 0x30, 0x13, 0x06, 0x07, 0x2A, 0x86, 0x48, - 0xCE, 0x3D, 0x02, 0x01, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, - 0x01, 0x07, 0x04, 0x24, 0x30, 0x22, 0x02, 0x01, 0x01, 0x04, 0x20, 0x18, - 0xCD, 0x6B, 0x61, 0x25, 0xB5, 0x37, 0x61, 0xB8, 0xF4, 0x6A, 0xBA, 0x42, - 0xAF, 0xA9, 0x70, 0x14, 0x9D, 0x72, 0x40, 0x6D, 0x84, 0x00, 0xA8, 0x40, - 0x02, 0x13, 0x86, 0x8C, 0xA9, 0x2D, 0x63 - }; - - struct aws_byte_cursor asn1_encoded_key = - aws_byte_cursor_from_array(asn1_encoded_key_raw, sizeof(asn1_encoded_key_raw)); - - return s_ecdsa_test_import_asn1_key_pair(allocator, asn1_encoded_key, AWS_CAL_ECDSA_P256); -} -AWS_TEST_CASE(ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair, s_ecdsa_p256_test_import_asn1_pkcs8_no_pub_key_pair_fn) - static int s_ecdsa_p384_test_import_asn1_key_pair_fn(struct aws_allocator *allocator, void *ctx) { (void)ctx; From 160e398a99b46e4ebb25bce664acac12b8097c8e Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Thu, 16 Oct 2025 11:26:43 -0700 Subject: [PATCH 28/28] error preconditions --- source/ecc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/ecc.c b/source/ecc.c index 3f157b7f..5784cae3 100644 --- a/source/ecc.c +++ b/source/ecc.c @@ -576,11 +576,11 @@ int aws_der_decoder_load_ecc_key_pair( struct aws_byte_cursor *out_private_d, enum aws_ecc_curve_name *out_curve_name) { - AWS_PRECONDITION(decoder); - AWS_PRECONDITION(out_public_x_coord); - AWS_PRECONDITION(out_public_y_coord); - AWS_PRECONDITION(out_private_d); - AWS_PRECONDITION(out_curve_name); + AWS_ERROR_PRECONDITION(decoder); + AWS_ERROR_PRECONDITION(out_public_x_coord); + AWS_ERROR_PRECONDITION(out_public_y_coord); + AWS_ERROR_PRECONDITION(out_private_d); + AWS_ERROR_PRECONDITION(out_curve_name); /** * Since this is a generic api to parse from ans1, we can encounter several key structures.