fix: kcp2k V1.35. secure cookie to prevent UDP spoofing. fixes: #3286 (#3445)

* fix: kcp2k V1.35 [2023-04-05]
- fix: KcpClients now need to validate with a secure cookie in order to protect against
  UDP spoofing. fixes:
  https://github.com/MirrorNetworking/Mirror/issues/3286
  [disclosed by IncludeSec]
- KcpClient/Server: change callbacks to protected so inheriting classes can use them too
- KcpClient/Server: change config visibility to protected

* credits

* credits

* link blog post

* 2019 compatibility

* Update Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs

Co-authored-by: MrGadget <9826063+MrGadget1024@users.noreply.github.com>

* Update Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs

Co-authored-by: MrGadget <9826063+MrGadget1024@users.noreply.github.com>

---------

Co-authored-by: MrGadget <9826063+MrGadget1024@users.noreply.github.com>

Assets/Mirror/Transports/KCP/kcp2k/VERSION.txt

@@ -1,3 +1,11 @@
+V1.35 [2023-04-05]
+- fix: KcpClients now need to validate with a secure cookie in order to protect against
+  UDP spoofing. fixes:
+  https://github.com/MirrorNetworking/Mirror/issues/3286
+  [disclosed by IncludeSec]
+- KcpClient/Server: change callbacks to protected so inheriting classes can use them too
+- KcpClient/Server: change config visibility to protected
+
 V1.34 [2023-03-15]
 - Send/SendTo/Receive/ReceiveFrom NonBlocking extensions.
   to encapsulate WouldBlock allocations, exceptions, etc.

Assets/Mirror/Transports/KCP/kcp2k/highlevel/Common.cs

@@ -1,5 +1,7 @@
+using System;
 using System.Net;
 using System.Net.Sockets;
+using System.Security.Cryptography;
 
 namespace kcp2k
 {
@@ -45,5 +47,29 @@ public static void ConfigureSocketBuffers(Socket socket, int recvBufferSize, int
 
             Log.Info($"Kcp: RecvBuf = {initialReceive}=>{socket.ReceiveBufferSize} ({socket.ReceiveBufferSize/initialReceive}x) SendBuf = {initialSend}=>{socket.SendBufferSize} ({socket.SendBufferSize/initialSend}x)");
         }
+
+        // generate a connection hash from IP+Port.
+        //
+        // NOTE: IPEndPoint.GetHashCode() allocates.
+        //  it calls m_Address.GetHashCode().
+        //  m_Address is an IPAddress.
+        //  GetHashCode() allocates for IPv6:
+        //  https://github.com/mono/mono/blob/bdd772531d379b4e78593587d15113c37edd4a64/mcs/class/referencesource/System/net/System/Net/IPAddress.cs#L699
+        //
+        // => using only newClientEP.Port wouldn't work, because
+        //    different connections can have the same port.
+        public static int ConnectionHash(EndPoint endPoint) =>
+            endPoint.GetHashCode();
+
+        // cookies need to be generated with a secure random generator.
+        // we don't want them to be deterministic / predictable.
+        // RNG is cached to avoid runtime allocations.
+        static readonly RNGCryptoServiceProvider cryptoRandom = new RNGCryptoServiceProvider();
+        static readonly byte[] cryptoRandomBuffer = new byte[4];
+        public static uint GenerateCookie()
+        {
+            cryptoRandom.GetBytes(cryptoRandomBuffer);
+            return BitConverter.ToUInt32(cryptoRandomBuffer, 0);
+        }
     }
 }

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpClient.cs

@@ -79,7 +79,8 @@ public void Connect(string address, ushort port)
             }
 
             // create fresh peer for each new session
-            peer = new KcpPeer(RawSend, OnAuthenticatedWrap, OnData, OnDisconnectedWrap, OnError, config);
+            // client doesn't need secure cookie.
+            peer = new KcpPeer(RawSend, OnAuthenticatedWrap, OnData, OnDisconnectedWrap, OnError, config, 0);
 
             // some callbacks need to wrapped with some extra logic
             void OnAuthenticatedWrap()

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs

@@ -15,6 +15,21 @@ public class KcpPeer
         // kcp reliability algorithm
         internal Kcp kcp;
 
