From c572246fa5c4fd67b03f349082b29bed8562cea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 19 Dec 2022 11:42:12 +0100 Subject: [PATCH 1/2] Use OPENSSL everywhere, not OPENSSL_CMD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These variables were both uses to select the default version of OpenSSL to use for tests: - when running compat.sh or ssl-opt.sh directly, OPENSSL_CMD was used; - when running all.sh, OPENSSL was used. This caused surprising situations if you had one but not the other set in your environment. For example I used to have OPENSSL_CMD set but not OPENSSL, so ssl-opt.sh was failing in some all.sh components but passing when I ran it manually in the same configuration and build, a rather unpleasant experience. The natural name would be OPENSSL, and that's what set in the Docker images used by the CI. However back in the 1.3.x days, that name was already used in library/Makefile, so it was preferable to pick a different one, hence OPENSSL_CMD. However the build system has not been using this name since at least Mbed TLS 2.0.0, so it's now free for use again (as demonstrated by the fact that it's been set in the CI without causing any trouble). So, unify things and use OPENSSL everywhere. Just leave an error message for the benefit of developers which might have OPENSSL_CMD, not OPENSSL, set in their environment from the old days. Signed-off-by: Manuel Pégourié-Gonnard --- tests/compat-in-docker.sh | 10 +++++----- tests/compat.sh | 27 +++++++++++++++++++-------- tests/scripts/all.sh | 10 +++++----- tests/scripts/basic-build-test.sh | 6 +++--- tests/ssl-opt-in-docker.sh | 10 +++++----- tests/ssl-opt.sh | 25 ++++++++++++++++++------- 6 files changed, 55 insertions(+), 33 deletions(-) diff --git a/tests/compat-in-docker.sh b/tests/compat-in-docker.sh index ad7358210..29c87877d 100755 --- a/tests/compat-in-docker.sh +++ b/tests/compat-in-docker.sh @@ -12,7 +12,7 @@ # # Notes for users # --------------- -# If OPENSSL_CMD, GNUTLS_CLI, or GNUTLS_SERV are specified the path must +# If OPENSSL, GNUTLS_CLI, or GNUTLS_SERV are specified the path must # correspond to an executable inside the Docker container. The special # values "next" (OpenSSL only) and "legacy" are also allowed as shorthand # for the installations inside the container. @@ -38,9 +38,9 @@ source tests/scripts/docker_env.sh -case "${OPENSSL_CMD:-default}" in - "legacy") export OPENSSL_CMD="/usr/local/openssl-1.0.1j/bin/openssl";; - "next") export OPENSSL_CMD="/usr/local/openssl-1.1.1a/bin/openssl";; +case "${OPENSSL:-default}" in + "legacy") export OPENSSL="/usr/local/openssl-1.0.1j/bin/openssl";; + "next") export OPENSSL="/usr/local/openssl-1.1.1a/bin/openssl";; *) ;; esac @@ -61,7 +61,7 @@ run_in_docker \ -e M_SRV \ -e GNUTLS_CLI \ -e GNUTLS_SERV \ - -e OPENSSL_CMD \ + -e OPENSSL \ -e OSSL_NO_DTLS \ tests/compat.sh \ $@ diff --git a/tests/compat.sh b/tests/compat.sh index 529c2c542..6f6d8f10e 100755 --- a/tests/compat.sh +++ b/tests/compat.sh @@ -39,10 +39,21 @@ SRVMEM=0 # default commands, can be overridden by the environment : ${M_SRV:=../programs/ssl/ssl_server2} : ${M_CLI:=../programs/ssl/ssl_client2} -: ${OPENSSL_CMD:=openssl} # OPENSSL would conflict with the build system +: ${OPENSSL:=openssl} # OPENSSL would conflict with the build system : ${GNUTLS_CLI:=gnutls-cli} : ${GNUTLS_SERV:=gnutls-serv} +# The OPENSSL variable used to be OPENSSL_CMD for historical reasons. +# To help the migration, error out if the old variable is set, +# but only if it has a different value than the new one. +if [ "${OPENSSL_CMD+set}" = set ]; then + # the variable is set, we can now check its value + if [ "$OPENSSL_CMD" != "$OPENSSL" ]; then + echo "Please use OPENSSL instead of OPENSSL_CMD." >&2 + exit 125 + fi +fi + # do we have a recent enough GnuTLS? if ( which $GNUTLS_CLI && which $GNUTLS_SERV ) >/dev/null 2>&1; then G_VER="$( $GNUTLS_CLI --version | head -n1 )" @@ -577,7 +588,7 @@ setup_arguments() # Mbed TLS wants >=1024, so force that for older versions. Don't force # it for newer versions, which reject a 1024-bit prime. Indifferently # force it or not for intermediate versions. - case $($OPENSSL_CMD version) in + case $($OPENSSL version) in "OpenSSL 1.0"*) O_SERVER_ARGS="$O_SERVER_ARGS -dhparam data_files/dhparams.pem" ;; @@ -601,7 +612,7 @@ setup_arguments() # OpenSSL 1.1.1f from Ubuntu 20.04. The syntax was only introduced in # OpenSSL 1.1.0 (21e0c1d23afff48601eb93135defddae51f7e2e3) and I can't find # a way to discover it from -help, so check the openssl version. - case $($OPENSSL_CMD version) in + case $($OPENSSL version) in "OpenSSL 0"*|"OpenSSL 1.0"*) :;; *) O_CLIENT_ARGS="$O_CLIENT_ARGS -cipher ALL@SECLEVEL=0" @@ -720,7 +731,7 @@ fi start_server() { case $1 in [Oo]pen*) - SERVER_CMD="$OPENSSL_CMD s_server $O_SERVER_ARGS" + SERVER_CMD="$OPENSSL s_server $O_SERVER_ARGS" ;; [Gg]nu*) SERVER_CMD="$GNUTLS_SERV $G_SERVER_ARGS --priority $G_SERVER_PRIO" @@ -810,7 +821,7 @@ run_client() { # run the command and interpret result case $1 in [Oo]pen*) - CLIENT_CMD="$OPENSSL_CMD s_client $O_CLIENT_ARGS -cipher $2" + CLIENT_CMD="$OPENSSL s_client $O_CLIENT_ARGS -cipher $2" log "$CLIENT_CMD" echo "$CLIENT_CMD" > $CLI_OUT printf 'GET HTTP/1.0\r\n\r\n' | $CLIENT_CMD >> $CLI_OUT 2>&1 & @@ -945,8 +956,8 @@ if [ ! -x "$M_CLI" ]; then fi if echo "$PEERS" | grep -i openssl > /dev/null; then - if which "$OPENSSL_CMD" >/dev/null 2>&1; then :; else - echo "Command '$OPENSSL_CMD' not found" >&2 + if which "$OPENSSL" >/dev/null 2>&1; then :; else + echo "Command '$OPENSSL' not found" >&2 exit 1 fi fi @@ -1009,7 +1020,7 @@ for VERIFY in $VERIFIES; do # help isn't accurate as of 1.0.2g: it supports DTLS 1.2 # but doesn't list it. But the s_server help seems to be # accurate.) - if ! $OPENSSL_CMD s_server -help 2>&1 | grep -q "^ *-$O_MODE "; then + if ! $OPENSSL s_server -help 2>&1 | grep -q "^ *-$O_MODE "; then continue; fi diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 584906a63..0695a8c14 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -724,7 +724,7 @@ pre_check_tools () { *" test_"*) # To avoid setting OpenSSL and GnuTLS for each call to compat.sh # and ssl-opt.sh, we just export the variables they require. - export OPENSSL_CMD="$OPENSSL" + export OPENSSL="$OPENSSL" export GNUTLS_CLI="$GNUTLS_CLI" export GNUTLS_SERV="$GNUTLS_SERV" # Avoid passing --seed flag in every call to ssl-opt.sh @@ -1584,10 +1584,10 @@ component_test_full_cmake_clang () { tests/ssl-opt.sh -f 'Default\|ECJPAKE\|SSL async private' msg "test: compat.sh NULL (full config)" # ~ 2 min - env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL' + env OPENSSL="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL' msg "test: compat.sh ARIA + ChachaPoly" - env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' + env OPENSSL="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' } skip_suites_without_constant_flow () { @@ -1955,10 +1955,10 @@ component_test_no_use_psa_crypto_full_cmake_asan() { tests/compat.sh msg "test: compat.sh NULL (full minus MBEDTLS_USE_PSA_CRYPTO)" - env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -f 'NULL' + env OPENSSL="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -f 'NULL' msg "test: compat.sh ARIA + ChachaPoly (full minus MBEDTLS_USE_PSA_CRYPTO)" - env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' + env OPENSSL="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' } component_test_psa_crypto_config_accel_ecdsa () { diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index a96254fd5..4aeeb238f 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -69,7 +69,7 @@ fi # To avoid setting OpenSSL and GnuTLS for each call to compat.sh and ssl-opt.sh # we just export the variables they require -export OPENSSL_CMD="$OPENSSL" +export OPENSSL="$OPENSSL" export GNUTLS_CLI="$GNUTLS_CLI" export GNUTLS_SERV="$GNUTLS_SERV" @@ -125,13 +125,13 @@ echo '################ compat.sh ################' echo echo '#### compat.sh: legacy (null)' - OPENSSL_CMD="$OPENSSL_LEGACY" \ + OPENSSL="$OPENSSL_LEGACY" \ GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" \ sh compat.sh -e '^$' -f 'NULL' echo echo '#### compat.sh: next (ARIA, ChaCha)' - OPENSSL_CMD="$OPENSSL_NEXT" sh compat.sh -e '^$' -f 'ARIA\|CHACHA' + OPENSSL="$OPENSSL_NEXT" sh compat.sh -e '^$' -f 'ARIA\|CHACHA' echo } | tee compat-test-$TEST_OUTPUT echo '^^^^^^^^^^^^^^^^ compat.sh ^^^^^^^^^^^^^^^^' diff --git a/tests/ssl-opt-in-docker.sh b/tests/ssl-opt-in-docker.sh index c8c669725..159be4c50 100755 --- a/tests/ssl-opt-in-docker.sh +++ b/tests/ssl-opt-in-docker.sh @@ -12,7 +12,7 @@ # # Notes for users # --------------- -# If OPENSSL_CMD, GNUTLS_CLI, or GNUTLS_SERV are specified, the path must +# If OPENSSL, GNUTLS_CLI, or GNUTLS_SERV are specified, the path must # correspond to an executable inside the Docker container. The special # values "next" and "legacy" are also allowed as shorthand for the # installations inside the container. @@ -38,9 +38,9 @@ source tests/scripts/docker_env.sh -case "${OPENSSL_CMD:-default}" in - "legacy") export OPENSSL_CMD="/usr/local/openssl-1.0.1j/bin/openssl";; - "next") export OPENSSL_CMD="/usr/local/openssl-1.1.1a/bin/openssl";; +case "${OPENSSL:-default}" in + "legacy") export OPENSSL="/usr/local/openssl-1.0.1j/bin/openssl";; + "next") export OPENSSL="/usr/local/openssl-1.1.1a/bin/openssl";; *) ;; esac @@ -62,6 +62,6 @@ run_in_docker \ -e P_PXY \ -e GNUTLS_CLI \ -e GNUTLS_SERV \ - -e OPENSSL_CMD \ + -e OPENSSL \ tests/ssl-opt.sh \ $@ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0d4ce6e18..0c1c03599 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -46,11 +46,22 @@ fi : ${P_CLI:=../programs/ssl/ssl_client2} : ${P_PXY:=../programs/test/udp_proxy} : ${P_QUERY:=../programs/test/query_compile_time_config} -: ${OPENSSL_CMD:=openssl} # OPENSSL would conflict with the build system +: ${OPENSSL:=openssl} : ${GNUTLS_CLI:=gnutls-cli} : ${GNUTLS_SERV:=gnutls-serv} : ${PERL:=perl} +# The OPENSSL variable used to be OPENSSL_CMD for historical reasons. +# To help the migration, error out if the old variable is set, +# but only if it has a different value than the new one. +if [ "${OPENSSL_CMD+set}" = set ]; then + # the variable is set, we can now check its value + if [ "$OPENSSL_CMD" != "$OPENSSL" ]; then + echo "Please use OPENSSL instead of OPENSSL_CMD." >&2 + exit 125 + fi +fi + guess_config_name() { if git diff --quiet ../include/mbedtls/mbedtls_config.h 2>/dev/null; then echo "default" @@ -62,8 +73,8 @@ guess_config_name() { : ${MBEDTLS_TEST_CONFIGURATION:="$(guess_config_name)"} : ${MBEDTLS_TEST_PLATFORM:="$(uname -s | tr -c \\n0-9A-Za-z _)-$(uname -m | tr -c \\n0-9A-Za-z _)"} -O_SRV="$OPENSSL_CMD s_server -www -cert data_files/server5.crt -key data_files/server5.key" -O_CLI="echo 'GET / HTTP/1.0' | $OPENSSL_CMD s_client" +O_SRV="$OPENSSL s_server -www -cert data_files/server5.crt -key data_files/server5.key" +O_CLI="echo 'GET / HTTP/1.0' | $OPENSSL s_client" G_SRV="$GNUTLS_SERV --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key" G_CLI="echo 'GET / HTTP/1.0' | $GNUTLS_CLI --x509cafile data_files/test-ca_cat12.crt" TCP_CLIENT="$PERL scripts/tcp_client.pl" @@ -509,7 +520,7 @@ requires_hash_alg() { # skip next test if OpenSSL doesn't support FALLBACK_SCSV requires_openssl_with_fallback_scsv() { if [ -z "${OPENSSL_HAS_FBSCSV:-}" ]; then - if $OPENSSL_CMD s_client -help 2>&1 | grep fallback_scsv >/dev/null + if $OPENSSL s_client -help 2>&1 | grep fallback_scsv >/dev/null then OPENSSL_HAS_FBSCSV="YES" else @@ -1639,8 +1650,8 @@ if [ "$MEMCHECK" -gt 0 ]; then exit 1 fi fi -if which $OPENSSL_CMD >/dev/null 2>&1; then :; else - echo "Command '$OPENSSL_CMD' not found" +if which $OPENSSL >/dev/null 2>&1; then :; else + echo "Command '$OPENSSL' not found" exit 1 fi @@ -1695,7 +1706,7 @@ fi # OpenSSL 1.1.1f from Ubuntu 20.04. The syntax was only introduced in # OpenSSL 1.1.0 (21e0c1d23afff48601eb93135defddae51f7e2e3) and I can't find # a way to discover it from -help, so check the openssl version. -case $($OPENSSL_CMD version) in +case $($OPENSSL version) in "OpenSSL 0"*|"OpenSSL 1.0"*) :;; *) O_CLI="$O_CLI -cipher ALL@SECLEVEL=0" From 6e666c2e79fe0ce76725790ccb0f5649249807d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 10 Jan 2023 09:38:58 +0100 Subject: [PATCH 2/2] Remove obsolete comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was explaining why we didn't use the OPENSSL name, but we are using it now... Signed-off-by: Manuel Pégourié-Gonnard --- tests/compat.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/compat.sh b/tests/compat.sh index 6f6d8f10e..fc2bfab7a 100755 --- a/tests/compat.sh +++ b/tests/compat.sh @@ -39,7 +39,7 @@ SRVMEM=0 # default commands, can be overridden by the environment : ${M_SRV:=../programs/ssl/ssl_server2} : ${M_CLI:=../programs/ssl/ssl_client2} -: ${OPENSSL:=openssl} # OPENSSL would conflict with the build system +: ${OPENSSL:=openssl} : ${GNUTLS_CLI:=gnutls-cli} : ${GNUTLS_SERV:=gnutls-serv}