Backport 2.28: Run ssl-opt.sh in more reduced configurations #5730
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
Signed-off-by: Gilles Peskine <[email protected]>
Some of the options have been moved around, but there are no semantic changes. Signed-off-by: Gilles Peskine <[email protected]>
1. Copy config-ccm-psk-tls1_2.h 2. Add DTLS support 3. Add some TLS and DTLS features that are useful in low-bandwidth, low-reliability networks 4. Reduce the SSL buffer to a very small size Signed-off-by: Gilles Peskine <[email protected]>
Since MBEDTLS_TIMING_C is enabled, mbedtls_entropy_init() adds the weak source MBEDTLS_ENTROPY_MAX_SOURCES(). With mbedtls_platform_entropy_poll(), this makes two sources. The unit tests need room for a third source. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Skip tests that require a specific version of the protocol if that version is disabled at compile time. This commit only partially does the job, mostly covering tests that check the protocol version in client or server logs. It is not intended to be exhaustive; in particular many uses of force_version are not covered (I think they should instead be covered automatically, but this is out of scope of the current commit). Signed-off-by: Gilles Peskine <[email protected]>
These tests ensure that a certain cipher suite is in use, so they fail in builds that lack one of the corresponding algorithms. Signed-off-by: Gilles Peskine <[email protected]>
This commit is not necessarily complete, but it's a step forward. Signed-off-by: Gilles Peskine <[email protected]>
In a PSK-only build: * Skip tests that rely on a specific non-PSK cipher suite. * Skip tests that exercise a certificate authentication feature. * Pass a pre-shared key in tests that don't mind the key exchange type. This commit only considers PSK-only builds vs builds with certificates. It does not aim to do something useful for builds with an asymmetric key exchange and a pre-shared key for authentication. Signed-off-by: Gilles Peskine <[email protected]>
Many test cases in ssl-opt.sh need error messages (MBEDTLS_ERROR_C) or SSL traces (MBEDTLS_DEBUG_C). Some sample configurations don't include these options. When running ssl-opt.sh on those configurations, enable the required options. They must be listed in the config*.h file, commented out. Run ssl-opt in the following configurations with debug options: ccm-psk-tls1_2, ccm-psk-dtls1_2, suite-b. Skip mini-tls1_1 for now because it requires significant improvements to ssl-opt.sh (lots of missing requires_xxx). Signed-off-by: Gilles Peskine <[email protected]>
Slightly reduce the amount of data so that the test passes with 512. Signed-off-by: Gilles Peskine <[email protected]>
User-visible changes: * With no argument, configurations are now tested in a deterministic order. * When given arguments, configurations are now tested in the order given. * When given arguments, if the same configuration is passed multiple times, it will now be tested multiple times. Signed-off-by: Gilles Peskine <[email protected]>
When doing builds with PSA enabled or with debug traces enabled, convey this in $MBEDTLS_TEST_CONFIGURATION and in the terminal logs. This fixes a bug that the outcome file did not distinguish entries from test cases run in a reference configuration with or without PSA. Signed-off-by: Gilles Peskine <[email protected]>
If MBEDTLS_SSL_EXTENDED_MASTER_SECRET is disabled or the feature is disabled at runtime, and if client authentication is not used, then calc_verify is not called, so don't require the corresponding debug trace. Signed-off-by: Gilles Peskine <[email protected]>
Some DTLS reordering tests rely on certificate authentication messages. It is probably possible to adapt them to rely on different messages, but for now, skip them in PSK-only builds. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
The log checks require a specific hash and a specific curve. Signed-off-by: Gilles Peskine <[email protected]>
Document that this is what it is. Don't allow made-up numerical values. Signed-off-by: Gilles Peskine <[email protected]>
Make sure that buf always has enough room for what it will contain. Before, this was not the case if the buffer was smaller than the default response, leading to memory corruption in ssl_server2. Signed-off-by: Gilles Peskine <[email protected]>
The added null byte was accounted for twice, once by taking opt.buffer_size+1 when allocating the buffer and once by taking opt.buffer-1 when filling the buffer. Make opt.buffer_size the size that is actually read, it's less confusing that way. Signed-off-by: Gilles Peskine <[email protected]>
Don't depend on the default sizes in the test programs: pass explicit request and buffer sizes. Don't depend on MAX_CONTENT_LEN (other than it not being extremely small: this commit assumes that it will never be less than 101). Signed-off-by: Gilles Peskine <[email protected]>
The automatic ciphersuite detection deliberately doesn't operate on test cases that verify that the test suite is rejected, but some RC4 test cases only apply to configurations where the algorithm must be enabled at compile time (otherwise the connection would fail in a different way). Signed-off-by: Gilles Peskine <[email protected]>
Rename maybe_requires_ciphersuite_enabled() to detect_required_features() and refactor its code a little. No intended behavior change. In subsequent commits, this function will detect other requirements in a similar way. Signed-off-by: Gilles Peskine <[email protected]>
No intended behavior change. Signed-off-by: Gilles Peskine <[email protected]>
When the client or server uses a specific protocol version, automatically require that version to be enabled at compile time. An explicit call is still needed in test cases that require a specific protocol version (due to analyzing version-specific behavior, or checking the version in logs), but do not force that specific protocol version, or that force a specific version only on the openssl/gnutls side. Signed-off-by: Gilles Peskine <[email protected]>
Automatically detect when an mbedtls or openssl client enables fallback SCSV. For test cases with a hard-coded ClientHello with FALLBACK_SCSV, declare the dependency manually. Remove the erroneous requirement on openssl in these test cases. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
tests/ssl-opt.sh
Outdated
Comment on lines
1727
to
1729
| requires_ciphersuite_enabled TLS-RSA-WITH-RC4-128-SHA | ||
| run_test "RC4: server enabled, client disabled" \ | ||
| "$P_SRV force_ciphersuite=TLS-RSA-WITH-RC4-128-SHA" \ |
There was a problem hiding this comment.
With this PR, this test case is no longer running in configurations that where this RC4 ciphersuite is usable, but disabled by default at runtime due to MBEDTLS_REMOVE_ARC4_CIPHERSUITES. This is a quirk of how ciphersuites are detected: ssl-opt.sh calls ssl_client2 help, which calls mbedtls_ssl_list_ciphersuites, which omits ciphersuites removed through MBEDTLS_REMOVE_ARC4_CIPHERSUITES.
This test case validates that a client won't try to use RC4 when MBEDTLS_REMOVE_ARC4_CIPHERSUITES is enabled, so it should run in configurations where the ciphersuite is available upon request but disabled by default, which includes the default configuration. So I'll change the dependencies and explain.
The same quirk exists for 3DES ciphersuites, but it doesn't make a difference to ssl-opt.sh since it never tries to use DES.
There was a problem hiding this comment.
I pushed a commit that fixes the RC4 test cases: 719a652.
I'm not very happy with this hack. How about expanding query_compile_time_config to list all ciphersuites (not just the ones enabled by default)?
There was a problem hiding this comment.
How about expanding query_compile_time_config to list all ciphersuites (not just the ones enabled by default)?
Done in https://github.com/gilles-peskine-arm/mbedtls/tree/ssl-opt-auto-psk-2.28-query_ciphersuites . But I'm not very happy with the way I've done it. It causes query_compile_time_config to require functions in libmbedtls (and so as it stands it'll fail in shared library builds). Rather, I should copy the ciphersuite_definitions array from ssl_ciphersuites.c.
When ARC4 ciphersuites are compiled in, but removed from the default list, requires_ciphersuite_enabled does not consider them to be enabled. Therefore test cases for MBEDTLS_REMOVE_ARC4_CIPHERSUITES, which must run in such configurations, must not use requires_ciphersuite_enabled. Instead, require the corresponding cryptographic mechanisms. In addition, for the test case "RC4: both enabled", bypass the automatic ciphersuite support detection based on force_ciphersuite= that would otherwise cause this test case to be skipped. (This automatic detection doesn't cause the negative tests to be skipped because it has an exception whenthe handshake is supposed to fail.) Signed-off-by: Gilles Peskine <[email protected]>
Comment on lines
-2821
to
3005
| requires_openssl_with_fallback_scsv | ||
| requires_config_enabled MBEDTLS_SSL_FALLBACK_SCSV | ||
| run_test "Fallback SCSV: beginning of list" \ |
There was a problem hiding this comment.
Fixing the dependency of this test case is causing a CI failure on FreeBSD. The failure isn't happening on the base branch because of the incorrect dependency. #5731
|
As of 719a652, the set of |
If the ssl-opt test case was skipped, the test was ineffective. Signed-off-by: Gilles Peskine <[email protected]>
This both simplifies parsing a little, and suppresses warnings. Suppressing warnings is both good and bad: on the one hand it resolves problems such as Mbed-TLS#5731, on the other hand it may hide clues as to why lsof wouldn't be working as expected. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
ssl-opt-auto-psk-2.28
branch
from
fd426fa to
36019d5
Compare
gilles-peskine-arm
added
the
needs-reviewer
gilles-peskine-arm
removed
the
needs-reviewer
gilles-peskine-arm
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.
This is an incremental improvement to the ability to run
tests/ssl-opt.shin reduced configurations. The main completeness goals is for it to pass onconfig/config-ccm-psk-dtls1_2.handconfig-suite-b.h, and to be roughly aligned with the 3.1 version. Beyond that, there are a few miscellaneous improvements in automatic requirement detection.Changes in the 2.28 version compared to the 3.1 version:
MBEDTLS_SHA224_C.MBEDTLS_SSL_MAX_CONTENT_LENinstead of {IN,OUT}.configs/config-ccm-psk-dtls1_2.hso that it's a DTLSadaptation of 2.28's
configs/config-ccm-psk-tls1_2.hrather than adirect drop-in of the 3.1 version.
configs/config-ccm-psk-dtls1_2.hhas an extra entropy source (hardclock)which was removed in 3.0.
ssl-opt.shhad fewerrequires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2so I made an additional commit for that. Most of those were introduced in 3.x in ab08290 from TLS1.3:Enable tls13 only build #5362, but that was about 1.2 vs 1.3 and many of the places that added "needs 1.2" are actually "needs ≤1.2" and thus should not be there in 2.28. As a result of not needing a specific version for many test cases, there are many rebase conflicts in commits that add some other config requirement to test cases.in 2.28 where the message in question exists.
MBEDTLS_SSL_DTLS_BADMAC_LIMIT(which was removed in 3.0) inssl-opt.sh. Enable it inconfigs/config-ccm-psk-dtls1_2.hanyway.developmentbut not backported the fix).An additional reference config exists:
configs/config-mini-tls1_1.h. It would be good to runssl-opt.shin this configuration, but this would require a lot of extra work inssl-opt.sh. I've started working on it, but it would delay and grow this backport significantly, so I've filed it for future work.