Backport 2.16: Make MBEDTLS_CHECK_PARAMS usable without defining mbedtls_param_failed #2701
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For unit tests and sample programs, CFLAGS=-m32 is enough to get a 32-bit build, because these programs are all compiled directly from *.c to the executable in one shot. But with makefile rules that first build object files and then link them, LDFLAGS=-m32 is also needed.
Don't use the macro name assert. It's technically permitted as long as <assert.h> is not included, but it's fragile, because it means the code and any header that it includes must not include <assert.h>.
Introduce a new configuration option MBEDTLS_CHECK_PARAMS_ASSERT, which is disabled by default. When this option is enabled, MBEDTLS_PARAM_FAILED defaults to assert rather than to a call to mbedtls_param_failed, and <assert.h> is included. This fixes Mbed-TLS#2671 (no easy way to make MBEDTLS_PARAM_FAILED assert) without breaking backward compatibility. With this change, `config.pl full` runs tests with MBEDTLS_PARAM_FAILED set to assert, so the tests will fail if a validation check fails, and programs don't need to provide their own definition of mbedtls_param_failed().
All sample and test programs had a definition of mbedtls_param_failed. This was necessary because we wanted to be able to build them in a configuration with MBEDTLS_CHECK_PARAMS set but without a definition of MBEDTLS_PARAM_FAILED. Now that we activate the sample definition of MBEDTLS_PARAM_FAILED in config.h when testing with MBEDTLS_CHECK_PARAMS set, this boilerplate code is no longer needed.
With the change to the full config, there were no longer any tests that exercise invalid-parameter behavior. The test suite exercises invalid-parameter behavior by calling TEST_INVALID_PARAM and friends, relying on the test suite's mbedtls_check_param function. This function is only enabled if MBEDTLS_CHECK_PARAMS is defined but not MBEDTLS_CHECK_PARAMS_ASSERT. Add a component to all.sh that enables MBEDTLS_CHECK_PARAMS but disables MBEDTLS_CHECK_PARAMS_ASSERT and doesn't define MBEDTLS_PARAM_FAILED. This way, the xxx_invalid_param() tests do run. Since sample programs don't provide a mbedtls_check_param function, this component doesn't build the sample programs.
gilles-peskine-arm
added
bug
mbed TLS team
needs-review
dgreen-arm
approved these changes
Jun 18, 2019
Patater
approved these changes
Jun 18, 2019
Patater
added
approved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tweak
config.handplatform_util.hso thatconfig.pl fullalso enables the definition ofMBEDTLS_PARAM_FAILEDasassert. Adaptall.shso that invalid-parameter tests are executed, but programs don't need to provide anmbedtls_param_failedfunction. Fix #2671This is a straightforward backport of #2697, including the
-m32commit, but not including theprogramsmake target fix which is not applicable to 2.16.