Update documentation of mbedtls_pk_sign and mbedtls_pk_verify #347

Closed
opened 2025-07-01 14:26:54 -05:00 by gilles-peskine-arm · 4 comments
gilles-peskine-arm commented 2025-07-01 14:26:54 -05:00 (Migrated from github.com)

Document that mbedtls_pk_sign() and mbedtls_pk_verify() are legacy functions, that perform the same algorithm that mbedtls_pk_get_psa_attributes() would pick in the key policy if given a sign or verify usage.

(Context: https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/docs/architecture/pk-4.md#changes-to-private-key-selection-in-tls)

Document that `mbedtls_pk_sign()` and `mbedtls_pk_verify()` are legacy functions, that perform the same algorithm that `mbedtls_pk_get_psa_attributes()` would pick in the key policy if given a sign or verify usage. (Context: https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/docs/architecture/pk-4.md#changes-to-private-key-selection-in-tls)
bjwtaylor commented 2025-08-13 05:55:01 -05:00 (Migrated from github.com)

I believe this is already documented in https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/docs/psa-transition.md#signature-operations, is anything required beyond this?

I believe this is already documented in https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/docs/psa-transition.md#signature-operations, is anything required beyond this?
mpg commented 2025-08-28 02:33:47 -05:00 (Migrated from github.com)

I think this task is to edit the doxygen documentation for those functions. Currently they each have a note about RSA keys that mentions "the padding mode of the underlying rsa context" which is no longer accurate (there is no longer such a thing as "the underlying rsa context) or appropriate (even if there was, users can't access it and shouldn't know about it). I guess this note should be replaced with something mentioning mbedtls_pk_get_psa_attributes() - but keep the part that says to use the _ext() version if you want to explicitly select an alg.

Checking just now, the documentation of mbedtls_pk_get_psa_attributes() also mentions an "underlying context" for RSA keys and again I don't think that's appropriate and it should be edited. I'm not sure off-hand what the new version should be, but I guess something like "if the key was just parsed, that's PKCS#1 v1.5, otherwise if the key was imported from PSA, whatever was set on the PSA side". (Might have to check the code.)

I think this task is to edit the doxygen documentation for those functions. Currently they each have a note about RSA keys that mentions "the padding mode of the underlying rsa context" which is no longer accurate (there is no longer such a thing as "the underlying rsa context) or appropriate (even if there was, users can't access it and shouldn't know about it). I guess this note should be replaced with something mentioning `mbedtls_pk_get_psa_attributes()` - but keep the part that says to use the `_ext()` version if you want to explicitly select an alg. Checking just now, the documentation of `mbedtls_pk_get_psa_attributes()` also mentions an "underlying context" for RSA keys and again I don't think that's appropriate and it should be edited. I'm not sure off-hand what the new version should be, but I guess something like "if the key was just parsed, that's PKCS#1 v1.5, otherwise if the key was imported from PSA, whatever was set on the PSA side". (Might have to check the code.)
bjwtaylor commented 2025-09-01 06:08:56 -05:00 (Migrated from github.com)

Is this not covered by this one https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/205? In which case is this a duplicate, or were we after something else here?

Is this not covered by this one https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/205? In which case is this a duplicate, or were we after something else here?
mpg commented 2025-09-02 02:34:24 -05:00 (Migrated from github.com)

Arguably #205 covers the part about removing references to "the underlying rsa context" as that's no longer public, indeed. It doesn't cover recommending the _ext variants, but OTOH that part is already present.

This issue's description also says to mention that the non-_ext() functions are legacy, which is not covered by #205 though.

Overall, I think it makes sense to do a single PR resolving both #205 and this one.

Arguably #205 covers the part about removing references to "the underlying rsa context" as that's no longer public, indeed. It doesn't cover recommending the `_ext` variants, but OTOH that part is already present. This issue's description also says to mention that the non-`_ext()` functions are legacy, which is not covered by #205 though. Overall, I think it makes sense to do a single PR resolving both #205 and this one.
Sign in to join this conversation.
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#347
No description provided.