Run ssl-opt.sh in more reduced configurations #5582

gilles-peskine-arm · 2022-02-25T20:55:26Z

This is an incremental improvement to the ability to run tests/ssl-opt.sh in reduced configurations. The main completeness goals is for it to pass on config/config-ccm-psk-dtls1_2.h and config-suite-b.h, and to be roughly aligned with the 2.28 backport. Beyond that, there are a few miscellaneous improvements in automatic requirement detection.

Add some missing requires_xxx statements.
Deduce a few more requirements automatically from the parameters passed to the client and the server.
In PSK-only builds, run some tests. (I don't seek to improve builds with PSK + asymmetric key exchange, but I don't think I'm making anything worse.)
New configuration config-ccm-psk-dtls1_2.h, based on config-ccm-psk-dtls1_2.h but with a few more DTLS options and a smaller SSL buffer, so more representative of IoT devices that don't do asymmetric crypto.
Minor improvements and fixes to test-ref-configs.pl.
Resolve lsof on FreeBSD might emit warnings and fail component_test_cmake_out_of_source #5731 (workaround for a problem with the test infrastructure).

Note on "Automatically detect protocol version requirement from force_version": I left a spurious requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 on "TLS 1.3: CertificateRequest check - openssl" to avoid a merge conflict.

Backport: #5730

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]>

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. 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]>

The message was removed in 6be9cf5 without a replacement. A failure would cause the test case to fail anyway, so this negative check is not really useful. Signed-off-by: Gilles Peskine <[email protected]>

Signed-off-by: Gilles Peskine <[email protected]>

tests/scripts/test-ref-configs.pl

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]>

gilles-peskine-arm · 2022-04-13T20:46:41Z

This PR and its backport are ready to review.

component_test_CID_no_debug was added specifically to be a non-regression test for Mbed-TLS#3998. Running compat.sh in the newly introduced config-ccm-psk-dtls1_2.h is also a non-regression test for that bug. Therefore component_test_CID_no_debug is redundant for its primary purpose. Of course every configuration is different, but the additional coverage from component_test_CID_no_debug is minimal, unlike config-ccm-psk-dtls1_2.h which is a plausible real-world configuration. In mbedtls-2.28, component_test_CID_no_debug was never added, and running the unit tests in that configuration does not trigger the Mbed-TLS#3998 bug, only compat.sh does. So, rather than backport component_test_CID_no_debug to 2.28.2, I am removing it from 3.2. Signed-off-by: Gilles Peskine <[email protected]>

mpg

LGTM. Thanks!

mpg · 2022-04-15T08:12:47Z

Did we check that this PR doesn't result in more tests being skipped (incorrectly, then) in the default config / full config?

gilles-peskine-arm · 2022-04-15T09:13:20Z

@mpg Good point, I did partial incremental checks but not a final comprehensive check. I'll go and dig CI logs.

mpg · 2022-04-15T09:14:06Z

Well, I've been doing local runs. The last one's almost finished, I'll post the results in a moment.

mpg · 2022-04-15T09:29:50Z

Full config:

before: PASSED (842 / 842 tests (31 skipped))
after: PASSED (844 / 844 tests (31 skipped))

Default config:

before: PASSED (842 / 842 tests (421 skipped))
after: PASSED (844 / 844 tests (421 skipped))

For "before" I used the merge-base which is 9a34b60

The number skipped are identical but the total number of tests reported is 2 more after this PR, which corresponds to the two mbedtls_ssl_get_bytes_avail cases added, so everything's looking as expected.

gilles-peskine-arm · 2022-04-15T13:13:30Z

On both this PR and the 2.28 backport, I went and compared logs from the latest pr-merge and the nearest nightly:

I extracted ssl-opt.sh PASS lines from the full logs, but unfortunately they're hard to use because the lines from parallel jobs are mixed together. The nightly runs ssl-opt for coverage, so additional lines from that are expected. So I sorted the log lines and checked for lines that have more than one more occurrence in the nightly log compared to the PR log. This is what I found:

