Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove policy enforcement from RSA #8492

Closed
gilles-peskine-arm opened this issue Nov 7, 2023 · 3 comments
Closed

Remove policy enforcement from RSA #8492

gilles-peskine-arm opened this issue Nov 7, 2023 · 3 comments
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-crypto Crypto primitives and low-level interfaces size-m Estimated task size: medium (~1w)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Nov 7, 2023

Remove policy enforcement from the RSA module. Instead of configuring a key to use a specific padding mode and hash, let each function call decide what to use.

Remove mbedtls_rsa_pkcs1_sign, mbedtls_rsa_pkcs1_verify, mbedtls_rsa_pkcs1_encrypt and mbedtls_rsa_pkcs1_decrypt. Let the caller call a v15/OAEP/PSS-specific function.

Create a separate mbedtls_pk_type_t value for OAEP (there are already separate types for RSA, meaning v15, and RSA-PSS). Use that to choose the function to dispatch to in pk.

PSA already knows the key's policy and the requested algorithm, so we just need to make it call the appropriate function.

Justification: having policy enforcement both in RSA (but partially and clumsily) and in higher-level modules is confusing. See for example the long discussion in #7930. It's simpler to let rsa.c focus on doing the calculations, and let pk.c and psa_crypto*.c worry about policies.

Prerequisite: #8452

@gilles-peskine-arm gilles-peskine-arm added component-crypto Crypto primitives and low-level interfaces api-break This issue/PR breaks the API and must wait for a new major version size-m Estimated task size: medium (~1w) labels Nov 7, 2023
@mpg
Copy link
Contributor

mpg commented Nov 20, 2023

Create a separate mbedtls_pk_type_t value for OAEP (there are already separate types for RSA, meaning v15, and RSA-PSS). Use that to choose the function to dispatch to in pk.

I'm not sure it is or should be the job of PK to enforce policies. We already have a well-designed API that handles policies: the PSA Crypto API.

The PK API on the other hand was not well designed and not with policies in mind. We are (probably) keeping it in 4.0 mostly for backwards compatibility purposes. If we try to extend it to enforce policies, we'll be partially negating the backwards compatibility benefits of keeping it, and we'll be fighting an uphill battle trying to extend a currently inconsistent API. I'm not sure that's something we want to do at this point.

Note: the RSA-PSS type is not actually a key type in that you can't create a key of that type; it's just a value you can pass to some _ext functions to specify what signature algorithm you want to use.

I think we probably need to have a holistic discussion about the future of PK at some point.

@gilles-peskine-arm
Copy link
Contributor Author

I'm not sure it is or should be the job of PK to enforce policies.

PK needs to know what algorithm to use when calling mbedtls_pk_{sign,verify,encrypt,decrypt}. ECC signature gets away with a compile-time variant choice because the only two supported variants (deterministic/randomized ECDSA) are functionally compatible. RSA needs the ext functions for PSS, but that defeats the original purpose of the PK design, and it would make more sense to encode the algorithm in the context object.

@gilles-peskine-arm
Copy link
Contributor Author

  • rsa.h is becoming internal, so changes in rsa.h are no longer API breaks, so this part is not a 4.0 issue.
  • The changes to pk.h are part of a larger redesign of pk.h which may or may not render the issue moot. Evolution of pk.h in 4.0 #8452

@gilles-peskine-arm gilles-peskine-arm closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2024
@github-project-automation github-project-automation bot moved this from Blocked to Done in Mbed TLS 4.0 planning Aug 8, 2024
@github-project-automation github-project-automation bot moved this to Mbed TLS 4.0 SHOULD in Backlog for Mbed TLS Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-break This issue/PR breaks the API and must wait for a new major version component-crypto Crypto primitives and low-level interfaces size-m Estimated task size: medium (~1w)
Projects
Status: Mbed TLS 4.0 SHOULD
Status: Done
Development

No branches or pull requests

2 participants