Plans for TF-PSA-Crypto legacy headers #145

gilles-peskine-arm · 2025-01-09T21:41:09Z

Work in progress on Mbed-TLS/mbedtls#9882.

Status: this is a work in progress that I'm progressively refining. However, for the parts that are already written, I'm not planning any restructuring or any changes other than clarifications or refinement (e.g. finer task breakdown, closing down open questions), so they are ready for review.

PR checklist

changelog not required because: doc only
framework PR not required
mbedtls PR not required
tests provided | not required because:

Covers high-level plans for header privatization. Signed-off-by: Gilles Peskine <[email protected]>

mpg

First draft is looking pretty good so far, thanks!

docs/architecture/0e-plans.md

Signed-off-by: Gilles Peskine <[email protected]>

It's public specifically for the sake of X.509, not for application code. Signed-off-by: Gilles Peskine <[email protected]>

Signed-off-by: Gilles Peskine <[email protected]>

gilles-peskine-arm · 2025-01-14T20:30:42Z

Thanks for the review. I've addressed your comments, but not yet added the missing sections that I'm still working on.

mpg

Thanks for addressing my comments!

docs/architecture/0e-plans.md

mpg · 2025-01-15T09:08:26Z

docs/architecture/0e-plans.md

+The following header files define cryptography-adjacent interfaces which we have no plans to replace.
+
+* `asn1.`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.
+* `base64.h`, `pem.h`: Encoding help, needed for key parsing/writing as well as for X.509. Arguably Base64 could be made private, for the use of PEM only, but it is currently used for non-PEM purposes in Mbed TLS. Thus I propose to keep it officially public in TF-PSA-Crypto 1.x.


Indeed. I'm not sure what the current status is, but I remember being deeply unsatisfied with our base64 API last time I looked at it (to the point of starting trying to fix it locally, which I evidently never got time to finish). The main problem being we're pretty inconsistent in where the reported sizes include the terminating nul byte or not, and things are not clearly documented. I don't remember if PEM suffer from similar issues these days.

Taking a step back, it could be argued that neither PEM nor base64 should rely on nul-termination, but rather only on size arguments.

I'm not sure if that's enough of a reason to not include them in 1.0 and re-include a better version later. (Obviously in an ideal world we'd have time to improve their API before the 1.0 and things would be easier, but we don't live in that world.)

Signed-off-by: Gilles Peskine <[email protected]>

davidhorstmann-arm

A few small comments, mostly nits. Looks good otherwise

davidhorstmann-arm · 2025-01-22T15:47:36Z

docs/architecture/0e-plans.md

+
+The following header files define cryptography-adjacent interfaces which we have no plans to replace.
+
+* `asn1.`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.


Nit:

Suggested change

* `asn1.`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.

* `asn1.h`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.

davidhorstmann-arm · 2025-01-22T16:59:37Z

docs/architecture/0e-plans.md

+
+`oid.h` and `MBEDTLS_OID_C` are in charge of defining all the OIDs used internally or by application code. This is a known problem which can result in applications wasting code size on many OIDs that they don't care about. We are planning a redesign: https://github.com/Mbed-TLS/mbedtls/issues/9380 .
+
+At this point, we do not consider this critical for TF-PSA-Crypto 1.0, and we are likely to work on it during the lifetime of TF-PSA-Crypto 1.x. This may change if we reevaluate the priority based on concrete use cases. In the meantime, `oid.h` will become private, but will remain used in Mbed TLS.


In the meantime, oid.h will become private, but will remain used in Mbed TLS.

What does this mean? In the categories of header described above, 'private' means that it can't be used by Mbed TLS, so this seems like a contradiction.

I've tried to consistently use “private” for unofficial interfaces that may still be exposed, and “internal” for interfaces where there is some technical measure limiting outside use (e.g. function only declared in a header in core or **/src). So there's no contradiction here: oid.h is not an official interface, but it's exposed to Mbed TLS. Did I get the terminology wrong somewhere above?

davidhorstmann-arm · 2025-01-22T17:09:04Z

docs/architecture/0e-plans.md

+
+### Private types in `psa_util.h`
+
+The functions `mbedtls_ecc_group_to_psa()` and mbedtls_ecc_group_from_psa()` are no longer relevant for public use since the legacy side of the conversion is no longer a public interface. They are not used in Mbed TLS. They should be moved to an internal header.


Nit: missing backtick

Suggested change

The functions `mbedtls_ecc_group_to_psa()` and mbedtls_ecc_group_from_psa()` are no longer relevant for public use since the legacy side of the conversion is no longer a public interface. They are not used in Mbed TLS. They should be moved to an internal header.

