From 3bafe74c0a2338d6c5e1dcf2be4100b3ec39a7a2 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Mon, 23 Sep 2024 23:18:36 +0200 Subject: [PATCH] More work on getting the protocol fixed --- Duplicati/Agent/Program.cs | 6 +- .../RemoteControl/ExchangeDataTypes.cs | 25 ++- .../RemoteControl/KeepRemoteConnection.cs | 152 ++++++++++++++++-- .../RemoteControl/RegisterForRemote.cs | 27 +++- .../Library/RemoteControl/SharedTypes.cs | 27 +--- .../controllers/SystemSettingsController.js | 2 +- .../RemoteControllerRegistrationService.cs | 1 + .../Services/RemoteControllerService.cs | 3 +- 8 files changed, 200 insertions(+), 43 deletions(-) diff --git a/Duplicati/Agent/Program.cs b/Duplicati/Agent/Program.cs index 29d4ff500..e4d0f02e9 100644 --- a/Duplicati/Agent/Program.cs +++ b/Duplicati/Agent/Program.cs @@ -159,12 +159,12 @@ public static class Program var settings = Settings.Load(); if (!string.IsNullOrWhiteSpace(keydata.JWT) && settings.JWT != keydata.JWT) settings = settings with { JWT = keydata.JWT }; - if (keydata.ServerCertificates.Any()) - settings = settings with { ServerCertificates = MiniServerCertificate.MergeCertificates(keydata.ServerCertificates, settings.ServerCertificates) }; + if (keydata.ServerCertificates != null && keydata.ServerCertificates.Any()) + settings = settings with { ServerCertificates = keydata.ServerCertificates }; if (!string.IsNullOrWhiteSpace(keydata.LocalEncryptionKey) && settings.SettingsEncryptionKey != keydata.LocalEncryptionKey) { - Log.WriteMessage(LogMessageType.Information, LogTag, "ReKey", "Changing the local settings encryption key"); + // Log.WriteMessage(LogMessageType.Information, LogTag, "ReKey", "Changing the local settings encryption key"); // TODO: Implement changing the database encryption key // FIXMEGlobal.Provider.GetRequiredService().ChangeDbKey(keydata.LocalEncryptionKey); // settings = settings with { SettingsEncryptionKey = keydata.LocalEncryptionKey }; diff --git a/Duplicati/Library/RemoteControl/ExchangeDataTypes.cs b/Duplicati/Library/RemoteControl/ExchangeDataTypes.cs index 3b60e3bb6..72b52ed61 100644 --- a/Duplicati/Library/RemoteControl/ExchangeDataTypes.cs +++ b/Duplicati/Library/RemoteControl/ExchangeDataTypes.cs @@ -110,6 +110,10 @@ internal sealed record EnvelopedMessage /// public string? Payload { get; init; } /// + /// The public key hash + /// + public string? PublicKeyHash { get; init; } + /// /// The signature of the payload /// public string? Signature { get; init; } @@ -185,6 +189,21 @@ internal sealed record EnvelopedMessage ).Replace("-", "").ToLower(); } + /// + /// Computes the signature of the payload + /// + /// The private key to use + /// The computed signature + public string? ComputePayloadSignature(RSA key) + { + if (key is null || Payload is null && MessageId is null) + return null; + + return BitConverter.ToString( + key.SignData(Encoding.UTF8.GetBytes($"{Payload}::{MessageId}"), HashAlgorithmName.SHA256, RSASignaturePadding.Pss) + ).Replace("-", "").ToLower(); + } + /// /// Validates the message signature /// @@ -203,10 +222,10 @@ internal sealed record EnvelopedMessage /// /// Creates the signature on the returned message /// - /// The private key to use + /// The private key to use /// The signed message - public EnvelopedMessage WithSignature(string? pemPrivatekey) - => this with { Signature = ComputePayloadSignature(pemPrivatekey) }; + public EnvelopedMessage WithSignature(RSA key) + => this with { Signature = ComputePayloadSignature(key) }; /// /// Creates a new message with a payload diff --git a/Duplicati/Library/RemoteControl/KeepRemoteConnection.cs b/Duplicati/Library/RemoteControl/KeepRemoteConnection.cs index 587cd9512..9d21fb065 100644 --- a/Duplicati/Library/RemoteControl/KeepRemoteConnection.cs +++ b/Duplicati/Library/RemoteControl/KeepRemoteConnection.cs @@ -22,6 +22,7 @@ using System.Net; using System.Security.Cryptography; using System.Text; +using System.Text.Json; using Duplicati.Library.Logging; namespace Duplicati.Library.RemoteControl; @@ -36,20 +37,32 @@ public class KeepRemoteConnection : IDisposable /// private static readonly string LogTag = Log.LogTagFromType(); + /// + /// The interval between reconnect attempts + /// + private static readonly TimeSpan ReconnectInterval = TimeSpan.FromSeconds(30); + /// /// The interval between heartbeats /// private static readonly TimeSpan HeartbeatInterval = TimeSpan.FromSeconds(5); + /// + /// The interval between certificate refreshes + /// + private static readonly TimeSpan CertificateRefreshInterval = TimeSpan.FromDays(7); + /// /// The client key to use for signing messages /// - private static readonly string? ClientKey = null; // TODO: Fill in + private static readonly RSA ClientKey = RSA.Create(2048); /// /// The client ID to use for identifying the client /// - private static readonly string ClientId = AutoUpdater.UpdaterManager.MachineID; + private static readonly string ClientId = string.IsNullOrWhiteSpace(AutoUpdater.UpdaterManager.MachineID) + ? Guid.NewGuid().ToString() + : AutoUpdater.UpdaterManager.MachineID; /// /// The stats the connection can be in @@ -94,6 +107,43 @@ public class KeepRemoteConnection : IDisposable /// The task that runs the connection /// private Task _runnerTask; + /// + /// The currently negotiated server certificate + /// + private MiniServerCertificate? _serverCertificate; + /// + /// The time the certificate was last refreshed + /// + private DateTime _lastCertificateRefresh = DateTime.UnixEpoch; + /// + /// Task for requesting certificate refresh + /// + private TaskCompletionSource _refreshCertificates = new TaskCompletionSource(); + /// + /// The callback to call when rekeying + /// + private readonly Func _onReKey; + /// + /// The callback to call when a message is received + /// + private readonly Func _onMessage; + + /// + /// The current JWT token + /// + private string _token; + /// + /// The server URL + /// + private string _serverUrl; + /// + /// The certificate URL + /// + private string _certificateUrl; + /// + /// The server keys + /// + private IEnumerable _serverKeys; /// /// Creates a new connection to the remote server @@ -104,8 +154,24 @@ public class KeepRemoteConnection : IDisposable /// The token to cancel the connection private KeepRemoteConnection(string serverUrl, string JWT, string certificateUrl, IEnumerable serverKeys, CancellationToken cancellationToken, Func onReKey, Func onMessage) { + _serverUrl = serverUrl; + _certificateUrl = certificateUrl; + _token = JWT; + _serverKeys = serverKeys; + _cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + _onReKey = onReKey; + _onMessage = onMessage; + _client = new Websocket.Client.WebsocketClient(new Uri(serverUrl)); - _client.ReconnectTimeout = TimeSpan.FromSeconds(30); + _runnerTask = RunMainLoop(); + } + + /// + /// Runs the inner loop of the connection + /// + private Task RunMainLoop() + { + _client.ReconnectTimeout = ReconnectInterval; _client.ReconnectionHappened.Subscribe(info => { @@ -115,8 +181,8 @@ public class KeepRemoteConnection : IDisposable _client.DisconnectionHappened.Subscribe(info => { - // TODO: If disconnected due to certifiate error, we should try to get fresh certificates _state = ConnectionState.NotConnected; + _serverCertificate = null; Log.WriteMessage(LogMessageType.Warning, LogTag, "WebsocketDisconnect", "Disconnected from the server"); }); @@ -127,8 +193,28 @@ public class KeepRemoteConnection : IDisposable try { var envelope = EnvelopedMessage.ForceParse(msg.Text); - var machineKey = serverKeys.FirstOrDefault(x => x.Identifier == envelope.From && x.Expiry > DateTimeOffset.Now)?.PublicKey; - envelope.ValidateSignature(machineKey); + if (_serverCertificate == null || _state == ConnectionState.Connected) + { + if (envelope.GetMessageType() != MessageType.Welcome) + throw new ProtocolViolationException("Expected welcome message"); + if (string.IsNullOrWhiteSpace(envelope.PublicKeyHash)) + throw new ProtocolViolationException("No public key hash in welcome message"); + _serverCertificate = _serverKeys.FirstOrDefault(x => x.PublicKeyHash == envelope.PublicKeyHash && x.Expiry > DateTimeOffset.Now); + + if (_serverCertificate == null) + { + _refreshCertificates.TrySetResult(true); + throw new ProtocolViolationException("No valid server certificate"); + } + } + + if (_serverCertificate == null) + { + _refreshCertificates.TrySetResult(true); + throw new ProtocolViolationException("No valid server certificate"); + } + + envelope.ValidateSignature(_serverCertificate?.PublicKey); if (_state == ConnectionState.Connected) { @@ -140,7 +226,7 @@ public class KeepRemoteConnection : IDisposable Log.WriteMessage(LogMessageType.Information, LogTag, "WebsocketAuthenticated", "Connected with the server"); _challenge = RandomNumberGenerator.GetHexString(64); - SendEnvelope(envelope.RespondWith(new AuthMessage(JWT, _challenge))); + SendEnvelope(envelope.RespondWith(new AuthMessage(_token, _challenge))); } else if (_state == ConnectionState.WelcomeReceived) { @@ -155,12 +241,15 @@ public class KeepRemoteConnection : IDisposable throw new ProtocolViolationException("Invalid Json message"); using RSA rsa = RSA.Create(); - rsa.ImportFromPem(machineKey); + rsa.ImportFromPem(_serverCertificate?.PublicKey); if (!rsa.VerifyData(Encoding.UTF8.GetBytes(_challenge!), Convert.FromHexString(authMessage.SignedChallenge), HashAlgorithmName.SHA256, RSASignaturePadding.Pss)) throw new EnvelopeJsonParsingException("Invalid Json message"); if ((authMessage.WillReplaceToken ?? false) && authMessage.NewToken != null) - await onReKey(new ClaimedClientData(authMessage.NewToken, serverUrl, certificateUrl, serverKeys, null)); + { + _token = authMessage.NewToken; + await InvokeReKey(); + } _state = ConnectionState.Authenticated; } @@ -172,7 +261,7 @@ public class KeepRemoteConnection : IDisposable break; case MessageType.Command: - await onMessage(new CommandMessage(envelope.GetPayload(), response => + await _onMessage(new CommandMessage(envelope.GetPayload(), response => { SendEnvelope(envelope.RespondWith(response)); return true; @@ -198,13 +287,16 @@ public class KeepRemoteConnection : IDisposable } }); - _cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - _runnerTask = Task.WhenAny( + return Task.WhenAny( _client.Start(), - RunHeartbeatLoop() + RunHeartbeatLoop(), + RunCertificateRefreshLoop() ); } + private Task InvokeReKey() + => _onReKey(new ClaimedClientData(_token, _serverUrl, _certificateUrl, _serverKeys, null)); + /// /// Creates a new connection to the remote server /// @@ -317,6 +409,40 @@ public class KeepRemoteConnection : IDisposable } } + /// + /// Runs a loop that refreshes the server certificates + /// + /// An awaitable task + private async Task RunCertificateRefreshLoop() + { + while (!_cancellationTokenSource.Token.IsCancellationRequested) + { + var t = await Task.WhenAny(_refreshCertificates.Task, Task.Delay(CertificateRefreshInterval, _cancellationTokenSource.Token)); + if (_cancellationTokenSource.Token.IsCancellationRequested) + return; + + if (t == _refreshCertificates.Task) + Interlocked.Exchange(ref _refreshCertificates, new TaskCompletionSource()); + + if (_lastCertificateRefresh.AddMinutes(5) < DateTime.Now) + { + using var client = new HttpClient(); + var response = await client.GetAsync(_certificateUrl); + if (response.IsSuccessStatusCode) + { + using var stream = await response.Content.ReadAsStreamAsync(_cancellationTokenSource.Token); + var serverKeys = await JsonSerializer.DeserializeAsync>(stream, cancellationToken: _cancellationTokenSource.Token); + if (serverKeys != null && serverKeys.Any()) + { + _lastCertificateRefresh = DateTime.Now; + _serverKeys = serverKeys; + await InvokeReKey(); + } + } + } + } + } + /// public void Dispose() { diff --git a/Duplicati/Library/RemoteControl/RegisterForRemote.cs b/Duplicati/Library/RemoteControl/RegisterForRemote.cs index 77f661979..b7527cb83 100644 --- a/Duplicati/Library/RemoteControl/RegisterForRemote.cs +++ b/Duplicati/Library/RemoteControl/RegisterForRemote.cs @@ -84,6 +84,26 @@ public class RegisterForRemote : IDisposable Disposed } + /// + /// Data returned when the machine is claimed + /// + /// True if the claim was successful + /// The status message for the claim + /// The JWT token for the machine + /// The URL for the remote server + /// The URL for getting new server certificates + /// The certificates for the remote server + /// The encryption key for the local settings + private sealed record EnvelopedClaimedClientData( + bool Success, + string StatusMessage, + string JWT, + string ServerUrl, + string CertificateUrl, + IEnumerable ServerCertificates, + string? LocalEncryptionKey + ); + /// /// The current state of the registration process /// @@ -230,8 +250,13 @@ public class RegisterForRemote : IDisposable var response = await _httpClient.PostAsync(_registerClientData!.StatusLink, CreateMachineData(), _cancellationTokenSource.Token); response.EnsureSuccessStatusCode(); - return await response.Content.ReadFromJsonAsync() + var result = await response.Content.ReadFromJsonAsync() ?? throw new Exception("Failed to read machine claim data"); + + if (!result.Success) + throw new Exception($"Failed to claim machine: {result.StatusMessage}"); + + return new ClaimedClientData(result.JWT, result.ServerUrl, result.CertificateUrl, result.ServerCertificates, result.LocalEncryptionKey); } /// diff --git a/Duplicati/Library/RemoteControl/SharedTypes.cs b/Duplicati/Library/RemoteControl/SharedTypes.cs index 665efbf06..f588d54a0 100644 --- a/Duplicati/Library/RemoteControl/SharedTypes.cs +++ b/Duplicati/Library/RemoteControl/SharedTypes.cs @@ -40,6 +40,8 @@ public sealed record RegisterClientData( /// /// Data returned when the machine is claimed /// +/// True if the claim was successful +/// The status message for the claim /// The JWT token for the machine /// The URL for the remote server /// The URL for getting new server certificates @@ -58,30 +60,13 @@ public sealed record ClaimedClientData( /// This data is serialized to various files, and chosen instead of X509 certificates. /// If we find a non-complex certificate format, we should switch to that. /// -/// The machine identifier the key is valid for +/// The hash of the certificate public key /// The certificate public key /// The date the certificate was obtained /// The expiry date of the certificate -/// The date the certificate was revoked, or null if not revoked public sealed record MiniServerCertificate( - string Identifier, + string PublicKeyHash, string PublicKey, DateTimeOffset Obtained, - DateTimeOffset Expiry, - DateTimeOffset? Revoked -) -{ - /// - /// Merges two sets of certificates, keeping the newest - /// - /// The new certificates - /// The old certificates - /// The merged certificates - public static IEnumerable MergeCertificates(IEnumerable? newcerts, IEnumerable? oldcerts) - => (newcerts ?? []).Concat(oldcerts ?? []) - .DistinctBy(x => x.Identifier) - .Where(x => x.Revoked == null) - .Where(x => x.Expiry > DateTimeOffset.UtcNow) - .ToArray(); - -} + DateTimeOffset Expiry +); \ No newline at end of file diff --git a/Duplicati/Server/webroot/ngax/scripts/controllers/SystemSettingsController.js b/Duplicati/Server/webroot/ngax/scripts/controllers/SystemSettingsController.js index ddbe9e74f..8af8f41e3 100644 --- a/Duplicati/Server/webroot/ngax/scripts/controllers/SystemSettingsController.js +++ b/Duplicati/Server/webroot/ngax/scripts/controllers/SystemSettingsController.js @@ -146,7 +146,7 @@ backupApp.controller('SystemSettingsController', function($rootScope, $scope, $r AppService.get('/remotecontrol/status').then(function(data) { mapRemoteControlStatus(data.data); - }, () => { }); + }, () => { }); } diff --git a/Duplicati/WebserverCore/Services/RemoteControllerRegistrationService.cs b/Duplicati/WebserverCore/Services/RemoteControllerRegistrationService.cs index 0ef80e28a..8af2c2a85 100644 --- a/Duplicati/WebserverCore/Services/RemoteControllerRegistrationService.cs +++ b/Duplicati/WebserverCore/Services/RemoteControllerRegistrationService.cs @@ -112,6 +112,7 @@ public class RemoteControllerRegistrationService(Connection connection, IHttpCli { Token = claimData.JWT, ServerCertificates = claimData.ServerCertificates, + CertificateUrl = claimData.CertificateUrl, ServerUrl = claimData.ServerUrl }); diff --git a/Duplicati/WebserverCore/Services/RemoteControllerService.cs b/Duplicati/WebserverCore/Services/RemoteControllerService.cs index 12b07ae09..c3a7fab05 100644 --- a/Duplicati/WebserverCore/Services/RemoteControllerService.cs +++ b/Duplicati/WebserverCore/Services/RemoteControllerService.cs @@ -79,6 +79,7 @@ public class RemoteControllerService(Connection connection, IHttpClientFactory h _keepRemoteConnection = KeepRemoteConnection.CreateRemoteListener( config.ServerUrl, config.Token, + config.CertificateUrl, config.ServerCertificates, CancellationToken.None, ReKey, @@ -100,7 +101,7 @@ public class RemoteControllerService(Connection connection, IHttpClientFactory h connection.ApplicationSettings.RemoteControlConfig = JsonConvert.SerializeObject(new RemoteControlConfig { Token = data.JWT, - ServerCertificates = MiniServerCertificate.MergeCertificates(data.ServerCertificates, oldCerts), + ServerCertificates = data.ServerCertificates ?? oldCerts ?? [], ServerUrl = data.ServerUrl, CertificateUrl = data.CertificateUrl });