Security Fix - Auth Server

12/14/2012 17:25 Super Aids#1

Notice this should be pretty common in the Conquer environment and especially at the auth state, but yet I've never seen anyone do it.

It's not much of a security problem, but it could be one.

Normally the client will disconnect after authentication, because there is only one packet to handle authentication, but what if the end-user is using a proxy and keeps spamming packets (dos?). That could be a problem and you would most likely want to avoid that.

After authentication you do not want to receive anymore packets.

There is two ways to get around this.

Never call your receive method after handling the first packet received (Unless co added more packets then after last packet.) at the auth server.

Or you can simply disconnect the client, but I am not sure if last one is allowed and will cause problem for the regular client. Never tested that.

Let's rely on the first one.

I assume most people uses a receive event like this: (Or something similar.)

Code:

public static BufferEvent OnReceive;

Where BufferEvent is a delegate. However in public sources I've noticed it's usually a delegate void. You want to change to that a bool.

Code:

public delegate bool BufferEvent(SocketClient sClient, DataPacket Packet);

Now in your receive's async callback you can do something like this:

Code:

							if (SocketEvents.OnReceive.Invoke(this, receiveData))
							{
								Receive(); // BeginReceive
							}

Now in your receive handler you can do something like this:

Code:

static bool SocketEvents_OnReceive(SocketClient sClient, DataPacket Packet)
		{
			// handle packets here
			return false; // return true if more packets should be received
		}

Notice that it returns a boolean. Whatever it returns also defines whether the server should still receive packets.

Not much of a security fix, but it should still be used.

This is all pseudo codes, so you have to use commonsense to actually implement something like this.

12/14/2012 17:44 nTL3fTy#2

Quote:
Originally Posted by Super Aids
Code:
public delegate bool BufferEvent(SocketClient sClient, DataPacket Packet);

Not to be too anal about naming conventions, but people still don't understand them?

12/14/2012 18:36 Korvacs#3

You can safely disconnect the client after receiving the second packet on the auth server.

12/19/2012 18:10 ryuchetval#4

Let's say we make a collection with all IPs that did not get through the game server authentication and if there are more than (x)...preferably 2 or 3 connections from that IP because the accept connections may be accessed 2 times for a single connection so we can make sure we don't disconnect the wrong client.

Instead of turning that void into a bool (to avoid calling the method once again when checking)
we can use the global collection to check if further data is to be sent or to disconnect the client.

Let me know if this way would work too.

12/20/2012 14:32 Super Aids#5

Quote:

Originally Posted by ryuchetval

Let's say we make a collection with all IPs that did not get through the game server authentication and if there are more than (x)...preferably 2 or 3 connections from that IP because the accept connections may be accessed 2 times for a single connection so we can make sure we don't disconnect the wrong client.

Instead of turning that void into a bool (to avoid calling the method once again when checking)
we can use the global collection to check if further data is to be sent or to disconnect the client.

Let me know if this way would work too.

It will never disconnect the wrong client. The problem does not lie in the acceptance of a connection, but in receiving data. The reason for this is to prevent endless packets being send to the auth-server.

It's only at the game-server that you want to actually keep receiving packets, because it does not handle authentication only, but the whole game processing.

Also the accept callback will only be called once per connection and if it gets called ore than once then there is something wrong with the way you've coded your sockets.

At last do not rely connections on their IPAddress :) Also only make a collection of the clients at the game server state when the client is actually logged in and authenticated (Packet-type 0x41c?), because then you can rely on the clients UID and the actual character name, rather than an IP. You do want to allow multiple connections per IP in most cases.