~/tmp/5582 % cat Makefile 
all: dev-nightly.pass dev-pr.pass lts-nightly.pass lts-pr.pass
all: dev-nightly-full.pass dev-pr-full.pass lts-nightly-full.pass lts-pr-full.pass

%.txt: %.html
        sed -n 's/^.*<\/style>//p' <$< >$@

%.pass: %.txt
        grep 'PASS$$' $< | grep -v '^test_suite_' | grep -v '^[mGO]->[mGO] ' |sort >$@
~/tmp/5582 % comm -23 dev-nightly.pass dev-pr.pass |uniq -d
PASS
Session resume using tickets, DTLS: openssl server ..................... PASS
mbedtls_ssl_get_bytes_avail: extra data ................................ PASS
~/tmp/5582 % comm -23 lts-nightly.pass lts-pr.pass |uniq -d
Fallback SCSV: disabled, openssl client ................................ PASS
Fallback SCSV: enabled, openssl client ................................. PASS
RC4: server enabled, client disabled ................................... PASS
mbedtls_ssl_get_bytes_avail: extra data ................................ PASS

So in the PR for development, there's the renamed test case (expected) and one flaky test which had a newline-RETRY-pass in the nightly, and all is good. In the 2.28 PR, there's the renamed test case (expected) and three test cases that are unexpected.

RC4: server enabled, client disabled is still running in full, but there's one configuration where it's no longer running. See https://github.com/Mbed-TLS/mbedtls/pull/5730/files#diff-54a2261aca14ebb2491a1584cc3351a458487c23c25f90df08de2573cd705e32R1727-R1729
Fallback SCSV: disabled, openssl client and Fallback SCSV: enabled, openssl client are no longer running in full. I made a typo in the requires line, which I've just pushed a fix for. (I should have checked “test case never executed” logs — I look forward to the day we can make it a CI failure).

Note that this analysis is imprecise: if a test case is now running in a config A but wasn't before, and was running in config B but isn't anymore, the two compensate.

I also checked the full configuration specifically (from the logs of test_full_cmake_gcc_asan), and the only difference is the two SCSV test cases in 2.28.

gilles-peskine-arm · 2022-04-15T16:41:03Z

Wouldn't it be nice if instead of digging through logs, there was a report from the CI containing a list of executed test cases with their outcome? 🤦

As before, comparing last night's pr-merge with last night's nightly:

$ cat Makefile
all: dev-nightly-ssl.csv dev-pr-ssl.csv lts-nightly-ssl.csv lts-pr-ssl.csv
%-ssl.csv: %-outcomes.csv.xz
        xzcat $< | grep ';ssl-opt;' | sort >$@
$ comm -23 dev-nightly-ssl.csv dev-pr-ssl.csv |grep -v ';mbedtls_ssl_get_bytes_avail: extra data;'
Linux-x86_64;config-thread.h;ssl-opt;ECJPAKE: working, DTLS, nolog;PASS;
$ comm -23 lts-nightly-ssl.csv lts-pr-ssl.csv |grep -v ';mbedtls_ssl_get_bytes_avail: extra data;'
FreeBSD-amd64;component_test_cmake_out_of_source;ssl-opt;Fallback SCSV: beginning of list;SKIP;
Linux-x86_64;component_test_asan_remove_peer_certificate;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_asan_remove_peer_certificate;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_asan_remove_peer_certificate;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_default_cmake_gcc_asan;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_default_cmake_gcc_asan;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_default_cmake_gcc_asan;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_full_cmake_gcc_asan;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_full_cmake_gcc_asan;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_m32_o2;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_m32_o2;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_malloc_0_null;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_malloc_0_null;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_memory_buffer_allocator;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_memory_buffer_allocator;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_memory_buffer_allocator;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_memsan;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_memsan;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_memsan;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_no_pem_no_fs;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_no_pem_no_fs;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_no_pem_no_fs;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_no_renegotiation;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_no_renegotiation;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_no_renegotiation;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_no_use_psa_crypto_full_cmake_asan;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_no_use_psa_crypto_full_cmake_asan;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_sslv3;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_sslv3;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_sslv3;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_CID;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_CID;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_CID;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_record_splitting;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_record_splitting;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_variable_ssl_in_out_buffer_len_record_splitting;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_zlib_cmake;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_zlib_cmake;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_zlib_cmake;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;component_test_zlib_make;ssl-opt;Fallback SCSV: disabled, openssl client;PASS;
Linux-x86_64;component_test_zlib_make;ssl-opt;Fallback SCSV: enabled, openssl client;PASS;
Linux-x86_64;component_test_zlib_make;ssl-opt;RC4: server enabled, client disabled;PASS;
Linux-x86_64;config-thread.h;ssl-opt;ECJPAKE: working, DTLS, nolog;PASS;

