Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skipped test cases in ssl-opt.sh in TLS 1.3-only configurations #9560

Open
gilles-peskine-arm opened this issue Sep 11, 2024 · 1 comment
Open

Comments

@gilles-peskine-arm
Copy link
Contributor

Many TLS 1.3 test cases are skipped in configurations where only TLS 1.3 is enabled.

scripts/config.py set MBEDTLS_USER_CONFIG_FILE="../tests/configs/tls13-only.h"'
make lib programs
tests/ssl-opt.sh -f '1.3'

Expectation: most test cases should pass. Only a few are expected to be skipped (e.g. test cases that want MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE disabled).

Actual behavior, as of mbedtls-3.6.1 (and in fact since #7921): most 1.3 test cases are skipped when TLS 1.2 is disabled.

A short example, where all 7 test cases should pass, but the ECDSA ones are unduly skipped when TLS 1.2 is disabled:

tests/ssl-opt.sh -f 'keyUsage cli 1.3:'
keyUsage cli 1.3: DigitalSignature, RSA: OK ............................ PASS
keyUsage cli 1.3: DigitalSignature+KeyEncipherment, RSA: OK ............ PASS
keyUsage cli 1.3: KeyEncipherment, RSA: fail (hard) .................... PASS
keyUsage cli 1.3: KeyAgreement, RSA: fail (hard) ....................... PASS
keyUsage cli 1.3: DigitalSignature, ECDSA: OK .......................... SKIP
keyUsage cli 1.3: KeyEncipherment, ECDSA: fail (hard) .................. SKIP
keyUsage cli 1.3: KeyAgreement, ECDSA: fail (hard) ..................... SKIP
------------------------------------------------------------------------
PASSED (7 / 7 tests (3 skipped))

The problem is what use_ext_tool_without_ecdh_support does or how it's used (I'm not sure exactly). run_test sets EXT_WO_ECDH=$(use_ext_tool_without_ecdh_support "$SRV_CMD" "$CLI_CMD") which is yes. Then, since EXT_WO_ECDH is not no, it decides that TLS_VERSION="TLS12". Then detect_required_features correctly requires a TLS 1.2 key exchange which is not present in the build.

A discussion on this problem started with #7921. #9556 tweaks the default TLS version, but does not tackle EXT_WO_ECDH and does not resolve this bug.

gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 11, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@mpg
Copy link
Contributor

mpg commented Sep 12, 2024

I think the faulty part is:

    # Guess the TLS version which is going to be used
    if [ "$EXT_WO_ECDH" = "no" ]; then
        TLS_VERSION=$(get_tls_version "$SRV_CMD" "$CLI_CMD")
    else
        TLS_VERSION="TLS12"
    fi

It doesn't make sense when you spell it out: if we're using a peer (GnuTLS, OpenSSL) which doesn't support TLS 1.2 static ECDH key echanges, then assume the TLS version is 1.2, otherwise guess it the normal way based on the command lines.

I'm not sure what we were trying to fix but clearly this was the wrong way. I'll have a try at fixing this.

gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 12, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 13, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 13, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 25, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Sep 25, 2024
Mbed-TLS#9560

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants