Add server side sanity checks of SNI/ALPN for use with 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 3d830a7b76f94222eace3e976a64cbf2ccd36ade..9d32366e1247b3ddf19b3870d2c4f8f5e1735aef 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -29,6 +29,7 @@ static int init_status_request(SSL *s, unsigned int context);
 static int init_npn(SSL *s, unsigned int context);
 #endif
 static int init_alpn(SSL *s, unsigned int context);
+static int final_alpn(SSL *s, unsigned int context, int sent, int *al);
 static int init_sig_algs(SSL *s, unsigned int context);
 static int init_certificate_authorities(SSL *s, unsigned int context);
 static EXT_RETURN tls_construct_certificate_authorities(SSL *s, WPACKET *pkt,
@@ -203,7 +204,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO
         | SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS,
         init_alpn, tls_parse_ctos_alpn, tls_parse_stoc_alpn,
-        tls_construct_stoc_alpn, tls_construct_ctos_alpn, NULL
+        tls_construct_stoc_alpn, tls_construct_ctos_alpn, final_alpn
     },
 #ifndef OPENSSL_NO_SRTP
     {
@@ -843,6 +844,8 @@ static int final_server_name(SSL *s, unsigned int context, int sent,
 
     case SSL_TLSEXT_ERR_NOACK:
         s->servername_done = 0;
+        if (s->session->ext.hostname != NULL)
+            s->ext.early_data_ok = 0;
         return 1;
 
     default:
@@ -940,6 +943,21 @@ static int init_alpn(SSL *s, unsigned int context)
     return 1;
 }
 
+static int final_alpn(SSL *s, unsigned int context, int sent, int *al)
+{
+    if (!s->server || !SSL_IS_TLS13(s))
+        return 1;
+
+    /*
+     * Call alpn_select callback if needed.  Has to be done after SNI and
+     * cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3
+     * we also have to do this before we decide whether to accept early_data.
+     * In TLSv1.3 we've already negotiated our cipher so we do this call now.
+     * For < TLSv1.3 we defer it until after cipher negotiation.
+     */
+    return tls_handle_alpn(s, al);
+}
+
 static int init_sig_algs(SSL *s, unsigned int context)
 {
     /* Clear any signature algorithms extension received */
@@ -1377,11 +1395,7 @@ static int final_early_data(SSL *s, unsigned int context, int sent, int *al)
             || s->session->ext.tick_identity != 0
             || s->early_data_state != SSL_EARLY_DATA_ACCEPTING
             || !s->ext.early_data_ok
-            || s->hello_retry_request
-            || s->s3->alpn_selected_len != s->session->ext.alpn_selected_len
-            || (s->s3->alpn_selected_len > 0
-                && memcmp(s->s3->alpn_selected, s->session->ext.alpn_selected,
-                          s->s3->alpn_selected_len) != 0)) {
+            || s->hello_retry_request) {
         s->ext.early_data = SSL_EARLY_DATA_REJECTED;
     } else {
         s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 2363c426e047b07b7d643e7a5d695432e319cd16..0dbec913038254ae94a3f4bcc40dbde71a9370c4 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -131,6 +131,9 @@ int tls_parse_ctos_server_name(SSL *s, PACKET *pkt, unsigned int context,
         s->servername_done = s->session->ext.hostname
             && PACKET_equal(&hostname, s->session->ext.hostname,
                             strlen(s->session->ext.hostname));
+
+        if (!s->servername_done && s->session->ext.hostname != NULL)
+            s->ext.early_data_ok = 0;
     }
 
     return 1;
@@ -745,7 +748,8 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
             memcpy(sess->sid_ctx, s->sid_ctx, s->sid_ctx_length);
             sess->sid_ctx_length = s->sid_ctx_length;
             ext = 1;
-            s->ext.early_data_ok = 1;
+            if (id == 0)
+                s->ext.early_data_ok = 1;
         } else {
             uint32_t ticket_age = 0, now, agesec, agems;
             int ret = tls_decrypt_ticket(s, PACKET_data(&identity),
@@ -774,7 +778,8 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
              * Therefore we add 1000ms to our age calculation to adjust for
              * rounding errors.
              */
-            if (sess->timeout >= (long)agesec
+            if (id == 0
+                    && sess->timeout >= (long)agesec
                     && agems / (uint32_t)1000 == agesec
                     && ticket_age <= agems + 1000
                     && ticket_age + TICKET_AGE_ALLOWANCE >= agems + 1000) {
@@ -791,6 +796,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
             /* The ciphersuite is not compatible with this session. */
             SSL_SESSION_free(sess);
             sess = NULL;
+            s->ext.early_data_ok = 0;
             continue;
         }
         break;

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 1f8c22dd236c2da3b19ec0f7df9c22583bb85d66..ae33fe5e5c2eba7735e67b46c70e023bad7c2ea7 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -390,3 +390,5 @@ int tls_parse_stoc_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx, int *al);
 int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                        size_t chainidx, int *al);
+
+int tls_handle_alpn(SSL *s, int *al);

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 8c5f77bce55876445f0c17709cccb5e81572599c..8c2a4e97acccd581939f2a40f7480e0f18e53e8f 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1938,7 +1938,7 @@ static int tls_handle_status_request(SSL *s, int *al)
  * Call the alpn_select callback if needed. Upon success, returns 1.
  * Upon failure, returns 0 and sets |*al| to the appropriate fatal alert.
  */
-static int tls_handle_alpn(SSL *s, int *al)
+int tls_handle_alpn(SSL *s, int *al)
 {
     const unsigned char *selected = NULL;
     unsigned char selected_len = 0;
@@ -1961,15 +1961,30 @@ static int tls_handle_alpn(SSL *s, int *al)
             /* ALPN takes precedence over NPN. */
             s->s3->npn_seen = 0;
 #endif
-        } else if (r == SSL_TLSEXT_ERR_NOACK) {
-            /* Behave as if no callback was present. */
+
+            /* Check ALPN is consistent with early_data */
+            if (s->ext.early_data_ok
+                    && (s->session->ext.alpn_selected == NULL
+                        || selected_len != s->session->ext.alpn_selected_len
+                        || memcmp(selected, s->session->ext.alpn_selected,
+                                  selected_len) != 0))
+                s->ext.early_data_ok = 0;
+
             return 1;
-        } else {
+        } else if (r != SSL_TLSEXT_ERR_NOACK) {
             *al = SSL_AD_NO_APPLICATION_PROTOCOL;
             return 0;
         }
+        /*
+         * If r == SSL_TLSEXT_ERR_NOACK then behave as if no callback was
+         * present.
+         */
     }
 
+    /* Check ALPN is consistent with early_data */
+    if (s->ext.early_data_ok && s->session->ext.alpn_selected != NULL)
+        s->ext.early_data_ok = 0;
+
     return 1;
 }
 
@@ -2059,9 +2074,11 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
         }
         /*
          * Call alpn_select callback if needed.  Has to be done after SNI and
-         * cipher negotiation (HTTP/2 restricts permitted ciphers).
+         * cipher negotiation (HTTP/2 restricts permitted ciphers). In TLSv1.3
+         * we already did this because cipher negotiation happens earlier, and
+         * we must handle ALPN before we decide whether to accept early_data.
          */
-        if (!tls_handle_alpn(s, &al)) {
+        if (!SSL_IS_TLS13(s) && !tls_handle_alpn(s, &al)) {
             SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
                    SSL_R_CLIENTHELLO_TLSEXT);
             goto f_err;