So in addition to the previously analyzed SCSV and RC4 test cases, there's one more difference:

Linux-x86_64;config-thread.h;ssl-opt;ECJPAKE: working, DTLS, nolog;PASS;

This difference comes from "test-ref-configs: clarify configuration-related traces", which fixed the bug in test-ref-configs.pl whereby it made indistinguishable reports from running tests in a certain configuration and running in that configuration plus PSA. So now, instead of seeing the line above twice, we have that line once and

Linux-x86_64;config-thread.h+PSA;ssl-opt;ECJPAKE: working, DTLS, nolog;PASS;

once. So all is good.

If the ssl-opt test case was skipped, the test was ineffective. Signed-off-by: Gilles Peskine <[email protected]>

component_test_cmake_out_of_source was running the ssl-opt.sh test case "Fallback SCSV: beginning of list", but this test case was removed in Mbed TLS 3.0, so ssl-opt.sh was running nothing, which is not an effective test. In 2.x, the test case was chosen because it uses an additional auxiliary program tests/scripts/tcp_client.pl. This auxiliary program is no longer used. So instead, run at least one test case that's sure to exist. 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]>

mpg

LGTM. Does this (and its backport) close #5731 then?

gilles-peskine-arm · 2022-04-19T08:27:47Z

@mpg Indeed it does. I've added the magic syntax to this PR's description.

paul-elliott-arm

LGTM

gilles-peskine-arm added 10 commits February 25, 2022 15:26

Fix the build when MBEDTLS_PLATFORM_C is unset

fd222da

Signed-off-by: Gilles Peskine <[email protected]>

Minimal TLS configuration: documentation improvements

7451e5a

Some of the options have been moved around, but there are no semantic changes. Signed-off-by: Gilles Peskine <[email protected]>

New sample/test configuration: small DTLS 1.2

168f17c

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]>

ssl-opt: automatically skip DTLS tests in builds without DTLS

82a4ab2

Signed-off-by: Gilles Peskine <[email protected]>

Automatically skip tests for some absent features: tickets, ALPN

a165b5c

Signed-off-by: Gilles Peskine <[email protected]>

Only run "Default" tests if the expected ciphersuite is enabled

3561526

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]>

Add some missing dependencies: EXTENDED_MASTER_SECRET, CACHE

2fe796f

This commit is not necessarily complete, but it's a step forward. Signed-off-by: Gilles Peskine <[email protected]>

Add a missing requires_max_content_len

588d7a7

Slightly reduce the amount of data so that the test passes with 512. Signed-off-by: Gilles Peskine <[email protected]>

gilles-peskine-arm added enhancement needs-work needs-ci Needs to pass CI tests component-test Test framework and CI scripts labels Feb 25, 2022

gilles-peskine-arm added the size-s Estimated task size: small (~2d) label Feb 25, 2022

gilles-peskine-arm added 4 commits March 14, 2022 19:49

