From: Andrew Zaborowski <andrew.zaborowski@intel.com>
To: ell@lists.linux.dev
Subject: [PATCH 1/4] tls: Allow ServerHello extensions when resuming session
Date: Wed, 9 Nov 2022 18:47:43 +0100 [thread overview]
Message-ID: <20221109174746.569046-1-andrew.zaborowski@intel.com> (raw)
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."
---
ell/tls-extensions.c | 9 +++++++++
ell/tls.c | 31 ++++++++++---------------------
2 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/ell/tls-extensions.c b/ell/tls-extensions.c
index 7796825..d03c331 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 98bc682..5f3b717 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 :
--
2.34.1
next reply other threads:[~2022-11-09 17:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 17:47 Andrew Zaborowski [this message]
2022-11-09 17:47 ` [PATCH 2/4] tls: Improve renegotiation Andrew Zaborowski
2022-11-09 17:47 ` [PATCH 3/4] tls: Implement RFC 5746 Secure Renegotiation Andrew Zaborowski
2022-11-09 17:47 ` [PATCH 4/4] tls: Add missing continue in tls_load_cached_server_session Andrew Zaborowski
2022-11-09 20:25 ` [PATCH 1/4] tls: Allow ServerHello extensions when resuming session Denis Kenzior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221109174746.569046-1-andrew.zaborowski@intel.com \
--to=andrew.zaborowski@intel.com \
--cc=ell@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).