Client side sanity check of ALPN after server has accepted early_data
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3926)
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 9d32366..b8ab5c8 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -844,7 +844,7 @@
case SSL_TLSEXT_ERR_NOACK:
s->servername_done = 0;
- if (s->session->ext.hostname != NULL)
+ if (s->server && s->session->ext.hostname != NULL)
s->ext.early_data_ok = 0;
return 1;
@@ -945,6 +945,9 @@
static int final_alpn(SSL *s, unsigned int context, int sent, int *al)
{
+ if (!s->server && !sent && s->session->ext.alpn_selected != NULL)
+ s->ext.early_data_ok = 0;
+
if (!s->server || !SSL_IS_TLS13(s))
return 1;
@@ -1387,9 +1390,25 @@
static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
{
- if (!s->server || !sent)
+ if (!sent)
return 1;
+ if (!s->server) {
+ if (context == SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS
+ && sent
+ && !s->ext.early_data_ok) {
+ /*
+ * If we get here then the server accepted our early_data but we
+ * later realised that it shouldn't have done (e.g. inconsistent
+ * ALPN)
+ */
+ *al = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+
+ return 1;
+ }
+
if (s->max_early_data == 0
|| !s->hit
|| s->session->ext.tick_identity != 0
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index cb7b211..bcbcbac 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -771,6 +771,7 @@
* extension, we set it to accepted.
*/
s->ext.early_data = SSL_EARLY_DATA_REJECTED;
+ s->ext.early_data_ok = 1;
return EXT_RETURN_SENT;
}
@@ -1400,15 +1401,22 @@
}
s->s3->alpn_selected_len = len;
- /* We also put a copy in the session */
- OPENSSL_free(s->session->ext.alpn_selected);
- s->session->ext.alpn_selected = OPENSSL_memdup(s->s3->alpn_selected,
- s->s3->alpn_selected_len);
- s->session->ext.alpn_selected_len = s->s3->alpn_selected_len;
-
- if (s->session->ext.alpn_selected == NULL) {
- *al = SSL_AD_INTERNAL_ERROR;
- return 0;
+ if (s->session->ext.alpn_selected != NULL
+ && (s->session->ext.alpn_selected_len != len
+ || memcmp(s->session->ext.alpn_selected, s->s3->alpn_selected,
+ len) != 0)) {
+ /* ALPN not consistent with the old session so cannot use early_data */
+ s->ext.early_data_ok = 0;
+ }
+ if (!s->hit) {
+ /* If a new session then update it with the selected ALPN */
+ s->session->ext.alpn_selected =
+ OPENSSL_memdup(s->s3->alpn_selected, s->s3->alpn_selected_len);
+ if (s->session->ext.alpn_selected == NULL) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+ s->session->ext.alpn_selected_len = s->s3->alpn_selected_len;
}
return 1;
@@ -1637,12 +1645,13 @@
return 0;
}
- if (s->ext.early_data != SSL_EARLY_DATA_REJECTED
+ if (!s->ext.early_data_ok
|| !s->hit
|| s->session->ext.tick_identity != 0) {
/*
* If we get here then we didn't send early data, or we didn't resume
- * using the first identity so the server should not be accepting it.
+ * using the first identity, or the SNI/ALPN is not consistent so the
+ * server should not be accepting it.
*/
*al = SSL_AD_ILLEGAL_PARAMETER;
return 0;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 8c2a4e9..a3a6bdf 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1962,14 +1962,26 @@
s->s3->npn_seen = 0;
#endif
- /* Check ALPN is consistent with early_data */
- if (s->ext.early_data_ok
- && (s->session->ext.alpn_selected == NULL
+ /* Check ALPN is consistent with session */
+ if (s->session->ext.alpn_selected == NULL
|| selected_len != s->session->ext.alpn_selected_len
|| memcmp(selected, s->session->ext.alpn_selected,
- selected_len) != 0))
+ selected_len) != 0) {
+ /* Not consistent so can't be used for early_data */
s->ext.early_data_ok = 0;
+ if (!s->hit) {
+ /* If a new session update it with the new ALPN value */
+ s->session->ext.alpn_selected = OPENSSL_memdup(selected,
+ selected_len);
+ if (s->session->ext.alpn_selected == NULL) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ return 0;
+ }
+ s->session->ext.alpn_selected_len = selected_len;
+ }
+ }
+
return 1;
} else if (r != SSL_TLSEXT_ERR_NOACK) {
*al = SSL_AD_NO_APPLICATION_PROTOCOL;
@@ -1981,9 +1993,11 @@
*/
}
- /* Check ALPN is consistent with early_data */
- if (s->ext.early_data_ok && s->session->ext.alpn_selected != NULL)
+ /* Check ALPN is consistent with session */
+ if (s->session->ext.alpn_selected != NULL) {
+ /* Not consistent so can't be used for early_data */
s->ext.early_data_ok = 0;
+ }
return 1;
}