+        // security cookie to prevent UDP spoofing.
+        // credits to IncludeSec for disclosing the issue.
+        //
+        // server passes the expected cookie to the client's KcpPeer.
+        // KcpPeer sends cookie to the connected client.
+        // KcpPeer only accepts packets which contain the cookie.
+        // => cookie can be a random number, but it needs to be cryptographically
+        //    secure random that can't be easily predicted.
+        // => cookie can be hash(ip, port) BUT only if salted to be not predictable
+        readonly uint cookie;
+
+        // this is the cookie that the other end received during handshake.
+        // store byte[] representation to avoid runtime int->byte[] conversions.
+        internal readonly byte[] receivedCookie = new byte[4];
+
         // IO agnostic
         readonly Action<ArraySegment<byte>> RawSend;
 
@@ -44,10 +59,12 @@ public class KcpPeer
         // Unity's time.deltaTime over long periods.
         readonly Stopwatch watch = new Stopwatch();
 
-        // we need to subtract the channel byte from every MaxMessageSize
+        // we need to subtract the channel and cookie bytes from every
-        // calculation.
+        // MaxMessageSize calculation.
         // we also need to tell kcp to use MTU-1 to leave space for the byte.
         const int CHANNEL_HEADER_SIZE = 1;
+        const int COOKIE_HEADER_SIZE = 4;
+        const int METADATA_SIZE = CHANNEL_HEADER_SIZE + COOKIE_HEADER_SIZE;
 
         // reliable channel (= kcp) MaxMessageSize so the outside knows largest
         // allowed message to send. the calculation in Send() is not obvious at
@@ -72,7 +89,7 @@ public class KcpPeer
         //            => sending UNRELIABLE max message size most of the time is
         //               best for performance (use that one for batching!)
         static int ReliableMaxMessageSize_Unconstrained(int mtu, uint rcv_wnd) =>
-            (mtu - Kcp.OVERHEAD - CHANNEL_HEADER_SIZE) * ((int)rcv_wnd - 1) - 1;
+            (mtu - Kcp.OVERHEAD - METADATA_SIZE) * ((int)rcv_wnd - 1) - 1;
 
         // kcp encodes 'frg' as 1 byte.
         // max message size can only ever allow up to 255 fragments.
@@ -84,7 +101,7 @@ public static int ReliableMaxMessageSize(int mtu, uint rcv_wnd) =>
 
         // unreliable max message size is simply MTU - channel header size
         public static int UnreliableMaxMessageSize(int mtu) =>
-            mtu - CHANNEL_HEADER_SIZE;
+            mtu - METADATA_SIZE;
 
         // buffer to receive kcp's processed messages (avoids allocations).
         // IMPORTANT: this is for KCP messages. so it needs to be of size:
