Improved estimation of wait in gnutls_session_get_data2tmp-improve-session-resumption

Previously we would wait an arbitrary value of 50ms for the
server to send session tickets. This change makes the client
wait for the estimated single trip time + 60 ms for the server
to calculate the session tickets. This improves the chance
to obtain tickets from internet servers during the call of
gnutls_session_get_data2().

Resolves: #706

Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org>

7 files changed, 139 insertions, 11 deletions

diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index ccbcc5b558..d7a4235f9a 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -1306,8 +1306,12 @@ typedef struct {
 	/* starting time of current handshake */
 	struct timespec handshake_start_time;
 
-	/* end time of current handshake */
-	struct timespec handshake_endtime;
+	/* expected end time of current handshake (start+timeout);
+	 * this is only filled if a handshake_time_ms is set. */
+	struct timespec handshake_abs_timeout;
+
+	/* An estimation of round-trip time under TLS1.3; populated in client side only */
+	unsigned ertt;
 
 	unsigned int handshake_timeout_ms;	/* timeout in milliseconds */
 	unsigned int record_timeout_ms;	/* timeout in milliseconds */
diff --git a/lib/handshake.c b/lib/handshake.c
index da1f87d183..45bf99a6f7 100644
--- a/lib/handshake.c
+++ b/lib/handshake.c
@@ -2782,7 +2782,7 @@ int gnutls_handshake(gnutls_session_t session)
 		gnutls_gettime(&session->internals.handshake_start_time);
 
 		tmo_ms = session->internals.handshake_timeout_ms;
-		end = &session->internals.handshake_endtime;
+		end = &session->internals.handshake_abs_timeout;
 		start = &session->internals.handshake_start_time;
 
 		if (tmo_ms && end->tv_sec == 0 && end->tv_nsec == 0) {
@@ -2835,6 +2835,18 @@ int gnutls_handshake(gnutls_session_t session)
 		_gnutls_epoch_bump(session);
 	}
 
+	/* Give an estimation of the round-trip under TLS1.3, used by gnutls_session_get_data2() */
+	if (!IS_SERVER(session) && vers->tls13_sem) {
+		struct timespec handshake_finish_time;
+		gnutls_gettime(&handshake_finish_time);
+
+		if (!(session->internals.hsk_flags & HSK_HRR_RECEIVED)) {
+			session->internals.ertt = timespec_sub_ms(&handshake_finish_time, &session->internals.handshake_start_time)/2;
+		} else {
+			session->internals.ertt = timespec_sub_ms(&handshake_finish_time, &session->internals.handshake_start_time)/4;
+		}
+	}
+
 	return 0;
 }
 
