Skip to content

PK: remove pk_can_do() -- part 1#615

Open
valeriosetti wants to merge 16 commits intoMbed-TLS:developmentfrom
valeriosetti:issue523
Open

PK: remove pk_can_do() -- part 1#615
valeriosetti wants to merge 16 commits intoMbed-TLS:developmentfrom
valeriosetti:issue523

Conversation

@valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented Dec 18, 2025 •
edited
Loading

Description

TL;DR

Addresses the first part of #523: modify mbedtls_pk_[sign|verify]_ext() so that they don't use mbedtls_pk_can_do().

Details

Commits in this PR can be divided into 3 blocks:

  1. Extending tests for mbedtls_pk_[sign|verify]_ext() in order to cover all possible cases
  2. Re-shape mbedtls_pk_[sign|verify]_ext() so that they really perform as an _ext function over the simpler mbedtls_pk_[sign|verify]() counterparts: i.e. try to sign|verify using the specified sigalg and PK context.
    • This removes a lot of "special case handling" that was done previously
    • This simplifies testing: many comments added at point (1) to explain why [sign|verify]_ext() was behaving differently than expected are removed.
    • For example, after this reshaping opaque RSA keys can perform signature verification while previously this was not possible. And this didn't require any special treatment for the opaque context: just treating all the PK context in the same way allowed to improve the functionality.
  3. Adapt tests that are failing after this change
    • In most of the cases it's just a change to the returned value.
    • As mentioned above, in some other cases there was an improvement in functionality so a test that was failing for lack of support now is passing.
    • There are only changes to PK tests, meaning that "users" of [sign|verify]_ext() are not affected by this functions' reshaping! To me this is a sort of proof that these changes might be potentially transparent for the end user.

Advantage of the reshape

Once this will be merged we can make mbedtls_pk_[sign|verify]() calling into their _ext() counterpart (instead of the restartable ones) as it it normally happens for other "standard"/"extended" functions in the library.

PR checklist

  • changelog not required because: mbedtls_pk_can_do is internal, so nothing to be notified
  • framework PR not required
  • **mbedtls not required because: no change there
  • mbedtls 3.6 PR not required because: no backport
  • tests provided

@valeriosetti valeriosetti added size-s Estimated task size: small (~2d) needs-reviewer This PR needs someone to pick it up for review needs-preceding-pr Requires another PR to be merged first needs-ci Needs to pass CI tests labels Dec 18, 2025
@valeriosetti valeriosetti marked this pull request as ready for review December 22, 2025 08:28
@valeriosetti valeriosetti removed the needs-preceding-pr Requires another PR to be merged first label Jan 23, 2026
@valeriosetti valeriosetti force-pushed the issue523 branch 7 times, most recently from 7cc1d09 to 78fe6c1 Compare February 2, 2026 13:25
@valeriosetti
tests: pk: rename pk_sign_ext
588c023 
The function is testing both "pk_sign_ext" and "pk_verify_ext" and in
follow-up commits it's going to be extended to become the main function
for testing these 2 functions. So this commits just rename it before
any real reshaping happens.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: use PK helpers in pk_sign_ext_verify_ext()
b97a74b 
This will allow to test the same functions with PK contexts generated in
different ways.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: specify PSA key algorithm in pk_sign_ext_verify_ext()
097bc27 
Add one exta param to allow specify which algorithm must be associated to
the PSA key.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: use correct mbedtls_pk_sigalg_t type in pk_sign_ext_verify…
635ebe7 
…_ext

