gh-Mbed-TLS/TF-PSA-Crypto

Watch

Fork 0

Guard removed mechanisms with TF_PSA_CRYPTO_REMOVED_MECHANISMS #233

New issue

Closed

opened 2025-03-23 18:38:14 -05:00 by gilles-peskine-arm · 5 comments

gilles-peskine-arm commented

2025-03-23 18:38:14 -05:00

(Migrated from github.com)

Define a configuration option TF_PSA_CRYPTO_REMOVED_MECHANISMS, disabled by default, but enabled in the full configuration. Document that users must not enable this, and we make no compatibility or security promise when this option is enabled. This option does not need to (in fact, probably should not) appear in the Doxygen rendered documentation.

This allows us to keep a cryptographic mechanism in the code base, but not have it as part of the public API.

Follow-ups:

Use this to privatize DES (used to decrypt test keys).
Use this to privatize small curves (used to test various parts of ECC and PK).

Planning note: I don't know how much work the follow-ups will be, e.g. with respect to depends.py. If it's easy, we might as well go ahead and to it in the same pull request.

Define a configuration option `TF_PSA_CRYPTO_REMOVED_MECHANISMS`, disabled by default, but enabled in the `full` configuration. Document that users must not enable this, and we make no compatibility or security promise when this option is enabled. This option does not need to (in fact, probably should not) appear in the Doxygen rendered documentation. This allows us to keep a cryptographic mechanism in the code base, but not have it as part of the public API. Follow-ups: * Use this to privatize [DES](https://github.com/Mbed-TLS/mbedtls/issues/9164) (used to decrypt test keys). * Use this to privatize [small curves](https://github.com/Mbed-TLS/mbedtls/issues/8136) (used to test various parts of ECC and PK). Planning note: I don't know how much work the follow-ups will be, e.g. with respect to `depends.py`. If it's easy, we might as well go ahead and to it in the same pull request.

gilles-peskine-arm commented

2025-06-24 08:56:53 -05:00

(Migrated from github.com)

disabled by default, but enabled in the full configuration

I'm not sure about that. In https://github.com/Mbed-TLS/mbedtls/issues/8136#issuecomment-2999891880 and https://github.com/Mbed-TLS/mbedtls/issues/9164#issuecomment-2999885985, I'd forgotten what I wrote here earlier and thought not to enable it in the full configuration.

> disabled by default, but enabled in the `full` configuration I'm not sure about that. In https://github.com/Mbed-TLS/mbedtls/issues/8136#issuecomment-2999891880 and https://github.com/Mbed-TLS/mbedtls/issues/9164#issuecomment-2999885985, I'd forgotten what I wrote here earlier and thought not to enable it in the `full` configuration.

gilles-peskine-arm commented

2025-08-06 03:51:34 -05:00

(Migrated from github.com)

August update: we're fully removing 224-bit curves and we're fully removing DES. We aren't removing 192-bit curves yet, however, due to the nontrivial work involved in porting test cases.

So the only crypto that's removed-but-not-really is the two 192-bit curves (secp192k1 and secp192r1). We don't need a generic mechanism for that. So I propose to keep it simple:

Keep PSA_WANT_ECC_SECP_K1_192 and PSA_WANT_ECC_SECP_R1_192 referenced in crypto_config.h, but disabled by default, and with documentation that says that these are not part of the API and will be removed.
Let the curves be picked up by full, so we keep the test coverage we want.
Optionally tweak depends.py so that it doesn't try to do builds where a 192-bit curve is the only curve — I'm not sure it matters.
Do remove all references to 192-bit curves on the mbedtls side (except if needed in depends.py).

I think at this point it's the easiest approach, but I haven't dug deep and I have no strong feelings about this. Whatever works to achieve the requirements, which are:

192-bit curves are clearly not part of the documented API of TF-PSA-Crypto 1.0.
Existing test cases using 192-bit curves are still executed by all.sh (we don't remove them and we don't add exceptions to analyze_outcomes.py).

August update: we're [fully removing 224-bit curves](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/408) and we're [fully removing DES](https://github.com/Mbed-TLS/mbedtls/issues/9164). We aren't removing 192-bit curves yet, however, due to the nontrivial work involved in porting test cases. So the only crypto that's removed-but-not-really is the two 192-bit curves (secp192k1 and secp192r1). We don't need a generic mechanism for that. So I propose to keep it simple: * Keep `PSA_WANT_ECC_SECP_K1_192` and `PSA_WANT_ECC_SECP_R1_192` referenced in `crypto_config.h`, but disabled by default, and with documentation that says that these are not part of the API and will be removed. * Let the curves be picked up by `full`, so we keep the test coverage we want. * Optionally tweak `depends.py` so that it doesn't try to do builds where a 192-bit curve is the only curve — I'm not sure it matters. * Do remove all references to 192-bit curves on the mbedtls side (except if needed in `depends.py`). I think at this point it's the easiest approach, but I haven't dug deep and I have no strong feelings about this. Whatever works to achieve the requirements, which are: * 192-bit curves are clearly not part of the documented API of TF-PSA-Crypto 1.0. * Existing test cases using 192-bit curves are still executed by `all.sh` (we don't remove them and we don't add exceptions to `analyze_outcomes.py`).

mpg commented

2025-09-25 03:28:40 -05:00

(Migrated from github.com)

@gilles-peskine-arm Looking at the issue description and comment above, this looks independent from https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/269 and could be picked up now. However when I asked, you said it's better to wait for #269 but I didn't understand the details. Can you enlighten me?

gilles-peskine-arm commented

2025-09-25 03:46:54 -05:00

(Migrated from github.com)

#269 starts complaining if you try to enable a macro that isn't listed in crypto_config.h, but appears in *adjust*.h. There's a way to disable this complaint, which is to define TF_PSA_CRYPTO_CONFIG_CHECK_BYPASS. So TF_PSA_CRYPTO_CONFIG_CHECK_BYPASS already does pretty much what this issue requires of TF_PSA_CRYPTO_REMOVED_MECHANISMS. So we don't really need a separate mechanism.

In any case, I think the state we're in now, where PSA_WANT_ECC_SECP_K1_192 and PSA_WANT_ECC_SECP_R1_192 still appear in crypto_config.h but have a comment saying not to enable them, is satisfactory, and we don't need a further technical mechanism. So I'm closing this issue, it's something we don't need as a distinct feature after all.

#269 starts complaining if you try to enable a macro that isn't listed in `crypto_config.h`, but appears in `*adjust*.h`. There's a way to disable this complaint, which is to define `TF_PSA_CRYPTO_CONFIG_CHECK_BYPASS`. So `TF_PSA_CRYPTO_CONFIG_CHECK_BYPASS` already does pretty much what this issue requires of `TF_PSA_CRYPTO_REMOVED_MECHANISMS`. So we don't really need a separate mechanism. In any case, I think the state we're in now, where `PSA_WANT_ECC_SECP_K1_192` and `PSA_WANT_ECC_SECP_R1_192` still appear in `crypto_config.h` but have a comment saying not to enable them, is satisfactory, and we don't need a further technical mechanism. So I'm closing this issue, it's something we don't need as a distinct feature after all.

mpg commented

2025-09-25 04:08:51 -05:00

(Migrated from github.com)

In any case, I think the state we're in now, where PSA_WANT_ECC_SECP_K1_192 and PSA_WANT_ECC_SECP_R1_192 still appear in crypto_config.h but have a comment saying not to enable them, is satisfactory,

I don't find that state fully satisfactory. What about people who had their own standalone crypto_config.h from 3.6 and are trying to use it unchanged with 1.0? Then in 1.0 they'd have these curves enabled and wouldn't notice they've been removed unless they spot them in the middle of the very long ChangeLog. Then in 1.1 the curves would disappear.

I think it would be nicer for these people to get a compile-time error in 1.0 already. I think the issue as it was described was a good way to achiever that.

> In any case, I think the state we're in now, where `PSA_WANT_ECC_SECP_K1_192` and `PSA_WANT_ECC_SECP_R1_192` still appear in `crypto_config.h` but have a comment saying not to enable them, is satisfactory, I don't find that state fully satisfactory. What about people who had their own standalone `crypto_config.h` from 3.6 and are trying to use it unchanged with 1.0? Then in 1.0 they'd have these curves enabled and wouldn't notice they've been removed unless they spot them in the middle of the very long ChangeLog. Then in 1.1 the curves would disappear. I think it would be nicer for these people to get a compile-time error in 1.0 already. I think the issue as it was described was a good way to achiever that.

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#233

No description provided.

Rows
Columns