From 55478523aaa152d73101bbf1761116d2e4ec4e9a Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Thu, 25 Sep 2025 13:11:39 +0200 Subject: [PATCH] Fixed masked passwords breaking test function This updates the masked password feature to be better at detecting variations of the password mask, such as removing one or more asterisks from the string. This also updates the "Test destination" endpoints to support a backupId so the call can unmask the target urls. --- Duplicati/Library/RestAPI/Database/Backup.cs | 2 +- .../Library/RestAPI/Database/Connection.cs | 36 +++++++++++++++---- .../RestAPI/Database/QuerystringMasking.cs | 4 +-- .../ngax/scripts/directives/backupEditUri.js | 6 ++-- .../webroot/ngax/templates/addoredit.html | 2 +- Duplicati/UnitTest/MaskingUnmaskingTest.cs | 11 ++++-- .../Dto/V2/DestinationTestRequestDto.cs | 5 +++ .../Endpoints/V1/RemoteOperation.cs | 29 +++++++++++---- .../Endpoints/V2/DestinationVerify.cs | 15 +++++++- 9 files changed, 87 insertions(+), 23 deletions(-) diff --git a/Duplicati/Library/RestAPI/Database/Backup.cs b/Duplicati/Library/RestAPI/Database/Backup.cs index 791436cbe..0eb658ed6 100644 --- a/Duplicati/Library/RestAPI/Database/Backup.cs +++ b/Duplicati/Library/RestAPI/Database/Backup.cs @@ -166,7 +166,7 @@ namespace Duplicati.Server.Database var prevSettings = previous.Settings.ToDictionary(x => x.Name, x => x.Value, StringComparer.OrdinalIgnoreCase); foreach (var setting in this.Settings) { - if (setting.Value == Connection.PASSWORD_PLACEHOLDER) + if (Connection.IsPasswordPlaceholder(setting.Value)) { if (prevSettings.TryGetValue(setting.Name, out var prevValue)) setting.Value = prevValue; diff --git a/Duplicati/Library/RestAPI/Database/Connection.cs b/Duplicati/Library/RestAPI/Database/Connection.cs index 11110241b..dee9642fa 100644 --- a/Duplicati/Library/RestAPI/Database/Connection.cs +++ b/Duplicati/Library/RestAPI/Database/Connection.cs @@ -33,6 +33,7 @@ using System.Data; using Duplicati.Library.Main.Database; using System.Globalization; using Microsoft.Extensions.DependencyInjection; +using System.Text.RegularExpressions; #nullable enable @@ -309,6 +310,29 @@ namespace Duplicati.Server.Database } } + /// + /// Checks if the given value is the password placeholder. + /// + /// The value to check. + /// true if the value is the password placeholder; otherwise false. + public static bool IsPasswordPlaceholder(string? value) + => string.IsNullOrWhiteSpace(value) + ? false + : value.Length > 3 && value.All(c => c == '*'); + + /// + /// Regular expression to match a URL password mask (3 or more asterisks or %2A) + /// + private static Regex UrlPasswordMaskRegex = new Regex(@"(\*|%2A){3,}", RegexOptions.Compiled | RegexOptions.IgnoreCase); + + /// + /// Checks if the given url contains a password placeholder. + /// + /// The url to check. + /// true if the value contains a password placeholder; otherwise false. + public static bool UrlContainsPasswordPlaceholder(string? url) + => !string.IsNullOrWhiteSpace(url) && UrlPasswordMaskRegex.IsMatch(url); + public ISetting[] GetSettings(long id) { lock (m_lock) @@ -346,7 +370,7 @@ namespace Duplicati.Server.Database cmd => cmd.SetCommandAndParameters(@"INSERT INTO ""Option"" (""BackupID"", ""Filter"", ""Name"", ""Value"") VALUES (@BackupId, @Filter, @Name, @Value)"), (cmd, f) => { - if (PASSWORD_PLACEHOLDER.Equals(f.Value)) + if (IsPasswordPlaceholder(f.Value)) throw new Exception("Attempted to save a property with the placeholder password"); cmd.SetParameterValue("@BackupId", id) @@ -543,10 +567,10 @@ namespace Duplicati.Server.Database if (string.IsNullOrWhiteSpace(item.TargetURL)) return "Missing a target"; - if (item.TargetURL.Contains(PASSWORD_PLACEHOLDER)) - return "TargetURL contains password placeholder value"; - if (item.Settings != null && item.Settings.Any(x => PASSWORD_PLACEHOLDER.Equals(x.Value))) - return "Settings contains password placeholder value"; + if (UrlContainsPasswordPlaceholder(item.TargetURL)) + return "TargetURL is password placeholder value"; + if (item.Settings != null && item.Settings.Any(x => IsPasswordPlaceholder(x.Value))) + return "Settings is password placeholder value"; if (item.Sources == null || item.Sources.Any(x => string.IsNullOrWhiteSpace(x)) || item.Sources.Length == 0) return "Invalid source list"; @@ -699,7 +723,7 @@ namespace Duplicati.Server.Database }, (cmd, n) => { - if (n.TargetURL.IndexOf(PASSWORD_PLACEHOLDER, StringComparison.Ordinal) >= 0) + if (UrlContainsPasswordPlaceholder(n.TargetURL)) throw new Exception("Attempted to save a backup with the password placeholder"); if (update && long.Parse(n.ID) <= 0) throw new Exception("Invalid update, cannot update application settings through update method"); diff --git a/Duplicati/Library/RestAPI/Database/QuerystringMasking.cs b/Duplicati/Library/RestAPI/Database/QuerystringMasking.cs index 00037210f..7bf5966d7 100644 --- a/Duplicati/Library/RestAPI/Database/QuerystringMasking.cs +++ b/Duplicati/Library/RestAPI/Database/QuerystringMasking.cs @@ -97,8 +97,8 @@ public static class QuerystringMasking var newValues = GetValuesCaseInsensitive(newQuery, key); if (newValues is null || newValues.Length == 0) continue; - // If any value is the mask, replace the entire set for that key from the previous URL - if (newValues.Any(v => string.Equals(v, Connection.PASSWORD_PLACEHOLDER, StringComparison.Ordinal))) + // If any value contains the mask, replace the entire set for that key from the previous URL + if (newValues.Any(v => Connection.IsPasswordPlaceholder(v))) { var prevValues = GetValuesCaseInsensitive(prevQuery, key); if (prevValues is null || prevValues.Length == 0) diff --git a/Duplicati/Server/webroot/ngax/scripts/directives/backupEditUri.js b/Duplicati/Server/webroot/ngax/scripts/directives/backupEditUri.js index d1e35318d..21a1b68cb 100644 --- a/Duplicati/Server/webroot/ngax/scripts/directives/backupEditUri.js +++ b/Duplicati/Server/webroot/ngax/scripts/directives/backupEditUri.js @@ -3,10 +3,11 @@ backupApp.directive('backupEditUri', function(gettextCatalog) { restrict: 'E', scope: { uri: '=uri', + backupId: '=backupId', setBuilduriFn: '&' }, templateUrl: 'templates/edituri.html', - controller: function($scope, AppService, AppUtils, SystemInfo, EditUriBackendConfig, DialogService, EditUriBuiltins) { + controller: function($scope, AppService, AppUtils, SystemInfo, EditUriBackendConfig, DialogService) { var scope = $scope; scope.AppUtils = AppUtils; @@ -41,7 +42,7 @@ backupApp.directive('backupEditUri', function(gettextCatalog) { dlg.dismiss(); dlg = DialogService.dialog(gettextCatalog.getString('Testing …'), gettextCatalog.getString('Testing connection …'), [], null, function() { - AppService.postJson('/remoteoperation/test', { path: uri }).then(function() { + AppService.postJson('/remoteoperation/test', { path: uri, backupId: scope.backupId }).then(function() { scope.Testing = false; dlg.dismiss(); @@ -202,6 +203,7 @@ backupApp.directive('backupEditUri', function(gettextCatalog) { } $scope.testConnection = function() { + console.log(scope); builduri(performConnectionTest); }; diff --git a/Duplicati/Server/webroot/ngax/templates/addoredit.html b/Duplicati/Server/webroot/ngax/templates/addoredit.html index c626bfd4d..0965f7ee8 100644 --- a/Duplicati/Server/webroot/ngax/templates/addoredit.html +++ b/Duplicati/Server/webroot/ngax/templates/addoredit.html @@ -107,7 +107,7 @@ - +
diff --git a/Duplicati/UnitTest/MaskingUnmaskingTest.cs b/Duplicati/UnitTest/MaskingUnmaskingTest.cs index b56dd9b0a..ecbdd74bd 100644 --- a/Duplicati/UnitTest/MaskingUnmaskingTest.cs +++ b/Duplicati/UnitTest/MaskingUnmaskingTest.cs @@ -21,7 +21,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Runtime.Serialization; using System.Web; using Duplicati.Server.Database; @@ -146,9 +145,15 @@ public class BackupConfigMaskingTests } [Test] - public void ValidateBackupDetectsPlaceholder() + public void ValidateBackupDetectsPlaceholder([Values(0, 1, 2)] int type) { - var placeholder = Connection.PASSWORD_PLACEHOLDER; + var placeholder = type switch + { + 0 => Connection.PASSWORD_PLACEHOLDER, + 1 => "***", + 2 => "%2A%2a%2A", + _ => throw new ArgumentOutOfRangeException(nameof(type)) + }; var backup = new Backup { diff --git a/Duplicati/WebserverCore/Dto/V2/DestinationTestRequestDto.cs b/Duplicati/WebserverCore/Dto/V2/DestinationTestRequestDto.cs index 60fe93426..dbf4d2ff2 100644 --- a/Duplicati/WebserverCore/Dto/V2/DestinationTestRequestDto.cs +++ b/Duplicati/WebserverCore/Dto/V2/DestinationTestRequestDto.cs @@ -30,6 +30,11 @@ public sealed record DestinationTestRequestDto /// public required string DestinationUrl { get; init; } + /// + /// The backup ID, if known + /// + public string? BackupId { get; init; } + /// /// Any additional options to pass to the destination /// diff --git a/Duplicati/WebserverCore/Endpoints/V1/RemoteOperation.cs b/Duplicati/WebserverCore/Endpoints/V1/RemoteOperation.cs index e8a4c32c7..6ef6735cd 100644 --- a/Duplicati/WebserverCore/Endpoints/V1/RemoteOperation.cs +++ b/Duplicati/WebserverCore/Endpoints/V1/RemoteOperation.cs @@ -32,7 +32,7 @@ namespace Duplicati.WebserverCore.Endpoints.V1 { public class RemoteOperation : IEndpointV1 { - private record RemoteOperationInput(string path); + private record RemoteOperationInput(string path, string? backupId); public static void Map(RouteGroupBuilder group) { @@ -41,11 +41,11 @@ namespace Duplicati.WebserverCore.Endpoints.V1 .RequireAuthorization(); group.MapPost("/remoteoperation/test", ([FromServices] Connection connection, [FromServices] IApplicationSettings applicationSettings, [FromQuery] bool? autocreate, [FromBody] RemoteOperationInput input, CancellationToken cancelToken) - => ExecuteTest(connection, applicationSettings, input.path, autocreate ?? false, cancelToken)) + => ExecuteTest(connection, applicationSettings, input.path, input.backupId, autocreate ?? false, cancelToken)) .RequireAuthorization(); - group.MapPost("/remoteoperation/create", ([FromBody] RemoteOperationInput input, CancellationToken cancelToken) - => ExecuteCreate(input.path, cancelToken)) + group.MapPost("/remoteoperation/create", ([FromServices] Connection connection, [FromBody] RemoteOperationInput input, CancellationToken cancelToken) + => ExecuteCreate(connection, input.path, input.backupId, cancelToken)) .RequireAuthorization(); } @@ -121,12 +121,26 @@ namespace Duplicati.WebserverCore.Endpoints.V1 } } - private static async Task ExecuteTest(Connection connection, IApplicationSettings applicationSettings, string url, bool autoCreate, CancellationToken cancelToken) + private static string UnmaskUrl(Connection connection, string maskedurl, string? backupId) + { + var previousUrl = !string.IsNullOrWhiteSpace(backupId) ? connection.GetBackup(backupId)?.TargetURL : null; + var unmasked = string.IsNullOrWhiteSpace(previousUrl) + ? maskedurl + : QuerystringMasking.Unmask(maskedurl, previousUrl); + + if (Connection.UrlContainsPasswordPlaceholder(unmasked)) + throw new ArgumentException("Unmasked URL contains password placeholder"); + + return unmasked; + } + + private static async Task ExecuteTest(Connection connection, IApplicationSettings applicationSettings, string maskedurl, string? backupId, bool autoCreate, CancellationToken cancelToken) { TupleDisposeWrapper? wrapper = null; try { + var url = UnmaskUrl(connection, maskedurl, backupId); wrapper = await GetBackend(connection, applicationSettings, url, cancelToken); using (var b = wrapper.Backend) @@ -181,11 +195,12 @@ namespace Duplicati.WebserverCore.Endpoints.V1 } - private static async Task ExecuteCreate(string uri, CancellationToken cancelToken) + private static async Task ExecuteCreate(Connection connection, string maskedurl, string? backupId, CancellationToken cancelToken) { try { - using (var b = Duplicati.Library.DynamicLoader.BackendLoader.GetBackend(uri, new Dictionary())) + var url = UnmaskUrl(connection, maskedurl, backupId); + using (var b = Duplicati.Library.DynamicLoader.BackendLoader.GetBackend(url, new Dictionary())) await b.CreateFolderAsync(cancelToken).ConfigureAwait(false); } catch (UserInformationException uex) diff --git a/Duplicati/WebserverCore/Endpoints/V2/DestinationVerify.cs b/Duplicati/WebserverCore/Endpoints/V2/DestinationVerify.cs index 8d041dceb..079be069b 100644 --- a/Duplicati/WebserverCore/Endpoints/V2/DestinationVerify.cs +++ b/Duplicati/WebserverCore/Endpoints/V2/DestinationVerify.cs @@ -108,6 +108,18 @@ public class DestinationVerify : IEndpointV2 } } + private static string UnmaskUrl(Connection connection, string maskedurl, string? backupId) + { + var previousUrl = !string.IsNullOrWhiteSpace(backupId) ? connection.GetBackup(backupId)?.TargetURL : null; + var unmasked = string.IsNullOrWhiteSpace(previousUrl) + ? maskedurl + : QuerystringMasking.Unmask(maskedurl, previousUrl); + + if (Connection.UrlContainsPasswordPlaceholder(unmasked)) + throw new ArgumentException("Unmasked URL contains password placeholder"); + + return unmasked; + } private static async Task ExecuteTest(Connection connection, IApplicationSettings applicationSettings, DestinationTestRequestDto input, CancellationToken cancelToken) { @@ -115,7 +127,8 @@ public class DestinationVerify : IEndpointV2 try { - wrapper = await GetBackend(connection, applicationSettings, input.DestinationUrl, cancelToken); + var url = UnmaskUrl(connection, input.DestinationUrl, input.BackupId); + wrapper = await GetBackend(connection, applicationSettings, url, cancelToken); using (var b = wrapper.Backend) {