Skip to content

Fix explicit Elliptic Curve parsing #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Marandil wants to merge 6 commits into
base: development
Choose a base branch
from
Open

Fix explicit Elliptic Curve parsing #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d5d81bd
Add failing tests
Marandil Mar 3, 2026
337e93d
Fix explicit curve import
Marandil Mar 4, 2026
8531ded
Add changelog
Marandil Mar 4, 2026
638764c
Add depdencency on MBEDTLS_PK_PARSE_EC_EXTENDED to the new (explicit …
Marandil Mar 5, 2026
d154ad8
Remove internal functions from Changelog
Marandil Mar 5, 2026
4c4fe7b
Cleanup changelog entry
Marandil Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.d/fix-pk_group_from_specified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Bugfix:
* Fix mbedtls_pk_parse_public_key failing to load keys with explicit curve parameters.
Fixes #4314

38 changes: 32 additions & 6 deletions extras/pkparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ static int pk_group_from_specified(const mbedtls_asn1_buf *params, mbedtls_ecp_g
unsigned char *p = params->p;
const unsigned char *const end = params->p + params->len;
const unsigned char *end_field, *end_curve;
mbedtls_mpi tmp;
size_t len;
size_t plen;
int ver;

/* SpecifiedECDomainVersion ::= INTEGER { 1, 2, 3 } */
Expand Down Expand Up @@ -184,6 +186,20 @@ static int pk_group_from_specified(const mbedtls_asn1_buf *params, mbedtls_ecp_g
(ret = mbedtls_mpi_read_binary(&grp->A, p, len)) != 0) {
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret);
}
/*
* If A == -3 mod p, the group is stored with an empty A parameter,
* see mbedtls_ecp_group_a_is_minus_3
*/
mbedtls_mpi_init(&tmp);
if((ret = mbedtls_mpi_sub_int(&tmp, &grp->P, 3)) != 0) {
mbedtls_mpi_free(&tmp);
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret);
}
if(mbedtls_mpi_cmp_mpi(&tmp, &grp->A) == 0) {
mbedtls_mpi_free(&grp->A);
mbedtls_mpi_init(&grp->A);
}
mbedtls_mpi_free(&tmp);

p += len;

Expand Down Expand Up @@ -211,20 +227,30 @@ static int pk_group_from_specified(const mbedtls_asn1_buf *params, mbedtls_ecp_g
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, ret);
}

