Skip to content

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jul 16, 2025

Set up a mechanism for erroring out if users try to use removed options in their configuration file. Resolves #10147.

Specification in Mbed-TLS/mbedtls-framework#184.

Status: ready.

Needed preceding PR: framework Mbed-TLS/mbedtls-framework#164

PR checklist

@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests needs-preceding-pr Requires another PR to be merged first size-s Estimated task size: small (~2d) labels Jul 16, 2025
@gilles-peskine-arm gilles-peskine-arm added the priority-high High priority - will be reviewed soon label Jul 16, 2025
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from bf54ded to 9995f03 Compare July 17, 2025 16:36
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from 9995f03 to f745c31 Compare July 18, 2025 13:00
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from f745c31 to 5a12986 Compare August 1, 2025 21:21
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from 5a12986 to 9cca937 Compare September 15, 2025 15:58
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from 9cca937 to 5582b64 Compare September 19, 2025 20:19
@gilles-peskine-arm gilles-peskine-arm added the needs-review Every commit must be reviewed by at least two team members, label Sep 22, 2025
@gilles-peskine-arm
Copy link
Contributor Author

This is ready for review. It isn't passing the CI yet, but fixing that should take just one more commit.

@gilles-peskine-arm gilles-peskine-arm added priority-very-high Highest priority - prioritise this over other review work and removed priority-high High priority - will be reviewed soon labels Sep 22, 2025
@gilles-peskine-arm
Copy link
Contributor Author

I will rebase this once #10382 is merged. This should resolve the Windows-mingw build failure since that job will be removed from the CI.

@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from cb8a4b1 to 109cf72 Compare September 23, 2025 08:12
@gilles-peskine-arm
Copy link
Contributor Author

I rebased a little too early (#10382 was still on the merge queue) so Windows-mingw is still failing. But anyway that job will go away, so the CI will pass once this is rebased again. This will need to be rebased again once the framework PR is merged, so I don't see any reason to rebase again right now. We'll have a green CI after the post-framework-merge rebase.

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.

Looking pretty good to me, just a question about a string, and a request for one more test case.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Read the list of historical config options in 3.6, compare that to 1.0/4.0
and emit the appropriate checkers.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix the build of libmbedx509 when generated files are not already present.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Indicate which config file has the most relevant tweak.

Duplicate a few test cases so that both the crypto config and the mbedtls
config are tested.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm force-pushed the config-error-on-removed-options-mbedtls branch from 0dafd09 to 3cee43e Compare September 24, 2025 15:21
@gilles-peskine-arm gilles-peskine-arm removed the needs-preceding-pr Requires another PR to be merged first label Sep 24, 2025
@gilles-peskine-arm
Copy link
Contributor Author

Rebased, hopefully for the last time, with the framework updated to the merge of Mbed-TLS/mbedtls-framework#164. The previous head is at https://github.com/gilles-peskine-arm/mbedtls/tree/config-error-on-removed-options-mbedtls-11

@gilles-peskine-arm gilles-peskine-arm removed the needs-ci Needs to pass CI tests label Sep 24, 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!

Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Sep 25, 2025
@gilles-peskine-arm gilles-peskine-arm 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 Sep 25, 2025
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Sep 25, 2025
Merged via the queue into Mbed-TLS:development with commit 3415d2d Sep 25, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Sep 25, 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 priority-very-high Highest priority - prioritise this over other review work size-s Estimated task size: small (~2d)
Development

Successfully merging this pull request may close these issues.

Mechanism for marking a compilation option as private
3 participants