Remove negative check for a message that no longer exists

aa162b5

The message was removed in 6be9cf5 without a replacement. A failure would cause the test case to fail anyway, so this negative check is not really useful. Signed-off-by: Gilles Peskine <[email protected]>

Fix unused function warning

4c3f240

Signed-off-by: Gilles Peskine <[email protected]>

gilles-peskine-arm commented Mar 14, 2022

View reviewed changes

tests/scripts/test-ref-configs.pl Show resolved Hide resolved

gilles-peskine-arm added 2 commits March 14, 2022 20:32

Skip some DTLS reordering tests in PSK-only builds

6f160ca

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]>

gilles-peskine-arm force-pushed the ssl-opt-auto-psk branch from 722e426 to 6f160ca Compare March 14, 2022 19:32

gilles-peskine-arm marked this pull request as ready for review March 14, 2022 19:32

gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review needs-backports Backports are missing or are pending review and approval. and removed needs-work labels Mar 14, 2022

gilles-peskine-arm added the needs-review Every commit must be reviewed by at least two team members, label Apr 13, 2022

gilles-peskine-arm mentioned this pull request Apr 13, 2022

Run ssl-opt.sh on config-mini-tls1_1.h #5734

Open

mpg previously approved these changes Apr 14, 2022

View reviewed changes

gilles-peskine-arm added the needs-work label Apr 15, 2022

gilles-peskine-arm added 3 commits April 16, 2022 11:30

test_cmake_out_of_source: validate that ssl-opt passed

7393ec5

If the ssl-opt test case was skipped, the test was ineffective. Signed-off-by: Gilles Peskine <[email protected]>

gilles-peskine-arm dismissed mpg’s stale review via 5bd0b51 April 16, 2022 09:36

gilles-peskine-arm removed the needs-work label Apr 16, 2022

gilles-peskine-arm requested a review from mpg April 16, 2022 12:00

mpg approved these changes Apr 19, 2022

View reviewed changes

paul-elliott-arm approved these changes Apr 20, 2022

View reviewed changes

mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Apr 21, 2022

gilles-peskine-arm merged commit afbfed9 into Mbed-TLS:development Apr 21, 2022

gilles-peskine-arm mentioned this pull request Sep 9, 2024

[3.6] ssl-opt: improve PSK mode detection #9546

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run ssl-opt.sh in more reduced configurations #5582

Run ssl-opt.sh in more reduced configurations #5582

gilles-peskine-arm commented Feb 25, 2022 •

edited

Loading

gilles-peskine-arm commented Apr 13, 2022

mpg left a comment

mpg commented Apr 15, 2022

gilles-peskine-arm commented Apr 15, 2022

mpg commented Apr 15, 2022

mpg commented Apr 15, 2022 •

edited

Loading

gilles-peskine-arm commented Apr 15, 2022 •

edited

Loading

gilles-peskine-arm commented Apr 15, 2022

mpg left a comment

gilles-peskine-arm commented Apr 19, 2022

paul-elliott-arm left a comment

Run ssl-opt.sh in more reduced configurations #5582

Run ssl-opt.sh in more reduced configurations #5582

Conversation

gilles-peskine-arm commented Feb 25, 2022 • edited Loading

gilles-peskine-arm commented Apr 13, 2022

mpg left a comment

Choose a reason for hiding this comment

mpg commented Apr 15, 2022

gilles-peskine-arm commented Apr 15, 2022

mpg commented Apr 15, 2022

mpg commented Apr 15, 2022 • edited Loading

gilles-peskine-arm commented Apr 15, 2022 • edited Loading

gilles-peskine-arm commented Apr 15, 2022

mpg left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Apr 19, 2022

paul-elliott-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Feb 25, 2022 •

edited

Loading

mpg commented Apr 15, 2022 •

edited

Loading

gilles-peskine-arm commented Apr 15, 2022 •

edited

Loading