tls: Allow ServerHello extensions when resuming session

While, as the spec notes, most extensions are not used during the
resumption of a cached session (are offered in the ClientHello but are
ignored and not be included in the ServerHello), this not a rule so allow
each extension to decide whether to skip adding itself to the ServerHello.

Specifically those extensions should rather be ignored by the server
when it is parsing the ClientHello but at that point we don't yet know
whether we'll allow the session to be resumed so instead the ServerHello
writer callbacks decide whether to reply or not.  As an example RFC 4492
Section 4 says: "In the case of session resumption, the server simply
ignores the Supported Elliptic Curves Extension and the Supported Point
Formats Extension appearing in the current ClientHello message.  These
extensions only play a role during handshakes negotiating a new session."

2 files changed, 19 insertions, 21 deletions

diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index 7796825a..d03c3318 100644
--- a/ell/tls-extensions.c
+++ b/ell/tls-extensions.c
@@ -31,6 +31,13 @@
 #include "cert.h"
 #include "tls-private.h"
 
+/* Most extensions are not used when resuming a cached session */
+#define SKIP_ON_RESUMPTION()	\
+	do {	\
+		if (tls->session_id_size && !tls->session_id_new)	\
+			return -ENOMSG;	\
+	} while (0);
+
 /* RFC 7919, Section A.1 */
 static const uint8_t tls_ffdhe2048_prime[] = {
 	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xad, 0xf8, 0x54, 0x58,
@@ -560,6 +567,8 @@ static bool tls_ec_point_formats_client_handle(struct l_tls *tls,
 static ssize_t tls_ec_point_formats_server_write(struct l_tls *tls,
 						uint8_t *buf, size_t len)
 {
+	SKIP_ON_RESUMPTION(); /* RFC 4492 Section 4 */
+
 	if (len < 2)
 		return -ENOMEM;
 
diff --git a/ell/tls.c b/ell/tls.c
index 98bc682f..5f3b7171 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -2053,11 +2053,8 @@ static void tls_handle_client_hello(struct l_tls *tls,
 			tls_load_cached_server_session(tls, buf + 35,
 							session_id_size)) {
 		/*
-		 * Attempt a session resumption but don't skip parsing Hello
-		 * extensions just yet because we may decide to start a new
-		 * session instead after our cipher suite and compression
-		 * method checks below and in that case we will need to
-		 * handle the extensions and include them in the Server Hello.
+		 * Attempt a session resumption but note later checks may
+		 * spoil this.
 		 */
 		resuming = true;
 		session_id_str = l_util_hexstring(tls->session_id,
@@ -2228,7 +2225,7 @@ static void tls_handle_client_hello(struct l_tls *tls,
 		l_getrandom(tls->session_id, 32);
 	}
 
-	if (!tls_send_server_hello(tls, resuming ? extensions_offered : NULL))
+	if (!tls_send_server_hello(tls, extensions_offered))
 		goto cleanup;
 
 	l_queue_destroy(extensions_offered, NULL);
@@ -2324,23 +2321,16 @@ static void tls_handle_server_hello(struct l_tls *tls,
 			TLS_DEBUG("Negotiated resumption of cached session %s",
 					session_id_str);
 			resuming = true;
-
-			/*
-			 * Skip parsing extensions as none of the ones we
-			 * support are used in session resumption.  We could
-			 * as well signal an error if the ServerHello has any
-			 * extensions, for now ignore them.
-			 */
-			goto check_version;
+		} else {
+			TLS_DEBUG("Server decided not to resume cached session "
+					"%s, sent %s session ID",
+					session_id_str,
+					session_id_size ? "a new" : "no");
+			tls->session_id_size = 0;
 		}
-
-		TLS_DEBUG("Server decided not to resume cached session %s, "
-				"sent %s session ID", session_id_str,
-				session_id_size ? "a new" : "no");
-		tls->session_id_size = 0;
 	}
 
-	if (session_id_size && tls->session_settings) {
+	if (session_id_size && !resuming && tls->session_settings) {
 		tls->session_id_new = true;
 		tls->session_id_size = session_id_size;
 		memcpy(tls->session_id, buf + 35, session_id_size);
@@ -2354,7 +2344,6 @@ static void tls_handle_server_hello(struct l_tls *tls,
 	if (!result)
 		return;
 
-check_version:
 	if (version < tls->min_version || version > tls->max_version) {
 		TLS_DISCONNECT(version < tls->min_version ?
 				TLS_ALERT_PROTOCOL_VERSION :