diff --git a/lib/handshake.h b/lib/handshake.h
index 2d56219591..f39b3a4571 100644
--- a/lib/handshake.h
+++ b/lib/handshake.h
@@ -115,7 +115,7 @@ int _gnutls13_handshake_hash_buffers_synth(gnutls_session_t session,
 /* return the remaining time in ms */
 inline static int handshake_remaining_time(gnutls_session_t session)
 {
-	struct timespec *end = &session->internals.handshake_endtime;
+	struct timespec *end = &session->internals.handshake_abs_timeout;
 
 	if (end->tv_sec || end->tv_nsec) {
 		struct timespec now;
diff --git a/lib/session.c b/lib/session.c
index 12f41797ec..10b19abb4e 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -28,6 +28,7 @@
 #include "buffers.h"
 #include "state.h"
 #include "ext/cert_types.h"
+#include <minmax.h>
 
 /**
  * gnutls_session_get_data:
@@ -99,8 +100,10 @@ gnutls_session_get_data(gnutls_session_t session,
  * is received by the client. To ensure that such a ticket has been received use
  * gnutls_session_get_flags() and check for flag %GNUTLS_SFLAGS_SESSION_TICKET;
  * if this flag is not set, this function will wait for a new ticket within
- * 50ms, and if not received will return dummy data which cannot lead to
- * resumption. To get notified when new tickets are received by the server
+ * an estimated rountrip, and if not received will return dummy data which
+ * cannot lead to resumption.
+ *
+ * To get notified when new tickets are received by the server
  * use gnutls_handshake_set_hook_function() to wait for %GNUTLS_HANDSHAKE_NEW_SESSION_TICKET
  * messages. Each call of gnutls_session_get_data2() after a ticket is
  * received, will return session resumption data corresponding to the last
@@ -120,8 +123,13 @@ gnutls_session_get_data2(gnutls_session_t session, gnutls_datum_t *data)
 	}
 
 	if (vers->tls13_sem && !(session->internals.hsk_flags & HSK_TICKET_RECEIVED)) {
-		/* wait for a message with timeout of 1ms */
-		ret = _gnutls_recv_in_buffers(session, GNUTLS_APPLICATION_DATA, -1, 50);
+		unsigned ertt = session->internals.ertt;
+		/* use our estimation of round-trip + some time for the server to calculate
+		 * the value(s). */
+		ertt += 60;
+
+		/* wait for a message with timeout */
+		ret = _gnutls_recv_in_buffers(session, GNUTLS_APPLICATION_DATA, -1, ertt);
 		if (ret < 0 && (gnutls_error_is_fatal(ret) && ret != GNUTLS_E_TIMEDOUT)) {
 			return gnutls_assert_val(ret);
 		}
diff --git a/lib/state.c b/lib/state.c
index 5a6991dfc8..db3262d52c 100644
--- a/lib/state.c
+++ b/lib/state.c
@@ -415,8 +415,8 @@ void _gnutls_handshake_internal_state_clear(gnutls_session_t session)
 
 	_gnutls_epoch_gc(session);
 
-	session->internals.handshake_endtime.tv_sec = 0;
-	session->internals.handshake_endtime.tv_nsec = 0;
+	session->internals.handshake_abs_timeout.tv_sec = 0;
+	session->internals.handshake_abs_timeout.tv_nsec = 0;
 	session->internals.handshake_in_progress = 0;
 
 	session->internals.tfo.connect_addrlen = 0;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8f778f57e5..97e63cdbae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -467,7 +467,7 @@ dist_check_SCRIPTS += fastopen.sh pkgconfig.sh starttls.sh starttls-ftp.sh start
 	ocsp-tests/ocsp-test cipher-listings.sh sni-hostname.sh server-multi-keys.sh \
 	psktool.sh ocsp-tests/ocsp-load-chain gnutls-cli-save-data.sh gnutls-cli-debug.sh \
 	sni-resume.sh ocsp-tests/ocsptool cert-reencoding.sh pkcs7-cat.sh long-crl.sh \
-	serv-udp.sh logfile-option.sh
+	serv-udp.sh logfile-option.sh gnutls-cli-resume.sh
 
 dist_check_SCRIPTS += gnutls-cli-self-signed.sh gnutls-cli-invalid-crl.sh
 
diff --git a/tests/gnutls-cli-resume.sh b/tests/gnutls-cli-resume.sh
new file mode 100755
index 0000000000..fe7ed1e029
--- /dev/null
+++ b/tests/gnutls-cli-resume.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# Author: Nikos Mavrogiannopoulos
+#
+# This file is part of GnuTLS.
+#
+# GnuTLS is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by the
+# Free Software Foundation; either version 3 of the License, or (at
+# your option) any later version.
+#
+# GnuTLS is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>
+#
+
+srcdir="${srcdir:-.}"
+SERV="${SERV:-../src/gnutls-serv${EXEEXT}}"
+CLI="${CLI:-../src/gnutls-cli${EXEEXT}}"
+unset RETCODE
+
+if ! test -x "${SERV}"; then
+	exit 77
+fi
+
+if ! test -x "${CLI}"; then
+	exit 77
+fi
+
+if test "${WINDIR}" != ""; then
+	exit 77
+fi
+
+SERV="${SERV} -q"
+
+. "${srcdir}/scripts/common.sh"
+
+KEY1=${srcdir}/../doc/credentials/x509/key-rsa.pem
+CERT1=${srcdir}/../doc/credentials/x509/cert-rsa.pem
+
+eval "${GETPORT}"
+launch_server $$ --echo --x509keyfile ${KEY1} --x509certfile ${CERT1}
+PID=$!
+wait_server ${PID}
+
+run_server_test() {
+	local priority=$1
+	local id=$2
+	local TMPFILE=resume.$$-$i.tmp
+
+	"${CLI}" -p "${PORT}" 127.0.0.1 --logfile=${TMPFILE} --priority="${priority}" --resume --insecure </dev/null >/dev/null || \
+		exit 1
+	grep -H "* This is a resumed session" ${TMPFILE} ||
+		exit 1
+
+	rm -f ${TMPFILE}
+
+	exit 0
+}
+
+echo "Checking whether session resumption works reliably under TLS1.3"
+PRIORITY="NORMAL:-VERS-ALL:+VERS-TLS1.3"
+WAITPID=""
+
+i=0
+while [ $i -lt 10 ]
+do
+	run_server_test "${PRIORITY}" $i &
+	WAITPID="$WAITPID $!"
+	i=`expr $i + 1`
+done
+
+for i in "$WAITPID";do
+	wait $i
+	test $? != 0 && exit 1
+done
+
+echo "Checking whether session resumption works reliably under TLS1.2"
+PRIORITY="NORMAL:-VERS-ALL:+VERS-TLS1.2"
+WAITPID=""
+
+i=0
+while [ $i -lt 10 ]
+do
+	run_server_test "${PRIORITY}" $i &
+	WAITPID="$WAITPID $!"
+	i=`expr $i + 1`
+done
+
+for i in "$WAITPID";do
+	wait $i
+	test $? != 0 && exit 1
+done
+
+kill ${PID}
+wait
+
+exit 0