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..c681f6d3c 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,8 @@ public async Task UpdateAsync()
{
var progress = new Progress(percent => DownloadPercent = percent);
installer = await DownloadInstallerAsync(Path.Combine(Path.GetTempPath(), "Filee-update"), progress);
+ // 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)
@@ -344,6 +357,33 @@ 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;
+ // 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)} {reason}");
+ }
+
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..31c0ecd4d 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,110 @@ 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 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()
+ {
+ 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()
{