Fix interoperability when MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is disabled #9628
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
The *.sh files in opt-testcases cannot be executed directly: they can only be sourced by ssl-opt.sh. So don't make them executable and don't give them a shebang line. Also make sure that the first paragraph of each file is a short description. Signed-off-by: Gilles Peskine <[email protected]>
For better searchability and readability, call requires_config_enabled or
requires_config_disabled for each option, instead of calling
requires_all_configs_enabled or requires_all_configs_disabled with a long
list of options.
```
perl -0777 -i -pe '
# With -0777, we act on the whole file.
# s[REGEXP][CODE]egm replaces every occurrence of REGEXP by the result
# of running CODE.
# The regexp matches "requires_all_configs_enabled" or
# "requires_all_configs_disabled" followed by a list of words ending
# with a line break. The words can be separated by a sequence of
# spaces and optionally a backslash-newline.
s[^requires_all_configs_(enabled|disabled) *((?:(?: \w+) *(?:\\\n)? *)+)\n][
$state = $1;
# Extract all the words from the list of words (/(\w+)/g). For each word,
# For each word, construct a line "requires_config_XXXabled WORD".
# The replacement text is the concatenation of these lines.
join("", map {"requires_config_$state $_\n"} $2 =~ /(\w+)/g)
]egm' tests/ssl-opt.sh tests/opt-testcases/*.sh
```
Signed-off-by: Gilles Peskine <[email protected]>
The compile-time option MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE gates both support for interoperability with a peer that uses middlebox compatibility mode, and support for activating that mode ourselves. Change code that is only needed for interoperability to be guarded by MBEDTLS_SSL_TLS1_3_ACCEPT_COMPATIBILITY_MODE. As of this commit, MBEDTLS_SSL_TLS1_3_ACCEPT_COMPATIBILITY_MODE is always enabled: there is no way to disable it, and there are no tests with it disabled. Signed-off-by: Gilles Peskine <[email protected]>
Adapt the test cases for TLS 1.3 middlebox compatibility mode, now that we always interoperate with peers that support it, regardless of whether MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is enabled. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is no longer required, except in test
cases that are specifically about it. This commit removes the requirement on
all test cases except those whose description contains "middlebox".
Exclude tls13-compat.sh which is automatically generated and will be handled
in a separate commit.
```
perl -0777 -i -pe '
# With -0777, we act on the whole file.
# s[REGEXP][EXPR]gm replaces every occurrence of REGEXP by EXPR.
# The regexp matches "requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE",
# followed by zero or more non-empty lines, followed by a line starting
# with "run_test" and not containing "middlebox".
# The replacement is everything matched except the first line.
s[^requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE\n((?:.+\n)*run_test (?!.*middlebox))]
[$1]gm' tests/ssl-opt.sh tests/opt-testcases/tls13-kex-modes.sh tests/opt-testcases/tls13-misc.sh
```
Signed-off-by: Gilles Peskine <[email protected]>
MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE is no longer required, except in test cases that are specifically about it. This commit removes the requirement in tls13-compat.sh (which does not have test cases that actually depend on the feature). Signed-off-by: Gilles Peskine <[email protected]>
With no options, update the output file (former behavior with -a). Pass -1 to generate a single test case. Also have the intended output file location as the default. This way, you can just run the script after updating it, without having to know the details of the directory structure. Signed-off-by: Gilles Peskine <[email protected]>
`tests/opt-testcases/tls13-compat.sh` is supposed to be automatically generated by `tests/scripts/generate_tls13_compat_tests.py`. So far, the output has been updated by running the script manually and committing the output. Switch to using our framework for generated files. Signed-off-by: Gilles Peskine <[email protected]>
Fix `tls13-compat.sh` changing based on exactly how `generate_tls13_compat_tests.py` was run (e.g. from which directory). This made `check-generated-files.sh` behave differently from `make`. The script has no official variations of the content of its output file, so we don't need to record the full command line. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
needs-backports
Merged
5 tasks
gilles-peskine-arm
added
needs-review
ronald-cron-arm
previously approved these changes
Sep 25, 2024
There was a problem hiding this comment.
I've been through the rebase of #9563 and ended up with the same tree as in this PR HEAD~1. The last commit looks good to me.
mpg
previously approved these changes
Sep 25, 2024
There was a problem hiding this comment.
LGTM as a faithful forward port of #9563, with an additional commit at the end addressing #9563 (comment)
mpg
reviewed
Sep 25, 2024
| @@ -78,6 +78,7 @@ if(GEN_FILES) | |||
| add_custom_target(tls13-compat.sh | |||
| DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/opt-testcases/tls13-compat.sh) | |||
| set_target_properties(tls13-compat.sh PROPERTIES EXCLUDE_FROM_ALL NO) | |||
| add_dependencies(${ssl_opt_target} tls13-compat.sh) | |||
There was a problem hiding this comment.
Actually wait, shouldn't we do something similar for make as well?
There was a problem hiding this comment.
Good point! I'd only thought about it from the point of view of having already run make generated_files, which doesn't exist with CMake. But I do expect all make targets to work from a fresh checkout. Fixed.
gilles-peskine-arm
added
approved
This is necessary if you haven't run `make generated_files` first. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
dismissed stale reviews from mpg and ronald-cron-arm
via
870ed2a
gilles-peskine-arm
added
needs-review
ronald-cron-arm
approved these changes
Sep 25, 2024
ronald-cron-arm
added
approved
Merged
via the queue into
Mbed-TLS:development
with commit Sep 25, 2024
2efb3da
4 of 5 checks passed
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.
When
MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODEis disabled, if an Mbed TLS client or server attempted a handshake with a peer that uses compatibility mode, it would abort the handshake. Fix that: now, even withMBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODEdisabled, Mbed TLS can communicate with e.g. a default OpenSSL or GnuTLS or Mbed TLS, as long as there is no middlebox in the way. Fixes #9551.Content of this PR:
requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODEexcept when testing middlebox compatibility mode.tests/opt-testcases/tls13-compat.shas a generated file.Differences with #9563:
make ssl-optgeneratetls13-compat.sh" takes care of Switch from test-ref-configs.pl to separate components #9565 (comment) since the other PR has just been merged. For 3.6, I'll do a follow-up PR (there's no harm now since none of the reference configurations enable TLS 1.3).PR checklist