Usage of mbedtls_pk_type_t type as input parameters to sign_ext/verify_ext
belong to the past. These functions nowadays use mbedtls_pk_sigalg_t
types.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: add expected return value params to pk_sign_ext_verify_ext
8865967 
Expected return values are added indipendently for both sign and verify
because wrapped keys might miss some functionality so having them
separate will become handy in future steps.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: remove tests cases with SHA-512 for pk_sign_ext_verify_ext()
83675fb 
There are already 2 hash algorithm being tested: SHA-256 and SHA-384.
Testing 2 is already enough and the 3rd one doesn't bring any useful
info when it comes to sign_ext/verify_ext.
Moreover tests are going to be expanded soon, so adding extra complexity
doesn't help.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: test PK context with wrapped key in pk_sign_ext_verify_ext()
48e1189 
Previous testing was limited to PK contexts that were copied from PSA.
Now we also test the wrapped case (i.e. what was once named "opaque").

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: remove pk_psa_wrap_sign_ext() test function and relative data
b93e6b8 
This kind of tests are now covered from "pk_sign_ext_verify_ext()".

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: complete testing of pk_sign_ext_verify_ext
310824e 
Add more positive and negative testing for all the relevant cases.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
drivers: pk: simplify and improve mbedtls_pk_sign_ext
c674cd2 
Just try to perform a signature with the given signature and MD algorithms.
Rely on psa_sign_hash() to check if the PSA key in the PK context supports
the operation of not.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
drivers: pk: simplify and improve mbedtls_pk_verify_ext()
4ed87d0 
Similarly to what has been done with mbedtls_pk_sign_ext() this commit
modifies mbedtls_pk_verify_ext() so that it tries to perform a signature
verification using the signature and MD algorithms provided as input.
psa_verify_hash() will determine if the provided PK context can perform
the requested operation or not.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: fix failures pk_rsa_verify_ext_test_vec()
eebc85f 
Adapt the test function to recent changes in mbedtls_pk_verify_ext().

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: fix failures for pk_sign_ext_verify_ext
5a5bb78 
Adapt pk_sign_ext_verify_ext to recent changes in sign_ext and verify_ext.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: fix failures in pk_rsa_overflow()
2813b6b 
Adapt pk_rsa_overflow() to recent changes in pk_sign_ext() and
pk_verify_ext().

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
drivers: pk: add new error code for signature failure
fe33401 
Now that "pk_[sign|verify]_ext()" work the same for all PK context types
it doesn't make sense to translate "PSA_ERROR_INVALID_SIGNATURE" to
"MBEDTLS_ERR_RSA_VERIFY_FAILED". Besides this translation is only available
when RSA key types are defined, which is not always true.

For this reason this commit introduces a more generic
"MBEDTLS_ERR_PK_INVALID_SIGNATURE" which is valid for any PK context handled
by PK module. The translation is trivially to "PSA_ERROR_INVALID_SIGNATURE"
of course.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti
tests: pk: add MBEDTLS_PSA_CRYPTO_C dependency for verify_ext with RS…
49c55b4 
…A-PSS

This change is related to recent changes that have been done to
mbedtls_pk_verify_ext() and the explanation is as follows.

Ideally the public key material in the RSA context would be OK to perform
RSA-PSS verification if this were the main algorithm associated with the
key once imported to PSA. However when an RSA key-pair is imported to
PK do a signature the main algorithm is always PKCS v1.5 and PSS is the
enrollment one (which is gated by MBEDTLS_PSA_CRYPTO_C).
As a consequence, for sake of symmetrical behavior, the same algorithms
should be used for verification, meaning that in order to have RSA-PSS
successfully working with verify_ext() it requires MBEDTLS_PSA_CRYPTO_C
to be enabled.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
@valeriosetti valeriosetti added needs-review Every commit must be reviewed by at least two team members priority-high High priority - will be reviewed soon and removed needs-ci Needs to pass CI tests labels Feb 3, 2026
@valeriosetti valeriosetti changed the title PK: remove pk_can_do() PK: remove pk_can_do() -- part 1 Feb 3, 2026
@bjwtaylor bjwtaylor self-requested a review February 3, 2026 11:41
bjwtaylor
bjwtaylor reviewed Feb 3, 2026
View reviewed changes
bjwtaylor
bjwtaylor reviewed Feb 5, 2026
View reviewed changes
Copy link
Contributor

@bjwtaylor bjwtaylor left a comment

Choose a reason for hiding this comment

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

as per #615 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bjwtaylor bjwtaylor bjwtaylor left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

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 priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)

Projects

Roadmap pull requests (new board)
Status: In Development

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@valeriosetti @bjwtaylor