feat: secure remote connections with tls

OpenPolytopia.Common/Network/ClientConnection.cs

@@ -1,7 +1,10 @@
 namespace OpenPolytopia.Common.Network;
 
 using System.Collections.Concurrent;
+using System.Net;
+using System.Net.Security;
 using System.Net.Sockets;
+using System.Security.Authentication;
 using Packets;
 
 /// <summary>
@@ -10,15 +13,29 @@ namespace OpenPolytopia.Common.Network;
 /// <remarks>
 /// Received packets are queued in <see cref="IncomingPackets"/> to let the consumer
 /// process them on its own thread; <see cref="KeepAlivePacket"/> gets answered automatically
-/// and the connection gets closed if the server doesn't send anything for longer than <see cref="TIMEOUT"/>
+/// and the connection gets closed if the server doesn't send anything for longer than <see cref="TIMEOUT"/>.
+/// Every connection that leaves the machine goes through TLS, validated against the certificate store
+/// of the system; there is no plaintext fallback, a server with a bad certificate is a server we don't talk to
 /// </remarks>
-public class ClientConnection(string address, int port) : IDisposable {
+/// <param name="address">the host name or ip address of the server</param>
+/// <param name="port">the port of the server</param>
+/// <param name="useTls">
+/// true to wrap the connection in TLS, false for plaintext;
+/// null to decide from <paramref name="address"/>, meaning TLS for everything but loopback
+/// </param>
+public class ClientConnection(string address, int port, bool? useTls = null) : IDisposable {
   private static readonly TimeSpan TIMEOUT = TimeSpan.FromSeconds(30);
   private static readonly TimeSpan TIMEOUT_CHECK_INTERVAL = TimeSpan.FromSeconds(5);
 
   private readonly TcpClient _client = new();
   private readonly CancellationTokenSource _cts = new();
   private NetworkConnection? _connection;
+  private int _disposed;
+
+  /// <summary>
+  /// true when the connection to the server is wrapped in TLS
+  /// </summary>
+  public bool UsesTls { get; } = useTls ?? !IsLoopback(address);
 
   /// <summary>
   /// Packets received from the server, waiting to be processed
@@ -38,11 +55,16 @@ public class ClientConnection(string address, int port) : IDisposable {
   /// <summary>
   /// Connects to the server and starts reading packets in background
   /// </summary>
+  /// <exception cref="AuthenticationException">
+  /// if the server doesn't present a certificate this machine trusts for <paramref name="address"/>
+  /// </exception>
   public async Task ConnectAsync() {
     PacketRegistrar.RegisterAllPackets();
     await _client.ConnectAsync(address, port, _cts.Token);
 
-    _connection = new NetworkConnection(0, _client);
+    var stream = UsesTls ? await AuthenticateAsync() : null;
+
+    _connection = new NetworkConnection(0, _client, stream);
     _connection.OnPacketReceived += PacketReceivedAsync;
     _connection.OnDisconnected += _ => OnDisconnected?.Invoke();
 
@@ -71,6 +93,43 @@ public void Disconnect() {
     _connection?.Close();
   }
 
+  /// <summary>
+  /// Checks if an address points to this same machine
+  /// </summary>
+  /// <remarks>
+  /// Loopback traffic never leaves the machine, so it's the only case where plaintext is acceptable;
+  /// anything that can't be recognized as loopback gets treated as remote and encrypted
+  /// </remarks>
+  /// <param name="address">the host name or ip address to check</param>
+  private static bool IsLoopback(string address) =>
+    string.Equals(address, "localhost", StringComparison.OrdinalIgnoreCase) ||
+    (IPAddress.TryParse(address, out var ip) && IPAddress.IsLoopback(ip));
+
+  /// <summary>
+  /// Wraps the connection in TLS
+  /// </summary>
+  /// <remarks>
+  /// The certificate gets validated the standard way, against the trust store of the operating system;
+  /// a failure throws and leaves the connection unusable, it never falls back to plaintext
+  /// </remarks>
+  /// <returns>the authenticated stream</returns>
+  private async Task<Stream> AuthenticateAsync() {
+    var ssl = new SslStream(_client.GetStream(), false);
+
+    try {
+      await ssl.AuthenticateAsClientAsync(
+        new SslClientAuthenticationOptions {
+          TargetHost = address, EnabledSslProtocols = SslProtocols.Tls12 | SslProtocols.Tls13
+        }, _cts.Token);
+    }
+    catch {
+      await ssl.DisposeAsync();
+      throw;
+    }
+
+    return ssl;
+  }
+
   private static async Task TimeoutLoopAsync(NetworkConnection connection, CancellationToken ct) {
     using var timer = new PeriodicTimer(TIMEOUT_CHECK_INTERVAL);
 
@@ -99,6 +158,7 @@ private async Task PacketReceivedAsync(NetworkConnection connection, IPacket pac
   }
 
   public void Dispose() {
+    if (Interlocked.Exchange(ref _disposed, 1) != 0) return;
     Disconnect();
     _cts.Dispose();
     _connection?.Dispose();

OpenPolytopia.Common/Network/NetworkConnection.cs

@@ -11,8 +11,14 @@ namespace OpenPolytopia.Common.Network;
 /// Used by the client for its connection to the server
 /// and by the server for every connected client
 /// </remarks>
-public class NetworkConnection(uint id, TcpClient client) : IDisposable {
-  private readonly NetworkStream _stream = client.GetStream();
+/// <param name="id">the id of this connection</param>
+/// <param name="client">the connected socket</param>
+/// <param name="stream">
+/// the stream to read and write packets on; null to use the plaintext stream of <paramref name="client"/>.
+/// Pass an already authenticated <see cref="System.Net.Security.SslStream"/> here to talk over TLS
+/// </param>
+public class NetworkConnection(uint id, TcpClient client, Stream? stream = null) : IDisposable {
+  private readonly Stream _stream = stream ?? client.GetStream();
   private readonly SemaphoreSlim _writeLock = new(1, 1);
   private int _closed;
 
@@ -116,6 +122,15 @@ public void Close() {
       return;
     }
 
+    // disposing the stream unblocks a pending read and, on TLS, sends the close notify;
+    // it can fail if the remote endpoint is already gone, which is not a problem here
+    try {
+      _stream.Dispose();
+    }
+    catch (Exception e) when (e is IOException or ObjectDisposedException or SocketException) {
+      // remote endpoint already gone
+    }
+
     client.Close();
     OnDisconnected?.Invoke(this);
   }

OpenPolytopia.Common/Network/ServerConnection.cs

@@ -1,7 +1,10 @@
 namespace OpenPolytopia.Common.Network;
 
 using System.Collections.Concurrent;
+using System.Net.Security;
 using System.Net.Sockets;
+using System.Security.Authentication;
+using System.Security.Cryptography.X509Certificates;
 using System.Threading.Channels;
 using Packets;
 
@@ -14,17 +17,29 @@ namespace OpenPolytopia.Common.Network;
 /// nor grow the memory of the server by never draining his queue.
 /// Every <see cref="KEEP_ALIVE_INTERVAL"/> it sends a <see cref="KeepAlivePacket"/> to every client
 /// and disconnects the ones that didn't send anything back for longer than <see cref="TIMEOUT"/>;
-/// clients that don't complete a handshake within <see cref="HANDSHAKE_TIMEOUT"/> get disconnected too
+/// clients that don't complete a handshake within <see cref="HANDSHAKE_TIMEOUT"/> get disconnected too.
+/// When a certificate is given every connection is wrapped in TLS before any packet is read;
+/// the TLS handshake runs in background so a slow or hostile client can't stall the accept loop
 /// </remarks>
 /// <param name="port">the port to listen on</param>
 /// <param name="bindAddress">the ip address to bind to; null to listen on every interface</param>
-public class ServerConnection(int port, string? bindAddress = null) : IDisposable {
+/// <param name="certificate">
+/// the certificate to serve TLS with; null to accept plaintext connections, which is only
+/// acceptable on loopback
+/// </param>
+public class ServerConnection(int port, string? bindAddress = null, X509Certificate2? certificate = null)
+  : IDisposable {
   private static readonly TimeSpan KEEP_ALIVE_INTERVAL = TimeSpan.FromSeconds(10);
   private static readonly TimeSpan TIMEOUT = TimeSpan.FromSeconds(30);
   private static readonly TimeSpan HANDSHAKE_TIMEOUT = TimeSpan.FromSeconds(10);
   private static readonly TimeSpan SEND_TIMEOUT = TimeSpan.FromSeconds(10);
   private static readonly TimeSpan ACCEPT_RETRY_DELAY = TimeSpan.FromSeconds(1);
 
+  /// <summary>
+  /// How long a client has to complete the TLS handshake before its socket gets dropped
+  /// </summary>
+  private static readonly TimeSpan TLS_HANDSHAKE_TIMEOUT = TimeSpan.FromSeconds(10);
+
   /// <summary>
   /// Max frames queued for a single client; way more than lobby traffic ever needs
   /// </summary>
@@ -37,6 +52,11 @@ public class ServerConnection(int port, string? bindAddress = null) : IDisposabl
   private readonly CancellationTokenSource _cts = new();
   private uint _nextId;
 
+  /// <summary>
+  /// true when connections get wrapped in TLS
+  /// </summary>
+  public bool TlsEnabled => certificate != null;
+
   /// <summary>
   /// Fired when a new client connects
   /// </summary>
@@ -83,20 +103,9 @@ public async Task RunAsync() {
           continue;
         }
 
-        var id = Interlocked.Increment(ref _nextId);
-
-        var connection = new NetworkConnection(id, tcpClient);
-        connection.OnPacketReceived += ClientPacketReceivedAsync;
-        connection.OnDisconnected += ClientDisconnected;
-
-        var client = new Client(connection);
-        _clients[id] = client;
-
-        OnClientConnected?.Invoke(connection);
-
-        // manage the client in background
-        _ = connection.RunAsync(_cts.Token);
-        _ = SenderLoopAsync(client, _cts.Token);
+        // the TLS handshake needs a round trip with the client, so it can't run here
+        // or one slow client would keep everybody else from connecting
+        _ = SetupClientAsync(tcpClient);
       }
     }
     catch (OperationCanceledException) {
@@ -116,6 +125,84 @@ public async Task RunAsync() {
   /// </summary>
   public void Stop() => _cts.Cancel();
 
+  /// <summary>
+  /// Completes the TLS handshake, if enabled, and registers the client
+  /// </summary>
+  /// <remarks>
+  /// Runs in background, one task per accepted socket; a socket that fails or takes longer than
+  /// <see cref="TLS_HANDSHAKE_TIMEOUT"/> to negotiate TLS gets dropped without ever becoming a client
+  /// </remarks>
+  /// <param name="tcpClient">the freshly accepted socket</param>
+  private async Task SetupClientAsync(TcpClient tcpClient) {
+    var stream = await AuthenticateAsync(tcpClient);
+
+    // the handshake failed, the socket is already gone
+    if (certificate != null && stream == null) {
+      return;
+    }
+
+    if (_cts.IsCancellationRequested) {
+      stream?.Dispose();
+      tcpClient.Dispose();
+      return;
+    }
+
+    var id = Interlocked.Increment(ref _nextId);
+
+    var connection = new NetworkConnection(id, tcpClient, stream);
+    connection.OnPacketReceived += ClientPacketReceivedAsync;
+    connection.OnDisconnected += ClientDisconnected;
+
+    var client = new Client(connection);
+    _clients[id] = client;
+
+    OnClientConnected?.Invoke(connection);
+
+    // manage the client in background
+    _ = connection.RunAsync(_cts.Token);
+    _ = SenderLoopAsync(client, _cts.Token);
+  }
+
+  /// <summary>
+  /// Wraps a socket in TLS
+  /// </summary>
+  /// <param name="tcpClient">the socket to wrap</param>
+  /// <returns>
+  /// the authenticated stream, null when TLS is disabled (plaintext) or when the handshake failed;
+  /// on failure the socket gets disposed
+  /// </returns>
+  private async Task<Stream?> AuthenticateAsync(TcpClient tcpClient) {
+    if (certificate == null) {
+      return null;
+    }
+
+    SslStream? ssl = null;
+    try {
+      ssl = new SslStream(tcpClient.GetStream(), false);
+
+      using var cts = CancellationTokenSource.CreateLinkedTokenSource(_cts.Token);
+      cts.CancelAfter(TLS_HANDSHAKE_TIMEOUT);
+
+      await ssl.AuthenticateAsServerAsync(
+        new SslServerAuthenticationOptions {
+          ServerCertificate = certificate,
+          ClientCertificateRequired = false,
+          EnabledSslProtocols = SslProtocols.Tls12 | SslProtocols.Tls13
+        }, cts.Token);
+
+      return ssl;
+    }
+    catch (Exception e) when (e is AuthenticationException or IOException or OperationCanceledException
+                                or ObjectDisposedException or SocketException) {
+      Console.Error.WriteLine($"TLS handshake failed: {e.Message}");
+
+      // nothing was ever handed out for this socket, drop it here
+      ssl?.Dispose();
+      tcpClient.Dispose();
+      return null;
+    }
+  }
+
   /// <summary>
   /// Marks a client as having completed the handshake
   /// </summary>

OpenPolytopia.Server/GameServer.cs

@@ -26,7 +26,8 @@ public class GameServer(int port, string? bindAddress = null) : IDisposable {
   /// </summary>
   private const int MAX_LOBBIES = 100;
 
-  private readonly ServerConnection _server = new(port, bindAddress);
+  private readonly System.Security.Cryptography.X509Certificates.X509Certificate2? _certificate = ServerTls.LoadAndValidate(bindAddress);
+  private ServerConnection _server = null!;
   private readonly LobbyManager _lobbyManager = new();
   private readonly GameManager _gameManager = new();
   private readonly Dictionary<uint, string> _playerNames = new();
@@ -45,6 +46,7 @@ public class GameServer(int port, string? bindAddress = null) : IDisposable {
   /// Runs the server until <see cref="Stop"/> gets called
   /// </summary>
   public async Task RunAsync() {
+    _server = new ServerConnection(port, bindAddress, _certificate);
     RegisterHandlers();
 
     _server.OnPacketReceived += ManagePacketAsync;
@@ -62,7 +64,7 @@ public async Task RunAsync() {
   /// </summary>
   public void Stop() {
     _cts.Cancel();
-    _server.Stop();
+    _server?.Stop();
   }
 
   private async Task ManagePacketAsync(NetworkConnection connection, IPacket packet) {
@@ -646,7 +648,8 @@ private async Task ManageEndTurnAsync(NetworkConnection connection, EndTurnPacke
   public void Dispose() {
     Stop();
     _cts.Dispose();
-    _server.Dispose();
+    _server?.Dispose();
+    _certificate?.Dispose();
     _stateLock.Dispose();
     GC.SuppressFinalize(this);
   }