From 8533b9833c25756c5106210cec8f6707c98b5761 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Thu, 9 Jul 2026 15:58:47 +0200 Subject: [PATCH 01/19] Improve TLS cert validation This addresses a logic issue in the previous check for TLS certs introduced with the option to ignore CRL failures. Since the call would end up calling `.Verify()` on the certificate, even toggling for soft-fail CRL issues, the certificate would be rejected. With the new logic we manually verify the chain. This is only done if the user has requested specific certificates should be validated as we otherwise rely on the OS cert validation. --- Duplicati/Library/Backend/WEBDAV/WEBDAV.cs | 6 +- .../Utility/SslCertificateValidator.cs | 76 ++++++++++++++++++- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/Duplicati/Library/Backend/WEBDAV/WEBDAV.cs b/Duplicati/Library/Backend/WEBDAV/WEBDAV.cs index e1d4586c0..5a1a4a104 100644 --- a/Duplicati/Library/Backend/WEBDAV/WEBDAV.cs +++ b/Duplicati/Library/Backend/WEBDAV/WEBDAV.cs @@ -365,11 +365,7 @@ namespace Duplicati.Library.Backend if (m_httpClient == null) { var httpHandler = new HttpClientHandler(); - - // Custom certificate validation to throw exception on failure - var validator = new SslCertificateValidator(m_certificateOptions.AcceptAllCertificates, m_certificateOptions.AcceptSpecificCertificateHashes, m_certificateOptions.IgnoreRevocationFailure); - httpHandler.ServerCertificateCustomValidationCallback = (sender, cert, chain, sslPolicyErrors) => - validator.ValidateServerCertificate(sender, cert, chain, sslPolicyErrors); + HttpClientHelper.ConfigureHandlerCertificateValidator(httpHandler, m_certificateOptions.AcceptAllCertificates, m_certificateOptions.AcceptSpecificCertificateHashes, m_certificateOptions.IgnoreRevocationFailure); if (m_useIntegratedAuthentication) { diff --git a/Duplicati/Library/Utility/SslCertificateValidator.cs b/Duplicati/Library/Utility/SslCertificateValidator.cs index cec74f9b7..8c2b320eb 100644 --- a/Duplicati/Library/Utility/SslCertificateValidator.cs +++ b/Duplicati/Library/Utility/SslCertificateValidator.cs @@ -59,7 +59,10 @@ public class SslCertificateValidator(bool acceptAll, string[]? validHashes, bool using var certificate = cert as X509Certificate2 ?? new X509Certificate2(cert ?? throw new ArgumentNullException(nameof(cert))); - // Validate date range before anything else, reject expired certs + // Validate date range before anything else, reject expired certs. + // NotBefore/NotAfter are returned by .NET as DateTime with Kind=Local (the UTC + // instant expressed in local time), so DateTime.Now (also Kind=Local) is the + // correct comparison; using UtcNow here would skew the window by the timezone offset. if (!IsDateValid(certificate, now)) return false; @@ -77,7 +80,11 @@ public class SslCertificateValidator(bool acceptAll, string[]? validHashes, bool } - // If requested, ignore revocation check failures (e.g. OCSP server offline or status unknown) + // If requested, ignore revocation check failures (e.g. OCSP server offline or status unknown). + // This strips revocation-only flags from the sslPolicyErrors that the TLS stack reported using + // its own chain object. The explicit chain built below operates on a separate X509Chain and + // applies the same soft-fail logic independently, so both the reported-policy path and the + // verify path honor ignoreRevocationFailure consistently. if (ignoreRevocationFailure) sslPolicyErrors = FilterRevocationErrors(sslPolicyErrors, chain); @@ -85,7 +92,70 @@ public class SslCertificateValidator(bool acceptAll, string[]? validHashes, bool throw new InvalidCertificateException(certificate.GetCertHashString(), sslPolicyErrors); // If no hash is found, perform the standard validations - return sslPolicyErrors == SslPolicyErrors.None && certificate.Verify(); + if (sslPolicyErrors != SslPolicyErrors.None) + return false; + + // certificate.Verify() builds its own X509Chain with a default policy that + // hard-fails when a revocation check cannot be completed (OCSP/CRL endpoint + // unreachable). The OS-native TLS stacks instead soft-fail in that situation: + // the certificate is accepted when the revocation status is merely unknown + // (not confirmed revoked). Build the chain explicitly so we can replicate that + // behavior and honor the ignoreRevocationFailure setting. + // + // Note: ChainPolicy.TrustMode is left at its default (UseSystemDefault), which + // resolves intermediate/root trust through the OS trust store exactly as + // certificate.Verify() did, so the trust-root behaviour of the previous + // implementation is preserved. + using var verifyChain = new X509Chain(); + // The primary mechanism for honoring ignoreRevocationFailure: skip the online + // revocation fetch entirely so an unreachable OCSP/CRL endpoint cannot fail the + // chain. The post-build soft-fail below is a safety net for platforms that + // still surface revocation status flags even under NoCheck, not the main path. + verifyChain.ChainPolicy.RevocationMode = + ignoreRevocationFailure ? X509RevocationMode.NoCheck : X509RevocationMode.Online; + bool chainValid = verifyChain.Build(certificate); + + // Soft-fail on unreachable revocation checks, but only when the caller has + // explicitly requested it via ignoreRevocationFailure. This replicates the + // OS-native TLS behavior of accepting a certificate whose revocation status is + // merely unknown (OCSP/CRL endpoint unreachable) rather than confirmed revoked. + // + // This block is a safety net for the NoCheck mode above: on some platforms + // Build() can still report revocation-related flags despite RevocationMode being + // NoCheck, so strip those flags here rather than relying on NoCheck alone. + // + // The guard requires a non-empty ChainStatus: if Build() returned false with no + // status flags (rare platform edge case), we must not silently accept the + // certificate, as that would widen trust beyond what any status indicates. + // A confirmed Revoked flag, PartialChain, or any other non-revocation status + // still fails the chain because the All() predicate would be false. The using + // scope ensures verifyChain is disposed even if Build() throws and the outer + // catch wraps the exception. + if (!chainValid && ignoreRevocationFailure && verifyChain.ChainStatus.Length > 0 && + verifyChain.ChainStatus.All(s => (s.Status & ~RevocationFailureFlags) == 0)) + chainValid = true; + + if (!chainValid) + { + var chainStatus = string.Join("; ", verifyChain.ChainStatus.Select(s => $"{s.Status}={s.StatusInformation?.Trim()}")); + var elementDetails = verifyChain.ChainElements.Count == 0 + ? "(no chain elements)" + : string.Join(" | ", verifyChain.ChainElements.Select(e => + { + var elemStatus = e.ChainElementStatus.Length == 0 + ? "(none)" + : string.Join(", ", e.ChainElementStatus.Select(s => $"{s.Status}={s.StatusInformation?.Trim()}")); + return $"{e.Certificate.Subject} [{elemStatus}]"; + })); + + Duplicati.Library.Logging.Log.WriteWarningMessage( + "SslCertificateValidator", + "VerifyChainFailed", + null, + $"Certificate chain validation failed for {certificate.Subject} (issuer={certificate.Issuer}, " + + $"thumbprint={certificate.Thumbprint}). Chain status: {chainStatus}. Per-element: {elementDetails}"); + } + return chainValid; } catch (InvalidCertificateException) { From 95b7ed29e2a8f4efe575d8c1fc43ede0935fd62a Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Fri, 10 Jul 2026 20:17:38 +0900 Subject: [PATCH 02/19] Avoid secondary transaction errors after repair failure --- .../Main/Database/ReusableTransaction.cs | 26 +++++++++-- Duplicati/UnitTest/Issue5827.cs | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 Duplicati/UnitTest/Issue5827.cs diff --git a/Duplicati/Library/Main/Database/ReusableTransaction.cs b/Duplicati/Library/Main/Database/ReusableTransaction.cs index 3e107369b..ac38d30b8 100644 --- a/Duplicati/Library/Main/Database/ReusableTransaction.cs +++ b/Duplicati/Library/Main/Database/ReusableTransaction.cs @@ -121,17 +121,37 @@ internal class ReusableTransaction(SqliteConnection con, SqliteTransaction? tran } catch (Exception ex) { - Logging.Log.WriteErrorMessage(LOGTAG, "ReusableTransaction dispose", ex, "Transaction disposed with error: {0}", ex.Message); - throw; + if (!IsInactiveTransactionException(ex)) + { + Logging.Log.WriteErrorMessage(LOGTAG, "ReusableTransaction dispose", ex, "Transaction disposed with error: {0}", ex.Message); + throw; + } + + Logging.Log.WriteVerboseMessage(LOGTAG, "ReusableTransactionAlreadyCompleted", ex, "Transaction was already completed during dispose: {0}", ex.Message); } finally { m_disposed = true; - await m_transaction.DisposeAsync().ConfigureAwait(false); + try + { + await m_transaction.DisposeAsync().ConfigureAwait(false); + } + catch (Exception ex) + { + if (!IsInactiveTransactionException(ex)) + throw; + + Logging.Log.WriteVerboseMessage(LOGTAG, "ReusableTransactionAlreadyDisposed", ex, "Transaction was already completed before dispose: {0}", ex.Message); + } } } } + private static bool IsInactiveTransactionException(Exception ex) + => ex.Message.Contains("No transaction is active", StringComparison.OrdinalIgnoreCase) + || ex.Message.Contains("transaction has completed", StringComparison.OrdinalIgnoreCase) + || ex.Message.Contains("no longer usable", StringComparison.OrdinalIgnoreCase); + /// /// Rolls back the transaction and restarts it. /// diff --git a/Duplicati/UnitTest/Issue5827.cs b/Duplicati/UnitTest/Issue5827.cs new file mode 100644 index 000000000..ec653818a --- /dev/null +++ b/Duplicati/UnitTest/Issue5827.cs @@ -0,0 +1,44 @@ +// Copyright (C) 2026, The Duplicati Team +// https://duplicati.com, hello@duplicati.com +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the "Software"), +// to deal in the Software without restriction, including without limitation +// the rights to use, copy, modify, merge, publish, distribute, sublicense, +// and/or sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Threading.Tasks; +using Duplicati.Library.Main.Database; +using Microsoft.Data.Sqlite; +using NUnit.Framework; + +namespace Duplicati.UnitTest; + +public class Issue5827 +{ + [Test] + public async Task DisposeDoesNotThrowWhenTransactionAlreadyCompletedAsync() + { + await using var connection = new SqliteConnection("Data Source=:memory:"); + await connection.OpenAsync(); + + var transaction = (SqliteTransaction)await connection.BeginTransactionAsync(); + await using var reusableTransaction = new ReusableTransaction(connection, transaction); + + await transaction.CommitAsync(); + + Assert.DoesNotThrowAsync(async () => await reusableTransaction.DisposeAsync().AsTask()); + } +} From 069c838ccbaf00e75248cffd913ccef8c92953e3 Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Fri, 10 Jul 2026 20:36:43 +0900 Subject: [PATCH 03/19] Address review: warn on inactive-transaction dispose, match by exception type - Log the "transaction already completed/disposed" cases as warnings instead of verbose messages, so they are captured in tests and the underlying issue can be fixed (per review feedback). - Replace the fragile message-string matching in IsInactiveTransactionException with an exception-type check. Microsoft.Data.Sqlite throws InvalidOperationException ("This SqliteTransaction has completed; it is no longer usable.") for an already-completed transaction; matching the type avoids breaking under localization or matching unintended messages (verified locally via the Issue5827 test, which catches System.InvalidOperationException). Co-Authored-By: Claude Opus 4.8 --- .../Library/Main/Database/ReusableTransaction.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Duplicati/Library/Main/Database/ReusableTransaction.cs b/Duplicati/Library/Main/Database/ReusableTransaction.cs index ac38d30b8..b9b36039f 100644 --- a/Duplicati/Library/Main/Database/ReusableTransaction.cs +++ b/Duplicati/Library/Main/Database/ReusableTransaction.cs @@ -127,7 +127,7 @@ internal class ReusableTransaction(SqliteConnection con, SqliteTransaction? tran throw; } - Logging.Log.WriteVerboseMessage(LOGTAG, "ReusableTransactionAlreadyCompleted", ex, "Transaction was already completed during dispose: {0}", ex.Message); + Logging.Log.WriteWarningMessage(LOGTAG, "ReusableTransactionAlreadyCompleted", ex, "Transaction was already completed during dispose: {0}", ex.Message); } finally { @@ -141,16 +141,17 @@ internal class ReusableTransaction(SqliteConnection con, SqliteTransaction? tran if (!IsInactiveTransactionException(ex)) throw; - Logging.Log.WriteVerboseMessage(LOGTAG, "ReusableTransactionAlreadyDisposed", ex, "Transaction was already completed before dispose: {0}", ex.Message); + Logging.Log.WriteWarningMessage(LOGTAG, "ReusableTransactionAlreadyDisposed", ex, "Transaction was already completed before dispose: {0}", ex.Message); } } } } + // Microsoft.Data.Sqlite throws InvalidOperationException ("This SqliteTransaction has completed; + // it is no longer usable.") when a transaction is rolled back or disposed after it has already + // been completed. Match on the exception type rather than the (localizable) message text. private static bool IsInactiveTransactionException(Exception ex) - => ex.Message.Contains("No transaction is active", StringComparison.OrdinalIgnoreCase) - || ex.Message.Contains("transaction has completed", StringComparison.OrdinalIgnoreCase) - || ex.Message.Contains("no longer usable", StringComparison.OrdinalIgnoreCase); + => ex is InvalidOperationException; /// /// Rolls back the transaction and restarts it. From c091b3e6fc78de55ef7ec2bc1820714413614a07 Mon Sep 17 00:00:00 2001 From: JamBalaya56562 Date: Fri, 10 Jul 2026 21:56:03 +0900 Subject: [PATCH 04/19] Make CLI find search all backup versions by default (fixes #2287) The `find` and `list` commands share the same handler. A bare filename is rewritten to a wildcard filter (e.g. `*/report.txt`), and the handler only searches every version for a Simple filter or when --all-versions is set, so a wildcard filter searches only the newest version. The all-versions fallback in Commands.List only triggers when the newest pass returns zero files, so a file present only in an older version was silently missed whenever the newest version still contained a same-named file (and full-path finds behaved differently from bare-name finds). Give `find` its own entry point that defaults to all-versions (respecting an explicit --all-versions/--version/--time), so `find ` reliably searches every backup version. `list` keeps its documented newest-only default. Update the help text and add a regression test (Issue2287) that a file present only in an older version is found by `find` but not by `list`. Co-Authored-By: Claude Opus 4.8 --- Duplicati/CommandLine/CLI/Commands.cs | 11 +++ Duplicati/CommandLine/CLI/Program.cs | 4 +- Duplicati/CommandLine/CLI/help.txt | 2 +- Duplicati/UnitTest/Issue2287.cs | 100 ++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 Duplicati/UnitTest/Issue2287.cs diff --git a/Duplicati/CommandLine/CLI/Commands.cs b/Duplicati/CommandLine/CLI/Commands.cs index b2376a3ec..f8e4f4892 100644 --- a/Duplicati/CommandLine/CLI/Commands.cs +++ b/Duplicati/CommandLine/CLI/Commands.cs @@ -498,6 +498,17 @@ namespace Duplicati.CommandLine return 0; } + public static int Find(TextWriter outwriter, Action setup, List args, Dictionary options, Library.Utility.IFilter filter) + { + // Unlike "list" (which shows only the newest version by default), "find" searches every + // backup version, so a file is located regardless of which version it appears in. + // Respect an explicit --all-versions / --version / --time supplied by the user. + if (!options.ContainsKey("all-versions") && !options.ContainsKey("version") && !options.ContainsKey("time")) + options["all-versions"] = "true"; + + return List(outwriter, setup, args, options, filter); + } + public static int List(TextWriter outwriter, Action setup, List args, Dictionary options, Library.Utility.IFilter filter) { filter = filter ?? new Duplicati.Library.Utility.FilterExpression(); diff --git a/Duplicati/CommandLine/CLI/Program.cs b/Duplicati/CommandLine/CLI/Program.cs index 4886e856f..1a2f0150b 100644 --- a/Duplicati/CommandLine/CLI/Program.cs +++ b/Duplicati/CommandLine/CLI/Program.cs @@ -76,7 +76,7 @@ namespace Duplicati.CommandLine ["help"] = Commands.Help, ["example"] = Commands.Examples, ["examples"] = Commands.Examples, - ["find"] = Commands.List, + ["find"] = Commands.Find, ["list"] = Commands.List, ["list-filesets"] = Commands.ListFilesets, ["list-folder-content"] = Commands.ListFolderContent, @@ -107,7 +107,7 @@ namespace Duplicati.CommandLine ["readlockinfo"] = Commands.ReadLockInfo, ["system-info"] = Commands.SystemInfo, ["systeminfo"] = Commands.SystemInfo, - ["send-mail"] = Commands.SendMail, + ["send-mail"] = Commands.SendMail, ["sync"] = Commands.Sync }; diff --git a/Duplicati/CommandLine/CLI/help.txt b/Duplicati/CommandLine/CLI/help.txt index 815996de9..5956711ce 100644 --- a/Duplicati/CommandLine/CLI/help.txt +++ b/Duplicati/CommandLine/CLI/help.txt @@ -103,7 +103,7 @@ Usage: %CLI_EXE% backup "" [] Usage: %CLI_EXE% find [""] [] - Finds specific files in specific backups. If is specified, all occurrences of in the backup are listed. can contain * and ? as wildcards. File names in [brackets] are interpreted as regular expression. Latest backup is searched by default. If entire path is specified, all available versions of the file are listed. If no is specified, a list of all available backups is shown. + Finds specific files in specific backups. If is specified, all occurrences of in the backup are listed. can contain * and ? as wildcards. File names in [brackets] are interpreted as regular expression. The "find" command searches all backup versions by default, while "list" shows only the latest version by default (use --all-versions with "list" to search every version). If no is specified, a list of all available backups is shown. --time=