Changes after code review
Signed-off-by: TRodziewicz <tomasz.rodziewicz@mobica.com>
diff --git a/ChangeLog.d/issue4313.txt b/ChangeLog.d/issue4313.txt
index 026a190..1fb6123 100644
--- a/ChangeLog.d/issue4313.txt
+++ b/ChangeLog.d/issue4313.txt
@@ -1,25 +1,4 @@
Removals
* Remove the following macros: MBEDTLS_CHECK_PARAMS,
MBEDTLS_CHECK_PARAMS_ASSERT, MBEDTLS_PARAM_FAILED,
- MBEDTLS_PARAM_FAILED_ALT, TEST_INVALID_PARAM, TEST_INVALID_PARAM_RET,
- the following macros have been inactivated MBEDTLS_INTERNAL_VALIDATE_RET
- and MBEDTLS_INTERNAL_VALIDATE, structures: param_failed_ctx_t,
- mbedtls_test_param_failed_location_record_t, functions:
- mbedtls_test_param_failed_get_location_record(),
- mbedtls_test_param_failed_expect_call(),
- mbedtls_test_param_failed_check_expected_call(),
- mbedtls_test_param_failed_get_state_buf(),
- mbedtls_test_param_failed_reset_state(),
- mbedtls_param_failed(). Remove the following functions from all.sh:
- component_test_check_params_functionality(),
- component_test_check_params_without_platform(),
- component_test_check_params_silent().
- Remove the following test functions from test_suite_*.function files:
- aes_check_params(), aria_invalid_param(), blowfish_invalid_param(),
- camellia_invalid_param(), ccm_invalid_param(), chacha20_bad_params(),
- chachapoly_bad_params(), cipher_invalid_param_conditional(),
- dhm_invalid_params(), ecdh_invalid_param(), ecdsa_invalid_param(),
- ecjpake_invalid_param(), ecp_invalid_param(), gcm_invalid_param(),
- mpi_invalid_param(), invalid_parameters() (pk), poly1305_bad_params(),
- rsa_invalid_param(), sha1_invalid_param(), sha256_invalid_param(),
- sha512_invalid_param(). Fixes #4313.
+ MBEDTLS_PARAM_FAILED_ALT. Fixes #4313.
diff --git a/docs/3.0-migration-guide.d/remove_mbedtls_check_params_option.md b/docs/3.0-migration-guide.d/remove_mbedtls_check_params_option.md
index 146b1c7..6f43aa3 100644
--- a/docs/3.0-migration-guide.d/remove_mbedtls_check_params_option.md
+++ b/docs/3.0-migration-guide.d/remove_mbedtls_check_params_option.md
@@ -1,10 +1,11 @@
Remove MBEDTLS_CHECK_PARAMS option
----------------------------------
-This change affects the way of how parameters are validated.
+This change does not affect users who use the default configuration; it only
+affects users who enabled that option.
-The option `MBEDTLS_CHECK_PARAMS` (disabled by default) enables certain kinds of
-“parameter validation”. It covers two kinds of validations:
+The option `MBEDTLS_CHECK_PARAMS` (disabled by default) enabled certain kinds
+of “parameter validation”. It covered two kinds of validations:
- In some functions that require a valid pointer, “parameter validation” checks
that the pointer is non-null. With the feature disabled, a null pointer is not
@@ -14,34 +15,17 @@
checks that the value is a valid one. With the feature disabled, an invalid
value causes a silent default to one of the valid values.
-The default reaction to a failed check is to call a function mbedtls_param_failed
-which the application must provide. If this function returns, its caller returns
-an error `MBEDTLS_ERR_xxx_BAD_INPUT_DATA`.
+The default reaction to a failed check was to call a function
+`mbedtls_param_failed()` which the application had to provide. If this function
+returned, its caller returned an error `MBEDTLS_ERR_xxx_BAD_INPUT_DATA`.
-This feature is only used in some classic (non-PSA) cryptography modules. It is
-not used in X.509, TLS or in PSA crypto, and it has not been implemented in all
+This feature was only used in some classic (non-PSA) cryptography modules. It was
+not used in X.509, TLS or in PSA crypto, and it was not implemented in all
classic crypto modules.
-Removal of `MBEDTLS_CHECK_PARAMS` and all dependent features means changing
-code that does something like this:
-```
-#if MBEDTLS_CHECK_PARAMS
-#define VALIDATE(cond) do {if(cond) return BAD_INPUT_DATA;} while (0)
-#else
-#define VALIDATE(cond) do {} while (0)
-#endif
-...
-VALIDATE(coin == HEADS || coin == TAILS);
-VALIDATE(data != NULL);
-if (coin == HEADS) heads();
-else tails();
-```
-to something like this:
-```
-if (coin == HEADS) heads();
-else if (coin == TAILS) tails();
-else return BAD_INPUT_DATA;
-```
+This feature has been removed. The library no longer checks for NULL pointers;
+checks for enum-like arguments will be kept or re-introduced on a case-by-case
+basis, but their presence will no longer be dependent on a compile-time option.
Validation of enum-like values is somewhat useful, but not extremely important,
because the parameters concerned are usually constants in applications.
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 552750c..c3dc6e7 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1021,10 +1021,7 @@
/*
* As the return error code may not be handled in case of multiple errors,
- * do our best to report an unexpected lock counter: if available
- * call MBEDTLS_PARAM_FAILED that may terminate execution (if called as
- * part of the execution of a test suite this will stop the test suite
- * execution).
+ * do our best to report an unexpected lock counter.
*/
if( slot->lock_count != 1 )
{
diff --git a/tests/include/test/macros.h b/tests/include/test/macros.h
index cad39aa..1c0e2bd 100644
--- a/tests/include/test/macros.h
+++ b/tests/include/test/macros.h
@@ -177,14 +177,6 @@
} \
} while( 0 )
-#if defined(MBEDTLS_CHECK_PARAMS) && !defined(MBEDTLS_PARAM_FAILED_ALT)
-#define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \
- do { if( ( TEST ) != ( PARAM_ERR_VALUE ) ) goto exit; } while( 0 )
-
-#define TEST_INVALID_PARAM( TEST ) \
- do { TEST; } while( 0 )
-#endif /* MBEDTLS_CHECK_PARAMS && !MBEDTLS_PARAM_FAILED_ALT */
-
/**
* \brief This macro tests the statement passed to it as a test step or
* individual test in a test case. The macro assumes the test will not fail.
diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function
index a740391..fe4dd3e 100644
--- a/tests/suites/test_suite_aes.function
+++ b/tests/suites/test_suite_aes.function
@@ -206,9 +206,6 @@
mbedtls_aes_xts_init( &ctx );
memset( data_unit, 0x00, sizeof( data_unit ) );
-
- /* Valid pointers are passed for builds with MBEDTLS_CHECK_PARAMS, as
- * otherwise we wouldn't get to the size check we're interested in. */
TEST_ASSERT( mbedtls_aes_crypt_xts( &ctx, MBEDTLS_AES_ENCRYPT, length, data_unit, src, output ) == retval );
}
/* END_CASE */
@@ -359,7 +356,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void aes_invalid_mode( )
{
mbedtls_aes_context aes_ctx;
diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function
index b24e498..5147064 100644
--- a/tests/suites/test_suite_aria.function
+++ b/tests/suites/test_suite_aria.function
@@ -23,7 +23,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void aria_invalid_param( )
{
mbedtls_aria_context ctx;
diff --git a/tests/suites/test_suite_blowfish.function b/tests/suites/test_suite_blowfish.function
index 5681a9e..fdecfb6 100644
--- a/tests/suites/test_suite_blowfish.function
+++ b/tests/suites/test_suite_blowfish.function
@@ -14,7 +14,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void blowfish_invalid_param( )
{
mbedtls_blowfish_context ctx;
diff --git a/tests/suites/test_suite_camellia.function b/tests/suites/test_suite_camellia.function
index fb0a349..0633b73 100644
--- a/tests/suites/test_suite_camellia.function
+++ b/tests/suites/test_suite_camellia.function
@@ -14,7 +14,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void camellia_invalid_param( )
{
mbedtls_camellia_context ctx;
diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function
index ea5fa2e..f1095b1 100644
--- a/tests/suites/test_suite_cipher.function
+++ b/tests/suites/test_suite_cipher.function
@@ -207,7 +207,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void cipher_invalid_param_conditional( )
{
mbedtls_cipher_context_t valid_ctx;
diff --git a/tests/suites/test_suite_ecdh.function b/tests/suites/test_suite_ecdh.function
index 5fced62..5fa5b67 100644
--- a/tests/suites/test_suite_ecdh.function
+++ b/tests/suites/test_suite_ecdh.function
@@ -50,7 +50,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void ecdh_invalid_param( )
{
mbedtls_ecdh_context ctx;
diff --git a/tests/suites/test_suite_ecjpake.function b/tests/suites/test_suite_ecjpake.function
index 2a6d893..311733b 100644
--- a/tests/suites/test_suite_ecjpake.function
+++ b/tests/suites/test_suite_ecjpake.function
@@ -98,7 +98,7 @@
* END_DEPENDENCIES
*/
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void ecjpake_invalid_param( )
{
mbedtls_ecjpake_context ctx;
diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function
index 9b8ba82..81a6514 100644
--- a/tests/suites/test_suite_ecp.function
+++ b/tests/suites/test_suite_ecp.function
@@ -38,7 +38,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void ecp_invalid_param( )
{
mbedtls_ecp_group grp;
diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function
index 35691f7..ae306b5 100644
--- a/tests/suites/test_suite_gcm.function
+++ b/tests/suites/test_suite_gcm.function
@@ -181,7 +181,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:NOT_DEFINED */
void gcm_invalid_param( )
{
mbedtls_gcm_context ctx;
diff --git a/tests/suites/test_suite_shax.data b/tests/suites/test_suite_shax.data
index 12eec84..1cc58af 100644
--- a/tests/suites/test_suite_shax.data
+++ b/tests/suites/test_suite_shax.data
@@ -1,9 +1,6 @@
SHA-1 - Valid parameters
sha1_valid_param:
-SHA-1 - Invalid parameters
-sha1_invalid_param:
-
# Test the operation of SHA-1 and SHA-2
SHA-1 Test Vector NIST CAVS #1
depends_on:MBEDTLS_SHA1_C
diff --git a/tests/suites/test_suite_shax.function b/tests/suites/test_suite_shax.function
index 1d4cf71..a8f5b18 100644
--- a/tests/suites/test_suite_shax.function
+++ b/tests/suites/test_suite_shax.function
@@ -11,46 +11,6 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_SHA1_C:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
-void sha1_invalid_param( )
-{
- mbedtls_sha1_context ctx;
- unsigned char buf[64] = { 0 };
- size_t const buflen = sizeof( buf );
-
- TEST_INVALID_PARAM( mbedtls_sha1_init( NULL ) );
-
- TEST_INVALID_PARAM( mbedtls_sha1_clone( NULL, &ctx ) );
- TEST_INVALID_PARAM( mbedtls_sha1_clone( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_starts_ret( NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_update_ret( NULL, buf, buflen ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_update_ret( &ctx, NULL, buflen ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_finish_ret( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_finish_ret( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_internal_sha1_process( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_internal_sha1_process( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_ret( NULL, buflen, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA,
- mbedtls_sha1_ret( buf, buflen, NULL ) );
-
-exit:
- return;
-}
-/* END_CASE */
-
/* BEGIN_CASE depends_on:MBEDTLS_SHA1_C */
void mbedtls_sha1( data_t * src_str, data_t * hash )
{
@@ -72,7 +32,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C:NOT_DEFINED */
void sha256_invalid_param( )
{
mbedtls_sha256_context ctx;
@@ -81,38 +41,10 @@
int valid_type = 0;
int invalid_type = 42;
- TEST_INVALID_PARAM( mbedtls_sha256_init( NULL ) );
-
- TEST_INVALID_PARAM( mbedtls_sha256_clone( NULL, &ctx ) );
- TEST_INVALID_PARAM( mbedtls_sha256_clone( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_starts_ret( NULL, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
+ TEST_EQUAL( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
mbedtls_sha256_starts_ret( &ctx, invalid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_update_ret( NULL, buf, buflen ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_update_ret( &ctx, NULL, buflen ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_finish_ret( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_finish_ret( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_internal_sha256_process( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_internal_sha256_process( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_ret( NULL, buflen,
- buf, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
- mbedtls_sha256_ret( buf, buflen,
- NULL, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
+ TEST_EQUAL( MBEDTLS_ERR_SHA256_BAD_INPUT_DATA,
mbedtls_sha256_ret( buf, buflen,
buf, invalid_type ) );
@@ -156,7 +88,7 @@
}
/* END_CASE */
-/* BEGIN_CASE depends_on:MBEDTLS_SHA512_C:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */
+/* BEGIN_CASE depends_on:MBEDTLS_SHA512_C:NOT_DEFINED */
void sha512_invalid_param( )
{
mbedtls_sha512_context ctx;
@@ -165,38 +97,10 @@
int valid_type = 0;
int invalid_type = 42;
- TEST_INVALID_PARAM( mbedtls_sha512_init( NULL ) );
-
- TEST_INVALID_PARAM( mbedtls_sha512_clone( NULL, &ctx ) );
- TEST_INVALID_PARAM( mbedtls_sha512_clone( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_starts_ret( NULL, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
+ TEST_EQUAL( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
mbedtls_sha512_starts_ret( &ctx, invalid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_update_ret( NULL, buf, buflen ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_update_ret( &ctx, NULL, buflen ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_finish_ret( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_finish_ret( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_internal_sha512_process( NULL, buf ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_internal_sha512_process( &ctx, NULL ) );
-
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_ret( NULL, buflen,
- buf, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
- mbedtls_sha512_ret( buf, buflen,
- NULL, valid_type ) );
- TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
+ TEST_EQUAL( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA,
mbedtls_sha512_ret( buf, buflen,
buf, invalid_type ) );