From c79d6021c4e10c39b323974a5439cf5eb3891f7b Mon Sep 17 00:00:00 2001 From: byron Date: Thu, 22 Oct 2026 23:09:29 -0400 Subject: [PATCH 1/2] Check the update installer's Authenticode signer before running it The installer and its SHA256SUMS file sit on the same GitHub release, so the checksum only shows the download is intact: whoever could replace the installer could replace the checksum too. The updater now also asks WinVerifyTrust whether the installer's signature is trusted (revocation included) and compares its signer with the publisher that signed the running Filee. A different or missing signer discards the installer and falls back to the download page. Copies that are not signed themselves (built from source) keep to the checksum. Co-Authored-By: Claude Opus 5.5 --- SECURITY.md | 3 +- src/Filee.App/Services/UpdateService.cs | 36 +++++- src/Filee.Platform.Windows/Authenticode.cs | 135 ++++++++++++++++++++ tests/Filee.App.Tests/UpdateServiceTests.cs | 87 +++++++++++++ 4 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 src/Filee.Platform.Windows/Authenticode.cs diff --git a/SECURITY.md b/SECURITY.md index 19661ef92..142c081c0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -41,7 +41,8 @@ Out of scope: ## How downloads are checked - **Updates:** an installed copy downloads the new installer from GitHub Releases and checks it against the release's - `SHA256SUMS` file before it runs it. + `SHA256SUMS` file before it runs it. It also checks that Windows trusts the installer's Authenticode signature and + that the publisher who signed the installed Filee signed it; an installer signed by anyone else is discarded. - **Engines:** every engine comes from the official location pinned in [`engines.json`](src/Filee.Engines/Infrastructure/engines.json), with its SHA-256 and size. A download that doesn't match is discarded. diff --git a/src/Filee.App/Services/UpdateService.cs b/src/Filee.App/Services/UpdateService.cs index 12d5d6171..339ebe2fb 100644 --- a/src/Filee.App/Services/UpdateService.cs +++ b/src/Filee.App/Services/UpdateService.cs @@ -1,5 +1,6 @@ // Update check against GitHub Releases. When a newer version is out the app says so (sidebar, notice, tray). In an -// installed copy "Update" downloads the new installer, checks it against the release's SHA256SUMS file and runs it; +// installed copy "Update" downloads the new installer, checks it against the release's SHA256SUMS file and that the +// publisher who signed this copy signed it too (Authenticode), and runs it; // Setup closes Filee, updates it in place (settings are kept) and starts it again. A portable copy, a copy run from // source, or a failed download opens a download page instead. The check works in every build. @@ -11,6 +12,7 @@ using System.Text.Json; using Avalonia.Threading; using CommunityToolkit.Mvvm.ComponentModel; +using Filee.Platform.Windows; using Microsoft.Extensions.Logging; namespace Filee.App.Services; @@ -96,6 +98,15 @@ internal static bool IsInstalledIn(string folder) => /// Whether installs by itself (; tests set it). internal bool InstallsItself { get; init; } = IsInstalled; + /// + /// Who signed a program, if Windows trusts the signature (; the flag also + /// checks revocation). Tests replace it. + /// + internal Func SignerOf { get; init; } = Authenticode.TrustedSigner; + + /// This copy of Filee, whose publisher must have signed the installer too (tests set it). + internal string? OwnProgram { get; init; } = Environment.ProcessPath; + /// The newer version found by the last check (e.g. "1.0.2"), or null when up to date / unknown. [ObservableProperty] [NotifyPropertyChangedFor(nameof(IsUpdateAvailable))] @@ -268,6 +279,7 @@ public async Task UpdateAsync() { var progress = new Progress(percent => DownloadPercent = percent); installer = await DownloadInstallerAsync(Path.Combine(Path.GetTempPath(), "Filee-update"), progress); + CheckPublisher(installer); } catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException or IOException or InvalidDataException or UnauthorizedAccessException) @@ -344,6 +356,28 @@ internal async Task DownloadInstallerAsync(string folder, IProgress return target; } + /// + /// Throws (and deletes the installer) unless the publisher that signed this copy + /// of Filee signed as well. The checksum file sits on the same release as the + /// installer, so it can't tell a replaced installer from the real one; the signature can. A copy that is not + /// signed itself (built from source) has no publisher to compare with and keeps to the checksum. + /// + internal void CheckPublisher(string installer) + { + var publisher = OwnProgram is null ? null : SignerOf(OwnProgram, false); + if (publisher is null) + { + _log.LogInformation("This copy of Filee is not signed: the installer is checked by its SHA-256 only"); + return; + } + var signer = SignerOf(installer, true); + if (signer == publisher) + return; + TryDelete(installer); + throw new InvalidDataException( + $"{Path.GetFileName(installer)} is signed by {signer ?? "no publisher Windows trusts"}, not by {publisher}"); + } + private async Task DownloadAsync(ReleaseFile file, string path, IProgress? progress, CancellationToken cancellationToken) { using var stall = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); diff --git a/src/Filee.Platform.Windows/Authenticode.cs b/src/Filee.Platform.Windows/Authenticode.cs new file mode 100644 index 000000000..118253bee --- /dev/null +++ b/src/Filee.Platform.Windows/Authenticode.cs @@ -0,0 +1,135 @@ +// Authenticode signatures of programs: whether Windows trusts a file's signature, and who signed it. +// +// The in-app updater checks the installer it downloaded with this before running it. The SHA-256 from the release's +// checksum file only shows the download arrived intact; whoever could replace the installer on the release could +// replace that file too. The signature shows the installer comes from the publisher that signed this copy of Filee. +// WinVerifyTrust checks the signature and its chain, and the signer is read from the same check, so the certificate +// compared is the one Windows verified. + +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; + +namespace Filee.Platform.Windows; + +/// Authenticode checks. Every member is safe to call on any OS. +public static partial class Authenticode +{ + /// + /// The subject of the certificate that signed (e.g. "CN=Open Source Developer, …"), or null + /// when the file is not signed, Windows does not trust its signature, or this is not Windows. + /// + /// + /// Also asks the certificate authorities whether a certificate in the chain was revoked (needs the network). + /// + public static string? TrustedSigner(string path, bool checkRevocation) + { + if (!OperatingSystem.IsWindows()) + return null; + try + { + return Verify(path, checkRevocation); + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException or CryptographicException) + { + return null; + } + } + + [SupportedOSPlatform("windows")] + private static unsafe string? Verify(string path, bool checkRevocation) + { + var action = GenericVerifyV2; + fixed (char* file = Path.GetFullPath(path)) + { + var fileInfo = new WINTRUST_FILE_INFO { cbStruct = (uint)sizeof(WINTRUST_FILE_INFO), pcwszFilePath = (nint)file }; + var data = new WINTRUST_DATA + { + cbStruct = (uint)sizeof(WINTRUST_DATA), + dwUIChoice = WTD_UI_NONE, + fdwRevocationChecks = checkRevocation ? WTD_REVOKE_WHOLECHAIN : WTD_REVOKE_NONE, + dwUnionChoice = WTD_CHOICE_FILE, + pFile = (nint)(&fileInfo), + dwStateAction = WTD_STATEACTION_VERIFY, + dwProvFlags = checkRevocation ? WTD_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT : 0, + }; + try + { + if (WinVerifyTrust(InvalidWindow, ref action, ref data) != 0) + return null; + // The signer's own certificate: the first one of the first signer's chain. + var provider = WTHelperProvDataFromStateData(data.hWVTStateData); + var signer = provider == 0 ? 0 : WTHelperGetProvSignerFromChain(provider, 0, false, 0); + var certificate = signer == 0 ? 0 : WTHelperGetProvCertFromChain(signer, 0); + if (certificate == 0) + return null; + // CRYPT_PROVIDER_CERT: a DWORD cbStruct, then (aligned to a pointer) the PCCERT_CONTEXT. + var context = Marshal.ReadIntPtr(certificate, IntPtr.Size); + if (context == 0) + return null; + using var x509 = new X509Certificate2(context); + return x509.Subject; + } + finally + { + data.dwStateAction = WTD_STATEACTION_CLOSE; + WinVerifyTrust(InvalidWindow, ref action, ref data); + } + } + } + + /// WINTRUST_ACTION_GENERIC_VERIFY_V2: the Authenticode policy. + private static readonly Guid GenericVerifyV2 = new("00AAC56B-CD44-11D0-8CC2-00C04FC295EE"); + + /// INVALID_HANDLE_VALUE: no window, so WinVerifyTrust never asks the user. + private const nint InvalidWindow = -1; + + private const uint WTD_UI_NONE = 2; + private const uint WTD_REVOKE_NONE = 0; + private const uint WTD_REVOKE_WHOLECHAIN = 1; + private const uint WTD_CHOICE_FILE = 1; + private const uint WTD_STATEACTION_VERIFY = 1; + private const uint WTD_STATEACTION_CLOSE = 2; + private const uint WTD_REVOCATION_CHECK_CHAIN_EXCLUDE_ROOT = 0x80; + + [StructLayout(LayoutKind.Sequential)] + private struct WINTRUST_FILE_INFO + { + public uint cbStruct; + public nint pcwszFilePath; + public nint hFile; + public nint pgKnownSubject; + } + + [StructLayout(LayoutKind.Sequential)] + private struct WINTRUST_DATA + { + public uint cbStruct; + public nint pPolicyCallbackData; + public nint pSIPClientData; + public uint dwUIChoice; + public uint fdwRevocationChecks; + public uint dwUnionChoice; + public nint pFile; + public uint dwStateAction; + public nint hWVTStateData; + public nint pwszURLReference; + public uint dwProvFlags; + public uint dwUIContext; + public nint pSignatureSettings; + } + + [LibraryImport("wintrust.dll")] + private static partial int WinVerifyTrust(nint hwnd, ref Guid action, ref WINTRUST_DATA data); + + [LibraryImport("wintrust.dll")] + private static partial nint WTHelperProvDataFromStateData(nint stateData); + + [LibraryImport("wintrust.dll")] + private static partial nint WTHelperGetProvSignerFromChain(nint providerData, uint signerIndex, + [MarshalAs(UnmanagedType.Bool)] bool counterSigner, uint counterSignerIndex); + + [LibraryImport("wintrust.dll")] + private static partial nint WTHelperGetProvCertFromChain(nint signer, uint certificateIndex); +} diff --git a/tests/Filee.App.Tests/UpdateServiceTests.cs b/tests/Filee.App.Tests/UpdateServiceTests.cs index 137abb56b..40e9f10af 100644 --- a/tests/Filee.App.Tests/UpdateServiceTests.cs +++ b/tests/Filee.App.Tests/UpdateServiceTests.cs @@ -1,10 +1,12 @@ using System.Net; using System.Net.Sockets; +using System.Runtime.InteropServices; using System.Security.Cryptography; using System.Text; using System.Text.Json; using Avalonia.Headless.XUnit; using Filee.App.Services; +using Filee.Platform.Windows; using Microsoft.Extensions.Logging.Abstractions; namespace Filee.App.Tests; @@ -174,6 +176,91 @@ await Assert.ThrowsAsync(() => updates.DownloadInstallerAsync(TempFolder(), null, TestContext.Current.CancellationToken)); } + private const string Publisher = "CN=Open Source Developer, Filee"; + + /// An update service whose own program is signed by and the installer by . + private static UpdateService Signed(string? own, string? installer) => + new(NullLogger.Instance, new FakeNetwork("no network")) + { + OwnProgram = "Filee.exe", + SignerOf = (path, _) => path == "Filee.exe" ? own : installer, + }; + + [Fact] + public void Installer_signed_by_the_same_publisher_is_kept() + { + var path = Path.Combine(TempFolder(), "Filee-9.9.9-win-Setup.exe"); + File.WriteAllText(path, "setup"); + + Signed(Publisher, Publisher).CheckPublisher(path); + + Assert.True(File.Exists(path)); + } + + [Theory] + [InlineData("CN=Someone Else")] + [InlineData(null)] // not signed, or signed with a certificate Windows does not trust + public void Installer_signed_by_anyone_else_is_thrown_away(string? signer) + { + var path = Path.Combine(TempFolder(), "Filee-9.9.9-win-Setup.exe"); + File.WriteAllText(path, "setup"); + + Assert.Throws(() => Signed(Publisher, signer).CheckPublisher(path)); + Assert.False(File.Exists(path)); + } + + [Fact] + public void Installer_is_checked_for_revocation_but_this_copy_is_not() + { + var checks = new List<(string Path, bool Revocation)>(); + var updates = new UpdateService(NullLogger.Instance, new FakeNetwork("no network")) + { + OwnProgram = "Filee.exe", + SignerOf = (path, revocation) => + { + checks.Add((path, revocation)); + return Publisher; + }, + }; + + updates.CheckPublisher("Setup.exe"); + + Assert.Equal([("Filee.exe", false), ("Setup.exe", true)], checks); + } + + [Fact] + public void Unsigned_copy_keeps_to_the_checksum() + { + // Built from source: there is no publisher to compare with, so the installer is not refused for its signature. + var path = Path.Combine(TempFolder(), "Filee-9.9.9-win-Setup.exe"); + File.WriteAllText(path, "setup"); + + Signed(null, null).CheckPublisher(path); + + Assert.True(File.Exists(path)); + } + + [Fact] + public void Unsigned_file_has_no_trusted_signer() + { + var path = Path.Combine(TempFolder(), "unsigned.exe"); + File.WriteAllBytes(path, RandomNumberGenerator.GetBytes(4_096)); + + Assert.Null(Authenticode.TrustedSigner(path, checkRevocation: false)); + } + + [Fact] + public void Signed_program_has_its_publisher_as_signer() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "Authenticode is Windows only"); + // dotnet.exe carries Microsoft's embedded signature; it sits three folders above the shared runtime + // (dotnet\shared\Microsoft.NETCore.App\). + var dotnet = Path.GetFullPath(Path.Combine(RuntimeEnvironment.GetRuntimeDirectory(), "..", "..", "..", "dotnet.exe")); + Assert.SkipUnless(File.Exists(dotnet), $"no {dotnet}"); + + Assert.Contains("Microsoft", Authenticode.TrustedSigner(dotnet, checkRevocation: false)); + } + [Fact] public void Installer_runs_without_the_wizard_and_without_restarting_windows() { From 3ebc862b733ed1ea94df403bd049db2d5cc9b1f9 Mon Sep 17 00:00:00 2001 From: KnifeLemon Date: Fri, 23 Oct 2026 18:15:51 +0900 Subject: [PATCH 2/2] Check the installer's signature off the UI thread; name a failed revocation check WinVerifyTrust hashes the whole installer and may ask the certificate authority about revocation, which froze the window for a few seconds on the ~125 MB Setup. Run CheckPublisher on the thread pool; its exceptions still reach the existing catch. When the installer's signature only fails the revocation check (offline, a proxy or a firewall, or a revoked certificate), the log now says so instead of "signed by no publisher Windows trusts". The installer is still refused. Co-Authored-By: Claude Opus 5.5 --- src/Filee.App/Services/UpdateService.cs | 12 +++++++++--- tests/Filee.App.Tests/UpdateServiceTests.cs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/Filee.App/Services/UpdateService.cs b/src/Filee.App/Services/UpdateService.cs index 339ebe2fb..c681f6d3c 100644 --- a/src/Filee.App/Services/UpdateService.cs +++ b/src/Filee.App/Services/UpdateService.cs @@ -279,7 +279,8 @@ public async Task UpdateAsync() { var progress = new Progress(percent => DownloadPercent = percent); installer = await DownloadInstallerAsync(Path.Combine(Path.GetTempPath(), "Filee-update"), progress); - CheckPublisher(installer); + // WinVerifyTrust hashes the whole installer and may ask the certificate authority about revocation. + await Task.Run(() => CheckPublisher(installer)); } catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException or IOException or InvalidDataException or UnauthorizedAccessException) @@ -373,9 +374,14 @@ internal void CheckPublisher(string installer) var signer = SignerOf(installer, true); if (signer == publisher) return; + // A signature that holds without the revocation check failed only that check: the certificate authority + // could not be reached (offline, a proxy or a firewall) or the certificate was revoked. + var reason = signer is null && SignerOf(installer, false) == publisher + ? $"is signed by {publisher}, but the revocation check failed (the certificate authority could not be " + + "reached, or the certificate was revoked)" + : $"is signed by {signer ?? "no publisher Windows trusts"}, not by {publisher}"; TryDelete(installer); - throw new InvalidDataException( - $"{Path.GetFileName(installer)} is signed by {signer ?? "no publisher Windows trusts"}, not by {publisher}"); + throw new InvalidDataException($"{Path.GetFileName(installer)} {reason}"); } private async Task DownloadAsync(ReleaseFile file, string path, IProgress? progress, CancellationToken cancellationToken) diff --git a/tests/Filee.App.Tests/UpdateServiceTests.cs b/tests/Filee.App.Tests/UpdateServiceTests.cs index 40e9f10af..31c0ecd4d 100644 --- a/tests/Filee.App.Tests/UpdateServiceTests.cs +++ b/tests/Filee.App.Tests/UpdateServiceTests.cs @@ -209,6 +209,25 @@ public void Installer_signed_by_anyone_else_is_thrown_away(string? signer) Assert.False(File.Exists(path)); } + [Fact] + public void Failed_revocation_check_is_named_in_the_error() + { + // Offline or behind a proxy, the signature is fine but the certificate authority can't be asked: the + // installer is still thrown away, and the log says why instead of blaming the publisher. + var path = Path.Combine(TempFolder(), "Filee-9.9.9-win-Setup.exe"); + File.WriteAllText(path, "setup"); + var updates = new UpdateService(NullLogger.Instance, new FakeNetwork("no network")) + { + OwnProgram = "Filee.exe", + SignerOf = (file, revocation) => file == "Filee.exe" || !revocation ? Publisher : null, + }; + + var error = Assert.Throws(() => updates.CheckPublisher(path)); + + Assert.Contains("revocation check failed", error.Message); + Assert.False(File.Exists(path)); + } + [Fact] public void Installer_is_checked_for_revocation_but_this_copy_is_not() {