Fix connection mishandling out of order initial packets with retrys (m…

…icrosoft#2200)
AliceYeh · Dec 2, 2021 · e48a14f · e48a14f
1 parent 04a6c3f
commit e48a14f
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 44 deletions.
diff --git a/src/core/binding.c b/src/core/binding.c
@@ -1211,45 +1211,6 @@ QuicBindingPreprocessDatagram(
     return TRUE;
 }
 
-//
-// Returns TRUE if the retry token was successfully decrypted and validated.
-//
-_IRQL_requires_max_(DISPATCH_LEVEL)
-BOOLEAN
-QuicBindingValidateRetryToken(
-    _In_ const QUIC_BINDING* const Binding,
-    _In_ const CXPLAT_RECV_PACKET* const Packet,
-    _In_ uint16_t TokenLength,
-    _In_reads_(TokenLength)
-        const uint8_t* TokenBuffer
-    )
-{
-    if (TokenLength != sizeof(QUIC_RETRY_TOKEN_CONTENTS)) {
-        QuicPacketLogDrop(Binding, Packet, "Invalid Retry Token Length");
-        return FALSE;
-    }
-
-    QUIC_RETRY_TOKEN_CONTENTS Token;
-    if (!QuicRetryTokenDecrypt(Packet, TokenBuffer, &Token)) {
-        QuicPacketLogDrop(Binding, Packet, "Retry Token Decryption Failure");
-        return FALSE;
-    }
-
-    if (Token.Encrypted.OrigConnIdLength > sizeof(Token.Encrypted.OrigConnId)) {
-        QuicPacketLogDrop(Binding, Packet, "Invalid Retry Token OrigConnId Length");
-        return FALSE;
-    }
-
-    const CXPLAT_RECV_DATA* Datagram =
-        CxPlatDataPathRecvPacketToRecvData(Packet);
-    if (!QuicAddrCompare(&Token.Encrypted.RemoteAddress, &Datagram->Route->RemoteAddress)) {
-        QuicPacketLogDrop(Binding, Packet, "Retry Token Addr Mismatch");
-        return FALSE;
-    }
-
-    return TRUE;
-}
-
 //
 // Returns TRUE if we should respond to the connection attempt with a Retry
 // packet.
@@ -1277,7 +1238,7 @@ QuicBindingShouldRetryConnection(
         //
         // Must always validate the token when provided by the client.
         //
-        if (QuicBindingValidateRetryToken(Binding, Packet, TokenLength, Token)) {
+        if (QuicPacketValidateRetryToken(Binding, Packet, TokenLength, Token)) {
             Packet->ValidToken = TRUE;
         } else {
             *DropPacket = TRUE;

diff --git a/src/core/connection.c b/src/core/connection.c
@@ -3519,10 +3519,23 @@ QuicConnRecvHeader(
         }
 
         QUIC_PATH* Path = &Connection->Paths[0];
-        if (!Path->IsPeerValidated && Packet->ValidToken) {
+        if (!Path->IsPeerValidated && (Packet->ValidToken || TokenLength != 0)) {
+
+            if (Packet->ValidToken) {
+                CXPLAT_DBG_ASSERT(TokenBuffer == NULL);
+                CXPLAT_DBG_ASSERT(TokenLength == 0);
+                QuicPacketDecodeRetryTokenV1(Packet, &TokenBuffer, &TokenLength);
+            } else {
+                CXPLAT_DBG_ASSERT(TokenBuffer != NULL);
+                if (!QuicPacketValidateRetryToken(
+                        Connection,
+                        Packet,
+                        TokenLength,
+                        TokenBuffer)) {
+                    return FALSE;
+                }
+            }
 
-            CXPLAT_DBG_ASSERT(TokenBuffer == NULL);
-            QuicPacketDecodeRetryTokenV1(Packet, &TokenBuffer, &TokenLength);
             CXPLAT_DBG_ASSERT(TokenBuffer != NULL);
             CXPLAT_DBG_ASSERT(TokenLength == sizeof(QUIC_RETRY_TOKEN_CONTENTS));
 
@@ -3581,7 +3594,6 @@ QuicConnRecvHeader(
                 Connection->OrigDestCID->Data,
                 Packet->DestCid,
                 Packet->DestCidLen);
-
         }
 
         Packet->KeyType = QuicPacketTypeToKeyType(Packet->LH->Type);

diff --git a/src/core/packet.c b/src/core/packet.c
@@ -473,6 +473,45 @@ QuicPacketDecodeRetryTokenV1(
     *TokenLength = (uint16_t)TokenLengthVarInt;
 }
 
+//
+// Returns TRUE if the retry token was successfully decrypted and validated.
+//
+_IRQL_requires_max_(DISPATCH_LEVEL)
+BOOLEAN
+QuicPacketValidateRetryToken(
+    _In_ const void* const Owner,
+    _In_ const CXPLAT_RECV_PACKET* const Packet,
+    _In_ uint16_t TokenLength,
+    _In_reads_(TokenLength)
+        const uint8_t* TokenBuffer
+    )
+{
+    if (TokenLength != sizeof(QUIC_RETRY_TOKEN_CONTENTS)) {
+        QuicPacketLogDrop(Owner, Packet, "Invalid Retry Token Length");
+        return FALSE;
+    }
+
+    QUIC_RETRY_TOKEN_CONTENTS Token;
+    if (!QuicRetryTokenDecrypt(Packet, TokenBuffer, &Token)) {
+        QuicPacketLogDrop(Owner, Packet, "Retry Token Decryption Failure");
+        return FALSE;
+    }
+
+    if (Token.Encrypted.OrigConnIdLength > sizeof(Token.Encrypted.OrigConnId)) {
+        QuicPacketLogDrop(Owner, Packet, "Invalid Retry Token OrigConnId Length");
+        return FALSE;
+    }
+
+    const CXPLAT_RECV_DATA* Datagram =
+        CxPlatDataPathRecvPacketToRecvData(Packet);
+    if (!QuicAddrCompare(&Token.Encrypted.RemoteAddress, &Datagram->Route->RemoteAddress)) {
+        QuicPacketLogDrop(Owner, Packet, "Retry Token Addr Mismatch");
+        return FALSE;
+    }
+
+    return TRUE;
+}
+
 _IRQL_requires_max_(DISPATCH_LEVEL)
 _Success_(return != FALSE)
 BOOLEAN

diff --git a/src/core/packet.h b/src/core/packet.h
@@ -283,6 +283,16 @@ QuicPacketDecodeRetryTokenV1(
     _Out_ uint16_t* TokenLength
     );
 
+_IRQL_requires_max_(DISPATCH_LEVEL)
+BOOLEAN
+QuicPacketValidateRetryToken(
+    _In_ const void* const Owner,
+    _In_ const CXPLAT_RECV_PACKET* const Packet,
+    _In_ uint16_t TokenLength,
+    _In_reads_(TokenLength)
+        const uint8_t* TokenBuffer
+    );
+
 _IRQL_requires_max_(DISPATCH_LEVEL)
 _Success_(return != FALSE)
 BOOLEAN