From 8f0a952d16e1e48fcb1d599d2750cf93f3cb404b Mon Sep 17 00:00:00 2001 From: Robin Rolf Date: Tue, 5 Mar 2024 17:13:09 +0000 Subject: [PATCH] fix: EncryptionTransport uses KDF to ensure fixed size key (#3773) * reset _time * fix: EncryptionTransport use KDF to ensure fixed size key * Expose pub key through transport * Old Unity compat --- .../EncryptionTransportConnectionTest.cs | 2 + .../Encryption/EncryptedConnection.cs | 62 +++++++++++++++---- .../Encryption/EncryptionTransport.cs | 2 + 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/Assets/Mirror/Tests/Editor/Transports/EncryptionTransportConnectionTest.cs b/Assets/Mirror/Tests/Editor/Transports/EncryptionTransportConnectionTest.cs index 4e505388a..418d9bc5b 100644 --- a/Assets/Mirror/Tests/Editor/Transports/EncryptionTransportConnectionTest.cs +++ b/Assets/Mirror/Tests/Editor/Transports/EncryptionTransportConnectionTest.cs @@ -51,6 +51,7 @@ public void Setup() clientValidateKey = null; clientRecv.Clear(); serverRecv.Clear(); + _time = 0; serverCreds = EncryptionCredentials.Generate(); server = new EncryptedConnection(serverCreds, false, @@ -119,6 +120,7 @@ private void Pump() server.TickNonReady(_time); } } + [TearDown] public void TearDown() { diff --git a/Assets/Mirror/Transports/Encryption/EncryptedConnection.cs b/Assets/Mirror/Transports/Encryption/EncryptedConnection.cs index 48a8a3cae..9515e74db 100644 --- a/Assets/Mirror/Transports/Encryption/EncryptedConnection.cs +++ b/Assets/Mirror/Transports/Encryption/EncryptedConnection.cs @@ -1,8 +1,10 @@ using System; using System.Security.Cryptography; +using System.Text; using Org.BouncyCastle.Crypto; using Org.BouncyCastle.Crypto.Agreement; -using Org.BouncyCastle.Crypto.Engines; +using Org.BouncyCastle.Crypto.Digests; +using Org.BouncyCastle.Crypto.Generators; using Org.BouncyCastle.Crypto.Modes; using Org.BouncyCastle.Crypto.Parameters; using UnityEngine.Profiling; @@ -11,6 +13,14 @@ namespace Mirror.Transports.Encryption { public class EncryptedConnection { + // 256-bit key + private const int KeyLength = 32; + // 512-bit salt for the key derivation function + private const int HkdfSaltSize = KeyLength * 2; + + // Info tag for the HKDF, this just adds more entropy + private static readonly byte[] HkdfInfo = Encoding.UTF8.GetBytes("Mirror/EncryptionTransport"); + // fixed size of the unique per-packet nonce. Defaults to 12 bytes/96 bits (not recommended to be changed) private const int NonceSize = 12; @@ -40,9 +50,14 @@ public class EncryptedConnection // use AesUtilities.CreateEngine here as it'll pick the hardware accelerated one if available (which is will not be unless on .net core) private static readonly GcmBlockCipher Cipher = new GcmBlockCipher(AesUtilities.CreateEngine()); + // Set up a global HKDF with a SHA-256 digest + private static readonly HkdfBytesGenerator Hkdf = new HkdfBytesGenerator(new Sha256Digest()); + // Global byte array to store nonce sent by the remote side, they're used immediately after private static readonly byte[] ReceiveNonce = new byte[NonceSize]; + // Buffer for the remote salt, as bouncycastle needs to take a byte[] *rolls eyes* + private static byte[] _tmpRemoteSaltBuffer = new byte[HkdfSaltSize]; // buffer for encrypt/decrypt operations, resized larger as needed // this is also the buffer that will be returned to mirror via ArraySegment // so any thread safety concerns would need to take extra care here @@ -93,6 +108,7 @@ enum State private Func _validateRemoteKey; // Our asymmetric credentials for the initial DH exchange private EncryptionCredentials _credentials; + private byte[] _hkdfSalt; // After no handshake packet in this many seconds, the handshake fails private double _handshakeTimeout; @@ -126,6 +142,11 @@ public EncryptedConnection(EncryptionCredentials credentials, { _credentials = credentials; _sendsFirst = isClient; + if (!_sendsFirst) + { + // salt is controlled by the server + _hkdfSalt = GenerateSecureBytes(HkdfSaltSize); + } _send = sendAction; _receive = receiveAction; _ready = readyAction; @@ -134,15 +155,15 @@ public EncryptedConnection(EncryptionCredentials credentials, } // Generates a random starting nonce - private static byte[] GenerateStartingNonce() + private static byte[] GenerateSecureBytes(int size) { - byte[] nonce = new byte[NonceSize]; + byte[] bytes = new byte[size]; using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) { - rng.GetBytes(nonce); + rng.GetBytes(bytes); } - return nonce; + return bytes; } public void OnReceiveRaw(ArraySegment data, int channel) @@ -203,7 +224,7 @@ public void OnReceiveRaw(ArraySegment data, int channel) _state = State.WaitingHandshakeReply; ResetTimeouts(); - CompleteExchange(reader.ReadBytesSegment(reader.Remaining)); + CompleteExchange(reader.ReadBytesSegment(reader.Remaining), _hkdfSalt); SendHandshakeAndPubKey(OpCodes.HandshakeAck); break; case OpCodes.HandshakeAck: @@ -228,7 +249,8 @@ public void OnReceiveRaw(ArraySegment data, int channel) _state = State.WaitingHandshakeReply; ResetTimeouts(); - CompleteExchange(reader.ReadBytesSegment(reader.Remaining)); + reader.ReadBytes(_tmpRemoteSaltBuffer, HkdfSaltSize); + CompleteExchange(reader.ReadBytesSegment(reader.Remaining), _tmpRemoteSaltBuffer); SendHandshakeFin(); break; case OpCodes.HandshakeFin: @@ -421,6 +443,10 @@ private void SendHandshakeAndPubKey(OpCodes opcode) using (NetworkWriterPooled writer = NetworkWriterPool.Get()) { writer.WriteByte((byte)opcode); + if (opcode == OpCodes.HandshakeAck) + { + writer.WriteBytes(_hkdfSalt, 0, HkdfSaltSize); + } writer.WriteBytes(_credentials.PublicKeySerialized, 0, _credentials.PublicKeySerialized.Length); _send(writer.ToArraySegment(), Channels.Unreliable); } @@ -435,7 +461,7 @@ private void SendHandshakeFin() } } - private void CompleteExchange(ArraySegment remotePubKeyRaw) + private void CompleteExchange(ArraySegment remotePubKeyRaw, byte[] salt) { AsymmetricKeyParameter remotePubKey; try @@ -468,10 +494,10 @@ private void CompleteExchange(ArraySegment remotePubKeyRaw) // It's like magic, but with math! ECDHBasicAgreement ecdh = new ECDHBasicAgreement(); ecdh.Init(_credentials.PrivateKey); - byte[] keyRaw; + byte[] sharedSecret; try { - keyRaw = ecdh.CalculateAgreement(remotePubKey).ToByteArrayUnsigned(); + sharedSecret = ecdh.CalculateAgreement(remotePubKey).ToByteArrayUnsigned(); } catch (Exception e) @@ -480,10 +506,24 @@ private void CompleteExchange(ArraySegment remotePubKeyRaw) return; } + if (salt.Length != HkdfSaltSize) + { + _error(TransportError.Unexpected, $"Salt is expected to be {HkdfSaltSize} bytes long, got {salt.Length}."); + return; + } + + Hkdf.Init(new HkdfParameters(sharedSecret, salt, HkdfInfo)); + + // Allocate a buffer for the output key + byte[] keyRaw = new byte[KeyLength]; + + // Generate the output keying material + Hkdf.GenerateBytes(keyRaw, 0, keyRaw.Length); + KeyParameter key = new KeyParameter(keyRaw); // generate a starting nonce - _nonce = GenerateStartingNonce(); + _nonce = GenerateSecureBytes(NonceSize); // we pass in the nonce array once (as it's stored by reference) so we can cache the AeadParameters instance // instead of creating a new one each encrypt/decrypt diff --git a/Assets/Mirror/Transports/Encryption/EncryptionTransport.cs b/Assets/Mirror/Transports/Encryption/EncryptionTransport.cs index 6c55aa28f..3ea0886c2 100644 --- a/Assets/Mirror/Transports/Encryption/EncryptionTransport.cs +++ b/Assets/Mirror/Transports/Encryption/EncryptionTransport.cs @@ -32,6 +32,8 @@ public enum ValidationMode new List(); private EncryptionCredentials _credentials; + public string EncryptionPublicKeyFingerprint => _credentials?.PublicKeyFingerprint; + public byte[] EncryptionPublicKey => _credentials?.PublicKeySerialized; private void ServerRemoveFromPending(EncryptedConnection con) {