From f5c7c66a25d6ee99c3356726f362277b1c0e5451 Mon Sep 17 00:00:00 2001
From: Skye <22365940+Skyedra@users.noreply.github.com>
Date: Fri, 11 Oct 2024 21:54:42 -0700
Subject: [PATCH] MITM Mitigation (#2)
---
Robust.Shared/Network/AuthManager.cs | 7 ++++
.../Handshake/MsgEncryptionResponse.cs | 3 ++
Robust.Shared/Network/NetEncryption.cs | 22 ++++++++++++
.../Network/NetManager.ClientConnect.cs | 34 +++++++++++++++++--
.../Network/NetManager.ServerAuth.cs | 23 +++++++++++++
5 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/Robust.Shared/Network/AuthManager.cs b/Robust.Shared/Network/AuthManager.cs
index 2c3a11cae68..4007b6ad428 100644
--- a/Robust.Shared/Network/AuthManager.cs
+++ b/Robust.Shared/Network/AuthManager.cs
@@ -13,6 +13,7 @@ internal interface IAuthManager
string? ServerPublicKey { get; set; }
string? UserPublicKey { get; set; }
string? UserJWT { get; set; }
+ string? SharedSecretBase64 { get; set; }
void LoadFromEnv();
}
@@ -22,6 +23,7 @@ internal sealed class AuthManager : IAuthManager
public string? ServerPublicKey { get; set; }
public string? UserPublicKey { get; set; }
public string? UserJWT { get; set; }
+ public string? SharedSecretBase64 { get; set; }
public void LoadFromEnv()
{
@@ -40,6 +42,11 @@ public void LoadFromEnv()
UserJWT = userJWT;
}
+ if (TryGetVar("ROBUST_SHARED_SECRET", out var sharedSecretBase64))
+ {
+ SharedSecretBase64 = sharedSecretBase64;
+ }
+
static bool TryGetVar(string var, [NotNullWhen(true)] out string? val)
{
val = Environment.GetEnvironmentVariable(var);
diff --git a/Robust.Shared/Network/Messages/Handshake/MsgEncryptionResponse.cs b/Robust.Shared/Network/Messages/Handshake/MsgEncryptionResponse.cs
index 383830bb386..a38f93406cc 100644
--- a/Robust.Shared/Network/Messages/Handshake/MsgEncryptionResponse.cs
+++ b/Robust.Shared/Network/Messages/Handshake/MsgEncryptionResponse.cs
@@ -15,6 +15,7 @@ internal sealed class MsgEncryptionResponse : NetMessage
public byte[] SealedData;
public string UserJWT;
public string UserPublicKey;
+ public ulong StartingNonce;
public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer serializer)
{
@@ -23,6 +24,7 @@ public override void ReadFromBuffer(NetIncomingMessage buffer, IRobustSerializer
UserJWT = buffer.ReadString();
UserPublicKey = buffer.ReadString();
+ StartingNonce = buffer.ReadUInt64();
}
public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer serializer)
@@ -32,6 +34,7 @@ public override void WriteToBuffer(NetOutgoingMessage buffer, IRobustSerializer
buffer.Write(UserJWT);
buffer.Write(UserPublicKey);
+ buffer.Write(StartingNonce);
}
}
}
diff --git a/Robust.Shared/Network/NetEncryption.cs b/Robust.Shared/Network/NetEncryption.cs
index 3c9df6b7241..cdbf41d2c69 100644
--- a/Robust.Shared/Network/NetEncryption.cs
+++ b/Robust.Shared/Network/NetEncryption.cs
@@ -17,6 +17,18 @@ internal sealed class NetEncryption
private ulong _nonce;
private readonly byte[] _key;
+ ///
+ /// How much to offset noonce during reconnect. Noonce+key combination should not be re-used per
+ /// https://doc.libsodium.org/secret-key_cryptography/aead/chacha20-poly1305/ietf_chacha20-poly1305_construction
+ /// "The public nonce npub should never ever be reused with the same key. The recommended way to generate it is to
+ /// use randombytes_buf() for the first message, and increment it for each subsequent message using the same key."
+ /// Since key is no longer randomly generated per connection, the noonce must be incremented. Rather than just
+ /// set this to exactly where it left off, I am padding it a bit. This is just in case there's some messages from
+ /// server -> client that the client never received. This way, I push this much farther into the unused future for
+ /// safety.
+ ///
+ public const ulong RECONNECT_NOONCE_PADDING = 2000000;
+
public NetEncryption(byte[] key, bool isServer)
{
if (key.Length != CryptoAeadXChaCha20Poly1305Ietf.KeyBytes)
@@ -119,4 +131,14 @@ public unsafe void Decrypt(NetIncomingMessage message)
if (!result)
throw new SodiumException("Decryption operation failed!");
}
+
+ public void SetNonce(ulong newValue)
+ {
+ Interlocked.Exchange(ref _nonce, newValue);
+ }
+
+ public ulong GetNonce()
+ {
+ return Interlocked.Read(ref _nonce);
+ }
}
diff --git a/Robust.Shared/Network/NetManager.ClientConnect.cs b/Robust.Shared/Network/NetManager.ClientConnect.cs
index 6ddb5098000..86185bcb21c 100644
--- a/Robust.Shared/Network/NetManager.ClientConnect.cs
+++ b/Robust.Shared/Network/NetManager.ClientConnect.cs
@@ -155,12 +155,39 @@ private async Task CCDoHandshake(NetPeerData peer, NetConnection connection, str
var encRequest = new MsgEncryptionRequest();
encRequest.ReadFromBuffer(response, _serializer);
- var sharedSecret = new byte[SharedKeyLength];
- RandomNumberGenerator.Fill(sharedSecret);
+ byte[] sharedSecret;
+
+ if (string.IsNullOrEmpty(_authManager.SharedSecretBase64))
+ {
+ // Generate new shared secret in robust client
+ sharedSecret = new byte[SharedKeyLength];
+ RandomNumberGenerator.Fill(sharedSecret); // In order for this to work, server must not be verifying JWT
+ }
+ else
+ {
+ // Use shared secret from launcher
+
+ // (Robust client does not have direct access to the user's private key for safety. The JWT needs to
+ // include the authhash to avoid a MITM. Thus, the launcher must generate the JWT and by extension
+ // the shared secret. Launcher will generate it and pass it to us in Base64 format.)
+ sharedSecret = System.Convert.FromBase64String(_authManager.SharedSecretBase64);
+
+ if (sharedSecret.Length != SharedKeyLength)
+ {
+ var msg = $"Invalid shared secret length from launcher. Expected {SharedKeyLength}, but was {sharedSecret.Length}.";
+ connection.Disconnect(msg);
+ throw new Exception(msg);
+ }
+ }
if (encrypt)
+ {
encryption = new NetEncryption(sharedSecret, isServer: false);
+ if (_clientEncryption != null)
+ encryption.SetNonce(_clientEncryption.GetNonce() + NetEncryption.RECONNECT_NOONCE_PADDING);
+ }
+
byte[] keyBytes;
if (hasServerPublicKey)
{
@@ -194,6 +221,9 @@ private async Task CCDoHandshake(NetPeerData peer, NetConnection connection, str
UserPublicKey = _authManager.UserPublicKey
};
+ if (encrypt && encryption != null)
+ encryptionResponse.StartingNonce = encryption.GetNonce() + 1;
+
var outEncRespMsg = peer.Peer.CreateMessage();
encryptionResponse.WriteToBuffer(outEncRespMsg, _serializer);
peer.Peer.SendMessage(outEncRespMsg, connection, NetDeliveryMethod.ReliableOrdered);
diff --git a/Robust.Shared/Network/NetManager.ServerAuth.cs b/Robust.Shared/Network/NetManager.ServerAuth.cs
index dc118f2ed17..2000a0b5b0d 100644
--- a/Robust.Shared/Network/NetManager.ServerAuth.cs
+++ b/Robust.Shared/Network/NetManager.ServerAuth.cs
@@ -136,7 +136,13 @@ private async void HandleHandshake(NetPeerData peer, NetConnection connection)
}
if (msgLogin.Encrypt)
+ {
encryption = new NetEncryption(sharedSecret, isServer: true);
+ encryption.SetNonce(msgEncResponse.StartingNonce);
+ }
+
+ var authHashBytes = MakeAuthHash(sharedSecret, CryptoPublicKey!);
+ var authHash = Base64Helpers.ConvertToBase64Url(authHashBytes);
// Validate the JWT
var userPublicKeyString = msgEncResponse.UserPublicKey ?? "";
@@ -219,6 +225,23 @@ private async void HandleHandshake(NetPeerData peer, NetConnection connection)
connection.Disconnect("JWT Validation Error\nJWT appears to be for another server.\nTry returning to launcher and reconnect.");
return;
}
+
+ // Also verify authhash matches.
+ // (This step helps deter a MITM/proxy attack, since even if traffic was proxied, it should also
+ // be encrypted.)
+ string authHashClaim = "";
+ var authHashClaimNode = jsonNode["authhash"];
+ if (authHashClaimNode == null)
+ {
+ connection.Disconnect("JWT Validation Error - No auth hash in JWT\n(Ensure you are using latest launcher version).");
+ return;
+ }
+ authHashClaim = (string) authHashClaimNode.GetValue();
+ if (authHashClaim != authHash)
+ {
+ connection.Disconnect("JWT Validation Error - Wrong auth hash in JWT\n(Check server address is correct).");
+ return;
+ }
}
_logger.Verbose(