send_client_hello: don't override version after HRR is received

When a Hello Retry Request is received, do not set our (transient)
version to TLS1.2 on the second client hello. That's because both
peers have already negotiated TLS1.3.

This addresses issue with peers which may send a changecipherspec
message at this stage, which is now allowed when our version is
set to be TLS1.2. Introduced test suite using openssl and resumption
using HRR which reproduces the issue.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

diff --git a/lib/handshake.c b/lib/handshake.c
index 5feaed99fd8d51faec67117741960660c2a012bc..d0c0f9dc9710c5033f9b2e1bb42c39d0d3760d8c 100644 (file)
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -2049,13 +2049,15 @@ static int send_client_hello(gnutls_session_t session, int again)
                        goto cleanup;
                }
 
-               /* Set the version we advertized as maximum 
-                * (RSA uses it).
-                */
-               set_adv_version(session, hver->major, hver->minor);
-               if (_gnutls_set_current_version(session, hver->id) < 0) {
-                       ret = gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
-                       goto cleanup;
+               /* if we are replying to an HRR the version is already negotiated */
+               if (!(session->internals.hsk_flags & HSK_HRR_RECEIVED) || !get_version(session)) {
+                       /* Set the version we advertized as maximum
+                        * (RSA uses it). */
+                       set_adv_version(session, hver->major, hver->minor);
+                       if (_gnutls_set_current_version(session, hver->id) < 0) {
+                               ret = gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
+                               goto cleanup;
+                       }
                }
 
                if (session->internals.priorities->min_record_version != 0) {

diff --git a/tests/suite/testcompat-tls13-openssl.sh b/tests/suite/testcompat-tls13-openssl.sh
index 957aa5fe055cde390d1c04c047986e2aa81c2d63..b28aad5bf0af7cb2da45a0ffd66b03a7a4e41307 100755 (executable)
--- a/tests/suite/testcompat-tls13-openssl.sh
+++ b/tests/suite/testcompat-tls13-openssl.sh
@@ -185,7 +185,6 @@ run_client_suite() {
        PID=$!
        wait_server ${PID}
 
-#      ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3${ADD}" --x509cafile "${CA_CERT}" </dev/null >>${OUTPUT} || \
        ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3${ADD}" --insecure </dev/null >>${OUTPUT} || \
                fail ${PID} "Failed"
 
@@ -198,7 +197,6 @@ run_client_suite() {
        PID=$!
        wait_server ${PID}
 
-#      ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3${ADD}" --x509cafile "${CA_CERT}" </dev/null >>${OUTPUT} || \
        ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3${ADD}" --insecure </dev/null >>${OUTPUT} || \
                fail ${PID} "Failed"
 
@@ -213,14 +211,28 @@ run_client_suite() {
        PID=$!
        wait_server ${PID}
 
-       # ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3:+GROUP-ALL${ADD}" --x509cafile "${CA_CERT}" --inline-commands | tee "${testdir}/client.out" >> ${OUTPUT}
        ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3:+GROUP-ALL${ADD}" --insecure --inline-commands <<< $(echo -e "^resume^\nGET / HTTP/1.0\r\n\r\n")| tee "${testdir}/client.out" >> ${OUTPUT}
        grep '^\*\*\* This is a resumed session' "${testdir}/client.out" || \
                fail ${PID} "Failed"
 
        kill ${PID}
        wait
-       rm -rf "$testdir"
+
+       # Try resumption with HRR
+       echo_cmd "${PREFIX}Checking TLS 1.3 with resumption and HRR..."
+       eval "${GETPORT}"
+       launch_bare_server $$ s_server -quiet -www -accept "${PORT}" -groups 'X25519:P-256' -keyform pem -certform pem ${OPENSSL_DH_PARAMS_OPT} -key "${RSA_KEY}" -cert "${RSA_CERT}" -CAfile "${CA_CERT}"
+       PID=$!
+       wait_server ${PID}
+
+       ${VALGRIND} "${CLI}" ${DEBUG} -p "${PORT}" 127.0.0.1 --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3:-GROUP-ALL:+GROUP-FFDHE2048:+GROUP-SECP256R1${ADD}" --single-key-share --insecure --inline-commands <<< $(echo -e "^resume^\nGET / HTTP/1.0\r\n\r\n")| tee "${testdir}/client.out" >> ${OUTPUT}
+       grep '^\*\*\* This is a resumed session' "${testdir}/client.out" || \
+               fail ${PID} "Failed"
+
+       kill ${PID}
+       wait
+
+       rm -rf "${testdir}"
 
 }
 
@@ -448,7 +460,25 @@ _EOF_
 
        kill ${PID}
        wait
-       rm -rf "$testdir"
+
+       echo_cmd "${PREFIX}Checking TLS 1.3 with resumption and HRR..."
+       eval "${GETPORT}"
+       launch_server $$ --priority "NORMAL:-VERS-ALL:+VERS-TLS1.3:-CIPHER-ALL:+AES-256-GCM:-GROUP-ALL:+GROUP-SECP384R1${ADD}" --x509certfile "${RSA_CERT}" --x509keyfile "${RSA_KEY}" --x509cafile "${CA_CERT}"  >>${OUTPUT} 2>&1
+       PID=$!
+       wait_server ${PID}
+
+       { echo a; sleep 1; } | \
+       ${OPENSSL_CLI} s_client -host localhost -port "${PORT}" -curves 'X25519:P-256:X448:P-521:P-384' -CAfile "${CA_CERT}" -sess_out "${testdir}/sess-hrr.pem" 2>&1 | grep "\:error\:" && \
+               fail ${PID} "Failed"
+       ${OPENSSL_CLI} s_client -host localhost -port "${PORT}" -curves 'X25519:P-256:X448:P-521:P-384' -CAfile "${CA_CERT}" -sess_in "${testdir}/sess-hrr.pem" </dev/null 2>&1 > "${testdir}/server.out"
+       grep "\:error\:" "${testdir}/server.out" && \
+               fail ${PID} "Failed"
+       grep "^Reused, TLSv1.3" "${testdir}/server.out" || \
+               fail ${PID} "Failed"
+
+       kill ${PID}
+       wait
+       rm -rf "${testdir}"
 
 }