Making Rpc Params Safe From Manipulation

Hello, I'm a student trying to understand server-client relations, and I'm working on making as much logic server-sided as possible. Today, I discovered that RpcParams can be easily altered by modified clients. Here’s an example of the issue:

[Rpc(SendTo.Server, RequireOwnership = false)]
public void RequestRecipeListDataServerRpc(CrewRoles role, RpcParams param = default)
{
    if (!DmrNetworkHelper.LegitCheckIsClientRegistered(param.Receive.SenderClientId) ||
        !DmrNetworkHelper.LegitCheckIsClientHasRoleSelected(param.Receive.SenderClientId))
    {
        return;
    }

    if (role == CrewRoles.None) return;

    RecipeListDataArrivedClientRpc(role, recipesByRoles[role].ToArray(), RpcTarget.Single(param.Receive.SenderClientId, RpcTargetUse.Temp));
}

In this code, the server is validating the parameters received from the client. I assumed that the param variable could be trusted, but I discovered that this isn’t the case. I was able to cheat by modifying the client to alter RpcParams. Here’s how it can be done:

In the NetworkBehaviour class, the method __endSendRpc constructs an RpcMessage:

var rpcMessage = new RpcMessage
{
    Metadata = new RpcMetadata
    {
        NetworkObjectId = NetworkObjectId,
        NetworkBehaviourId = NetworkBehaviourId,
        NetworkRpcMethodId = rpcMethodId,
    },
    SenderClientId = NetworkManager.LocalClientId, // Changing this to another player's ClientId
    WriteBuffer = bufferWriter
};

By changing the SenderClientId to another player’s ClientId (which is often predictable or can be brute-forced), a modified client can send requests on behalf of other players.

To address this issue, implementing a token-based validation system could be a solution. However, it is important to note that blocking such parameter tampering should be a default security feature.

This appears to be the case, I'm surprised to see the clientId being sent as part of the message. I'd assumed the clientId was tied to the transport's connection identifier.

@AhmetDmr02

Your assessment is correct... it would require one to modify the DLL which if a bad actor is modifying DLLs then they could modify all messages.

For the example you provided, there are many ways to further secure things like RPCs. Here is one (very simple/could be greatly expanded upon) way to prevent that from happening:

        private float m_KeyUpdateFrequency;
        private float m_NextKeyUpdate;
        private int m_MaxKeyBuffer;
        private List<long> m_PreviousKeys = new List<long>();
        private NetworkVariable<int> m_ClientKey = new NetworkVariable<int>(0, NetworkVariableReadPermission.Owner, NetworkVariableWritePermission.Server);

        private void Update()
        {
            if (!IsSpawned || !IsServer)
            {
                return;
            }

            if (m_NextKeyUpdate < Time.realtimeSinceStartup)
            {
                if (m_NextKeyUpdate != 0.0f)
                {
                    m_PreviousKeys.Add(m_ClientKey.Value);
                    if (m_PreviousKeys.Count > m_MaxKeyBuffer)
                    {
                        m_PreviousKeys.RemoveAt(0);
                    }
                }
                m_NextKeyUpdate = Time.realtimeSinceStartup + m_KeyUpdateFrequency;
                m_ClientKey.Value = Random.Range(int.MinValue, int.MaxValue);
            }
        }

        private bool m_IsRequestingRoleUpdate;
        public void RequestUpdateRoles(CrewRoles role)
        {
            m_IsRequestingRoleUpdate = true;
            RequestRecipeListDataServerRpc(role, m_ClientKey.Value);
        }

        [Rpc(SendTo.Server, RequireOwnership = false)]
        public void RequestRecipeListDataServerRpc(CrewRoles role,int clientKey, RpcParams param = default)
        {
            if (m_ClientKey.Value != clientKey && !m_PreviousKeys.Contains(clientKey))
            {
                // Perform any logging actions or the like here

                // Then exit
                return;
            }
            if (!DmrNetworkHelper.LegitCheckIsClientRegistered(param.Receive.SenderClientId) ||
                !DmrNetworkHelper.LegitCheckIsClientHasRoleSelected(param.Receive.SenderClientId))
            {
                return;
            }

            if (role == CrewRoles.None) return;

            RecipeListDataArrivedClientRpc(role, recipesByRoles[role].ToArray(), RpcTarget.Single(param.Receive.SenderClientId, RpcTargetUse.Temp));
        }

        [Rpc(SendTo.SpecifiedInParams)]
        private void RecipeListDataArrivedClientRpc(CrewRoles role, Recipe[] recipes, RpcParams rpcParams = default)
        {
            if (!m_IsRequestingRoleUpdate)
            {
                // You could send a notification to your server, log something, or determine any other action you want to take here

                //Then exit
                return;
            }

            // Otherwise, you were expecting the update so you can just process it here

            m_IsRequestingRoleUpdate = false;
        }