if ((ret = mbedtls_ecp_point_read_binary(grp, &grp->G,
(const unsigned char *) p, len)) != 0) {
/*
* We can't read the point using mbedtls_ecp_point_read_binary,
* as it recursively uses grp->G form to select the key format.
* Instead we manually parse the point.
*/
plen = mbedtls_mpi_size(&grp->P);
if (len == 2 * plen + 1 && p[0] == 0x04) {
if (mbedtls_mpi_read_binary(&grp->G.X, p + 1, plen) != 0 ||
mbedtls_mpi_read_binary(&grp->G.Y, p + 1 + plen, plen) != 0 ||
mbedtls_mpi_lset(&grp->G.Z, 1) != 0) {
return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT;
}
} else if (len == plen + 1 && (p[0] == 0x02 || p[0] == 0x03)) {
/*
* If we can't read the point because it's compressed, cheat by
* reading only the X coordinate and the parity bit of Y.
*/
if (ret != MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE ||
(p[0] != 0x02 && p[0] != 0x03) ||
len != mbedtls_mpi_size(&grp->P) + 1 ||
mbedtls_mpi_read_binary(&grp->G.X, p + 1, len - 1) != 0 ||
if (mbedtls_mpi_read_binary(&grp->G.X, p + 1, plen) != 0 ||
mbedtls_mpi_lset(&grp->G.Y, p[0] - 2) != 0 ||
mbedtls_mpi_lset(&grp->G.Z, 1) != 0) {
return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT;
}
} else {
return MBEDTLS_ERR_PK_KEY_INVALID_FORMAT;
}

p += len;
Expand Down
2 changes: 1 addition & 1 deletion framework
Open in desktop
Submodule framework updated 13 files
+48 −0 data_files/Makefile
+9 −0 data_files/ec_224_pub.expl.pem
+ − data_files/ec_256_pub.expl.der
+9 −0 data_files/ec_256_pub.expl.pem
+12 −0 data_files/ec_384_pub.expl.pem
+ − data_files/ec_521_pub.expl.der
+15 −0 data_files/ec_521_pub.expl.pem
+9 −0 data_files/ec_bp256_pub.expl.pem
+12 −0 data_files/ec_bp384_pub.expl.pem
+ − data_files/ec_bp512_pub.expl.der
+14 −0 data_files/ec_bp512_pub.expl.pem
+ − data_files/ec_pub.expl.der
+8 −0 data_files/ec_pub.expl.pem
36 changes: 36 additions & 0 deletions tests/suites/test_suite_pkparse.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,14 @@ Parse Public EC Key #4b (RFC 5480, secp256r1, DER)
depends_on:PSA_WANT_ECC_SECP_R1_256
pk_parse_public_keyfile_ec:"../framework/data_files/ec_256_pub.der":0

Parse Public EC Key #4c (RFC 5480, secp256r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_256_pub.expl.pem":0

Parse Public EC Key #4d (RFC 5480, secp256r1, explicit, DER)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED

@valeriosetti valeriosetti May 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MBEDTLS_PEM_PARSE_C but the file is in .der format.

pk_parse_public_keyfile_ec:"../framework/data_files/ec_256_pub.expl.der":0

Parse Public EC Key #5 (RFC 5480, secp384r1)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_SECP_R1_384
pk_parse_public_keyfile_ec:"../framework/data_files/ec_384_pub.pem":0
Expand All @@ -1199,6 +1207,10 @@ Parse Public EC Key #5a (RFC 5480, secp384r1, compressed)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_PARSE_EC_COMPRESSED:MBEDTLS_ECP_DP_SECP384R1_ENABLED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_384_pub.comp.pem":0

Parse Public EC Key #5b (RFC 5480, secp384r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_384_pub.expl.pem":0

Parse Public EC Key #6 (RFC 5480, secp521r1)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_SECP_R1_521
pk_parse_public_keyfile_ec:"../framework/data_files/ec_521_pub.pem":0
Expand All @@ -1207,6 +1219,14 @@ Parse Public EC Key #6a (RFC 5480, secp521r1, compressed)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_PARSE_EC_COMPRESSED:MBEDTLS_ECP_DP_SECP521R1_ENABLED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_521_pub.comp.pem":0

Parse Public EC Key #6b (RFC 5480, secp521r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_521_pub.expl.pem":0

Parse Public EC Key #6c (RFC 5480, secp521r1, explicit, DER)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED

@valeriosetti valeriosetti May 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MBEDTLS_PEM_PARSE_C but the file is in .der format.

pk_parse_public_keyfile_ec:"../framework/data_files/ec_521_pub.expl.der":0

Parse Public EC Key #7 (RFC 5480, brainpoolP256r1)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_BRAINPOOL_P_R1_256
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp256_pub.pem":0
Expand All @@ -1215,6 +1235,10 @@ Parse Public EC Key #7a (RFC 5480, brainpoolP256r1, compressed)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_PARSE_EC_COMPRESSED:MBEDTLS_ECP_DP_BP256R1_ENABLED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp256_pub.comp.pem":0

Parse Public EC Key #7b (RFC 5480, brainpoolP256r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_BP256R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp256_pub.expl.pem":0

Parse Public EC Key #8 (RFC 5480, brainpoolP384r1)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_BRAINPOOL_P_R1_384
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp384_pub.pem":0
Expand All @@ -1223,6 +1247,10 @@ Parse Public EC Key #8a (RFC 5480, brainpoolP384r1, compressed)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_PARSE_EC_COMPRESSED:MBEDTLS_ECP_DP_BP384R1_ENABLED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp384_pub.comp.pem":0

Parse Public EC Key #8b (RFC 5480, brainpoolP384r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_BP384R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp384_pub.expl.pem":0

Parse Public EC Key #9 (RFC 5480, brainpoolP512r1)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_BRAINPOOL_P_R1_512
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp512_pub.pem":0
Expand All @@ -1231,6 +1259,14 @@ Parse Public EC Key #9a (RFC 5480, brainpoolP512r1, compressed)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_PARSE_EC_COMPRESSED:MBEDTLS_ECP_DP_BP512R1_ENABLED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp512_pub.comp.pem":0

Parse Public EC Key #9b (RFC 5480, brainpoolP512r1, explicit)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_BP512R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED
pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp512_pub.expl.pem":0

Parse Public EC Key #9c (RFC 5480, brainpoolP512r1, explicit, DER)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_BP512R1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED

@valeriosetti valeriosetti May 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MBEDTLS_PEM_PARSE_C but the file is in .der format.

pk_parse_public_keyfile_ec:"../framework/data_files/ec_bp512_pub.expl.der":0

Parse Public EC Key #10 (RFC 8410, DER, X25519)
depends_on:MBEDTLS_PEM_PARSE_C:PSA_WANT_ECC_MONTGOMERY_255
pk_parse_public_keyfile_ec:"../framework/data_files/ec_x25519_pub.der":0
Expand Down