Support ALPN List Change via ConnectionSetConfiguration function (#2972)

microsoft · Aug 13, 2022 · 3ca4fc4 · 3ca4fc4
1 parent 8d9bec9
commit 3ca4fc4
Show file tree

Hide file tree

Showing 16 changed files with 123 additions and 171 deletions.
diff --git a/docs/api/ConnectionSetConfiguration.md b/docs/api/ConnectionSetConfiguration.md
@@ -31,7 +31,7 @@ The function returns a [QUIC_STATUS](QUIC_STATUS.md). The app may use `QUIC_FAIL
 
 # Remarks
 
-`ConnectionSetConfiguration` can be called in the `QUIC_LISTENER_EVENT_NEW_CONNECTION` callback, or outside of it if the connection was accepted. It's generally recommended to call `ConnectionSetConfiguration` in the `QUIC_LISTENER_EVENT_NEW_CONNECTION` callback unless the server application needs to do asynchronous processing to decide which configuration to use on a connection.
+`ConnectionSetConfiguration` can be called in the `QUIC_LISTENER_EVENT_NEW_CONNECTION` callback, or outside of it if the connection was accepted. It's generally recommended to call `ConnectionSetConfiguration` in the `QUIC_LISTENER_EVENT_NEW_CONNECTION` callback unless the server application needs to do asynchronous processing to decide which configuration to use on a connection. You can use this function to set the ALPN list for the server connection.
 
 # See Also
 

diff --git a/docs/api/QUIC_LISTENER_EVENT.md b/docs/api/QUIC_LISTENER_EVENT.md
@@ -42,31 +42,6 @@ This field indicates the [QUIC_NEW_CONNECTION_INFO](QUIC_NEW_CONNECTION_INFO.md)
 
 This field indicates the valid handle to the new incoming connection.
 
-`NewNegotiatedAlpn`
-
-This field is optional to set. If you want to set, this field **must** indicate a valid address on the `ClientAlpnList` of the `QUIC_NEW_CONNECTION_INFO`.
-If you do not set this field, server will use `NegotiatedAlpn` in the `QUIC_NEW_CONNECTION_INFO`.
-If you set this field, server will use the ALPN in this field.
-If you set this field incorrectly, server will **fail** the connection with `QUIC_STATUS_INTERNAL_ERROR`.
-
-### Example Usage of NewNegotiatedAlpn Field
-
-```C
-uint8_t NewAlpn[] = "msquic1";
-uint8_t NewAlpnLength = sizeof(NewAlpn)/sizeof(*NewAlpn) - 1;
-uint16_t AlpnListLength = Event->NEW_CONNECTION.Info->ClientAlpnListLength;
-const uint8_t* AlpnList = Event->NEW_CONNECTION.Info->ClientAlpnList;
-while (AlpnListLength != 0) {
-    if (AlpnList[0] == NewAlpnLength &&
-        memcmp(AlpnList+1, NewAlpn, NewAlpnLength) == 0) {
-        Event->NEW_CONNECTION.NewNegotiatedAlpn = AlpnList;
-        break;
-    }
-    AlpnListLength -= AlpnList[0] + 1;
-    AlpnList += (size_t)AlpnList[0] + (size_t)1;
-}
-```
-
 ## QUIC_LISTENER_EVENT_STOP_COMPLETE
 
 This event is delivered when server app wants to stop receiving new incoming connections.

diff --git a/src/core/binding.c b/src/core/binding.c
@@ -542,6 +542,8 @@ QuicBindingAcceptConnection(
     }
     CxPlatCopyMemory(NegotiatedAlpn, Info->NegotiatedAlpn - 1, NegotiatedAlpnLength);
     Connection->Crypto.TlsState.NegotiatedAlpn = NegotiatedAlpn;
+    Connection->Crypto.TlsState.ClientAlpnList = Info->ClientAlpnList;
+    Connection->Crypto.TlsState.ClientAlpnListLength = Info->ClientAlpnListLength;
 
     //
     // Allow for the listener to decide if it wishes to accept the incoming

diff --git a/src/core/connection.c b/src/core/connection.c
@@ -2516,6 +2516,17 @@ QuicConnSetConfiguration(
             Status = QUIC_STATUS_INVALID_PARAMETER;
             goto Error;
         }
+
+        Status =
+            QuicCryptoReNegotiateAlpn(
+                Connection,
+                Connection->Configuration->AlpnListLength,
+                Connection->Configuration->AlpnList);
+        if (QUIC_FAILED(Status)) {
+            goto Error;
+        }
+        Connection->Crypto.TlsState.ClientAlpnList = NULL;
+        Connection->Crypto.TlsState.ClientAlpnListLength = 0;
     }
 
     Status = QuicConnGenerateLocalTransportParameters(Connection, &LocalTP);

diff --git a/src/core/crypto.c b/src/core/crypto.c
@@ -2542,3 +2542,83 @@ QuicCryptoDecodeClientTicket(
 Error:
     return Status;
 }
+
+QUIC_STATUS
+QuicCryptoReNegotiateAlpn(
+    _In_opt_ QUIC_CONNECTION* Connection,
+    _In_ uint16_t AlpnListLength,
+    _In_reads_bytes_(AlpnListLength)
+        const uint8_t* AlpnList
+    )
+{
+    CXPLAT_DBG_ASSERT(Connection != NULL);
+    CXPLAT_DBG_ASSERT(AlpnList != NULL);
+    CXPLAT_DBG_ASSERT(AlpnListLength > 0);
+
+    const uint8_t* PrevNegotiatedAlpn = Connection->Crypto.TlsState.NegotiatedAlpn;
+    if (AlpnList[0] == PrevNegotiatedAlpn[0]) {
+        if (memcmp(AlpnList + 1, PrevNegotiatedAlpn + 1, AlpnList[0]) == 0) {
+            return QUIC_STATUS_SUCCESS;
+        }
+    }
+
+    const uint8_t* NewNegotiatedAlpn = NULL;
+    while (AlpnListLength != 0) {
+        const uint8_t* Result =
+            CxPlatTlsAlpnFindInList(
+                Connection->Crypto.TlsState.ClientAlpnListLength,
+                Connection->Crypto.TlsState.ClientAlpnList,
+                AlpnList[0],
+                AlpnList + 1);
+        if (Result != NULL) {
+            NewNegotiatedAlpn = AlpnList;
+            break;
+        }
+        AlpnListLength -= AlpnList[0] + 1;
+        AlpnList += AlpnList[0] + 1;
+    }
+
+    if (NewNegotiatedAlpn == NULL) {
+        QuicTraceEvent(
+            ConnError,
+            "[conn][%p] ERROR, %s.",
+            Connection,
+            "No ALPN match found");
+        QuicConnTransportError(
+            Connection,
+            QUIC_ERROR_INTERNAL_ERROR);
+        return QUIC_STATUS_INVALID_PARAMETER;
+    }
+
+    //
+    // Free current ALPN buffer if it's allocated on heap.
+    //
+    if (Connection->Crypto.TlsState.NegotiatedAlpn != Connection->Crypto.TlsState.SmallAlpnBuffer) {
+        CXPLAT_FREE(Connection->Crypto.TlsState.NegotiatedAlpn, QUIC_POOL_ALPN);
+        Connection->Crypto.TlsState.NegotiatedAlpn = NULL;
+    }
+
+    uint8_t* NegotiatedAlpn = NULL;
+    uint8_t NegotiatedAlpnLength = NewNegotiatedAlpn[0];
+    if (NegotiatedAlpnLength < TLS_SMALL_ALPN_BUFFER_SIZE) {
+        NegotiatedAlpn = Connection->Crypto.TlsState.SmallAlpnBuffer;
+    } else {
+        NegotiatedAlpn = CXPLAT_ALLOC_NONPAGED(NegotiatedAlpnLength + sizeof(uint8_t), QUIC_POOL_ALPN);
+        if (NegotiatedAlpn == NULL) {
+            QuicTraceEvent(
+                AllocFailure,
+                "Allocation of '%s' failed. (%llu bytes)",
+                "NegotiatedAlpn",
+                NegotiatedAlpnLength);
+            QuicConnTransportError(
+                Connection,
+                QUIC_ERROR_INTERNAL_ERROR);
+            return QUIC_STATUS_OUT_OF_MEMORY;
+        }
+    }
+    NegotiatedAlpn[0] = NegotiatedAlpnLength;
+    CxPlatCopyMemory(NegotiatedAlpn + 1, NewNegotiatedAlpn + 1, NegotiatedAlpnLength);
+    Connection->Crypto.TlsState.NegotiatedAlpn = NegotiatedAlpn;
+
+    return QUIC_STATUS_SUCCESS;
+}
diff --git a/src/core/crypto.h b/src/core/crypto.h
@@ -398,6 +398,17 @@ QuicCryptoDecodeClientTicket(
     _Out_ uint32_t* QuicVersion
     );
 
+//
+// Helper function to NegotiateAlpn.
+//
+QUIC_STATUS
+QuicCryptoReNegotiateAlpn(
+    _In_opt_ QUIC_CONNECTION* Connection,
+    _In_ uint16_t AlpnListLength,
+    _In_reads_bytes_(AlpnListLength)
+        const uint8_t* AlpnList
+    );
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/src/core/listener.c b/src/core/listener.c
@@ -613,7 +613,6 @@ QuicListenerClaimConnection(
     Event.Type = QUIC_LISTENER_EVENT_NEW_CONNECTION;
     Event.NEW_CONNECTION.Info = Info;
     Event.NEW_CONNECTION.Connection = (HQUIC)Connection;
-    Event.NEW_CONNECTION.NewNegotiatedAlpn = NULL;
 
     QuicListenerAttachSilo(Listener);
 
@@ -644,55 +643,6 @@ QuicListenerClaimConnection(
         return FALSE;
     }
 
-    const uint8_t* NewNegotiatedAlpn = Event.NEW_CONNECTION.NewNegotiatedAlpn;
-    if (NewNegotiatedAlpn) {
-        const uint8_t* ListStart = Info->ClientAlpnList;
-        const uint8_t* ListEnd = Info->ClientAlpnList + Info->ClientAlpnListLength;
-        if (ListStart > NewNegotiatedAlpn ||
-            ListEnd <= NewNegotiatedAlpn ||
-            NewNegotiatedAlpn + NewNegotiatedAlpn[0] + sizeof(uint8_t) > ListEnd) {
-            QuicTraceEvent(
-                ListenerError,
-                "[list][%p] ERROR, %s.",
-                Listener,
-                "'NewNegotiatedAlpn' field is out of bounds of the 'ClientAlpnList' buffer.");
-            QuicConnTransportError(
-                Connection,
-                QUIC_ERROR_INTERNAL_ERROR);
-            return FALSE;
-        }
-
-        //
-        // Free current ALPN buffer if it's allocated on heap.
-        //
-        if (Connection->Crypto.TlsState.NegotiatedAlpn != Connection->Crypto.TlsState.SmallAlpnBuffer) {
-            CXPLAT_FREE(Connection->Crypto.TlsState.NegotiatedAlpn, QUIC_POOL_ALPN);
-            Connection->Crypto.TlsState.NegotiatedAlpn = NULL;
-        }
-
-        uint8_t* NegotiatedAlpn = NULL;
-        uint8_t NegotiatedAlpnLength = Event.NEW_CONNECTION.NewNegotiatedAlpn[0];
-        if (NegotiatedAlpnLength < TLS_SMALL_ALPN_BUFFER_SIZE) {
-            NegotiatedAlpn = Connection->Crypto.TlsState.SmallAlpnBuffer;
-        } else {
-            NegotiatedAlpn = CXPLAT_ALLOC_NONPAGED(NegotiatedAlpnLength + sizeof(uint8_t), QUIC_POOL_ALPN);
-            if (NegotiatedAlpn == NULL) {
-                QuicTraceEvent(
-                    AllocFailure,
-                    "Allocation of '%s' failed. (%llu bytes)",
-                    "NegotiatedAlpn",
-                    NegotiatedAlpnLength);
-                QuicConnTransportError(
-                    Connection,
-                    QUIC_ERROR_INTERNAL_ERROR);
-                return FALSE;
-            }
-        }
-        NegotiatedAlpn[0] = NegotiatedAlpnLength;
-        CxPlatCopyMemory(NegotiatedAlpn + 1, Event.NEW_CONNECTION.NewNegotiatedAlpn + 1, NegotiatedAlpnLength);
-        Connection->Crypto.TlsState.NegotiatedAlpn = NegotiatedAlpn;
-    }
-
     //
     // The application layer has accepted the connection.
     //

diff --git a/src/cs/lib/msquic_generated.cs b/src/cs/lib/msquic_generated.cs
@@ -1820,9 +1820,6 @@ internal unsafe partial struct _NEW_CONNECTION_e__Struct
 
                 [NativeTypeName("HQUIC")]
                 internal QUIC_HANDLE* Connection;
-
-                [NativeTypeName("const uint8_t *")]
-                internal byte* NewNegotiatedAlpn;
             }
 
             internal partial struct _STOP_COMPLETE_e__Struct

diff --git a/src/generated/linux/listener.c.clog.h b/src/generated/linux/listener.c.clog.h
@@ -307,26 +307,6 @@ tracepoint(CLOG_LISTENER_C, ListenerRundown , arg2, arg3);\
 
 
 
-/*----------------------------------------------------------
-// Decoder Ring for ListenerError
-// [list][%p] ERROR, %s.
-// QuicTraceEvent(
-                ListenerError,
-                "[list][%p] ERROR, %s.",
-                Listener,
-                "'NewNegotiatedAlpn' field is out of bounds of the 'ClientAlpnList' buffer.");
-// arg2 = arg2 = Listener = arg2
-// arg3 = arg3 = "'NewNegotiatedAlpn' field is out of bounds of the 'ClientAlpnList' buffer." = arg3
-----------------------------------------------------------*/
-#ifndef _clog_4_ARGS_TRACE_ListenerError
-#define _clog_4_ARGS_TRACE_ListenerError(uniqueId, encoded_arg_string, arg2, arg3)\
-tracepoint(CLOG_LISTENER_C, ListenerError , arg2, arg3);\
-
-#endif
-
-
-
-
 /*----------------------------------------------------------
 // Decoder Ring for ConnError
 // [conn][%p] ERROR, %s.

diff --git a/src/generated/linux/listener.c.clog.h.lttng.h b/src/generated/linux/listener.c.clog.h.lttng.h
@@ -324,29 +324,6 @@ TRACEPOINT_EVENT(CLOG_LISTENER_C, ListenerRundown,
 
 
 
-/*----------------------------------------------------------
-// Decoder Ring for ListenerError
-// [list][%p] ERROR, %s.
-// QuicTraceEvent(
-                ListenerError,
-                "[list][%p] ERROR, %s.",
-                Listener,
-                "'NewNegotiatedAlpn' field is out of bounds of the 'ClientAlpnList' buffer.");
-// arg2 = arg2 = Listener = arg2
-// arg3 = arg3 = "'NewNegotiatedAlpn' field is out of bounds of the 'ClientAlpnList' buffer." = arg3
-----------------------------------------------------------*/
-TRACEPOINT_EVENT(CLOG_LISTENER_C, ListenerError,
-    TP_ARGS(
-        const void *, arg2,
-        const char *, arg3), 
-    TP_FIELDS(
-        ctf_integer_hex(uint64_t, arg2, arg2)
-        ctf_string(arg3, arg3)
-    )
-)
-
-
-
 /*----------------------------------------------------------
 // Decoder Ring for ConnError
 // [conn][%p] ERROR, %s.

diff --git a/src/inc/msquic.h b/src/inc/msquic.h
@@ -966,7 +966,6 @@ typedef struct QUIC_LISTENER_EVENT {
         struct {
             const QUIC_NEW_CONNECTION_INFO* Info;
             HQUIC Connection;
-            const uint8_t* NewNegotiatedAlpn;
         } NEW_CONNECTION;
         struct {
             BOOLEAN AppCloseInProgress  : 1;

diff --git a/src/inc/quic_tls.h b/src/inc/quic_tls.h
@@ -326,6 +326,12 @@ typedef struct CXPLAT_TLS_PROCESS_STATE {
     //
     QUIC_PACKET_KEY* WriteKeys[QUIC_PACKET_KEY_COUNT];
 
+    //
+    // (Server-Connection-Only) ClientAlpnList cache params (in TLS format)
+    //
+    const uint8_t* ClientAlpnList;
+    uint16_t ClientAlpnListLength;
+
 } CXPLAT_TLS_PROCESS_STATE;
 
 typedef

diff --git a/src/test/lib/HandshakeTest.cpp b/src/test/lib/HandshakeTest.cpp
@@ -3101,15 +3101,15 @@ QuicTestChangeAlpn(
     {
         const char* FirstAlpns[] = {"quic1", "quic1", "VerifyThisAsQuicALPN", "VerifyThisAsQuicALPN", "quic1"};
         const char* SecondAlpns[] = {"MsQuicTest", "MsQuicVerifyThisAsQuicALPN", "MsQuicTest", "MsQuicVerifyThisAsQuicALPN", "MsQuicTest"};
-        for (int idx = 0; idx < 4; ++idx) {
+        for (uint32_t idx = 0; idx < ARRAYSIZE(FirstAlpns); ++idx) {
             MsQuicAlpn Alpn(FirstAlpns[idx], SecondAlpns[idx]);
 
-            AlpnHelper NewAlpn(SecondAlpns[idx], true);
+            MsQuicAlpn NewAlpn(SecondAlpns[idx]);
 
             MsQuicSettings Settings;
             Settings.SetIdleTimeoutMs(3000);
 
-            MsQuicConfiguration ServerConfiguration(Registration, Alpn, Settings, ServerSelfSignedCredConfig);
+            MsQuicConfiguration ServerConfiguration(Registration, NewAlpn, Settings, ServerSelfSignedCredConfig);
             TEST_TRUE(ServerConfiguration.IsValid());
 
             MsQuicCredentialConfig ClientCredConfig;
@@ -3119,7 +3119,7 @@ QuicTestChangeAlpn(
             QUIC_ADDRESS_FAMILY QuicAddrFamily = QUIC_ADDRESS_FAMILY_INET;
 
             {
-                TestListener Listener(Registration, ListenerAcceptConnection, ServerConfiguration, &NewAlpn);
+                TestListener Listener(Registration, ListenerAcceptConnection, ServerConfiguration);
                 TEST_TRUE(Listener.IsValid());
                 QuicAddr ServerLocalAddr(QuicAddrFamily);
                 TEST_QUIC_SUCCEEDED(Listener.Start(Alpn, &ServerLocalAddr.SockAddr));
@@ -3154,33 +3154,32 @@ QuicTestChangeAlpn(
                         }
                         TEST_TRUE(Server->GetIsConnected());
 
-                        auto& AlpnBuffer = NewAlpn;
+                        auto& AlpnBuffer = NewAlpn[0];
 
                         TEST_EQUAL(Server->GetNegotiatedAlpnLength(), AlpnBuffer.Length);
                         for (uint32_t i = 0; i < AlpnBuffer.Length; i++) {
-                            TEST_EQUAL(Server->GetNegotiatedAlpn()[i], AlpnBuffer.Alpn[i]);
+                            TEST_EQUAL(Server->GetNegotiatedAlpn()[i], AlpnBuffer.Buffer[i]);
                         }
 
                         TEST_EQUAL(Client.GetNegotiatedAlpnLength(), AlpnBuffer.Length);
                         for (uint32_t i = 0; i < AlpnBuffer.Length; i++) {
-                            TEST_EQUAL(Client.GetNegotiatedAlpn()[i], AlpnBuffer.Alpn[i]);
+                            TEST_EQUAL(Client.GetNegotiatedAlpn()[i], AlpnBuffer.Buffer[i]);
                         }
                     }
                 }
             }
         }
     }
-
     // Failure cases
     {
         MsQuicAlpn Alpn("quic1", "MsQuicTest");
 
-        AlpnHelper NewAlpn("MsQuicTest", false);
+        MsQuicAlpn NewAlpn("MsQuicTest23");
 
         MsQuicSettings Settings;
         Settings.SetIdleTimeoutMs(3000);
 
-        MsQuicConfiguration ServerConfiguration(Registration, Alpn, Settings, ServerSelfSignedCredConfig);
+        MsQuicConfiguration ServerConfiguration(Registration, NewAlpn, Settings, ServerSelfSignedCredConfig);
         TEST_TRUE(ServerConfiguration.IsValid());
 
         MsQuicCredentialConfig ClientCredConfig;
@@ -3190,7 +3189,7 @@ QuicTestChangeAlpn(
         QUIC_ADDRESS_FAMILY QuicAddrFamily = QUIC_ADDRESS_FAMILY_INET;
 
         {
-            TestListener Listener(Registration, ListenerAcceptConnection, ServerConfiguration, &NewAlpn);
+            TestListener Listener(Registration, ListenerAcceptConnection, ServerConfiguration);
             TEST_TRUE(Listener.IsValid());
             QuicAddr ServerLocalAddr(QuicAddrFamily);
             TEST_QUIC_SUCCEEDED(Listener.Start(Alpn, &ServerLocalAddr.SockAddr));