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

Detect invalid padding parameters when setting up an RSA context #4523

Closed
gilles-peskine-arm opened this issue May 17, 2021 · 0 comments · Fixed by #4618
Closed

Detect invalid padding parameters when setting up an RSA context #4523

gilles-peskine-arm opened this issue May 17, 2021 · 0 comments · Fixed by #4618
Assignees
Labels
component-crypto Crypto primitives and low-level interfaces enhancement size-s Estimated task size: small (~2d)

Comments

@gilles-peskine-arm
Copy link
Contributor

Context

mbedtls_rsa_init takes two arguments padding and hash_id which can be invalid. However, as an initialization function, it must return void, since it is often called in contexts that cannot jump to error handling because other objects are not initialized yet.

In Mbed TLS 2.16+, there was validation through MBEDTLS_PARAM_FAILED if MBEDTLS_CHECK_PARAMS was enabled. With the removal of MBEDTLS_CHECK_PARAMS, there is no way to enable any validation anymore.

Goal

Change to the API of the RSA module. Important requirements:

  • Make a small change. We don't have much time.
  • mbedtls_rsa_init cannot return errors. (But mbedtls_rsa_set_padding can.)
  • An invalid padding value must be caught, preferably in a way that is easy to debug. In the current API, the padding can be passed to either mbedtls_rsa_init or mbedtls_rsa_set_padding, and it is stored in the RSA context object.
  • Preferably an invalid hash should be caught too. (But note that MBEDTLS_MD_NONE is ok for V15 padding.)
  • Although it is good hygiene for a key to only be used with a single kind of padding, this is not a strong requirement. It may be possible to stop storing the padding parameters in the context altogether.

Possible solutions

This is not an exhaustive list.

  • Stop storing the padding in the context altogether?
  • Have mbedtls_rsa_init set the padding to some documented default, and require the caller to call mbedtls_rsa_set_padding if some other default is desired. Change mbedtls_rsa_set_padding to return MBEDTLS_ERR_RSA_BAD_INPUT_DATA if the arguments are invalid.
  • Other, specify.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-crypto Crypto primitives and low-level interfaces enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants