feat!(SocketLayer): adding connection key based on mirage version

This will ensure that client connecting is on the same major version as the client.

This key can be overridden by setting the value in Config

BREAKING CHANGE: Mismatched server/client versions will no longer be able to connect to each other

Assets/Mirage/Runtime/ClientStoppedReason.cs

@@ -14,6 +14,8 @@ namespace Mirage
     [Serializable]
     public enum ClientStoppedReason
     {
+        // IMPORTANT: When adding new values, check all numbers, they might not be in order!
+
         /// <summary>No reason given</summary>
         None = 0,
 
@@ -33,6 +35,8 @@ public enum ClientStoppedReason
         ConnectingTimeout = 5,
         /// <summary>Disconnect called locally before server replies with connected</summary>
         ConnectingCancel = 6,
+        /// <summary>Key given with first message did not match the value on the server, See SocketLayer Config</summary>
+        KeyInvalid = 9,
 
         /// <summary>Disconnect called when server was stopped in host mode</summary>
         HostModeStopped = 7,
@@ -62,6 +66,7 @@ public static ClientStoppedReason ToClientStoppedReason(this RejectReason reason
                 case RejectReason.None: return ClientStoppedReason.None;
                 case RejectReason.Timeout: return ClientStoppedReason.ConnectingTimeout;
                 case RejectReason.ServerFull: return ClientStoppedReason.ServerFull;
+                case RejectReason.KeyInvalid: return ClientStoppedReason.KeyInvalid;
                 case RejectReason.ClosedByPeer: return ClientStoppedReason.ConnectingCancel;
             }
         }

Assets/Mirage/Runtime/SocketLayer/Config.cs

@@ -36,6 +36,13 @@ public class Config
 
 
         #region shared
+        /// <summary>
+        /// Key sent with connection message (defaults to Major version of assmebly)
+        /// <para>Used to validate that server and client are same application/version</para>
+        /// <para>NOTE: key will be ASCII encoded</para>
+        /// </summary>
+        public string key = null;
+
         /// <summary>
         /// How long after disconnect before connection is fully removed from Peer
         /// </summary>

Assets/Mirage/Runtime/SocketLayer/ConnectKeyValidator.cs

@@ -1,3 +1,5 @@
+using System.Text;
+
 namespace Mirage.SocketLayer
 {
     /// <summary>
@@ -6,22 +8,49 @@ namespace Mirage.SocketLayer
     /// </summary>
     internal class ConnectKeyValidator
     {
-        // todo pass in key instead of having constant
-        readonly byte key = (byte)'H';
+        readonly byte[] key;
+        public readonly int KeyLength;
+        const int OFFSET = 2;
 
-        public int KeyLength => 1;
+        public ConnectKeyValidator(byte[] key)
+        {
+            this.key = key;
+            KeyLength = key.Length;
+        }
+
+        static byte[] GetKeyBytes(string key)
+        {
+            // default to mirage version
+            if (string.IsNullOrEmpty(key))
+            {
+                string version = typeof(ConnectKeyValidator).Assembly.GetName().Version.Major.ToString();
+                key = $"Mirage V{version}";
+            }
+
+            return Encoding.ASCII.GetBytes(key);
+        }
+        public ConnectKeyValidator(string key) : this(GetKeyBytes(key))
+        {
+        }
 
         public bool Validate(byte[] buffer)
         {
-            byte keyByte = buffer[2];
+            for (int i = 0; i < KeyLength; i++)
+            {
+                byte keyByte = buffer[i + OFFSET];
+                if (keyByte != key[i])
+                    return false;
+            }
 
-            return keyByte == key;
+            return true;
         }
 
         public void CopyTo(byte[] buffer)
         {
-            buffer[2] = key;
+            for (int i = 0; i < KeyLength; i++)
+            {
+                buffer[i + OFFSET] = key[i];
+            }
         }
-        public byte GetKey() => key;
     }
 }

Assets/Mirage/Runtime/SocketLayer/Enums/RejectReason.cs

@@ -24,5 +24,10 @@ public enum RejectReason
         /// Closed called locally before connect
         /// </summary>
         ClosedByPeer = 3,
+
+        /// <summary>
+        /// Key given with first message did not match the value on the server
+        /// </summary>
+        KeyInvalid = 4,
     }
 }

Assets/Mirage/Runtime/SocketLayer/Peer.cs

@@ -69,7 +69,7 @@ public Peer(ISocket socket, IDataHandler dataHandler, Config config = null, ILog
             this.dataHandler = dataHandler ?? throw new ArgumentNullException(nameof(dataHandler));
             time = new Time();
 
-            connectKeyValidator = new ConnectKeyValidator();
+            connectKeyValidator = new ConnectKeyValidator(this.config.key);
 
             bufferPool = new Pool<ByteBuffer>(ByteBuffer.CreateNew, this.config.MaxPacketSize, this.config.BufferPoolStartSize, this.config.BufferPoolMaxSize, this.logger);
             Application.quitting += Application_quitting;
@@ -178,7 +178,12 @@ internal void SendCommandUnconnected(IEndPoint endPoint, Commands command, byte?
 
         internal void SendConnectRequest(Connection connection)
         {
-            SendCommand(connection, Commands.ConnectRequest, connectKeyValidator.GetKey());
+            using (ByteBuffer buffer = bufferPool.Take())
+            {
+                int length = CreateCommandPacket(buffer, Commands.ConnectRequest, null);
+                connectKeyValidator.CopyTo(buffer.array);
+                Send(connection, buffer.array, length + connectKeyValidator.KeyLength);
+            }
         }
 
         internal void SendCommand(Connection connection, Commands command, byte? extra = null)
@@ -369,7 +374,12 @@ private void HandleNewConnection(IEndPoint endPoint, Packet packet)
             // if invalid, then reject without reason
             if (!Validate(packet)) { return; }
 
-            if (AtMaxConnections())
+
+            if (!connectKeyValidator.Validate(packet.buffer.array))
+            {
+                RejectConnectionWithReason(endPoint, RejectReason.KeyInvalid);
+            }
+            else if (AtMaxConnections())
             {
                 RejectConnectionWithReason(endPoint, RejectReason.ServerFull);
             }
@@ -397,9 +407,6 @@ private bool Validate(Packet packet)
             if (packet.command != Commands.ConnectRequest)
                 return false;
 
-            if (!connectKeyValidator.Validate(packet.buffer.array))
-                return false;
-
             //if (!hashCashValidator.Validate(packet))
             //    return false;
 

Assets/Tests/Runtime/ClientServer/NetworkClientDisconnectTest.cs

@@ -151,6 +151,7 @@ public IEnumerator DisconnectEventWhenFull()
             Assert.That(called, Is.EqualTo(1));
         }
     }
+
     public class NetworkClientConnectFailedTest : ClientServerSetup<MockComponent>
     {
         protected override bool AutoConnectClient => false;
@@ -211,4 +212,30 @@ public IEnumerator DisconnectEventWhenCanceled()
             Assert.That(called, Is.EqualTo(1));
         }
     }
+
+    public class NetworkClientConnectFailedBadKeyTest : ClientServerSetup<MockComponent>
+    {
+        protected override Config ServerConfig => new Config { key = "Server Key" };
+        protected override Config ClientConfig => new Config { key = "Client Key" };
+        protected override bool AutoConnectClient => false;
+
+        [UnityTest]
+        public IEnumerator DisconnectEventWhenFull()
+        {
+            client.Connect("localhost");
+
+            int called = 0;
+            client.Disconnected.AddListener((reason) =>
+            {
+                called++;
+                Assert.That(reason, Is.EqualTo(ClientStoppedReason.KeyInvalid));
+            });
+
+            // wait 2 frames so that messages can go from client->server->client
+            yield return null;
+            yield return null;
+
+            Assert.That(called, Is.EqualTo(1));
+        }
+    }
 }

