search

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.26k stars 2.57k forks source link

Check that all test cases are executed at least once #2691

Open gilles-peskine-arm opened 5 years ago

gilles-peskine-arm commented 5 years ago

We have test cases that are never executed on our CI because they depend on a particular configuration that we don't test. This is a gap in test coverage: if we wrote those tests, it's presumably because they're useful for something.

We should check that when we run all.sh, every test case in every .data file is executed at least once, and every test case in `ssl-opt.sh is executed at least once.

For example, I think (by visual inspection) that entropy_nv_seed in test_suite_entropy has not been executed on CI for a long time, since it depends on MBEDTLS_PLATFORM_NV_SEED_ALT but we never set that on CI until https://github.com/ARMmbed/mbedtls/pull/2684 (it wasn't set by full). And it also depends on MBEDTLS_ENTROPY_SHA512_ACCUMULATOR so before https://github.com/ARMmbed/mbedtls/pull/2684 you'd have to set MBEDTLS_PLATFORM_NV_SEED_ALT and not set MBEDTLS_ENTROPY_FORCE_SHA256.

Goal of this issue: if a CI job succeeds, it guarantees that all test cases have been executed, except for a curated whitelist of test cases that may be skipped.

Prerequisites:

Remaining tasks for this issue:

gilles-peskine-arm commented 4 years ago

https://github.com/ARMmbed/mbedtls/pull/3458 adds a script that reports the test cases that are not executed at least once. They are (on development):

Warning: Test case not executed: ssl-opt;DTLS client reconnect from same port: reconnect, nbio, valgrind
Warning: Test case not executed: ssl-opt;DTLS fragmenting: 3d, openssl client, DTLS 1.0
Warning: Test case not executed: ssl-opt;DTLS fragmenting: 3d, openssl client, DTLS 1.2
Warning: Test case not executed: ssl-opt;DTLS fragmenting: 3d, openssl server, DTLS 1.0
Warning: Test case not executed: ssl-opt;DTLS fragmenting: 3d, openssl server, DTLS 1.2
Warning: Test case not executed: ssl-opt;DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d (drop, delay, duplicate), \"short\" PSK handshake
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d, \"short\" (no ticket, no cli_auth) FS handshake
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d, \"short\" RSA handshake
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d, openssl server
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d, openssl server, fragmentation
Warning: Test case not executed: ssl-opt;DTLS proxy: 3d, openssl server, fragmentation, nbio
Warning: Test case not executed: ssl-opt;Handshake memory usage (MFL $1)
Warning: Test case not executed: ssl-opt;PSA - ECDH with $1
Warning: Test case not executed: ssl-opt;PSA-supported ciphersuite: $1
Warning: Test case not executed: ssl-opt;RC4: server disabled, client enabled
Warning: Test case not executed: ssl-opt;RC4: server half, client enabled
Warning: Test case not executed: test_suite_cipher.gcm;AES GCM Decrypt empty buffer
Warning: Test case not executed: test_suite_psa_crypto;PSA key policy algorithm2: CTR, CBC
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too big
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too small using MBEDTLS_ENTROPY_BLOCK_SIZE
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too small using MBEDTLS_ENTROPY_MIN_PLATFORM
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: before and after crypto_init
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: good, max size
Warning: Test case not executed: test_suite_psa_crypto_entropy;PSA validate entropy injection: good, minimum size
Warning: Test case not executed: test_suite_psa_crypto_metadata;Asymmetric signature: SHA-256 + deterministic DSA using SHA-256 [#1]
Warning: Test case not executed: test_suite_psa_crypto_metadata;Asymmetric signature: SHA-256 + randomized DSA SHA-256 using SHA-256
Warning: Test case not executed: test_suite_psa_crypto_metadata;Asymmetric signature: deterministic DSA with wildcard hash [#1]
Warning: Test case not executed: test_suite_psa_crypto_metadata;Asymmetric signature: randomized DSA with wildcard hash
Warning: Test case not executed: test_suite_psa_crypto_metadata;Cipher: ChaCha20
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-2 SHA-512/224
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-2 SHA-512/256
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-224
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-256
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-384
Warning: Test case not executed: test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-512
Warning: Test case not executed: test_suite_psa_crypto_metadata;Key type: DSA key pair
Warning: Test case not executed: test_suite_psa_crypto_metadata;Key type: DSA public key
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA-512/224
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA-512/256
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-224
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-256
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-384
Warning: Test case not executed: test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-512
Warning: Test case not executed: test_suite_ssl;SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_SHA256 SHA-256 not enabled

See a subsequent comment for analysis.

The complete outcomes.csv from running all.sh on Linux as of #3458 (i.e. shortly before Mbed TLS 2.23): outcomes.csv.gz

gilles-peskine-arm commented 4 years ago

Analysis of the cases reported as not executed

Deliberately skipped SSL tests

Some test cases are prefixed by skip_next_test. They are skipped deliberately. They should not be considered to be available.

All of these test cases trigger a bug in OpenSSL that is fixed in 1.0.2q, 1.1.1a and 3.0.0 (upcoming). On our CI we currently only run ssl-opt.sh with 1.0.2g (and 1.0.1j for legacy), and we only have 1.1.1 (next) besides that.

ssl-opt;DTLS fragmenting: 3d, openssl client, DTLS 1.0
ssl-opt;DTLS fragmenting: 3d, openssl client, DTLS 1.2
ssl-opt;DTLS fragmenting: 3d, openssl server, DTLS 1.0
ssl-opt;DTLS fragmenting: 3d, openssl server, DTLS 1.2
ssl-opt;DTLS proxy: 3d, openssl server
ssl-opt;DTLS proxy: 3d, openssl server, fragmentation
ssl-opt;DTLS proxy: 3d, openssl server, fragmentation, nbio

SSL tests specifically for valgrind

We don't run ssl-opt.sh with valgrind. all.sh can do it but only with an additional option --memory.

ssl-opt;DTLS client reconnect from same port: reconnect, nbio, valgrind
ssl-opt;DTLS fragmenting: proxy MTU: auto-reduction (with valgrind)

SSL tests with a double quote in the description

In ssl-opt.sh, the test cases are in double-quoted string literals. Some test cases have a description that contains a double quote, which is written \". There is a defect in check_test_cases.py in that it treats \" as being part of the description. This didn't matter for description validity and unicity checking, but it matters in the new analysis where the string is compared with the description. A simple fix would be to strip the backslash in check_test_cases.py, but we might as well not bother with that since a fix that works for parametrized SSL tests would also fix this as a side effect.

ssl-opt;DTLS proxy: 3d (drop, delay, duplicate), \"short\" PSK handshake
ssl-opt;DTLS proxy: 3d, \"short\" (no ticket, no cli_auth) FS handshake
ssl-opt;DTLS proxy: 3d, \"short\" RSA handshake

Parametrized SSL tests

Some test cases in ssl-opt.sh are parametrized. The analysis script (via check_test_cases.py) treats the uninterpolated shell literal as the test case description, with $1 instead of the parameter value. This isn't good enough to know the test case descriptions at runtime. 

ssl-opt;Handshake memory usage (MFL $1)
ssl-opt;PSA - ECDH with $1
ssl-opt;PSA-supported ciphersuite: $1

RC4

These test cases check that a ciphersuite is disabled. But there is a generic mechanism that skips tests that use a disabled ciphersuite (requires_ciphersuite_enabled), so these test cases never run. This is a bug in ssl-opt.sh. Fixed in #3463.

ssl-opt;RC4: server disabled, client enabled
ssl-opt;RC4: server half, client enabled

Mistakes in dependencies

Fixed in #3463.

test_suite_cipher.gcm;AES GCM Decrypt empty buffer
test_suite_psa_crypto;PSA key policy algorithm2: CTR, CBC
test_suite_psa_crypto_metadata;Cipher: ChaCha20

PSA entropy injection

This feature was developed for the needs of Pelion. In production, it requires additional functions, and we don't have a mock of these functions for testing. The feature needs a redesign, but the functions aren't very difficult to mock so we should do this.

test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too big
test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too small using MBEDTLS_ENTROPY_BLOCK_SIZE
test_suite_psa_crypto_entropy;PSA validate entropy injection: bad, too small using MBEDTLS_ENTROPY_MIN_PLATFORM
test_suite_psa_crypto_entropy;PSA validate entropy injection: before and after crypto_init
test_suite_psa_crypto_entropy;PSA validate entropy injection: good, max size
test_suite_psa_crypto_entropy;PSA validate entropy injection: good, minimum size

PSA crypto metadata

The PSA crypto metadata test cases only run if the corresponding algorithm might be enabled. The reason is that the macros that calculate the metadata might not give correct results for algorithms that are enabled. There are test cases for algorithms that are not implemented. Arguably either these test cases should be removed (if the metadata isn't correct) or they should be enabled in all configurations (if the metadata is correct).

Fixed in #3463 by removing the test cases.

test_suite_psa_crypto_metadata;Asymmetric signature: SHA-256 + deterministic DSA using SHA-256 [#1]
test_suite_psa_crypto_metadata;Asymmetric signature: SHA-256 + randomized DSA SHA-256 using SHA-256
test_suite_psa_crypto_metadata;Asymmetric signature: deterministic DSA with wildcard hash [#1]
test_suite_psa_crypto_metadata;Asymmetric signature: randomized DSA with wildcard hash
test_suite_psa_crypto_metadata;Hash: SHA-2 SHA-512/224
test_suite_psa_crypto_metadata;Hash: SHA-2 SHA-512/256
test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-224
test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-256
test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-384
test_suite_psa_crypto_metadata;Hash: SHA-3 SHA3-512
test_suite_psa_crypto_metadata;Key type: DSA key pair
test_suite_psa_crypto_metadata;Key type: DSA public key
test_suite_psa_crypto_metadata;MAC: HMAC-SHA-512/224
test_suite_psa_crypto_metadata;MAC: HMAC-SHA-512/256
test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-224
test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-256
test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-384
test_suite_psa_crypto_metadata;MAC: HMAC-SHA3-512

TLS without SHA-256

depends-hashes.pl excludes SSL. This is deliberate but perhaps not desirable. One test case ends up never running as a consequence.

test_suite_ssl;SSL TLS_PRF MBEDTLS_SSL_TLS_PRF_SHA256 SHA-256 not enabled
gilles-peskine-arm commented 4 years ago

I haven't done any analysis for LTS branches (2.16, 2.7) because they don't have the outcome file machinery, so we don't have a way to collect the data. I think that's acceptable: if we backport any applicable fix from development that affects test coverage, LTS branches shouldn't lack coverage.

gilles-peskine-arm commented 3 years ago

Note that in 3.0, many test cases in ssl-opt.sh are not executed because the corresponding feature has been removed, but the test cases are still present. This is expected and tracked in https://github.com/ARMmbed/mbedtls/issues/4564.