PK: inline & simplify verify functions #519

New issue

Open

opened 2025-10-14 05:57:55 -05:00 by mpg · 0 comments

mpg commented

2025-10-14 05:57:55 -05:00

(Migrated from github.com)

This is the same as as #515 but for verify_func. The high-level steps are as follows:

Review existing tests (see below) and possibly complete them.
Refactor
Optional: if the refactoring results in more cases being naturally supported, add corresponding test cases.

For step 1, here's what should be tested:

Non-wrapping context (created by parsing or similar) :
- Positive: RSA (v1.5), ECC - context holding a public key -> check that valid signatures are accepted
- Positive: RSA, ECC - context holds a key pair (private key) -> check that valid signatures are accepted
- Negative: RSA, ECC -> check that invalid signatures of the expected length are rejected: for example, flip a bit in a valid signature
- Negative: RSA, ECC -> check that (invalid) signatures that are too short are rejected cleanly (no overread): for example copy a prefix or a valid signature in a buffer that's one byte shorter.
- Negative: ECC: trying to verify with a key that only allows DH (ECKEY_DH) -> according to lcov this is missing!
Wrapping context (created by pk_wrap_psa()):
- Negative: not supported, should return a clean error.

For step 2, remove use of verify_func pointers then follow your heart (can look at #515 and #518 for inspiration).

For step 3, it may happen that in step 2 we had to add an artificial special case "if wrapping context return not-supported" and the code would actually be simpler without it. In that case, consider removing that now-artificial limitation. This requires thinking about the test plan for this new part, and adding the tests. If that seems too much for a single PR (in addition to the previous steps), it's also OK to skip this now and just open an issue to record your thoughts on the topic (especially the test plan). This step is really optional and should only be done as part of this PR is there's an easy win.

Does not really depend on #515, but likely to have contextual conflicts in pk_wrap.[ch].

This is the same as as #515 but for `verify_func`. The high-level steps are as follows: 1. Review existing tests (see below) and possibly complete them. 2. Refactor 3. Optional: if the refactoring results in more cases being naturally supported, add corresponding test cases. For step 1, here's what should be tested: - Non-wrapping context (created by parsing or similar) : - Positive: RSA (v1.5), ECC - context holding a public key -> check that valid signatures are accepted - Positive: RSA, ECC - context holds a key pair (private key) -> check that valid signatures are accepted - Negative: RSA, ECC -> check that invalid signatures of the expected length are rejected: for example, flip a bit in a valid signature - Negative: RSA, ECC -> check that (invalid) signatures that are too short are rejected cleanly (no overread): for example copy a prefix or a valid signature in a buffer that's one byte shorter. - Negative: ECC: trying to verify with a key that only allows DH (`ECKEY_DH`) -> according to lcov this is missing! - Wrapping context (created by `pk_wrap_psa()`): - Negative: not supported, should return a clean error. For step 2, remove use of `verify_func` pointers then follow your heart (can look at #515 and #518 for inspiration). For step 3, it may happen that in step 2 we had to add an artificial special case "if wrapping context return not-supported" and the code would actually be simpler without it. In that case, consider removing that now-artificial limitation. This requires thinking about the test plan for this new part, and adding the tests. If that seems too much for a single PR (in addition to the previous steps), it's also OK to skip this now and just open an issue to record your thoughts on the topic (especially the test plan). This step is really optional and should only be done as part of this PR is there's an easy win. Does not really depend on #515, but likely to have contextual conflicts in `pk_wrap.[ch]`.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto#519

No description provided.

Rows
Columns