Assets/Tests/SocketLayer/ConnectKeyValidatorTest.cs

@@ -3,11 +3,63 @@
 namespace Mirage.SocketLayer.Tests
 {
     [Category("SocketLayer")]
+    [TestFixture("hello")]
+    [TestFixture("Mirage V123")]
     public class ConnectKeyValidatorTest
     {
-        [Test] public void KeyIsSameEachCall() { Assert.Ignore("not implemented"); }
-        [Test] public void ValidateReturnsTrueIfKeyIsCorrect() { Assert.Ignore("not implemented"); }
-        [Test] public void ValidateReturnsFalseIfKeyIsCorrect() { Assert.Ignore("not implemented"); }
-        [Test] public void CopySetsKeyIn2ndElementOfBuffer() { Assert.Ignore("not implemented"); }
+        ConnectKeyValidator validator;
+
+        public ConnectKeyValidatorTest(string key)
+        {
+            validator = new ConnectKeyValidator(key);
+        }
+
+        [Test]
+        public void KeyIsSameEachCall()
+        {
+            byte[] buffer1 = new byte[50];
+            byte[] buffer2 = new byte[50];
+            validator.CopyTo(buffer1);
+            validator.CopyTo(buffer2);
+
+            Assert.That(buffer1, Is.EquivalentTo(buffer2), "buffers should have same values");
+        }
+
+        [Test]
+        public void ValidateReturnsTrueIfKeyIsCorrect()
+        {
+            byte[] buffer = new byte[50];
+            validator.CopyTo(buffer);
+
+            bool valid = validator.Validate(buffer);
+            Assert.IsTrue(valid);
+        }
+
+        [Test]
+        public void ValidateReturnsFalseIfKeyIsCorrect()
+        {
+            byte[] buffer = new byte[50];
+            validator.CopyTo(buffer);
+            // corrupt 1 byte
+            buffer[4] = 0;
+
+            bool valid = validator.Validate(buffer);
+            Assert.IsFalse(valid);
+        }
+
+        [Test]
+        [TestCase(1, 2)]
+        [TestCase(10, 220)]
+        public void DoesNotOverWriteFirst2Indexes(byte index1, byte index2)
+        {
+            // use tests cases so we check that it doesn't change atleast 2 sets of values
+            byte[] buffer = new byte[50];
+            buffer[0] = index1;
+            buffer[1] = index2;
+            validator.CopyTo(buffer);
+
+            Assert.That(buffer[0], Is.EqualTo(index1));
+            Assert.That(buffer[1], Is.EqualTo(index2));
+        }
     }
 }

Assets/Tests/SocketLayer/PeerTestBase.cs

@@ -13,12 +13,7 @@ namespace Mirage.SocketLayer.Tests.PeerTests
     public class PeerTestBase
     {
         public const int maxConnections = 5;
-        protected static readonly byte[] connectRequest = new byte[3]
-        {
-            (byte)PacketType.Command,
-            (byte)Commands.ConnectRequest,
-            new ConnectKeyValidator().GetKey(),
-        };
+        protected byte[] connectRequest;
 
         PeerInstance instance;
         protected Action<IConnection> connectAction;
@@ -45,6 +40,17 @@ public void SetUp()
             peer.OnConnected += connectAction;
             peer.OnConnectionFailed += connectFailedAction;
             peer.OnDisconnected += disconnectAction;
+
+            CreateConnectPacket();
+        }
+
+        private void CreateConnectPacket()
+        {
+            var keyValidator = new ConnectKeyValidator(instance.config.key);
+            connectRequest = new byte[2 + keyValidator.KeyLength];
+            connectRequest[0] = (byte)PacketType.Command;
+            connectRequest[1] = (byte)Commands.ConnectRequest;
+            keyValidator.CopyTo(connectRequest);
         }
     }