Commit graph

15550 commits

Author SHA1 Message Date
Gilles Peskine
f29019f9cc Build tests with -Wdocumentation when using Clang
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
5a7702e76d Fix some C function documentation in the test framework
The primary goal of this commit is to fix various comments where
`clang -Wdocumentation` identified a discrepancy between the actual
function parameters and the documented parameters. The discrepancies
were due to copypasta, formatting issues or documentation that had
diverged from the implementation.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
c86a16548c Don't use STATIC_ASSERT_EXPR on non-GCC-compatible compilers
ARRAY_LENGTH has a portable but unsafe implementation, and a
non-portable implementation that causes a compile-time error if the
macro is accidentally used on a pointer.

The safety check was only implemented for __GCC__-defining compilers,
but the part that triggered the compile-time error was always used. It
turns out that this part triggers a build warning with MSVC (at least
with some versions: observed with Visual Studio 2013).
```
C:\builds\workspace\mbed-tls-pr-head_PR-4141-head\src\tests\src\psa_crypto_helpers.c(52): error C2220: warning treated as error - no 'object' file generated [C:\builds\workspace\mbed-tls-pr-head_PR-4141-head\src\mbedtls_test.vcxproj]
C:\builds\workspace\mbed-tls-pr-head_PR-4141-head\src\tests\src\psa_crypto_helpers.c(52): warning C4116: unnamed type definition in parentheses [C:\builds\workspace\mbed-tls-pr-head_PR-4141-head\src\mbedtls_test.vcxproj]
```

Since a compile-time error is never triggered when the compile-time
check for the argument type is not implemented, just use the unsafe
macro directly when there's no safety check.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
6b362e6f01 Fix copypasta for the type of a variable
MSVC started (rightfully) complaining after moving the code to a
separate .c file.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
6e0d5bd00d Increment the test step number when invalidating a key
This makes failure messages easier to understand.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
65048ad648 Destroy recorded persistent keys in PSA_DONE()
This ensures that test cases won't leave persistent files behind even
on failure, provided they use TEST_USES_KEY_ID(). Test cases that
don't use this macro are unaffected.

Tests that use PSA_DONE() midway and expect persistent keys to survive
must use PSA_SESSION_DONE() instead.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
aae718caca New test helper to purge persistent key from memory
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
e09ef87364 Document the newly exported storage cleanup macros and functions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
313ffb8f90 Move PSA storage cleanup out of the slot_management test suite
Merge the two identical definitions of TEST_USES_KEY_ID and
mbedtls_test_psa_purge_key_storage from
test_suite_psa_crypto_slot_management.function and
test_suite_psa_crypto_se_driver_hal.function into a single copy in
common test code so that it can be used in all test suites.

No semantic change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:36:07 +01:00
Gilles Peskine
b9ad79417d Push back on divergence of duplicated code
Persistent storage common code from
test_suite_psa_crypto_slot_management.function had been duplicated in
test_suite_psa_crypto_se_driver_hal.function and the copy had slightly
diverged. Re-align the copy in preparation from moving the code to a
common module and using that sole copy in both test suites.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
e95a643839 Prepare to move persistent storage cleanup to common code
Rename functions to mbedtls_test_xxx and make them non-static if
they're going to be exported.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
2385f71abd Fix and simplify test assertions
mbedtls_test_fail does not copy the failure explanation string, so
passing a string on the stack doesn't work. This fixes a garbage
message that would appear if a test triggered a non-implemented code
path.

More generally, just use TEST_ASSERT instead of explicitly calling
mbedtls_test_fail, since we aren't playing any tricks with the error
location.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
cc9db30851 exported_key_sanity_check: make checks slightly more systematic
Shuffle the logic in mbedtls_test_psa_exported_key_sanity_check()
somewhat. The resulting behavior changes are:

* Always check the exported length against PSA_EXPORT_KEY_OUTPUT_SIZE,
  even for unstructured key types.
* Always complain if a key type is not explicitly covered, not just
  for public keys.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