Since Netcode for GameObjects is public and open for anyone to view, providing an "out of the box" type of "secure transaction system" would be a bit futile since we would be presenting the blueprints to any bad actors. The better approach is to leave this part up to each developer so each project, that has such concerns, implements their own approach and algorithms. How developers protect that portion of their code is better left up to the developer (i.e. don't "provide the blue prints" for bad actors).

One could create a self extracting package/DLL that contains the "client key generation" portion of your project's code and is loaded into the application domain... you could encrypt the package/DLL with a key and there are many ways to do this too... whether you obtain a key from a remote service specific to your project or have some way to generate a key based on certain CRCs of certain files/assets...that is better left to your own design and best kept in a non-public repository (etc).

But... in the end... the minute something is "open source" it immediately becomes vulnerable...no matter how good the algorithm/methods/processes are to try and "lock down" all aspects of it. So, it is a trade off between free and "open to view and or modify" vs something that attempts to make internet communication "secure"...which often isn't free nor open to view and modify.

So, a decision to primarily focus on the netcode framework side of things was made a long time ago and we will continue to keep this focus while providing the developers/users of the netcode solution different general avenues they can use, if that is their concern/desire, to implement their own unique way to secure such scenarios.

Does this help?

As a side note: We could check the SenderId against the inbound message, but that is still "a place that could be modified"... so the reality is if it is a C# compiled DLL, then it is fully accessible to bad actors that could make modifications to them.

Of course, you could also add CRC checks against specific DLLs to the client connection data. If you inject code into a DLL then most likely you will inject more code than was there in the first place. With the example you provided, a bad actor would not just inject a specific client ID at that specific location but would most likely provide some additional code that might invoke a method external to the DLL in question or assign a value from a different location/property... which the CRC for the modified DLL will very likely not match the expected CRC... so you could just prevent bad actors from connecting to sessions by sending that information:

When the client first connects (first level prevention)
- Con: More advanced bad actors might modify that code depending upon where it exists and if it is protected
When you detect mismatches in say your keys (i.e. the example above I provided) you could:
- Have a more secure library that uses a different transport/means of communication and forces all clients to send the server CRCs of (insert DLLs of concern here)...and/or that library could also track assemblies loaded in the domain and provide that information too (a bit more complex as you would need to compare against OS specific sets of data etc)... and if one of the clients sends the wrong information then you can disconnect that client.

The idea is that there are a lot of ways to detect modifications but there are also a lot of ways to circumvent detection... it just depends upon what kind of security you think your product needs and then what kind of effort you can allocate towards implementing ways to further secure your product... in the end if you have bad actors performing "advanced techniques" it most likely means your product is very successful/popular which means (other than you hopefully profiting from the success) you might come up with several stages ahead of time for various ways to protect your product, allocate time to implement those you think fits your budget (time/money), and if you find yourself in a place where bad actors "overcame" your first stage...you then implement the next one... post an update... and give them "more things to spend time trying to figure out"...while you think of even more stages/ways to secure your product...and that is pretty much the life cycle of many popular products out in the wild that are popular enough for bad actors to feel like "it is worth it" to spend their time hacking your product. 😸

You could also use addressables to handle some of the "updating" portions...

Anyway, thought I would add these additional ideas/thoughts to my response with the hope that it will provide you with a different perspective, possibly some ideas (for "stage 1"), and maybe reduce your concerns a bit.

👍

@NoelStephensUnity

First of all, thank you very much for writing such a detailed description and providing examples. As I mentioned, I'm just a student interested in learning server-authoritative behaviors, not a expert by any means. What I meant by the token validation system is something like this:

When a client connects to the server, the server generates a token using secure, unpredictable libraries, such as Cryptography, for both client-to-server and server-to-client communication. This ensures that no one can impersonate the server or the client. The key remains secret, and the only ways to compromise it would be through backdooring the server or intercepting packets during the connection. During RPCs, I will validate the incoming key against the key generated during the handshake. (the token is paired to clientId on connection) If they match, it confirms that the client possesses the correct key.

However, as I said, I'm not a security expert, so I'm not sure if this technique is as safe as I think it is. But if I validate the key I created on the server, wouldn't that make the server requests DLL safe (just for the clientId parameters)? I also haven't looked deeply into how the old legacy ServerRpc works but I couldn't see any SenderClientId sent by the packet, so I'm not sure how senderClientId is handled there.

Lastly, what can I actually trust in NGO? For example, can I trust structures like NetworkVariableReadPermission.Owner? Anyway, thank you so much for your detailed explanation. If this token system could be implemented into core RPC, it would be very reassuring (assuming it would be safe enough). Honestly, if only the senderClientId were safe, that would be good enough since other parameters will be checked on the server

When a client connects to the server, the server generates a token using secure, unpredictable libraries, such as Cryptography, for both client-to-server and server-to-client communication. This ensures that no one can impersonate the server or the client.

This would be something that, in my opinion, would be best implemented by the project developer as opposed to having in open source... even a protected library will have C# methods that invoke other methods (or the like) which if you are talking about modifying a DLL (like your original example) it has the same issues.

The original ServerRpc and ClientRpc still had the same kind of RpcParams used (ServerRpcParams & ClientRpcParams) it was just injected during the ILPP process.

Lastly, what can I actually trust in NGO? For example, can I trust structures like NetworkVariableReadPermission.Owner?

In a client-server network topology, you can rely upon the NetworkVariableReadPermission.Owner and NetworkVariableWritePermission.Server since any changes have to originate from the server and it will only send those changes to the owner of the associated NetworkObject. That is why my changing key example used that configuration as no other client would know what the keys were at any given moment. The means of how the key value is generated and how often it is generated would be something that you (or any project developer with these types of concerns) would want to implement in order to keep that part private (i.e. not part of a public repository).

If this token system could be implemented into core RPC, it would be very reassuring (assuming it would be safe enough). Honestly, if only the senderClientId were safe, that would be good enough since other parameters will be checked on the server.

It would be much safer to have each unique project have their own means to securing Rpc communication than it would be to have a predefined way to securing this information (circling back to: "if it is public, then a bad actor will find a way to circumvent it if their desire (and skills) is high enough.").

You can also use addressables that contain unique assets used to authenticate, have several versions that use different algorithms, and then have the server randomly pick one of the addressables available for each session. I would recommend using NetworkVariables to handle conveying any keys to each owner/client/player with the server write and owner read permissions (i.e. the server is the only thing that can write, it will reject any changes from all clients, and will only forward changes to the owner). This type of approach would be a good "stage-1/stage-2" type of start to assuring you have secure communication...if it was me and I was truly concerned then I would also incorporate some form of CRC checking of specific DLLs with random "spot checks" to assure all clients were running the correct DLLs. There are also ways to combine named messages as part of the authentication process (mixture of the server write owner read NetworkVariable that provides a channel name to use to provide other information between the client and server).

A combination of these types of approaches would be a good start. Netcode for GameObjects is a netcode SDK. While it does provide some aspect to help secure a product, its primary goal is focused on the netcode SDK side of things and not securing communication between two points.

@NoelStephensUnity

Understood, Thank you for providing additional information and for taking the time to write everything out so thoroughly.

Unity-Technologies / com.unity.netcode.gameobjects

Making Rpc Params Safe From Manipulation #3015