The functions `mbedtls_ecc_group_to_psa()` and `mbedtls_ecc_group_from_psa()` are no longer relevant for public use since the legacy side of the conversion is no longer a public interface. They are not used in Mbed TLS. They should be moved to an internal header.

davidhorstmann-arm · 2025-01-22T17:10:13Z

docs/architecture/0e-plans.md

+| `hkdf.h` | `mbedtls_hkdf_` | Delete | https://github.com/Mbed-TLS/mbedtls/issues/9150 |
+| `hmac_drbg.h` | `mbedtls_hmac_drbg_` | Private | [can be made fully private](#headers-that-can-be-made-fully-private) with a little work for [RNG header privatization](#rng-header-privatization) |
+| `lms.h` | `mbedtls_lms_` | Public | [no PSA equivalent](#cryptographic-mechanisms-with-no-PSA-equivalent) |
+| `md.h` | `mbedtls_md_` | Expose | [context types](#headers-with-context-types), but likely [Public hash-only `md.h`](#public-hash-only-md.h) |


Signed-off-by: Gilles Peskine <[email protected]>

I had reorganized some sections without updating their links. Also fix some incorrect title-to-fragment conversions (trying to follow GitHub markdown rules). Signed-off-by: Gilles Peskine <[email protected]>

Signed-off-by: Gilles Peskine <[email protected]>

mpg · 2025-01-23T09:11:29Z

docs/architecture/0e-plans.md

+* Inside the Mbed TLS library, to parse and write X.509 objects.
+* In application code, very occasionally. (Examples: [Fire-evacuation-guidance-system-IoT](https://github.com/2nd-Chance/Fire-evacuation-guidance-system-IoT/blob/33031a8255fe1ae516ddd58f1baa808801cd3abf/iotivity/resource/csdk/security/src/credresource.c#L3185) (dead project), [SiLabs Bluetooth attestation server](https://github.com/SiliconLabs/bluetooth_applications/blob/3eb0f3c9e234ada1f10714fb9376fcbc8e95807f/bluetooth_secure_attestation/bt_secure_attestation_server/src/ecdh_util3.c#L375))
+
+The use of PEM inside the Mbed TLS is intrinsic. It doesn't leak through the API of Mbed TLS, but Mbed TLS cannot be implemented without PEM. This is different from other private modules that Mbed TLS currently calls internally, but will no longer need to call once Mbed TLS has fully migrated to PSA. Thus, in the long term, TF-PSA-Crypto needs to expose its PEM API to Mbed TLS. (We reject the hypotheses of independent PEM implementations, or of making PEM its own library, as too much maintenance work.)


Nit: "the Mbed TLS"

mpg · 2025-01-23T09:17:21Z

docs/architecture/0e-plans.md

+
+* Inside the crypto library, to implement PEM.
+* In the `pem2der` sample program.
+* In Mbed TLS SSL test programs, for context serialization. (It's not clear to me why we encode in Base64 rather than binary.)


Not clear to me either, seems completely arbitrary to me. (And if we want something printable, we already have utility functions for hex in the test programs and could use that instead.)

davidhorstmann-arm

Noticed a missing header while investigating

davidhorstmann-arm · 2025-01-23T16:15:53Z

docs/architecture/0e-plans.md

+| `sha256.h` | `mbedtls_sha256_` | Expose | [context types](#headers-with-context-types) |
+| `sha3.h` | `mbedtls_sha3_` | Expose | [context types](#headers-with-context-types) |
+| `sha512.h` | `mbedtls_sha512_` | Expose | [context types](#headers-with-context-types) |
+| `threading.h` | `mbedtls_threading_` | Public | [Platform headers](#platform-headers) |


This list is missing timing.h I believe. What will happen to this header?

First draft

7b43b55

Covers high-level plans for header privatization. Signed-off-by: Gilles Peskine <[email protected]>

mpg reviewed Jan 14, 2025

View reviewed changes

gilles-peskine-arm added 6 commits January 14, 2025 21:24

Fix lexicographic order

7d2bce0

Signed-off-by: Gilles Peskine <[email protected]>

Improve disucssion of PEM

07bb2b8

It's public specifically for the sake of X.509, not for application code. Signed-off-by: Gilles Peskine <[email protected]>

drivers/builtin/include/mbedtls/build_info.h is actually transitional

5a60b88

Signed-off-by: Gilles Peskine <[email protected]>

Make section headers more uniform

dd612ae

Signed-off-by: Gilles Peskine <[email protected]>

Clarify some points about Doxygen

e108748

Signed-off-by: Gilles Peskine <[email protected]>

Note a current use of dhm.h slated for removal

89e71b5

Signed-off-by: Gilles Peskine <[email protected]>

mpg reviewed Jan 15, 2025

View reviewed changes

gilles-peskine-arm added 2 commits January 16, 2025 17:24

Fix copypasta

b589b36

Signed-off-by: Gilles Peskine <[email protected]>

Expand the discussion of Base64 and PEM

780b08f

Signed-off-by: Gilles Peskine <[email protected]>

davidhorstmann-arm requested changes Jan 22, 2025

View reviewed changes

gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members needs-work priority-high High priority - will be reviewed soon labels Jan 22, 2025

gilles-peskine-arm added 3 commits January 22, 2025 18:55

Typos

8223056

Signed-off-by: Gilles Peskine <[email protected]>

Update cross-references

941ba99

I had reorganized some sections without updating their links. Also fix some incorrect title-to-fragment conversions (trying to follow GitHub markdown rules). Signed-off-by: Gilles Peskine <[email protected]>

Note that the md.h plans are basically MBEDTLS_MD_LIGHT

4f9f866

Signed-off-by: Gilles Peskine <[email protected]>

mpg reviewed Jan 23, 2025

View reviewed changes

davidhorstmann-arm requested changes Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plans for TF-PSA-Crypto legacy headers #145

Plans for TF-PSA-Crypto legacy headers #145

gilles-peskine-arm commented Jan 9, 2025 •

edited

Loading

mpg left a comment

gilles-peskine-arm commented Jan 14, 2025

mpg left a comment

mpg Jan 15, 2025

davidhorstmann-arm left a comment

davidhorstmann-arm Jan 22, 2025

davidhorstmann-arm Jan 22, 2025

gilles-peskine-arm Jan 22, 2025

davidhorstmann-arm Jan 22, 2025

davidhorstmann-arm Jan 22, 2025

mpg Jan 23, 2025

mpg Jan 23, 2025

davidhorstmann-arm left a comment

davidhorstmann-arm Jan 23, 2025 •

edited

Loading


		The following header files define cryptography-adjacent interfaces which we have no plans to replace.

		* `asn1.`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.

	* `asn1.`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.
	* `asn1.h`, `asn1write.h`: ASN.1, needed for key parsing/writing as well as for X.509.


		`oid.h` and `MBEDTLS_OID_C` are in charge of defining all the OIDs used internally or by application code. This is a known problem which can result in applications wasting code size on many OIDs that they don't care about. We are planning a redesign: https://github.com/Mbed-TLS/mbedtls/issues/9380 .

		At this point, we do not consider this critical for TF-PSA-Crypto 1.0, and we are likely to work on it during the lifetime of TF-PSA-Crypto 1.x. This may change if we reevaluate the priority based on concrete use cases. In the meantime, `oid.h` will become private, but will remain used in Mbed TLS.


		### Private types in `psa_util.h`

		The functions `mbedtls_ecc_group_to_psa()` and mbedtls_ecc_group_from_psa()` are no longer relevant for public use since the legacy side of the conversion is no longer a public interface. They are not used in Mbed TLS. They should be moved to an internal header.

	\| `md.h` \| `mbedtls_md_` \| Expose \| [context types](#headers-with-context-types), but likely [Public hash-only `md.h`](#public-hash-only-md.h) \|
	\| `md.h` \| `mbedtls_md_` \| Expose \| [context types](#headers-with-context-types), but likely [Public hash-only `md.h`](#changes-to-public-crypto-headers) \|

Plans for TF-PSA-Crypto legacy headers #145

Are you sure you want to change the base?

Plans for TF-PSA-Crypto legacy headers #145

Conversation

gilles-peskine-arm commented Jan 9, 2025 • edited Loading

PR checklist

mpg left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Jan 14, 2025

mpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidhorstmann-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidhorstmann-arm left a comment

Choose a reason for hiding this comment

davidhorstmann-arm Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

gilles-peskine-arm commented Jan 9, 2025 •

edited

Loading

davidhorstmann-arm Jan 23, 2025 •

edited

Loading