5c2665b164 Use const pointers on parsing functions
The const-ness has to be cast away when calling mbedtls_asn1_xxx
parsing functions. This is a known flaw in the mbedtls API
(https://github.com/ARMmbed/mbedtls/issues/803).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
ad557e58bf exported_key_sanity_check: simplify the logic for public keys
Remove a conditional imbrication level. Get rid of some minor overhead
for ECC public keys dating back from when they had ASN.1 wrapping.

No behavior change.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
e50b578218 Document functions and macros that are now exported
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
e78b00210a Move exercise_key and related functions to their own module
Move mbedtls_test_psa_exercise_key() (formerly exercise_key()) and
related functions to its own module. Export the few auxiliary
functions that are also called directly.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
8e94efe4ba Move asn1_skip_integer to the asn1_helpers module
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
57948369c4 Create a separate test module for ASN.1 test helper functions
The code will be moved in a subsequent commit.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
ea38a926c1 Remove a redundant export step in import_export
exercise_export_key() exports the key and does sanity checks on the
result. Here we've already just exported the key, so just run the
sanity checks.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
c18e25f6df Prepare to move exercise_key and friends to their own module
Rename functions to mbedtls_test_psa_xxx if they're going to be
exported. Declare functions as static if they're aren't meant to be
called directly from test code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
66e7b903ce Create a separate test module for PSA exercise_key
The code will be moved in a subsequent commit.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
2091f3afb7 Fix wrong \file name in Doxygen comments
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 20:29:38 +01:00
Gilles Peskine
a0fd0f8531
Merge pull request #4104 from gilles-peskine-arm/test-mutex-usage-count-development
Test and fix mutex usage
2021-02-23 15:14:43 +01:00
Gilles Peskine
b15832160b Make entropy double-free work
Although the library documentation does not guarantee that calling
mbedtls_entropy_free() twice works, it's a plausible assumption and it's
natural to write code that frees an object twice. While this is uncommon for
an entropy context, which is usually a global variable, it came up in our
own unit tests (random_twice tests in test_suite_random).

Announce this in the same changelog entry as for RSA because it's the same
bug in the two modules.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-23 11:27:03 +01:00
Gilles Peskine
3d979f781e Add init-free tests for entropy
These tests validate that an entropy object can be reused and that
calling mbedtls_entropy_free() twice is ok.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 21:24:02 +01:00
Gilles Peskine
60fe6606bf Only define test_hooks_xxx under MBEDTLS_TEST_HOOKS
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
00d0ad4036 Clarify the advice about reporting errors in test hooks
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
76e9c64c3e Clarify the advice about reporting errors in test hooks
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
414e717036 Deinitialize the PSA subsystem
The PSA subsystem may consume global resources. It currently doesn't
consume any heap when no keys are registered, but it may do so in the
future. It does consume mutexes, which are reported as leaks when
mutex usage checking is enabled.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
e374b95fe1 Detect and report mutex usage errors in SSL test programs
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
d0a46e5c7f ssl_server2: don't check test hooks failure in query_config mode
Test hook failure checks may print information to stdout, which messes
up the usage of query_config mode. Nothing interesting happens in
query_config mode anyway, so that's no loss.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
53dea743d5 SSL test programs: allow for test hooks init and error reports
Create utility functions to set up test hooks and report errors that
the test hooks might detect. Call them in ssl_client2 and ssl_server2.

Test hooks are potentially enabled by compiling with
MBEDTLS_TEST_HOOKS.

This commit only sets up the functions. It doesn't make them do
anything yet.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
4337a9cb18 Document mutex usage for RSA
The mutex is now initialized iff ver != 0.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
1226ecef01 Changelog entry for RSA mutex usage fix
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
5e40a7cfa0 Fix mutex leak in RSA
mbedtls_rsa_gen_key() was not freeing the RSA object, and specifically
not freeing the mutex, in some error cases.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
eb94059edd Fix mutex double-free in RSA
When MBEDTLS_THREADING_C is enabled, RSA code protects the use of the
key with a mutex. mbedtls_rsa_free() frees this mutex by calling
mbedtls_mutex_free(). This does not match the usage of
mbedtls_mutex_free(), which in general can only be done once.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
914afe1fdb Add init-free tests for RSA
These tests are trivial except when compiling with MBEDTLS_THREADING_C
and a mutex implementation that are picky about matching each
mbedtls_mutex_init() with exactly one mbedtls_mutex_free().

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
7aba036154 Add missing cleanup in test function
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
71edf749e1 Changelog entry for DRBG mutex usage fix
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
478847cca3 Document thread safety for HMAC_DRBG
random(), and only this function, is thread-safe.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
e39b2192e5 Document mutex invariant for HMAC_DRBG
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
b791dc66ce Fix mutex leak in HMAC_DRBG
mbedtls_hmac_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.

To fix this, mbedtls_hmac_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_hmac_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_hmac_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
f305d92480 Document thread safety for CTR_DRBG
random(), and only this function, is thread-safe.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
da290f9bcd Document mutex invariant for CTR_DRBG
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
f4b3429782 Fix mutex leak in CTR_DRBG
mbedtls_ctr_drbg_free() left a mutex in the initialized state. This
caused a resource leak on platforms where mbedtls_mutex_init()
allocates resources.

To fix this, mbedtls_ctr_drbg_free() no longer reinitializes the
mutex. To preserve the property that mbedtls_ctr_drbg_free() leaves
the object in an initialized state, which is generally true throughout
the library except regarding mutex objects on some platforms, no
longer initialize the mutex in mbedtls_ctr_drbg_init(). Since the
mutex is only used after seeding, and seeding is only permitted once,
call mbedtls_mutex_init() as part of the seeding process.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
39a1a26d0b Explain the usage of is_valid in pthread mutexes
Document the usage inside the library, and relate it with how it's
additionally used in the test code.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
f96d3d8b20 Count and report non-freed mutexes
Subtract the number of calls to mbedtls_mutex_free() from the number
of calls to mbedtls_mutex_init(). A mutex leak will manifest as a
positive result at the end of the test case.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:24:03 +01:00
Gilles Peskine
2a4c598859 Detect and report mutex usage errors
If the mutex usage verification framework is enabled and it detects a
mutex usage error, report this error and mark the test as failed.

This detects most usage errors, but not all cases of using
uninitialized memory (which is impossible in full generality) and not
leaks due to missing free (which will be handled in a subsequent commit).

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:08:14 +01:00
Gilles Peskine
1061ec6782 Mutex usage testing: set up wrapper functions
When using pthread mutexes (MBEDTLS_THREADING_C and
MBEDTLS_THREADING_PTHREAD enabled), and when test hooks are
enabled (MBEDTLS_TEST_HOOKS), set up wrappers around the
mbedtls_mutex_xxx abstraction. In this commit, the wrapper functions
don't do anything yet.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2021-02-22 19:08:14 +01:00