Skip to content

Commit

Permalink
feat: Secure messages that require authentication, fixes #720 (#1089)
Browse files Browse the repository at this point in the history 
* feat: Secure messages that require authentication, fixes #720

* Rename the parameter for clarity

* Update Assets/Mirror/Runtime/MessagePacker.cs

Co-Authored-By: vis2k <info@noobtuts.com>

* Update Assets/Mirror/Runtime/MessagePacker.cs

Co-Authored-By: MrGadget <chris@clevertech.net>
  • Loading branch information
@paulpach @miwarnec
2 people authored and miwarnec committed Sep 17, 2019
1 parent 8308502 commit 7ac43cd
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 16 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Authenticators/BasicAuthenticator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ public class AuthResponseMessage : MessageBase
public override void OnStartServer()
{
// register a handler for the authentication request we expect from client
NetworkServer.RegisterHandler<AuthRequestMessage>(OnAuthRequestMessage);
NetworkServer.RegisterHandler<AuthRequestMessage>(OnAuthRequestMessage, false);
}

public override void OnStartClient()
{
// register a handler for the authentication response we expect from server
NetworkClient.RegisterHandler<AuthResponseMessage>(OnAuthResponseMessage);
NetworkClient.RegisterHandler<AuthResponseMessage>(OnAuthResponseMessage, false);
}

public override void OnServerAuthenticate(NetworkConnection conn)
Expand Down
10 changes: 9 additions & 1 deletion Assets/Mirror/Runtime/MessagePacker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static bool UnpackMessage(NetworkReader messageReader, out int msgType)
}
}