@@ -153,7 +170,8 @@ public KcpPeer(
             Action<ArraySegment<byte>, KcpChannel> OnData,
             Action OnDisconnected,
             Action<ErrorCode, string> OnError,
-            KcpConfig config)
+            KcpConfig config,
+            uint cookie)
         {
             // initialize callbacks first to ensure they can be used safely.
             this.OnAuthenticated = OnAuthenticated;
@@ -165,6 +183,9 @@ public KcpPeer(
             // set up kcp over reliable channel (that's what kcp is for)
             kcp = new Kcp(0, RawSendReliable);
 
+            // security cookie
+            this.cookie = cookie;
+
             // set nodelay.
             // note that kcp uses 'nocwnd' internally so we negate the parameter
             kcp.SetNoDelay(config.NoDelay ? 1u : 0u, config.Interval, config.FastResend, !config.CongestionWindow);
@@ -174,7 +195,7 @@ public KcpPeer(
             // message. so while Kcp.MTU_DEF is perfect, we actually need to
             // tell kcp to use MTU-1 so we can still put the header into the
             // message afterwards.
-            kcp.SetMtu((uint)config.Mtu - CHANNEL_HEADER_SIZE);
+            kcp.SetMtu((uint)config.Mtu - METADATA_SIZE);
 
             // create mtu sized send buffer
             rawSendBuffer = new byte[config.Mtu];
@@ -320,8 +341,22 @@ void TickIncoming_Connected(uint time)
                     {
                         // we were waiting for a handshake.
                         // it proves that the other end speaks our protocol.
-                        // GetType() shows Server/ClientConn instead of just Connection.
+
-                        Log.Info($"KcpPeer: received handshake");
+                        // parse the cookie
+                        if (message.Count != 4)
+                        {
+                            // pass error to user callback. no need to log it manually.
+                            OnError(ErrorCode.InvalidReceive, $"KcpPeer: received invalid handshake message with size {message.Count} != 4. Disconnecting the connection.");
+                            Disconnect();
+                            return;
+                        }
+
+                        // store the cookie bytes to avoid int->byte[] conversions when sending.
+                        // still convert to uint once, just for prettier logging.
+                        Buffer.BlockCopy(message.Array, message.Offset, receivedCookie, 0, 4);
+                        uint prettyCookie = BitConverter.ToUInt32(message.Array, message.Offset);
+
+                        Log.Info($"KcpPeer: received handshake with cookie={prettyCookie}");
                         state = KcpState.Authenticated;
                         OnAuthenticated?.Invoke();
                         break;
@@ -570,8 +605,21 @@ public void RawInput(ArraySegment<byte> segment)
             // byte channel = segment[0]; ArraySegment[i] isn't supported in some older Unity Mono versions
             byte channel = segment.Array[segment.Offset + 0];
 
+            // parse cookie
+            uint messageCookie = BitConverter.ToUInt32(segment.Array, segment.Offset + 1);
+
+            // compare cookie to protect against UDP spoofing.
+            // messages won't have a cookie until after handshake.
+            // so only compare if we are authenticated.
+            // simply drop the message if the cookie doesn't match.
+            if (state == KcpState.Authenticated && messageCookie != cookie)
+            {
+                Log.Warning($"KcpPeer: dropped message with invalid cookie: {messageCookie} expected: {cookie}.");
+                return;
+            }
+
             // parse message
-            ArraySegment<byte> message = new ArraySegment<byte>(segment.Array, segment.Offset + 1, segment.Count - 1);
+            ArraySegment<byte> message = new ArraySegment<byte>(segment.Array, segment.Offset + 1 + 4, segment.Count - 1 - 4);
 
             switch (channel)
             {
@@ -599,12 +647,20 @@ public void RawInput(ArraySegment<byte> segment)
         // raw send called by kcp
         void RawSendReliable(byte[] data, int length)
         {
-            // copy channel header, data into raw send buffer, then send
+            // write channel header
+            // from 0, with 1 byte
             rawSendBuffer[0] = (byte)KcpChannel.Reliable;
-            Buffer.BlockCopy(data, 0, rawSendBuffer, 1, length);
+
+            // write handshake cookie to protect against UDP spoofing.
+            // from 1, with 4 bytes
+            Buffer.BlockCopy(receivedCookie, 0, rawSendBuffer, 1, 4);
+
+            // write data
+            // from 5, with N bytes
+            Buffer.BlockCopy(data, 0, rawSendBuffer, 1+4, length);
 
             // IO send
-            ArraySegment<byte> segment = new ArraySegment<byte>(rawSendBuffer, 0, length + 1);
+            ArraySegment<byte> segment = new ArraySegment<byte>(rawSendBuffer, 0, length + 1 + 4);
             RawSend(segment);
         }
 
@@ -619,8 +675,10 @@ void SendReliable(KcpHeader header, ArraySegment<byte> content)
                 return;
             }
 
-            // copy header, content (if any) into send buffer
+            // write channel header
             kcpSendBuffer[0] = (byte)header;
+
+            // write data (if any)
             if (content.Count > 0)
                 Buffer.BlockCopy(content.Array, content.Offset, kcpSendBuffer, 1, content.Count);
 
@@ -644,12 +702,22 @@ void SendUnreliable(ArraySegment<byte> message)
                 return;
             }
 
-            // copy channel header, data into raw send buffer, then send
+            // write channel header
+            // from 0, with 1 byte
             rawSendBuffer[0] = (byte)KcpChannel.Unreliable;
-            Buffer.BlockCopy(message.Array, message.Offset, rawSendBuffer, 1, message.Count);
+
+            // write handshake cookie to protect against UDP spoofing.
+            // from 1, with 4 bytes
+            Buffer.BlockCopy(receivedCookie, 0, rawSendBuffer, 1, 4);
+
+            // write data
+            // from 5, with N bytes
+            Buffer.BlockCopy(message.Array, message.Offset, rawSendBuffer, 1 + 4, message.Count);
+
+            Log.Warning($"KcpPeer: SendUnreliable with receivedCookie={BitConverter.ToUInt32(receivedCookie, 0)}");
 
             // IO send
-            ArraySegment<byte> segment = new ArraySegment<byte>(rawSendBuffer, 0, message.Count + 1);
+            ArraySegment<byte> segment = new ArraySegment<byte>(rawSendBuffer, 0, message.Count + 1 + 4);
             RawSend(segment);
         }
 
@@ -661,9 +729,18 @@ void SendUnreliable(ArraySegment<byte> message)
         // => handshake info needs to be delivered, so it goes over reliable.
         public void SendHandshake()
         {
+            // server includes a random cookie in handshake.
+            // client is expected to include in every message.
+            // this avoid UDP spoofing.
+            // KcpPeer simply always sends a cookie.
+            // in case client -> server cookies are ever implemented, etc.
+
+            // TODO nonalloc
+            byte[] cookieBytes = BitConverter.GetBytes(cookie);
+
             // GetType() shows Server/ClientConn instead of just Connection.
-            Log.Info($"KcpPeer: sending Handshake to other end!");
+            Log.Info($"KcpPeer: sending Handshake to other end with cookie={cookie}!");
-            SendReliable(KcpHeader.Handshake, default);
+            SendReliable(KcpHeader.Handshake, new ArraySegment<byte>(cookieBytes));
         }
 
         public void SendData(ArraySegment<byte> data, KcpChannel channel)

Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs

@@ -157,15 +157,7 @@ protected virtual bool RawReceiveFrom(out ArraySegment<byte> segment, out int co
                 if (socket.ReceiveFromNonBlocking(rawReceiveBuffer, out segment, ref newClientEP))
                 {
                     // set connectionId to hash from endpoint
-                    // NOTE: IPEndPoint.GetHashCode() allocates.
+                    connectionId = Common.ConnectionHash(newClientEP);
-                    //  it calls m_Address.GetHashCode().
-                    //  m_Address is an IPAddress.
-                    //  GetHashCode() allocates for IPv6:
-                    //  https://github.com/mono/mono/blob/bdd772531d379b4e78593587d15113c37edd4a64/mcs/class/referencesource/System/net/System/Net/IPAddress.cs#L699
-                    //
-                    // => using only newClientEP.Port wouldn't work, because
-                    //    different connections can have the same port.
-                    connectionId = newClientEP.GetHashCode();
                     return true;
                 }
             }
@@ -214,8 +206,12 @@ protected virtual KcpServerConnection CreateConnection(int connectionId)
             // afterwards we assign the peer.
             KcpServerConnection connection = new KcpServerConnection(newClientEP);
 
+            // generate a random cookie for this connection to avoid UDP spoofing.
+            // needs to be random, but without allocations to avoid GC.
+            uint cookie = Common.GenerateCookie();
+
             // set up peer with callbacks
-            KcpPeer peer = new KcpPeer(RawSendWrap, OnAuthenticatedWrap, OnDataWrap, OnDisconnectedWrap, OnErrorWrap, config);
+            KcpPeer peer = new KcpPeer(RawSendWrap, OnAuthenticatedWrap, OnDataWrap, OnDisconnectedWrap, OnErrorWrap, config, cookie);
 
             // assign peer to connection
             connection.peer = peer;

README.md

@@ -243,6 +243,7 @@ A lot of projects use Mirror in production. If you found a critical bug / exploi
 
 **Credits / past findings / fixes:**
 * 2020, fholm: fuzzing ConnectMessage to stop further connects [[#2397](https://github.com/vis2k/Mirror/pull/2397)]
+* 2023-04-05: IncludeSec: [kcp2k UDP spoofing](http://blog.includesecurity.com/?p=1407) [[#3286](https://github.com/vis2k/Mirror/pull/2397)]
 
 ---
 # Credits & Thanks 🙏