Unfortunately, I don't think I agree with that change: some private key operations might need to use the public exponent for counter-measures against some attacks. For example, currently our code for PKCS#1 v1.5 signing uses the public exponent in order to verify the signature as a counter-measure against Lenstra's attack, via a call to mbedtls_rsa_public(): github.com/ARMmbed/mbed-crypto@4fca6b1a15/library/rsa.c (L2074)

So I don't think it would be appropriate to remove this check for private keys. Is there a particular reason why you wanted to remove this check?

Hi @mvardan, and thanks for your contribution! Unfortunately, I don't think I agree with that change: some private key operations might need to use the public exponent for counter-measures against some attacks. For example, currently our code for PKCS#1 v1.5 signing uses the public exponent in order to verify the signature as a counter-measure against [Lenstra's attack](https://people.redhat.com/~fweimer/rsa-crt-leaks.pdf), via a call to `mbedtls_rsa_public()`: https://github.com/ARMmbed/mbed-crypto/blob/4fca6b1a15fb3575f7f3dfc284edd6ca39f79170/library/rsa.c#L2074 So I don't think it would be appropriate to remove this check for private keys. Is there a particular reason why you wanted to remove this check?

mvardan commented

2020-03-23 12:28:22 -05:00

(Migrated from github.com)

Hi @mpg, thank you for your detailed feedback,

I have source code that currently works with custom HW, I wanted to try mbed_tls as an alternative SW implementation. In my case, I have API which is wrapping mbedtls_rsa_private() and that API doesn't have public exponent.

As a user, in some cases, I would like to have the possibility to perform mbedtls_rsa_*** APIs with minimal input. Is it an option to make this configurable? If so I will be happy to refactor this and add a new configuration.

BTW: In the particular use case which was described by you (PKCS#1 v1.5 signing), mbedtls_rsa_public() will still fail, it will error out from here.

Hi @mpg, thank you for your detailed feedback, I have source code that currently works with custom HW, I wanted to try mbed_tls as an alternative SW implementation. In my case, I have API which is wrapping mbedtls_rsa_private() and that API doesn't have public exponent. As a user, in some cases, I would like to have the possibility to perform mbedtls_rsa_*** APIs with minimal input. Is it an option to make this configurable? If so I will be happy to refactor this and add a new configuration. BTW: In the particular use case which was described by you (PKCS#1 v1.5 signing), mbedtls_rsa_public() will still fail, it will error out from [here](https://github.com/ARMmbed/mbed-crypto/blob/4fca6b1a15fb3575f7f3dfc284edd6ca39f79170/library/rsa.c#L2092).

gilles-peskine-arm (Migrated from github.com) requested changes

2020-03-23 13:22:03 -05:00

gilles-peskine-arm (Migrated from github.com) left a comment

Building an RSA key object which doesn't have the public exponent isn't explicitly forbidden, but you have to fill the mbedtls_rsa_context structure manually. We're planning to remove this ability in Mbed TLS 3.0, primarily because it's incompatible with hardware-accelerated alternative implementations that use a different key representation. If you use mbedtls_rsa_import to fill the key structure, you must finish by calling mbedtls_rsa_complete which will reject a key without E.

We could expand mbedtls_rsa_complete to support key structures that have N, D, P and Q but not E. But I'm reluctant to do that because keys without E are rare in practice, so it would cost code size and development effort for very limited benefits.

If you don't mind discussing it publicly, what environment are you working in? What interface do you need to implement: do you have only N and D or also the CRT parameters? Is E only ever 3 or 65537 or can it have other values?

Building an RSA key object which doesn't have the public exponent isn't explicitly forbidden, but you have to fill the `mbedtls_rsa_context` structure manually. We're planning to remove this ability in Mbed TLS 3.0, primarily because it's incompatible with hardware-accelerated alternative implementations that use a different key representation. If you use `mbedtls_rsa_import` to fill the key structure, [you must finish by calling `mbedtls_rsa_complete`](https://github.com/ARMmbed/mbed-crypto/blob/mbedcrypto-3.0.1/include/mbedtls/rsa.h#L179) which will reject a key without E. We could expand `mbedtls_rsa_complete` to support key structures that have N, D, P and Q but not E. But I'm reluctant to do that because keys without E are rare in practice, so it would cost code size and development effort for very limited benefits. If you don't mind discussing it publicly, what environment are you working in? What interface do you need to implement: do you have only N and D or also the CRT parameters? Is E only ever 3 or 65537 or can it have other values?

mvardan commented

2020-03-23 17:37:06 -05:00

(Migrated from github.com)

Unfortunately, I can't provide more details about my environment.

I have added a test-case with a possible use-case. I am not pretending to have this merged.
Currently mbedtls_rsa_import() allows filling mbedtls_rsa_context having only N and D without E and with this trick, it will correctly perform mbedtls_rsa_private().

I understand that it will be costly to provide a solid solution, but if you'll decide to add support for this use case, I will be happy to spend some time contributing to open-source.

Unfortunately, I can't provide more details about my environment. I have added a test-case with a possible use-case. I am not pretending to have this merged. Currently `mbedtls_rsa_import()` allows filling `mbedtls_rsa_context` having only N and D without E and with this trick, it will correctly perform `mbedtls_rsa_private()`. I understand that it will be costly to provide a solid solution, but if you'll decide to add support for this use case, I will be happy to spend some time contributing to open-source.

gilles-peskine-arm (Migrated from github.com) requested changes

2020-03-24 03:54:00 -05:00

gilles-peskine-arm (Migrated from github.com) left a comment

I am against a change that is only useful if you call mbedtls_rsa_import without calling mbedtls_rsa_complete afterwards, since this is explicitly forbidden by the documentation.

I'm open to changing mbedtls_rsa_complete to reconstruct E from the modulus and the CRT parameters. But I'm a bit worried about the code size increase. A compile-time option is of course a possibility, but we're trying to cut down on those because each compile-time option adds to the maintenance and testing burden.

I am against a change that is only useful if you call `mbedtls_rsa_import` without calling `mbedtls_rsa_complete` afterwards, since this is explicitly forbidden by the documentation. I'm open to changing `mbedtls_rsa_complete` to reconstruct E from the modulus and the CRT parameters. But I'm a bit worried about the code size increase. A compile-time option is of course a possibility, but we're trying to cut down on those because each compile-time option adds to the maintenance and testing burden.

tests/suites/test_suite_rsa.function

The documentation of mbedtls_rsa_import states that you must call mbedtls_rsa_complete afterwards. I disagree with lifting this restriction.

C being C, it isn't currently enforced at the language level. But we don't guarantee that it will keep working in the next version of Mbed TLS. And please note that the next major version (3.0) is likely to remove these low-level functions from the public interface.

The documentation of `mbedtls_rsa_import` states that you must call `mbedtls_rsa_complete` afterwards. I disagree with lifting this restriction. C being C, it isn't currently enforced at the language level. But we don't guarantee that it will keep working in the next version of Mbed TLS. And please note that the next major version (3.0) is likely to remove these low-level functions from the public interface.

gilles-peskine-arm commented

2023-05-24 13:42:31 -05:00

(Migrated from github.com)

This pull request has been abandoned and the maintener team has no intention of pursuing this feature.

Mbed TLS (which has reintegrated Mbed Crypto) is moving away from exposing the RSA API directly: a future major version will remove rsa.h from the public API. Cryptography should go through the PSA API. If you need to do something that the PSA Crypto API does not allow, please raise an issue against Mbed TLS or against the PSA API.

This pull request has been abandoned and the maintener team has no intention of pursuing this feature. Mbed TLS (which has reintegrated Mbed Crypto) is moving away from exposing the RSA API directly: a future major version will remove `rsa.h` from the public API. Cryptography should go through the PSA API. If you need to do something that the PSA Crypto API does not allow, please raise an issue against [Mbed TLS](https://github.com/Mbed-TLS/mbedtls/issues) or against [the PSA API](https://github.com/ARM-software/psa-api/issues).