Move ALPN handling from finalizer to delayed call

Commit 02f0274e8c0596dcf7e2d104250232a42c650b96 moved ALPN processing
into an extension finalization function, as the only documented ordering
requirement from previous commits was that ALPN processing occur after
SNI processing, and SNI processing is performed before the extension
finalization step.  However, it is useful for applications'
alpn_select callbacks to run after ciphersuite selection as well -- at
least one application protocol specification (HTTP/2) imposes restrictions
on which ciphersuites are usable with that protocol.  Since it is generally
more preferrable to have a successful TLS connection with a default application
protocol than to fail the TLS connection and not be able to have the preferred
application protocol, it is good to give the alpn_select callback information
about the ciphersuite to be used, so that appropriate restrctions can be
enforced in application code.

Accordingly, split the ALPN handling out into a separate tls_handl_alpn()
function akin to tls_handle_status_request(), called from
tls_post_process_client_hello().  This is an alternative to resuscitating
ssl_check_clienthello_tlsext_late(), something of an awkwward name itself.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4070)

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 2268b271ce0daf82804a5c1da9e86cc2be52cafc..a5dda45a96e7ed76a3cda707a1383c0e2a6b97a8 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -28,7 +28,6 @@ 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,
@@ -207,7 +206,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, final_alpn
+        tls_construct_stoc_alpn, tls_construct_ctos_alpn, NULL
     },
 #ifndef OPENSSL_NO_SRTP
     {
@@ -937,44 +936,6 @@ static int init_alpn(SSL *s, unsigned int context)
     return 1;
 }
 
-static int final_alpn(SSL *s, unsigned int context, int sent, int *al)
-{
-    const unsigned char *selected = NULL;
-    unsigned char selected_len = 0;
-
-    if (!s->server)
-        return 1;
-
-    if (s->ctx->ext.alpn_select_cb != NULL && s->s3->alpn_proposed != NULL) {
-        int r = s->ctx->ext.alpn_select_cb(s, &selected, &selected_len,
-                                           s->s3->alpn_proposed,
-                                           (unsigned int)s->s3->alpn_proposed_len,
-                                           s->ctx->ext.alpn_select_cb_arg);
-
-        if (r == SSL_TLSEXT_ERR_OK) {
-            OPENSSL_free(s->s3->alpn_selected);
-            s->s3->alpn_selected = OPENSSL_memdup(selected, selected_len);
-            if (s->s3->alpn_selected == NULL) {
-                *al = SSL_AD_INTERNAL_ERROR;
-                return 0;
-            }
-            s->s3->alpn_selected_len = selected_len;
-#ifndef OPENSSL_NO_NEXTPROTONEG
-            /* 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. */
-            return 1;
-        } else {
-            *al = SSL_AD_NO_APPLICATION_PROTOCOL;
-            return 0;
-        }
-    }
-
-    return 1;
-}
-
 static int init_sig_algs(SSL *s, unsigned int context)
 {
     /* Clear any signature algorithms extension received */

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index fad339adb2bd2cf5c410f76deab707f4df0d5c00..8c5f77bce55876445f0c17709cccb5e81572599c 100644 (file)
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1934,6 +1934,45 @@ static int tls_handle_status_request(SSL *s, int *al)
     return 1;
 }
 
+/*
+ * 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)
+{
+    const unsigned char *selected = NULL;
+    unsigned char selected_len = 0;
+
+    if (s->ctx->ext.alpn_select_cb != NULL && s->s3->alpn_proposed != NULL) {
+        int r = s->ctx->ext.alpn_select_cb(s, &selected, &selected_len,
+                                           s->s3->alpn_proposed,
+                                           (unsigned int)s->s3->alpn_proposed_len,
+                                           s->ctx->ext.alpn_select_cb_arg);
+
+        if (r == SSL_TLSEXT_ERR_OK) {
+            OPENSSL_free(s->s3->alpn_selected);
+            s->s3->alpn_selected = OPENSSL_memdup(selected, selected_len);
+            if (s->s3->alpn_selected == NULL) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                return 0;
+            }
+            s->s3->alpn_selected_len = selected_len;
+#ifndef OPENSSL_NO_NEXTPROTONEG
+            /* 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. */
+            return 1;
+        } else {
+            *al = SSL_AD_NO_APPLICATION_PROTOCOL;
+            return 0;
+        }
+    }
+
+    return 1;
+}
+
 WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
 {
     int al = SSL_AD_HANDSHAKE_FAILURE;
@@ -2018,6 +2057,15 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
                    SSL_R_CLIENTHELLO_TLSEXT);
             goto f_err;
         }
+        /*
+         * Call alpn_select callback if needed.  Has to be done after SNI and
+         * cipher negotiation (HTTP/2 restricts permitted ciphers).
+         */
+        if (!tls_handle_alpn(s, &al)) {
+            SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
+                   SSL_R_CLIENTHELLO_TLSEXT);
+            goto f_err;
+        }
 
         wst = WORK_MORE_C;
     }