Skip to content

Commit afdc584

Browse files
committed
fix: install the device helper from the MSI, owned by the console user
1 parent b73acde commit afdc584

6 files changed

Lines changed: 367 additions & 5 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,46 @@ jobs:
287287
exit 1
288288
fi
289289
290+
# ── MSI post-processing ───────────────────────────────────────────────────
291+
# Runs AFTER pack (which is where signing happens) and therefore INVALIDATES the MSI's
292+
# Authenticode signature - the re-sign below is not optional, it is part of this pair.
293+
#
294+
# See scripts/msi-elevate-hooks.ps1 for what it changes and why. Short version: vpk emits
295+
# the install hook as a deferred IMPERSONATED custom action, so `sc create` inside it runs
296+
# on the user's filtered token and the background service is silently never registered.
297+
- name: Elevate the MSI install hooks (Windows)
298+
if: matrix.rid == 'win-x64'
299+
shell: pwsh
300+
run: scripts/msi-elevate-hooks.ps1 -MsiPath releases/OrgZ-win.msi
301+
302+
# Only the MSI is re-signed; every other artifact still carries the signature vpk gave
303+
# it. Conditional on the same secret as the rest, so an unsigned build stays a build.
304+
- name: Re-sign the patched MSI (Windows)
305+
if: ${{ matrix.rid == 'win-x64' && env.HAS_AZURE_SIGNING == 'true' }}
306+
uses: azure/artifact-signing-action@v2
307+
with:
308+
endpoint: ${{ secrets.AZURE_TS_ENDPOINT }}
309+
signing-account-name: ${{ secrets.AZURE_TS_ACCOUNT }}
310+
certificate-profile-name: ${{ secrets.AZURE_TS_PROFILE }}
311+
files-folder: ${{ github.workspace }}/releases
312+
files-folder-filter: msi
313+
file-digest: SHA256
314+
timestamp-rfc3161: http://timestamp.acs.microsoft.com
315+
timestamp-digest: SHA256
316+
317+
# A patched-but-unsigned MSI is worse than either end state: the service works and every
318+
# user gets a SmartScreen block. Fail the release here rather than publish that.
319+
- name: Verify the MSI signature survived (Windows)
320+
if: ${{ matrix.rid == 'win-x64' && env.HAS_AZURE_SIGNING == 'true' }}
321+
shell: pwsh
322+
run: |
323+
$status = (Get-AuthenticodeSignature 'releases/OrgZ-win.msi').Status
324+
Write-Host "MSI signature: $status"
325+
if ($status -ne 'Valid') {
326+
Write-Error "MSI signature is '$status' after patching - the re-sign did not take."
327+
exit 1
328+
}
329+
290330
- name: Upload artifacts
291331
uses: actions/upload-artifact@v4
292332
with:

‎OrgZ.Tests/ShareHostingTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ public void Status_line_reads_plainly_in_each_state()
297297
{
298298
Assert.Equal("Not sharing", SettingsDialog.DescribeShareState(true, null));
299299
Assert.Equal("Not sharing", SettingsDialog.DescribeShareState(true, new DeviceHelperClient.ShareState(false, null, 0)));
300-
Assert.Equal("Sharing “Fox Library” on port 7391", SettingsDialog.DescribeShareState(true, new DeviceHelperClient.ShareState(true, "Fox Library", 7391)));
300+
Assert.Equal("Sharing “Fox Library”", SettingsDialog.DescribeShareState(true, new DeviceHelperClient.ShareState(true, "Fox Library", 7391)));
301301

302302
// A share with no reported name still says something sensible.
303303
Assert.Contains("This library", SettingsDialog.DescribeShareState(true, new DeviceHelperClient.ShareState(true, " ", 7391)));

‎Services/DeviceHelper/DeviceHelperInstaller.cs‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,29 @@ internal static string WindowsFirewallRemoveCommands() =>
198198
return null;
199199
}
200200

201-
// LocalSystem, LocalService, NetworkService - nobody to own the helper.
201+
// LocalSystem, LocalService, NetworkService - nobody is BEHIND this process, so
202+
// its own SID is the wrong answer twice over: stamping SYSTEM as the owner locks
203+
// out the human who just installed OrgZ, and recording nothing leaves the helper
204+
// fail-open to any authenticated caller.
205+
//
206+
// This is the normal case, not an edge one. The MSI's install hook is deliberately
207+
// NoImpersonate (see scripts/msi-elevate-hooks.ps1) because `sc create` needs the
208+
// elevated token, which means EVERY machine-wide install arrives here. So ask the
209+
// session instead of the token: the person at the console is the person installing.
202210
if (sid.IsWellKnown(System.Security.Principal.WellKnownSidType.LocalSystemSid)
203211
|| sid.IsWellKnown(System.Security.Principal.WellKnownSidType.LocalServiceSid)
204212
|| sid.IsWellKnown(System.Security.Principal.WellKnownSidType.NetworkServiceSid))
205213
{
206-
_log.Information("Install is running as a machine account; the device helper will accept any authenticated caller");
214+
var consoleSid = WindowsConsoleUserSid();
215+
if (consoleSid is not null)
216+
{
217+
_log.Information("Install is running as a machine account; recording the console user {Sid} as the device helper's owner", consoleSid);
218+
return consoleSid;
219+
}
220+
221+
// Unattended, or nobody logged in - an SCCM push, an image build. There is
222+
// genuinely no owner to record, so this stays the legacy fail-open case.
223+
_log.Warning("Install is running as a machine account with no console user; the device helper will accept any authenticated caller");
207224
return null;
208225
}
209226

@@ -216,6 +233,76 @@ internal static string WindowsFirewallRemoveCommands() =>
216233
}
217234
}
218235

236+
/// <summary>
237+
/// The SID of the user signed in at the physical console, or null when there isn't one.
238+
///
239+
/// Used only when the installer itself is a machine account. WTS is asked for the user
240+
/// NAME rather than a token on purpose: <c>WTSQueryUserToken</c> would need SE_TCB_NAME
241+
/// and hand back a handle to own and close, while the name translates to a SID through
242+
/// plain managed code. LocalSystem can read this without any privilege adjustment.
243+
/// </summary>
244+
internal static string? WindowsConsoleUserSid()
245+
{
246+
if (!OperatingSystem.IsWindows())
247+
{
248+
return null;
249+
}
250+
251+
try
252+
{
253+
var session = WtsNativeMethods.WTSGetActiveConsoleSessionId();
254+
255+
// 0xFFFFFFFF means no session is attached to the console - the machine is at the
256+
// lock screen with nobody logged in, or this is a headless/RDP-only box.
257+
if (session == 0xFFFFFFFF)
258+
{
259+
return null;
260+
}
261+
262+
var user = WtsQuery(session, WtsNativeMethods.WtsInfoClass.WTSUserName);
263+
if (string.IsNullOrWhiteSpace(user))
264+
{
265+
return null;
266+
}
267+
268+
var domain = WtsQuery(session, WtsNativeMethods.WtsInfoClass.WTSDomainName);
269+
var account = string.IsNullOrWhiteSpace(domain) ? user : $@"{domain}\{user}";
270+
271+
var identifier = (System.Security.Principal.SecurityIdentifier)
272+
new System.Security.Principal.NTAccount(account).Translate(typeof(System.Security.Principal.SecurityIdentifier));
273+
274+
return identifier.Value;
275+
}
276+
catch (Exception ex)
277+
{
278+
_log.Debug(ex, "Could not determine the console user's SID");
279+
return null;
280+
}
281+
}
282+
283+
[System.Runtime.Versioning.SupportedOSPlatform("windows")]
284+
private static string? WtsQuery(uint session, WtsNativeMethods.WtsInfoClass info)
285+
{
286+
var buffer = IntPtr.Zero;
287+
288+
try
289+
{
290+
if (!WtsNativeMethods.WTSQuerySessionInformationW(IntPtr.Zero, session, info, out buffer, out _))
291+
{
292+
return null;
293+
}
294+
295+
return System.Runtime.InteropServices.Marshal.PtrToStringUni(buffer);
296+
}
297+
finally
298+
{
299+
if (buffer != IntPtr.Zero)
300+
{
301+
WtsNativeMethods.WTSFreeMemory(buffer);
302+
}
303+
}
304+
}
305+
219306
/// <summary>
220307
/// Records the owner SID in the service's own environment. A Windows service has no
221308
/// equivalent of launchd's EnvironmentVariables or systemd's Environment=, but the SCM
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2026 FoxCouncil (https://github.com/FoxCouncil/OrgZ)
2+
3+
using System.Runtime.InteropServices;
4+
using System.Runtime.Versioning;
5+
6+
namespace OrgZ.Services.DeviceHelper;
7+
8+
/// <summary>
9+
/// The two Terminal Services calls needed to answer "who is sitting at this machine".
10+
///
11+
/// Only reached when OrgZ is running as LocalSystem during the MSI install hook, where
12+
/// <see cref="System.Security.Principal.WindowsIdentity.GetCurrent"/> describes the machine
13+
/// rather than the person - see <see cref="DeviceHelperInstaller.WindowsConsoleUserSid"/>.
14+
/// </summary>
15+
[SupportedOSPlatform("windows")]
16+
internal static partial class WtsNativeMethods
17+
{
18+
internal enum WtsInfoClass
19+
{
20+
WTSUserName = 5,
21+
WTSDomainName = 7,
22+
}
23+
24+
/// <summary>
25+
/// The session currently attached to the physical console, or 0xFFFFFFFF when none is.
26+
/// Fast User Switching moves this, which is the point: it names the session in front of
27+
/// the screen right now, not the first one that logged in.
28+
/// </summary>
29+
[LibraryImport("kernel32.dll")]
30+
internal static partial uint WTSGetActiveConsoleSessionId();
31+
32+
/// <summary>
33+
/// <c>WTS_CURRENT_SERVER_HANDLE</c> is IntPtr.Zero - the local machine. The returned
34+
/// buffer belongs to WTS and must go back through <see cref="WTSFreeMemory"/>.
35+
/// </summary>
36+
[LibraryImport("wtsapi32.dll", SetLastError = true)]
37+
[return: MarshalAs(UnmanagedType.Bool)]
38+
internal static partial bool WTSQuerySessionInformationW(
39+
IntPtr server,
40+
uint sessionId,
41+
WtsInfoClass infoClass,
42+
out IntPtr buffer,
43+
out uint bytesReturned);
44+
45+
[LibraryImport("wtsapi32.dll")]
46+
internal static partial void WTSFreeMemory(IntPtr memory);
47+
}

‎Views/SettingsDialog.axaml.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,11 @@ private void LoadSettings()
8686

8787
// Services
8888
ShareNameInput.Text = Settings.Get("OrgZ.Services.Sharing.Name", $"{Environment.MachineName} Library");
89+
// Seeded from settings so the box is not blank for the length of a service probe; the
90+
// probe below corrects it from what the service is ACTUALLY doing. Not refreshed here
91+
// as well - RefreshServiceStatusAsync now ends by re-reading the share, and firing both
92+
// would race two probes at the same checkbox on every open.
8993
ShareEnabledCheck.IsChecked = Settings.Get("OrgZ.Services.Sharing.Enabled", false);
90-
_ = RefreshShareStatusAsync();
9194

9295
// No "Disc burning" row: a burn always survives the GUI (the service or the elevated
9396
// helper process owns the write), and stopping one mid-write would only ruin the disc.
@@ -636,7 +639,7 @@ internal static string DescribeShareState(bool serviceAvailable, Services.Device
636639
}
637640

638641
var name = string.IsNullOrWhiteSpace(state.Name) ? "This library" : state.Name;
639-
return $"Sharing “{name}” on port {state.Port}";
642+
return $"Sharing “{name}”";
640643
}
641644

642645
private async Task RefreshShareStatusAsync()
@@ -745,6 +748,14 @@ private async Task RefreshServiceStatusAsync()
745748
}
746749

747750
SetServiceButtons(buttons);
751+
752+
// The share card reads the SAME service this method just changed the state of, so it
753+
// has to be re-read here rather than only when the dialog opens. Without this, starting
754+
// or stopping the service left the sharing checkbox showing whatever was true when the
755+
// dialog was constructed - disabled after a start that had just made it available,
756+
// still ticked after a stop that had already torn the share down - and the only way to
757+
// correct it was to close and reopen the app.
758+
await RefreshShareStatusAsync();
748759
}
749760

750761
private void SetServiceButtons(ServiceButtons buttons)

0 commit comments

Comments
 (0)