Merge pull request #5717 from icing/sni-alpn-order

Changing ALPN selection to a deterministic point in the handshake.

src/internal.c

@@ -6829,7 +6829,8 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
     ssl->max_fragment = MAX_RECORD_SIZE;
 #endif
 #ifdef HAVE_ALPN
-    ssl->alpn_client_list = NULL;
+    ssl->alpn_peer_requested = NULL;
+    ssl->alpn_peer_requested_length = 0;
     #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY)
         ssl->alpnSelect    = ctx->alpnSelect;
         ssl->alpnSelectArg = ctx->alpnSelectArg;
@@ -7659,9 +7660,10 @@ void SSL_ResourceFree(WOLFSSL* ssl)
     TLSX_FreeAll(ssl->extensions, ssl->heap);
 #endif /* !NO_TLS */
 #ifdef HAVE_ALPN
-    if (ssl->alpn_client_list != NULL) {
+    if (ssl->alpn_peer_requested != NULL) {
-        XFREE(ssl->alpn_client_list, ssl->heap, DYNAMIC_TYPE_ALPN);
+        XFREE(ssl->alpn_peer_requested, ssl->heap, DYNAMIC_TYPE_ALPN);
-        ssl->alpn_client_list = NULL;
+        ssl->alpn_peer_requested = NULL;
+        ssl->alpn_peer_requested_length = 0;
     }
 #endif
 #endif /* HAVE_TLS_EXTENSIONS */
@@ -33305,6 +33307,10 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
                 if((ret=SNI_Callback(ssl)))
                     goto out;
     #endif
+    #ifdef HAVE_ALPN
+                if((ret=ALPN_Select(ssl)))
+                    goto out;
+    #endif
 
                 i += totalExtSz;
 #else

src/ssl.c

@@ -3108,22 +3108,37 @@ int wolfSSL_ALPN_GetProtocol(WOLFSSL* ssl, char **protocol_name, word16 *size)
 
 int wolfSSL_ALPN_GetPeerProtocol(WOLFSSL* ssl, char **list, word16 *listSz)
 {
-    if (list == NULL || listSz == NULL)
+    int i, len;
+    char *p;
+    byte *s;
+
+    if (ssl == NULL || list == NULL || listSz == NULL)
         return BAD_FUNC_ARG;
 
-    if (ssl->alpn_client_list == NULL)
+    if (ssl->alpn_peer_requested == NULL
+        || ssl->alpn_peer_requested_length == 0)
         return BUFFER_ERROR;
 
-    *listSz = (word16)XSTRLEN(ssl->alpn_client_list);
+    /* ssl->alpn_peer_requested are the original bytes sent in a ClientHello,
-    if (*listSz == 0)
+     * formatted as (len-byte chars+)+. To turn n protocols into a
-        return BUFFER_ERROR;
+     * comma-separated C string, one needs (n-1) commas and a final 0 byte
-
+     * which has the same length as the original.
-    *list = (char *)XMALLOC((*listSz)+1, ssl->heap, DYNAMIC_TYPE_TLSX);
+     * The returned length is the strlen() of the C string, so -1 of that. */
-    if (*list == NULL)
+    *listSz = ssl->alpn_peer_requested_length-1;
+    *list = p = (char *)XMALLOC(ssl->alpn_peer_requested_length, ssl->heap,
+                                DYNAMIC_TYPE_TLSX);
+    if (p == NULL)
         return MEMORY_ERROR;
 
-    XSTRNCPY(*list, ssl->alpn_client_list, (*listSz)+1);
+    for (i = 0, s = ssl->alpn_peer_requested, len = 0;
-    (*list)[*listSz] = 0;
+         i < ssl->alpn_peer_requested_length;
+         p += len, i += len) {
+        if (i)
+            *p++ = ',';
+        len = s[i++];
+        XSTRNCPY(p, (char *)(s + i), len);
+    }
+    *p = 0;
 
     return WOLFSSL_SUCCESS;
 }

src/tls.c

@@ -1579,16 +1579,116 @@ static int TLSX_SetALPN(TLSX** extensions, const void* data, word16 size,
     return WOLFSSL_SUCCESS;
 }
 
+static int ALPN_find_match(WOLFSSL *ssl, TLSX **pextension,
+                           const byte **psel, byte *psel_len,
+                           const byte *alpn_val, word16 alpn_val_len)
+{
+    TLSX    *extension;
+    ALPN    *alpn, *list;
+    const byte *sel = NULL, *s;
+    byte sel_len = 0, wlen;
+
+    extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL);
+    if (extension == NULL)
+        extension = TLSX_Find(ssl->ctx->extensions,
+                              TLSX_APPLICATION_LAYER_PROTOCOL);
+
+    /* No ALPN configured here */
+    if (extension == NULL || extension->data == NULL)
+        return 0;
+
+    list = (ALPN*)extension->data;
+    for (s = alpn_val, wlen = 0;
+         (s - alpn_val) < alpn_val_len;
+         s += wlen) {
+        wlen = *s++; /* bounds already checked on save */
+        alpn = TLSX_ALPN_Find(list, (char*)s, wlen);
+        if (alpn != NULL) {
+            WOLFSSL_MSG("ALPN protocol match");
+            sel = s,
+            sel_len = wlen;
+            break;
+        }
+    }
+
+    if (sel == NULL) {
+        WOLFSSL_MSG("No ALPN protocol match");
+
+        /* do nothing if no protocol match between client and server and option
+         is set to continue (like OpenSSL) */
+        if (list->options & WOLFSSL_ALPN_CONTINUE_ON_MISMATCH) {
+            WOLFSSL_MSG("Continue on mismatch");
+        }
+        else {
+            SendAlert(ssl, alert_fatal, no_application_protocol);
+            WOLFSSL_ERROR_VERBOSE(UNKNOWN_ALPN_PROTOCOL_NAME_E);
+            return UNKNOWN_ALPN_PROTOCOL_NAME_E;
+        }
+    }
+
+    *pextension = extension;
+    *psel = sel;
+    *psel_len = sel_len;
+    return 0;
+}
+
+int ALPN_Select(WOLFSSL *ssl)
+{
+    TLSX *extension;
+    const byte *sel = NULL;
+    byte sel_len = 0;
+    int r = 0;
+
+    WOLFSSL_ENTER("ALPN_Select");
+    if (ssl->alpn_peer_requested == NULL)
+        return 0;
+
+#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY)
+    if (ssl->alpnSelect != NULL && ssl->options.side == WOLFSSL_SERVER_END) {
+        if (ssl->alpnSelect(ssl, &sel, &sel_len, ssl->alpn_peer_requested,
+                            ssl->alpn_peer_requested_length,
+                            ssl->alpnSelectArg) == 0) {
+            WOLFSSL_MSG_EX("ALPN protocol match");
+        }
+        else {
+            sel = NULL;
+            sel_len = 0;
+        }
+    }
+#endif
+
+    if (sel == NULL) {
+        r = ALPN_find_match(ssl, &extension, &sel, &sel_len,
+                            ssl->alpn_peer_requested,
+                            ssl->alpn_peer_requested_length);
+        if (r != 0)
+            return r;
+    }
+
+    if (sel != NULL) {
+        /* set the matching negotiated protocol */
+        r = TLSX_SetALPN(&ssl->extensions, sel, sel_len, ssl->heap);
+        if (r != WOLFSSL_SUCCESS) {
+            WOLFSSL_MSG("TLSX_SetALPN failed");
+            return BUFFER_ERROR;
+        }
+        /* reply to ALPN extension sent from peer */
+#ifndef NO_WOLFSSL_SERVER
+        TLSX_SetResponse(ssl, TLSX_APPLICATION_LAYER_PROTOCOL);
+#endif
+    }
+    return 0;
+}
+
 /** Parses a buffer of ALPN extensions and set the first one matching
  * client and server requirements */
 static int TLSX_ALPN_ParseAndSet(WOLFSSL *ssl, const byte *input, word16 length,
                                  byte isRequest)
 {
-    word16  size = 0, offset = 0, idx = 0;
+    word16  size = 0, offset = 0, wlen;
     int     r = BUFFER_ERROR;
-    byte    match = 0;
     TLSX    *extension;
-    ALPN    *alpn = NULL, *list;
+    const byte *s;
 
     if (OPAQUE16_LEN > length)
         return BUFFER_ERROR;
@@ -1596,116 +1696,56 @@ static int TLSX_ALPN_ParseAndSet(WOLFSSL *ssl, const byte *input, word16 length,
     ato16(input, &size);
     offset += OPAQUE16_LEN;
 
-    if (size == 0)
-        return BUFFER_ERROR;
-
-    extension = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL);
-    if (extension == NULL)
-        extension = TLSX_Find(ssl->ctx->extensions,
-                              TLSX_APPLICATION_LAYER_PROTOCOL);
-
-#if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY)
-    if (ssl->alpnSelect != NULL && ssl->options.side == WOLFSSL_SERVER_END) {
-        const byte* out;
-        unsigned char outLen;
-
-        if (ssl->alpnSelect(ssl, &out, &outLen, input + offset, size,
-                            ssl->alpnSelectArg) == 0) {
-            WOLFSSL_MSG("ALPN protocol match");
-            /* clears out all current ALPN extensions set */
-            TLSX_Remove(&ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL, ssl->heap);
-            extension = NULL;
-            if (TLSX_UseALPN(&ssl->extensions, (char*)out, outLen, 0, ssl->heap)
-                                                           == WOLFSSL_SUCCESS) {
-                extension = TLSX_Find(ssl->extensions,
-                                      TLSX_APPLICATION_LAYER_PROTOCOL);
-            }
-        }
-    }
-#endif
-
-    if (extension == NULL || extension->data == NULL) {
-        return isRequest ? 0
-                         : TLSX_HandleUnsupportedExtension(ssl);
-    }
-
     /* validating alpn list length */
-    if (length != OPAQUE16_LEN + size)
+    if (size == 0 || length != OPAQUE16_LEN + size)
         return BUFFER_ERROR;
 
-    list = (ALPN*)extension->data;
+    /* validating length of entries before accepting */
-
+    for (s = input + offset, wlen = 0; (s - input) < size; s += wlen) {
-    /* keep the list sent by client */
+        wlen = *s++;
-    if (isRequest) {
+        if (wlen == 0 || (s + wlen - input) > length)
-        if (ssl->alpn_client_list != NULL)
+            return BUFFER_ERROR;
-            XFREE(ssl->alpn_client_list, ssl->heap, DYNAMIC_TYPE_ALPN);
-
-        ssl->alpn_client_list = (char *)XMALLOC(size, ssl->heap,
-                                                DYNAMIC_TYPE_ALPN);
-        if (ssl->alpn_client_list == NULL)
-            return MEMORY_ERROR;
     }
 
-    for (size = 0; offset < length; offset += size) {
+    if (isRequest) {
-
+        /* keep the list sent by peer, if this is from a request. We
-        size = input[offset++];
+         * use it later in ALPN_Select() for evaluation. */
-        if (offset + size > length || size == 0)
+        if (ssl->alpn_peer_requested != NULL) {
-            return BUFFER_ERROR;
+            XFREE(ssl->alpn_peer_requested, ssl->heap, DYNAMIC_TYPE_ALPN);
-
+            ssl->alpn_peer_requested_length = 0;
-        if (isRequest) {
-            XMEMCPY(ssl->alpn_client_list+idx, (char*)input + offset, size);
-            idx += size;
-            ssl->alpn_client_list[idx++] = ',';
         }
+        ssl->alpn_peer_requested = (byte *)XMALLOC(size, ssl->heap,
+                                                   DYNAMIC_TYPE_ALPN);
+        if (ssl->alpn_peer_requested == NULL) {
+            return MEMORY_ERROR;
+        }
+        ssl->alpn_peer_requested_length = size;
+        XMEMCPY(ssl->alpn_peer_requested, (char*)input + offset, size);
+    }
+    else {
+        /* a response, we should find the value in our config */
+        const byte *sel = NULL;
+        byte sel_len = 0;
 
-        if (!match) {
+        r = ALPN_find_match(ssl, &extension, &sel, &sel_len, input + offset, size);
-            alpn = TLSX_ALPN_Find(list, (char*)input + offset, size);
+        if (r != 0)
-            if (alpn != NULL) {
+            return r;
-                WOLFSSL_MSG("ALPN protocol match");
-                match = 1;
 
-                /* skip reading other values if not required */
+        if (sel != NULL) {
-                if (!isRequest)
+            /* set the matching negotiated protocol */
-                    break;
+            r = TLSX_SetALPN(&ssl->extensions, sel, sel_len, ssl->heap);
+            if (r != WOLFSSL_SUCCESS) {
+                WOLFSSL_MSG("TLSX_SetALPN failed");
+                return BUFFER_ERROR;
             }
         }
-    }
+        /* If we had nothing configured, the response is unexpected */
-
+        else if (extension == NULL) {
-    if (isRequest)
+            r = TLSX_HandleUnsupportedExtension(ssl);
-        ssl->alpn_client_list[idx-1] = 0;
+            if (r != 0)
-
+                return r;
-    if (!match) {
-        WOLFSSL_MSG("No ALPN protocol match");
-
-        /* do nothing if no protocol match between client and server and option
-         is set to continue (like OpenSSL) */
-        if (list->options & WOLFSSL_ALPN_CONTINUE_ON_MISMATCH) {
-            WOLFSSL_MSG("Continue on mismatch");
-            return 0;
         }
-
-        SendAlert(ssl, alert_fatal, no_application_protocol);
-        WOLFSSL_ERROR_VERBOSE(UNKNOWN_ALPN_PROTOCOL_NAME_E);
-        return UNKNOWN_ALPN_PROTOCOL_NAME_E;
     }
-
-    /* set the matching negotiated protocol */
-    r = TLSX_SetALPN(&ssl->extensions,
-                     alpn->protocol_name,
-                     (word16)XSTRLEN(alpn->protocol_name),
-                     ssl->heap);
-    if (r != WOLFSSL_SUCCESS) {
-        WOLFSSL_MSG("TLSX_SetALPN failed");
-        return BUFFER_ERROR;
-    }
-
-    /* reply to ALPN extension sent from client */
-    if (isRequest) {
-#ifndef NO_WOLFSSL_SERVER
-        TLSX_SetResponse(ssl, TLSX_APPLICATION_LAYER_PROTOCOL);
-#endif
-    }
-
     return 0;
 }
 

src/tls13.c

@@ -5863,7 +5863,7 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 
 #ifdef HAVE_SNI
         if ((ret = SNI_Callback(ssl)) != 0)
-            return ret;
+            goto exit_dch;
         ssl->options.side = WOLFSSL_SERVER_END;
 #endif
 
@@ -5950,6 +5950,12 @@ int DoTls13ClientHello(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
 #endif
     }
 
+#ifdef HAVE_ALPN
+    /* With PSK and all other things validated, it's time to
+     * select the ALPN protocol, if so requested */
+    if ((ret = ALPN_Select(ssl)) != 0)
+        goto exit_dch;
+#endif
     /* Advance state and proceed */
     ssl->options.asyncState = TLS_ASYNC_BUILD;
     } /* case TLS_ASYNC_BEGIN */

tests/quic.c

@@ -1174,6 +1174,98 @@ static int test_quic_server_hello(int verbose) {
     return ret;
 }
 
+/* This has gotten a bit out of hand. */
+#if (defined(OPENSSL_ALL) || (defined(OPENSSL_EXTRA) && \
+    (defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || \
+    defined(HAVE_LIGHTY) || defined(WOLFSSL_HAPROXY) || \
+    defined(WOLFSSL_OPENSSH) || defined(HAVE_SBLIM_SFCB)))) \
+    && defined(HAVE_ALPN) && defined(HAVE_SNI)
+#define REALLY_HAVE_ALPN_AND_SNI
+#else
+#undef REALLY_HAVE_ALPN_AND_SNI
+#endif
+
+#ifdef REALLY_HAVE_ALPN_AND_SNI
+static int inspect_SNI(WOLFSSL *ssl, int *ad, void *baton)
+{
+    char *stripe = baton;
+
+    (void)ssl;
+    *ad = 0;
+    strcat(stripe, "S");
+    return 0;
+}
+
+static int select_ALPN(WOLFSSL *ssl,
+            const unsigned char **out,
+            unsigned char *outlen,
+            const unsigned char *in,
+            unsigned int inlen,
+            void *baton)
+{
+    char *stripe = baton;
+
+    (void)ssl;
+    (void)inlen;
+    /* just select the first */
+    *out = in + 1;
+    *outlen = in[0];
+    strcat(stripe, "A");
+    return 0;
+}
+
+static int test_quic_alpn(int verbose) {
+    WOLFSSL_CTX *ctx_c, *ctx_s;
+    int ret = 0;
+    QuicTestContext tclient, tserver;
+    QuicConversation conv;
+    char stripe[256];
+    unsigned char alpn_protos[256];
+
+    AssertNotNull(ctx_c = wolfSSL_CTX_new(wolfTLSv1_3_client_method()));
+    AssertNotNull(ctx_s = wolfSSL_CTX_new(wolfTLSv1_3_server_method()));
+    AssertTrue(wolfSSL_CTX_use_certificate_file(ctx_s, svrCertFile, WOLFSSL_FILETYPE_PEM));
+    AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx_s, svrKeyFile, WOLFSSL_FILETYPE_PEM));
+
+    stripe[0] = '\0';
+    wolfSSL_CTX_set_servername_callback(ctx_s, inspect_SNI);
+    wolfSSL_CTX_set_servername_arg(ctx_s, stripe);
+    wolfSSL_CTX_set_alpn_select_cb(ctx_s, select_ALPN, stripe);
+
+    /* setup ssls */
+    QuicTestContext_init(&tclient, ctx_c, "client", verbose);
+    QuicTestContext_init(&tserver, ctx_s, "server", verbose);
+
+    /* set SNI and ALPN callbacks on server side,
+     * provide values on client side */
+    wolfSSL_UseSNI(tclient.ssl, WOLFSSL_SNI_HOST_NAME,
+                   "wolfssl.com", sizeof("wolfssl.com")-1);
+    /* connect */
+    QuicConversation_init(&conv, &tclient, &tserver);
+
+    strcpy((char*)(alpn_protos + 1), "test");
+    alpn_protos[0] = 4;
+    wolfSSL_set_alpn_protos(tclient.ssl, alpn_protos, 5);
+
+    QuicConversation_do(&conv);
+    AssertIntEQ(tclient.output.len, 0);
+    AssertIntEQ(tserver.output.len, 0);
+
+    /* SNI callback needs to be called before ALPN callback */
+    AssertStrEQ(stripe, "SA");
+
+    QuicTestContext_free(&tclient);
+    QuicTestContext_free(&tserver);
+
+    wolfSSL_CTX_free(ctx_c);
+    wolfSSL_CTX_free(ctx_s);
+    printf("    test_quic_alpn: %s\n", (ret == 0)? passed : failed);
+
+    return ret;
+}
+#endif /* REALLY_HAVE_ALPN_AND_SNI */
+
+
 #ifdef HAVE_SESSION_TICKET
 
 static int test_quic_key_share(int verbose) {
@@ -1536,6 +1628,9 @@ int QuicTest(void)
     if ((ret = test_quic_crypt()) != 0) goto leave;
     if ((ret = test_quic_client_hello(verbose)) != 0) goto leave;
     if ((ret = test_quic_server_hello(verbose)) != 0) goto leave;
+#ifdef REALLY_HAVE_ALPN_AND_SNI
+    if ((ret = test_quic_alpn(verbose)) != 0) goto leave;
+#endif /* REALLY_HAVE_ALPN_AND_SNI */
 #ifdef HAVE_SESSION_TICKET
     if ((ret = test_quic_key_share(verbose)) != 0) goto leave;
     if ((ret = test_quic_resumption(verbose)) != 0) goto leave;

wolfssl/internal.h

@@ -1975,6 +1975,10 @@ WOLFSSL_LOCAL int SNI_Callback(WOLFSSL* ssl);
 #endif
 #endif
 
+#ifdef HAVE_ALPN
+WOLFSSL_LOCAL int ALPN_Select(WOLFSSL* ssl);
+#endif
+
 WOLFSSL_LOCAL int ChachaAEADEncrypt(WOLFSSL* ssl, byte* out, const byte* input,
                               word16 sz); /* needed by sniffer */
 
@@ -5084,7 +5088,9 @@ struct WOLFSSL {
         SecureRenegotiation* secure_renegotiation; /* valid pointer indicates */
     #endif                                         /* user turned on */
     #ifdef HAVE_ALPN
-        char*   alpn_client_list;  /* keep the client's list */
+        byte *alpn_peer_requested; /* the ALPN bytes requested by peer, sequence
+                                    * of length byte + chars */
+        word16 alpn_peer_requested_length; /* number of bytes total */
         #if defined(OPENSSL_ALL) || defined(WOLFSSL_NGINX)  || \
             defined(WOLFSSL_HAPROXY) || defined(WOLFSSL_QUIC)
             CallbackALPNSelect alpnSelect;