Skip to content

PK: inline & simplify sign_func #515

@mpg

Description

@mpg

This is an internal refactoring that should be completely transparent to users. Before the refactoring with want to double-check that we have sufficient testing, and add tests if necessary.

Things that should be tested:

  • Non-wrapping context:
    • Positive: RSA (v1.5), ECC (see "common" below)
    • Negative: trying to sign with a public key (seems missing?)
    • Negative: trying to sign with a key that doesn't allow signing (ECKEY_DH) -> according to lcov this is missing!
  • Wrapping context (created with pk_wrap_psa()):
    • Positive: RSA (v1.5 and v2.1 (anysalt or not)), ECC (plain ECDSA or deterministic) (see "common" below)
    • Negative: the key's usage bits don't allow signing
    • Negative: the key's algorithm is not a signature algorithm (eg RSA decrypt or ECDH)
  • Common:
    • Positive: check the result with pk_verify() or verify_ext(). When we know deterministic ECDSA will be used, check the signature output against test vectors.
    • Negative: the output buffer is too small (seems missing?)

Once testing is in good shape, we want to:

  • Move the body of all sign_func functions to the body of pk_sign_restartable().
  • Merge as much as possible of the various branches.
  • Avoid reliance on the PK type as much as possible.
  • Do any other cleanup or simplification we can think of.

See Manuel's prototype: #518

Note: we should not change observable behaviour, except that on error cases we can return a different error code, as long as the documentation was not promising a specific error code for this particular case (and the new code makes sense of course).

Scope

This issue is about the public function mbedtls_pk_sign() and the corresponding sign_func function pointers. The fact the mbedtls_pk_sign() happens to be implemented as the non-restartable case of mbedtls_pk_sign_restartable() is an implementation detail. The testing part of this issue is all about ensuring mbedtls_pk_sign() is tested appropriately.

Other issues in this series:

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestsize-sEstimated task size: small (~2d)

Type

Projects

Status

PK Rework Completion

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions