LLAPI - Big issue? Is / Can we have the IncomingMessageQueue be kept separate per connection?

I think currently the IncomingMessageQueue contains messages from all connections, which means a single connection, whether through malicious intent or other reasons, could send a bunch of small messages to fill up the currently default max queue size of 1024 which would cause all other connections messages to be dropped.
Keep in mind, at least as far as I know, that messages != packets. That means someone could create a single packet of 1000 bytes that contains 300 messages in it, no need for lots of bandwidth usage.
Combine that with a local variable such as "maxEventsPerFrame", of which is used to limit how many calls to receive is made within a frame, and a single user could bring down a game room with not much bandwidth needed, depending on framerate, expected messages, and the values set for maxEventsPerFrame. For example, with a framerate of 30 and maxEventsPerFrame of 500 (which I think is what unitys HLAPI has it set to), 60 packets a second of 1000 bytes per packet for 300 messages = 600 messages a frame = no more game. Of course a ddos could bring down a game, but this is just 60 kilobytes a second... voice chat alone could be 30 kilobytes a second per person.

Ideally I would want there to be a separate message queue / packet queue per connection, with the ability to call something like NetworkTransport.ReceiveFromHostFromConnection(int hostID, int connectionID, .....)

Or I could be wrong and theres nothing to worry about =).

 

By using a program called RawCap for windows to capture packets sent to localhost and using wireshark to read it, I see that unets unreliable packets have a header size of 12 and then the rest of the bytes seem to be an array of "channel + length + message". The length is only 1 byte when the message length is small such as a length of 1.

So I did a test by sending a "malicious" packet of messages with a length of 0, which means each message takes 2 bytes (1 byte for channel and 1 byte for length of 0). The 0 length messages do take up space in the IncomingMessageQueue, as well as calling NetworkTransport.Receive will give you the 0 length message.
This means a packet with byte data size of 1000 bytes can send about 500 messages within that single packet, which is even less bandwidth required then in my explanation above. However, even if unity discards 0 length messages, my above post still holds with when messages could be 1 byte long.

This wouldnt be an issue if the IncomingQueueSize was separate per connection and we had a NetworkTransport.ReceiveFromHostFromConnection method since then we can keep all connections separate and if one connection does malicious things then it will mainly only effect their own connection.
(From my tests IncomingQueueSize is shared with each connection, but if my tests were wrong then I apologies =] )

(You cant send 0 length messages to the NetworkTransport via code, so I had to do this in a weird way using c# sockets)

 

Welp, there goes LLAPI stability.

 

It should hopefully be an easy fix though, without breaking any current code.

 

This is pretty much something that has to be fixed. Hopefully Unity will put this on their Top Priorities

 

That might be a valid reason to not have a separate queue for a thousand users, however, I cant help but assume a high majority of users arent making a game with more than 64 players and those that are making a mmo are more than likely going to use some other network library anyways. Though in trying to keep unet flexible for all types of games, I can see why youd choose to have a single queue.

I dont know much about this so ill take your word for it =)

The issue is that we are reading all the messages from connection 3, but connection 3 decided to be malicious and send 500 empty messages in a single packet to clog up the message queue so that now the only messages we are reading are their fake messages while legit messages from other connections are being dropped.

I guess the only way to avoid this would be to check to see if within a single frame we have read more than X amount of messages from a single user and if so we disconnect them? I worry that might be a bit unreliable since perhaps there were legit reasons that user happened to go over their max events for that frame. I guess I would need to have a strike system where 3 strikes and your out?
Theyll just be able to keep reconnecting though, as currently there is no way to block users from connecting (that I know of), of which I mention in this thread https://forum.unity3d.com/threads/llapi-how-to-refuse-unwanted-connections-before-connecting.474654/

 

I was more so just thinking about hacking it into my own code. I am not sure of how to properly handle this to be put into the api.
Perhaps a better way to handle this would be to be able to set a maxQueuedMessagesPerConnection in GlobalConfig or ConnectionConfig that unets backend will be able to simply keep track of how many messages per connection is currently in the incoming queue, and if it detects a single connection already reached the maxQueuedMessagesPerConnection then it just drops the message so that they cant fill the queue? A maxQueuedMessagesPerConnection of 0 means there is no limit?
If this sounds good, I could write up a feature request with that.
Maybe also be able to set the max on a per connection basis and to be able to dynamically change it when expecting more messages from a connection?
Though the maxQueuedMessagesPerConnection might not save you if a lot of people decide to be malicious at once, but I guess that depends on your maxQueuedMessagesPerConnection value and your IncomingMessageQueue buffer size. Its probably fine actually so long as you set those values properly. For example, having a maxQueuedMessagesPerConnection value of 50 and a IncomingMessageQueue buffer size of 200 is no different from having a separate queue for each connection with a size of 50 for a max of 4 connections.
edit - Though it still wont be as good as handling each connection separately since then we can decide how many events per connection we want to run, whereas maxQueuedMessagesPerConnection can still be vulnerable if multiple people spam messages.

So with the connect request buffer we can simply ignore a connection request and unity will just not do anything with that connection? For example, if I decide to refuse/ignore a connection request, I want the other side to not get any messages at all so they dont know anything.

 

Yes, we need a callback or something in the LLAPI that we received a connection request. We could then check the IP address and whatever data the connection request sent to us and decide if we want to accept the connection or just silently ignore the connection request. If we accept, then everything will connect as it does now, but if we silently ignore/refuse, no message will be sent. Maybe give the option to send a refuse message, but you should be able to also just silently ignore the request.

Also note - The maxQueuedMessagesPerConnection option would still be vulnerable if multiple people are sending many fake messages since they would fill up the maxEventsPerFrame, however, so long as we have the maxQueuedMessagesPerConnection, we would be able to reliably work around the issue by creating our own queue of the messages after receiving it from NetworkTransport.Receive. We couldnt just reliably call NetworkTransport.Receive and queue all messages to avoid the issue without a maxQueuedMessagesPerConnection since messages might have been dropped already since the IncomingMessageQueue would be full before we would of had access to them through NetworkTransport.Receive.
Perhaps a way to check the next connectionID in the NetworkTransport.Receive and the ability to skip it this frame and next frame it will be first in line, but that might be too difficult to do with all the synchronizing of the queues and what not, and the fact that its probably a queue that you cant just insert into the front.
If anyone can think of a better way to handle this, please do tell =)
However, a maxQueuedMessagesPerConnection is a great start.

Edit - Also, looking at GlobalConfig.ReactorMaximumSentMessages and ConnectionConfig.MaxSentMessageQueueSize, it would make more sense to have the maxQueuedMessagesPerConnection be in ConnectionConfig and be called MaxReceiveMessageQueueSize

 

Just thought I would add my two cents:
If you are making a MMO. Surely, it might become a problem. But isn't it just to lower the queue per connection?
And the easiest approach, would be to have a option to simply have different queues per connection. Probably should default to true.

But I guess for the time being.

That will have to be the solution. It's a bit dirty. But it will work. But if we get backlogged by some CPU thortling or something. Well, everyone will probably get disconnected.

And also pretty much agree with this:

This is what lidgren does. If they don't have the right ticket.Or they have been knocked out for sending to much traffic. They will simply be ignored.
This way, if someone clogs our queue. We can add them to a blacklist.
But the best way is probably to have different queues for everyone.
Or even better, both

 

With the maxReceivedQueuedMessages you should be able to reliably create your own queues per connection yourself if needed. You can just call Receive, sort the messages into their proper queue per connection, and then do a round robin dequeue per queue or something.
It would be nice for it to be just built in since doing it ourselves would be less performant with all the copying of bytes, but at least this gives us the ability to do it.
Perhaps to save on byte copying you can first check if anything is in that connections queue, if not then read the message right there and then and keep reading messages for that connection for how ever max you want, but once you reach the max that frame you would then queue the messages.

 

Well, say this:
Our server can only process 150 messages per frame at 60fps forexample. If the Unet queue is 1024.
If somebody malicious sends just enough messages so the processing is slower than their send rate. They have blocked everyones connection. Cause we still have to add them to our queue in that frame update.

If instead, they NEVER shared queue. This could not happend. They would fill their queue, and their packets would be dropped. Sucks for them.

 

In the case that I gave, you would need to call NetworkTransport.Receive until the queue is empty. You dont have to proccess all the messages that frame since you could store them in your own queue, but you would need to at least grab them all. This is actually how c# sockets work (and probably any udp library). They have a thread or something that is made specifically just for grabbing messages from the udp packets queue. No proccessing of the messages should be done in that thread since if it slows down to much you might miss packets.
Now, unity is probably doing just that and storing everything in a queue as fast as it can and then we can grab it from the queue whenever we want. Now if we wanted a queue per connection we would need to basically emulate all that and call Receive as fast as we can and just store all the messages to either be proccessed now or later.
Its probably less performant to do ourself, yea, but at least it should be possible and not that bad for just 16 players or something.

 