internal static NetworkMessageDelegate MessageHandler<T>(Action<NetworkConnection, T> handler) where T : IMessageBase, new() => networkMessage =>
internal static NetworkMessageDelegate MessageHandler<T>(Action<NetworkConnection, T> handler, bool requireAuthenication) where T : IMessageBase, new() => networkMessage =>
{
// protect against DOS attacks if attackers try to send invalid
// data packets to crash the server/client. there are a thousand
Expand All @@ -122,6 +122,14 @@ public static bool UnpackMessage(NetworkReader messageReader, out int msgType)
T message = default;
try
{
if (requireAuthenication && !networkMessage.conn.isAuthenticated)
{
// message requires authentication, but the connection was no authnticated
Debug.LogWarning($"Closing connection: {networkMessage.conn.connectionId}. Received message {typeof(T)} that required authentication, but the user has not authenticated yet");
networkMessage.conn.Disconnect();
return;
}
message = networkMessage.ReadMessage<T>();
}
catch (Exception exception)
Expand Down
9 changes: 5 additions & 4 deletions Assets/Mirror/Runtime/NetworkClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ internal static void RegisterSystemHandlers(bool localClient)
{
RegisterHandler<ObjectDestroyMessage>(ClientScene.OnLocalClientObjectDestroy);
RegisterHandler<ObjectHideMessage>(ClientScene.OnLocalClientObjectHide);
RegisterHandler<NetworkPongMessage>((conn, msg) => { });
RegisterHandler<NetworkPongMessage>((conn, msg) => { }, false);
RegisterHandler<SpawnPrefabMessage>(ClientScene.OnLocalClientSpawnPrefab);
RegisterHandler<SpawnSceneObjectMessage>(ClientScene.OnLocalClientSpawnSceneObject);
RegisterHandler<ObjectSpawnStartedMessage>((conn, msg) => { }); // host mode doesn't need spawning
Expand All @@ -362,7 +362,7 @@ internal static void RegisterSystemHandlers(bool localClient)
{
RegisterHandler<ObjectDestroyMessage>(ClientScene.OnObjectDestroy);
RegisterHandler<ObjectHideMessage>(ClientScene.OnObjectHide);
RegisterHandler<NetworkPongMessage>(NetworkTime.OnClientPong);
RegisterHandler<NetworkPongMessage>(NetworkTime.OnClientPong, false);
RegisterHandler<SpawnPrefabMessage>(ClientScene.OnSpawnPrefab);
RegisterHandler<SpawnSceneObjectMessage>(ClientScene.OnSpawnSceneObject);
RegisterHandler<ObjectSpawnStartedMessage>(ClientScene.OnObjectSpawnStarted);
Expand Down Expand Up @@ -402,14 +402,15 @@ public static void RegisterHandler(MsgType msgType, NetworkMessageDelegate handl
/// </summary>
/// <typeparam name="T">The message type to unregister.</typeparam>
/// <param name="handler"></param>
public static void RegisterHandler<T>(Action<NetworkConnection, T> handler) where T : IMessageBase, new()
/// <param name="requireAuthentication">true if the message requires an authenticated connection</param>
public static void RegisterHandler<T>(Action<NetworkConnection, T> handler, bool requireAuthentication = true) where T : IMessageBase, new()
{
int msgType = MessagePacker.GetId<T>();
if (handlers.ContainsKey(msgType))
{
if (LogFilter.Debug) Debug.Log("NetworkClient.RegisterHandler replacing " + handler + " - " + msgType);
}
handlers[msgType] = MessagePacker.MessageHandler<T>(handler);
handlers[msgType] = MessagePacker.MessageHandler<T>(handler, requireAuthentication);
}

/// <summary>
Expand Down
12 changes: 6 additions & 6 deletions Assets/Mirror/Runtime/NetworkManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,12 @@ public virtual void OnValidate()

void RegisterServerMessages()
{
NetworkServer.RegisterHandler<ConnectMessage>(OnServerConnectInternal);
NetworkServer.RegisterHandler<DisconnectMessage>(OnServerDisconnectInternal);
NetworkServer.RegisterHandler<ConnectMessage>(OnServerConnectInternal, false);
NetworkServer.RegisterHandler<DisconnectMessage>(OnServerDisconnectInternal, false);
NetworkServer.RegisterHandler<ReadyMessage>(OnServerReadyMessageInternal);
NetworkServer.RegisterHandler<AddPlayerMessage>(OnServerAddPlayerInternal);
NetworkServer.RegisterHandler<RemovePlayerMessage>(OnServerRemovePlayerMessageInternal);
NetworkServer.RegisterHandler<ErrorMessage>(OnServerErrorInternal);
NetworkServer.RegisterHandler<ErrorMessage>(OnServerErrorInternal, false);
}

/// <summary>
Expand Down Expand Up @@ -443,10 +443,10 @@ public bool StartServer()

void RegisterClientMessages()
{
NetworkClient.RegisterHandler<ConnectMessage>(OnClientConnectInternal);
NetworkClient.RegisterHandler<DisconnectMessage>(OnClientDisconnectInternal);
NetworkClient.RegisterHandler<ConnectMessage>(OnClientConnectInternal, false);
NetworkClient.RegisterHandler<DisconnectMessage>(OnClientDisconnectInternal, false);
NetworkClient.RegisterHandler<NotReadyMessage>(OnClientNotReadyMessageInternal);
NetworkClient.RegisterHandler<ErrorMessage>(OnClientErrorInternal);
NetworkClient.RegisterHandler<ErrorMessage>(OnClientErrorInternal, false);
NetworkClient.RegisterHandler<SceneMessage>(OnClientSceneInternal);

if (playerPrefab != null)
Expand Down
7 changes: 4 additions & 3 deletions Assets/Mirror/Runtime/NetworkServer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ internal static void RegisterMessageHandlers()
RegisterHandler<ReadyMessage>(OnClientReadyMessage);
RegisterHandler<CommandMessage>(OnCommandMessage);
RegisterHandler<RemovePlayerMessage>(OnRemovePlayerMessage);
RegisterHandler<NetworkPingMessage>(NetworkTime.OnServerPing);
RegisterHandler<NetworkPingMessage>(NetworkTime.OnServerPing, false);
}

/// <summary>
Expand Down Expand Up @@ -550,14 +550,15 @@ public static void RegisterHandler(MsgType msgType, NetworkMessageDelegate handl
/// </summary>
/// <typeparam name="T">Message type</typeparam>
/// <param name="handler">Function handler which will be invoked for when this message type is received.</param>
public static void RegisterHandler<T>(Action<NetworkConnection, T> handler) where T: IMessageBase, new()
/// <param name="requireAuthentication">True if the message requires an authenticated connection</param>
public static void RegisterHandler<T>(Action<NetworkConnection, T> handler, bool requireAuthentication = true) where T: IMessageBase, new()
{
int msgType = MessagePacker.GetId<T>();
if (handlers.ContainsKey(msgType))
{
if (LogFilter.Debug) Debug.Log("NetworkServer.RegisterHandler replacing " + msgType);
}
handlers[msgType] = MessagePacker.MessageHandler<T>(handler);
handlers[msgType] = MessagePacker.MessageHandler<T>(handler, requireAuthentication);
}

/// <summary>
Expand Down

0 comments on commit 7ac43cd

Please sign in to comment.