Skip to content

OID: make X.509 independent from crypto #10173

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

Merged
merged 20 commits into from
Jun 4, 2025

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented May 12, 2025

Make OIDs in Mbed TLS independent from OID support in crypto. Resolves #10124.

  • Mbed TLS no longer cares about MBEDTLS_OID_C.
  • Mbed TLS no longer references mbedtls/oid.h from TF-PSA-Crypto.
  • Mbed TLS no longer relies on functions from oid.c in TF-PSA-Crypto.

Unblocks Mbed-TLS/TF-PSA-Crypto#291.

PR checklist

@gilles-peskine-arm gilles-peskine-arm added component-x509 needs-ci Needs to pass CI tests size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels May 12, 2025
@gilles-peskine-arm gilles-peskine-arm marked this pull request as ready for review May 14, 2025 07:33
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels May 14, 2025
@felixc-arm felixc-arm self-requested a review May 14, 2025 08:06
felixc-arm
felixc-arm previously approved these changes May 14, 2025
Copy link
Contributor

@felixc-arm felixc-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, + minor optional nit 👍

felixc-arm
felixc-arm previously approved these changes May 14, 2025
@gilles-peskine-arm gilles-peskine-arm moved this from In Development to Has Approval in Roadmap pull requests (new board) May 28, 2025
@mpg mpg removed the needs-reviewer This PR needs someone to pick it up for review label Jun 3, 2025
@mpg mpg self-requested a review June 3, 2025 10:29
mpg
mpg previously approved these changes Jun 3, 2025
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! (Note: at 626dead - in case you need to rebase.)

Just has a conflict with the submodule update that needs resolving.

Stop referring to low-level APIs that are becoming private.

Also drop the requirement on supporting what is now
PSA_ALG_RSA_PKCS1V15_SIGN_RAW. That was needed for TLS 1.0/1.1 which signs
MD5||SHA1, but is no longer needed since Mbed TLS 3.0 dropped support for
these protocol versions.

Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Keep "mbedtls/oid.h" in code that only uses OID macros.

```
git grep -l mbedtls_oid_ '**/*.[hc]' tests/suites/*.function | xargs perl -i -pe 's!["<]mbedtls/oid\.h[">]!"x509_oid.h"!g'
```

Signed-off-by: Gilles Peskine <[email protected]>
Avoid clashes with the functions and the type that are still defined in
TF-PSA-Crypto. They are now internal names, so it doesn't really matter, but
having the same name as the ones declared in TF-PSA-Crypto's `oid.h` would
cause problems during the transition.

Remove the unused name for `struct mbedtls_oid_descriptor_t`, and rename the
rest:

```
perl -i -pe 's/mbedtls_oid_/mbedtls_x509_oid_/g' library/x509_oid.[hc]
./framework/scripts/code_style.py --fix library/x509_oid.[hc]
```

Signed-off-by: Gilles Peskine <[email protected]>
```
git grep -l -P 'mbedtls_oid_get_(?!numeric_string\b)' | xargs perl -i -pe 's/\bmbedtls_oid_get_(?!numeric_string\b)/mbedtls_x509_oid_get_/'
./framework/scripts/code_style.py --since HEAD~1 --fix
```

Signed-off-by: Gilles Peskine <[email protected]>
They're just aliases for the corresponding MBEDTLS_X509_EXT_xxx. We don't
need separate names.

Signed-off-by: Gilles Peskine <[email protected]>
Remove the definition of `MBEDTLS_ERR_OID_BUF_TOO_SMALL` in `x509_oid.h`,
and use the corresponding PSA error instead.

```
git grep -l MBEDTLS_ERR_OID_BUF_TOO_SMALL | xargs perl -i -pe 's/\bMBEDTLS_ERR_OID_BUF_TOO_SMALL\b/PSA_ERROR_BUFFER_TOO_SMALL/p'
edit library/x509_oid.h
```

Signed-off-by: Gilles Peskine <[email protected]>
Replace the non-X.509-named error code `MBEDTLS_ERR_OID_NOT_FOUND` with
`MBEDTLS_ERR_X509_UNKNOWN_OID`, which already exists and is currently not
used for anything.

Public functions in X.509 propagate this error code, so it needs to have a
public name.

Remove the definition of `MBEDTLS_ERR_OID_NOT_FOUND` in `x509_oid.h`, then

```
git grep -l MBEDTLS_ERR_OID_NOT_FOUND | xargs perl -i -pe 's/\bMBEDTLS_ERR_OID_NOT_FOUND\b/MBEDTLS_ERR_X509_UNKNOWN_OID/g'
```

Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
This will be a subset of the former `<mbedtls/oid.h>`, with only macro
definitions, no function declarations.

Signed-off-by: Gilles Peskine <[email protected]>
Some code that parses or writes X.509 needs to know OID values. We provide a
convenient list. Don't remove this list from the public interface of the
library.

For user convenience, expose these values in the same header as before and
with the same name as before: `MBEDTLS_OID_xxx` in `<mbedtls/oid.h>`.

Signed-off-by: Gilles Peskine <[email protected]>
mbedtls_oid_get_md_alg() is used in X.509, but
mbedtls_oid_get_oid_by_md() is only used in crypto.

Signed-off-by: Gilles Peskine <[email protected]>
For each function in `x509_oid.c`, determine where it is used and only
include it in the build if it is needed by the X.509 code. Define the
corresponding internal tables only when they are consumed by a function.

This makes Mbed TLS completely independent of the compilation option
`MBEDTLS_OID_C`. This option remains present only in sample configs for
crypto, where it must stay until TF-PSA-Crypto no longer relies on this
option.

Signed-off-by: Gilles Peskine <[email protected]>
The header `mbedtls/oid.h` now belongs to the X.509 library. Move the
declarations of `mbedtls_oid_get_numeric_string()` and
`mbedtls_oid_from_numeric_string()` back to this header, which is where they
were in all previous releases of Mbed TLS. This avoids gratuitously breaking
backward compatibility.

Signed-off-by: Gilles Peskine <[email protected]>
Otherwise Doxygen complains about two `\file` with the same name.

This is a temporary exclusion which can be removed once crypto no longer has
an oid.h.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm
Copy link
Contributor Author

gilles-peskine-arm commented Jun 3, 2025

I've rebased this branch on top of development with two changes:

The previous version is in
https://github.com/gilles-peskine-arm/mbedtls/tree/oid-split-x509-4

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Jun 4, 2025
@mpg mpg added this pull request to the merge queue Jun 4, 2025
Merged via the queue into Mbed-TLS:development with commit e806134 Jun 4, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-x509 priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Development

Successfully merging this pull request may close these issues.

Move X.509 OIDs from crypto to X.509
3 participants