But all the processing would still have to be done in main thead, if we want to use Unity stuff. Which we probably want. Or we have to do the math calculations and such on a separate thread, then passing the results back to main thread to apply it. We are probably just shooting ourselves in the foot due to the wait to sync between threads.

 

Yes, you would do it all in the main thread. That should be fine so long as your IncomingMessageQueue is large enough. I think the reason when using c# sockets, or working with the udp packets on a low level like unity is doing behind the scenes, you need to grab and store the udp packets right away since they dont get buffered. If you dont read and store it, it will just get lost.
However, since unity is storing it already in a queue for us, working with just the main thread should be fine, as well as doing processes on the messages as we are receiving them.
The important thing is to not count calls to Receive as Events that we want to have a limit on. Calls to Receive should be done a lot more than actually processing the messages since otherwise the buffer may eventually get full. This is mainly if you want to queue the messages yourself per connection.
All of this depends on how many messages you are sending, how costly processing those messages are, how many fps your expecting the game to run at, etc...
For example, with a 16 player game it might be fine to just give each connection a maxReceivedQueuedMessages of 50 and a maxEventPerFrame of 400, with the average total expected messages a frame to be something like 40. This means you can handle a couple of people doing malicious things before they start to actually effect things ((400 - 40) / 50 = can handle about 7 players doing malicious things). No need to have your own queue per connection in that case if thats enough. However, in all honesty thats probably not enough if you want to be sure safe, in which case you can either raise the maxEventPerFrame as well as lower the maxReceivedQueuedMessages per connection, or just implement your own queue per connection, which would probably be less performant as well as take up more memory, but at least its possible and should be fine for 64 players or less.

 

Its not meant to prevent ddos, its just meant to prevent people from joining the game when they are not meant to.
For example..
-Someone joins the game and send a connect request to everyone.
-Everyone accepts the request.
-Later in the game everyone decides to kick this player due to cheating.
-Player send connection request and everyone blindly accepts...
-Client then realizes player is meant to have the player kicked and automatically re-kicks the player.
-Player keeps sending connect requests and rejoining, potentially blocking other players from joining.

I think you misunderstood. What I was saying was that there is a ReactorMaximumSentMessages in GlobalConfig and a MaxSentMessageQueueSize in ConnectionConfig, and those 2 work together to put limits on the total max send messages in the single send queue all connections share, as well as the max send messages a single connection can queue in that shared single queue for all connections.
We basically want the same thing for the Receiving queue. When we look at the Receiving queue, we see there is a ReactorMaximumReceivedMessages in GlobalConfig to limit the total max received messages that can be queued which is shared by all connections. However, there is no "MaxReceivedMessageQueueSize" in ConnectionConfig like there is for MaxSentMessageQueueSize. So I was saying we should have a MaxReceivedMessageQueueSize in ConnectionConfig that limits how many messages a single connection can queue in the received queue just like how we can limit how many send messages a single connection can place in the send queue.

So what you mean is that all received messages will still go into the single receive queue, but if we set some kind of configuration then unity will later take all the messages out of that queue and sort it for all connections before we call Receive?
Or do you mean we would bypass the single queue all together and messages will directly go to their separate queues per connection?
If we will bypass the single queue and messages will go directly to their separate queue per connection, then that should be all good since now a single connection cant affect another connection.
However, if we are still going to be using the single queue, then it can be possible that a user sent so many messages maliciously that they filled the queue so other messages from other connections were dropped before the queue was sorted to their respected connections queues. In this case, we would still need the MaxReceivedMessageQueueSize described above, but having the receive message sorting per connection be done on the low level would still be good to have for performance reasons (as well as less code for us to write ^_~).

I will gladly write up a feature request if you think my thoughts above are accurate =).

 

Kicking / Banning / Filtering can all be done on the user level, however, there would need to be the ability to not allow a connection before a connection is made.
The 2 main reasons I want this ability is
1 - If I Kick / Ban a player, I do not want to allow them to reconnect until I decide otherwise.
2 - When working with Steam and Steamworks, I only want to allow players to join if they are actually in the same steam lobby as me.
Both of these would be handled through user code, but we would need at least the ability to check connection requests and decide if we want to accept them.
The steamworks api has something like this. A user sends a request, the other side gets a request callback and can then call a "Accept" method to accept the request if desired, or just ignore it.

