From ee957f3fcd1ca45d569e28ed0ac7867eef8e7cfd Mon Sep 17 00:00:00 2001 From: mischa <16416509+vis2k@users.noreply.github.com> Date: Thu, 6 Apr 2023 04:46:26 +0200 Subject: [PATCH] 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> --- .../Mirror/Transports/KCP/kcp2k/VERSION.txt | 8 ++ .../Transports/KCP/kcp2k/highlevel/Common.cs | 26 ++++ .../KCP/kcp2k/highlevel/KcpClient.cs | 3 +- .../Transports/KCP/kcp2k/highlevel/KcpPeer.cs | 113 +++++++++++++++--- .../KCP/kcp2k/highlevel/KcpServer.cs | 16 +-- README.md | 1 + 6 files changed, 138 insertions(+), 29 deletions(-) diff --git a/Assets/Mirror/Transports/KCP/kcp2k/VERSION.txt b/Assets/Mirror/Transports/KCP/kcp2k/VERSION.txt index 58399adf7..c872e4d7d 100755 --- a/Assets/Mirror/Transports/KCP/kcp2k/VERSION.txt +++ b/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. diff --git a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/Common.cs b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/Common.cs index cc310a08b..ea5140f14 100644 --- a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/Common.cs +++ b/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); + } } } diff --git a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpClient.cs b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpClient.cs index 833dfdac3..788d065cf 100644 --- a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpClient.cs +++ b/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() diff --git a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs index 89a67423d..5bcbaaf8f 100644 --- a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpPeer.cs +++ b/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> 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 - // calculation. + // we need to subtract the channel and cookie bytes from every + // 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, KcpChannel> OnData, Action OnDisconnected, Action 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 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 message = new ArraySegment(segment.Array, segment.Offset + 1, segment.Count - 1); + ArraySegment message = new ArraySegment(segment.Array, segment.Offset + 1 + 4, segment.Count - 1 - 4); switch (channel) { @@ -599,12 +647,20 @@ public void RawInput(ArraySegment 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 segment = new ArraySegment(rawSendBuffer, 0, length + 1); + ArraySegment segment = new ArraySegment(rawSendBuffer, 0, length + 1 + 4); RawSend(segment); } @@ -619,8 +675,10 @@ void SendReliable(KcpHeader header, ArraySegment 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 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 segment = new ArraySegment(rawSendBuffer, 0, message.Count + 1); + ArraySegment segment = new ArraySegment(rawSendBuffer, 0, message.Count + 1 + 4); RawSend(segment); } @@ -661,9 +729,18 @@ void SendUnreliable(ArraySegment 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!"); - SendReliable(KcpHeader.Handshake, default); + Log.Info($"KcpPeer: sending Handshake to other end with cookie={cookie}!"); + SendReliable(KcpHeader.Handshake, new ArraySegment(cookieBytes)); } public void SendData(ArraySegment data, KcpChannel channel) diff --git a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs index fba6aa9a6..b9e56c66b 100644 --- a/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs +++ b/Assets/Mirror/Transports/KCP/kcp2k/highlevel/KcpServer.cs @@ -157,15 +157,7 @@ protected virtual bool RawReceiveFrom(out ArraySegment segment, out int co if (socket.ReceiveFromNonBlocking(rawReceiveBuffer, out segment, ref newClientEP)) { // set connectionId to hash from endpoint - // 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. - connectionId = newClientEP.GetHashCode(); + connectionId = Common.ConnectionHash(newClientEP); 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; diff --git a/README.md b/README.md index 359d8b37a..1387433c2 100644 --- a/README.md +++ b/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 🙏