Inline check_pair functions #580

bjwtaylor · 2025-11-21T11:41:32Z

Description

Inline check_pair functions resolves #521

PR checklist

changelog not required because: No public changes
framework PR not required
mbedtls development PR not required because: No changes
mbedtls 3.6 PRnot required because: No changes
tests provided

davidhorstmann-arm

A few small things and questions, but looks good otherwise!

davidhorstmann-arm · 2025-12-10T11:36:24Z

tests/suites/test_suite_pk.function

        TEST_EQUAL(mbedtls_pk_check_pair(&pub, &prv), ret);
    } else {
-        TEST_EQUAL(mbedtls_pk_check_pair(&pub, &prv), MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE);
+        TEST_EQUAL(mbedtls_pk_check_pair(&pub, &prv), MBEDTLS_ERR_ASN1_BUF_TOO_SMALL);


Why did this need changing? Was this code path not exercised before this change?

So, this was because the previous implementation was able to check the function pointer to check_pair in the structure to see if it was supported. With the new architecture this isn't available, so it progresses further before failing with a different error. You make a good point though, so I've added additional logic to return the same error as before.

davidhorstmann-arm · 2025-12-10T13:52:51Z

drivers/builtin/src/pk.c


-    return prv->pk_info->check_pair_func((mbedtls_pk_context *) pub,
-                                         (mbedtls_pk_context *) prv);
+    if ((prv->pk_info->type == MBEDTLS_PK_RSA) || (prv->pk_info->type == MBEDTLS_PK_RSASSA_PSS)) {


Since #532 has now been merged, could you change this to use mbedtls_pk_get_key_type()?

Suggested change

if ((prv->pk_info->type == MBEDTLS_PK_RSA) || (prv->pk_info->type == MBEDTLS_PK_RSASSA_PSS)) {

if (PSA_KEY_TYPE_IS_RSA(mbedtls_pk_get_key_type(prv))) {

I've replaced these calls with the new mbedtls_pk_get_key_type function.

davidhorstmann-arm · 2025-12-10T14:33:25Z

drivers/builtin/src/pk.c

    if ((prv->pk_info->type != MBEDTLS_PK_OPAQUE) &&
-        (pub->pk_info != prv->pk_info)) {
+        (pub->pk_info != prv->pk_info) &&
+        (pub->pk_info->type != prv->pk_info->type)) {


What was this check for originally and why does it need a new explicit check for the type?

Are there cases where the pk_info pointers are different but the types are the same? My understanding was that this could not happen as all of the possible pk_info structs are initialized statically (I think it's in pk_wrap.c).

Yes, this is a good point. I've removed the type check.

davidhorstmann-arm · 2025-12-10T14:50:08Z

Module	Base	This PR	Delta	% Delta
pk	3963	4043	+80	+2.02%
pk_wrap	2090	1958	-132	-6.32%
TOTAL	209749	209697	-52	-0.02%

52B code size reduction, thanks!

Add additional tests for the check_pair functionality, as some test cases are currently not covered. This is done first to validate that the refactoring and inline in future commits is correct. Signed-off-by: Ben Taylor <[email protected]>

Add the new check pair functionality, this is a refactored and inlined version of all of the original check pair functions. It therefore provides the same functionality with a smaller code size. Signed-off-by: Ben Taylor <[email protected]>

Remove the legacy functions and pointers as these are now replaced by the new inlined check_pair functionality. Thisis functionaly equivelent to the original code with the exception of some error codes. Signed-off-by: Ben Taylor <[email protected]>

Signed-off-by: Ben Taylor <[email protected]>

…lready checked Signed-off-by: Ben Taylor <[email protected]>

Signed-off-by: Ben Taylor <[email protected]>

bjwtaylor force-pushed the inline-check_pair branch 3 times, most recently from 1e12dba to 22b0400 Compare December 1, 2025 09:00

bjwtaylor marked this pull request as ready for review December 1, 2025 14:06

bjwtaylor added needs-review Every commit must be reviewed by at least two team members needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Dec 1, 2025

bjwtaylor force-pushed the inline-check_pair branch from 22b0400 to f231ae8 Compare December 3, 2025 13:19

bjwtaylor added needs-ci Needs to pass CI tests needs-work labels Dec 3, 2025

bjwtaylor force-pushed the inline-check_pair branch from f231ae8 to 851cfba Compare December 3, 2025 15:51

valeriosetti self-requested a review December 5, 2025 13:18

valeriosetti removed needs-work needs-ci Needs to pass CI tests labels Dec 5, 2025

davidhorstmann-arm self-requested a review December 8, 2025 09:42

davidhorstmann-arm removed the needs-reviewer This PR needs someone to pick it up for review label Dec 8, 2025

davidhorstmann-arm requested changes Dec 10, 2025

View reviewed changes

bjwtaylor added the needs-work label Jan 7, 2026

bjwtaylor force-pushed the inline-check_pair branch from d7dc92f to fcec227 Compare January 7, 2026 09:33

Ben Taylor added 5 commits January 8, 2026 15:23

Add additional tests for check_pair

23dc4ed

Add additional tests for the check_pair functionality, as some test cases are currently not covered. This is done first to validate that the refactoring and inline in future commits is correct. Signed-off-by: Ben Taylor <[email protected]>

Add new check_pair functionality

37d21b0

Add the new check pair functionality, this is a refactored and inlined version of all of the original check pair functions. It therefore provides the same functionality with a smaller code size. Signed-off-by: Ben Taylor <[email protected]>

Remove legacy functions and pointers

19a518d

Remove the legacy functions and pointers as these are now replaced by the new inlined check_pair functionality. Thisis functionaly equivelent to the original code with the exception of some error codes. Signed-off-by: Ben Taylor <[email protected]>

restore PSA_WANT_ECC_SECP_R1_512

df8b90f

Signed-off-by: Ben Taylor <[email protected]>

Corrected typo in definitions for check pair tests in pk

2810786

Signed-off-by: Ben Taylor <[email protected]>

Ben Taylor added 3 commits January 8, 2026 15:23

Added both 521 and 256 bit ecc curves

7401c13

Signed-off-by: Ben Taylor <[email protected]>

Add additional code to deal with unsupported configurations

3d881dc

Signed-off-by: Ben Taylor <[email protected]>

Update to use the newly added function mbedtls_pk_get_key_type

14c3b8a

Signed-off-by: Ben Taylor <[email protected]>

bjwtaylor force-pushed the inline-check_pair branch from fb547b3 to aceaf43 Compare January 8, 2026 15:24

Remove check for pk type, as it is not required when the pk_info is a…

753d5dd

…lready checked Signed-off-by: Ben Taylor <[email protected]>

bjwtaylor force-pushed the inline-check_pair branch from aceaf43 to 753d5dd Compare January 8, 2026 15:30

Ben Taylor added 2 commits January 8, 2026 15:33

Add check for key size

0321a06

Signed-off-by: Ben Taylor <[email protected]>

Correcting code style

c99c4f4

Signed-off-by: Ben Taylor <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Inline check_pair functions #580

Inline check_pair functions #580

bjwtaylor commented Nov 21, 2025

Uh oh!

davidhorstmann-arm left a comment

Uh oh!

davidhorstmann-arm Dec 10, 2025

Uh oh!

bjwtaylor Jan 8, 2026

Uh oh!

davidhorstmann-arm Dec 10, 2025

Uh oh!

bjwtaylor Jan 8, 2026 •

edited

Loading

Uh oh!

davidhorstmann-arm Dec 10, 2025

Uh oh!

bjwtaylor Jan 8, 2026

Uh oh!

davidhorstmann-arm commented Dec 10, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if ((prv->pk_info->type == MBEDTLS_PK_RSA) \|\| (prv->pk_info->type == MBEDTLS_PK_RSASSA_PSS)) {
	if (PSA_KEY_TYPE_IS_RSA(mbedtls_pk_get_key_type(prv))) {

Inline check_pair functions #580

Are you sure you want to change the base?

Inline check_pair functions #580

Conversation

bjwtaylor commented Nov 21, 2025

Description

PR checklist

Uh oh!

davidhorstmann-arm left a comment

Choose a reason for hiding this comment

Uh oh!

davidhorstmann-arm Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

bjwtaylor Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

davidhorstmann-arm Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

bjwtaylor Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidhorstmann-arm Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

bjwtaylor Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

davidhorstmann-arm commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bjwtaylor Jan 8, 2026 •

edited

Loading

davidhorstmann-arm commented Dec 10, 2025 •

edited

Loading