That would be great and is exactly what we are looking for =), however, what do you mean by "user MUST to pope events fro connections by himself". Arent we already poping events from connections ourselves? We make NetworkTransport.ReceiveFromHost calls which gives a Networking.NetworkEventType that we check and take action based on. How would it be done with the new configuration?

 

The problem with that is that by the time we got the Connect event, the connection was already successfully made (at least from my test on localhost). Is there a way we can have it so we can decide this before the connection was made? In other words, I dont want to be sending packets to people I am going to be refusing a connection to later on anyways.
-If there are 9 people in the room with a max of 10 players
-Someone joins and become the 10th player, name is "joe"
-We kick that player "joe" due to cheating
-Player "joe" re-sends connection request
-LLAPI blindly accepts connect request and sends out a connect accept to "joe"
-NetworkTransport.Receive connect event has not been called yet
-A new good player named "sam" tries to join game, but game is seen as full due to connection made with "joe"
-Main thread runs and NetworkTransport.Receive connect event is called
-We detect "joe" should be kicked and call Disconnect
-Player "joe" re-sends connection request.....
etc...

Maybe my thought process is wrong?

Does the connection need to be dropped right away? Cant we just drop new received packets if the queue is full like how it is currently with NetworkTransport.Receive, and have the connection be dropped after a certain time? At least I think thats how things currently are.

Is this how the code will be like?

Code (CSharp):

1.  
2. List<int> connectionIDs;
3. int maxEventsPerFramePerConnection = 50;
4.  
5. void Update()
6. {
7.     int outHostId;
8.     int outConnectionId;
9.     int outChannelId;
10.  
11.     int receivedSize;
12.     byte error;
13.  
14.     do
15.     {
16.         NetworkEventType eventType = NetworkTransport.ReceiveFromHost(hostId, out outConnectionId, out outChannelId, buffer, buffer.Length, out receivedSize, out error);
17.  
18.         switch (eventType)
19.         {
20.             case NetworkEventType.ConnectEvent:
21.             {
22.                 OnConnect(outConnectionId, (NetworkError)error);
23.                 break;
24.             }
25.             case NetworkEventType.DisconnectEvent:
26.             {
27.                 OnDisconnect(outConnectionId, (NetworkError)error);
28.                 break;
29.             }
30.         }
31.     }while(eventType != NetworkEventType.Nothing);
32.  
33.     foreach(int connectionID in connectionIDs)
34.     {
35.         int eventCount = 0;
36.  
37. //Example 1
38.         //ReceiveDataFromHostFromConnection returns true if there is data, false if there isnt
39.         while(eventCount++ < maxEventsPerFramePerConnection &&
40.               NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error))
41.         {
42.             OnData(connectionID, buffer, receivedSize);
43.         }
44.  
45. //Example 2
46.         ////Or it can return a NetworkEventType, doesnt really matter.
47.         //do
48.         //{
49.         //  NetworkEventType eventType = NetworkTransport.ReceiveDataFromHostFromConnection(hostID, connectionID, out outChannelId, buffer, buffer.Length, out receivedSize, out error);
50.  
51.         //  if(eventType == NetworkEventType.Data)
52.         //  {
53.         //      OnData(connectionID, buffer, receivedSize);
54.         //  }
55.         //}while(eventCount++ < maxEventsPerFramePerConnection &&
56.         //       eventType != NetworkEventType.Nothing);
57.     }
58. }
59.  

If so then that should be fine =)

 

@HiddenMonk I suggest a design for aabra on this thread:
https://forum.unity3d.com/threads/i...ntication-system-in-unet.343217/#post-3166995
Would be nice if you could add to that / agree and/or disagree with it. Would partially solve some of the problems in this post (Even tho a per connection queue would be super sweet, atleast to optionally turn on)

 

I have written up the feature requests on the feedback site.

Feedback for having a MaxReceivedMessageQueueSize.
https://feedback.unity3d.com/sugges...vedmessagequeuesize-in-connectionconfig-class

Feedback for having a serparate incoming message queue per connection.
https://feedback.unity3d.com/sugges...gmessagequeue-be-kept-separate-per-connection

Feedback for authenticating connection requests
https://feedback.unity3d.com/suggestions/llapi-ability-to-authenticate-connection-requests