PC window over Remote Desktop; Windows Wake-on-LAN check and fix

.github/workflows/build.yml

@@ -152,8 +152,8 @@ jobs:
       - uses: actions/checkout@v7
       - uses: actions/setup-go@v7
         with: { go-version-file: helper/go.mod, cache-dependency-path: helper/go.sum }
-      - name: Unit tests for Windows-only code (DPAPI sealing, NanoKVM client)
-        run: go test -count=1 ./internal/secret/ ./internal/nanokvm/...
+      - name: Unit tests for Windows-only code (DPAPI sealing, NanoKVM client, window access, PowerShell)
+        run: go test -count=1 -v -run . ./internal/secret/ ./internal/nanokvm/... ./internal/winsvc/ ./internal/setup/
       - name: Build PowerDeck.exe
         run: |
           go install github.com/tc-hib/[email protected]

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 2.2.1
+
+- **PC window over Remote Desktop.** An administrator of the PC can now open the PowerDeck window over Remote Desktop without running it as administrator. Standard accounts in another session are still refused, and the message now says what to do.
+- **Wake-on-LAN on Windows.** The check could report Wake-on-LAN as off while it was on, and **Fix** failed with a block of XML ("the driver reports magic-packet wake as <Objs…"): PowerShell's progress messages, such as "Preparing modules for first use", were being read as the answer. PowerDeck now reads only the result, and a real error shows PowerShell's own message. A failed `powercfg` step no longer stops the fix.
+
 ## 2.2.0
 
 KVMs move to an always-on PowerDeck computer, which can now reset a frozen one.

SECURITY.md

@@ -16,7 +16,7 @@ PowerDeck can shut computers down, wake them, and (through a NanoKVM) press thei
 - **A KVM wired wrongly** could press the wrong button. PowerDeck presses to turn on only when the computer's helper doesn't answer and the power light reads off, and only after it has seen that light on (an unwired LED reads off forever). Linking checks that the light agrees with the computer's helper. Whether the power and reset switches are wired is learned from what a press does, never assumed.
 - **A KVM press can lose work.** A reset or a forced power-off is refused while the computer's helper still answers (unless the owner forces it), needs a power light that is known to be wired and reads on, and is for owner devices only: no automation key, and no agent relayed through an owner device. Pressing power to shut down a computer whose helper doesn't answer is owner-only too, since nobody can check whether someone is using it. The hub can't drive the KVM of the machine it runs on.
 - **The hub holds NanoKVM passwords.** The always-on helper that drives the KVMs needs their passwords. The app sends each one over the hub's pinned TLS connection; the hub seals it (below) and only uses it to reach the NanoKVM by its Tailscale name or address. It never returns it. Give PowerDeck a NanoKVM account without administrator rights ([docs/nanokvm-hardening.md](docs/nanokvm-hardening.md)).
-- **Other people on the same PC** can't use the PowerDeck window: on Windows it answers only programs of the person signed in at the screen, or elevated administrators. It also needs a per-run key embedded in the page and refuses foreign `Host` headers (DNS rebinding).
+- **Other people on the same PC** can't use the PowerDeck window: on Windows it answers only programs of the person signed in at the screen, or of an administrator of the PC (elevated or not, e.g. over Remote Desktop: UAC doesn't keep an administrator out, so this doesn't either). A standard account in another session (fast user switching, Remote Desktop) is refused. It also needs a per-run key embedded in the page and refuses foreign `Host` headers (DNS rebinding).
 - **Websites in a browser** must not drive a helper. Requests carrying an `Origin` header are refused. The Windows window on `127.0.0.1` checks the `Host` header (DNS rebinding) and needs a per-run key embedded only in the page, with a CSP that forbids framing.
 - **Release downloads** could be tampered with. `install.sh` checks the SHA-256 and the Ed25519 signature; `install.ps1` checks the SHA-256. The helpers' updaters install only files whose Ed25519 signature verifies against the public key compiled in. Android only accepts updates signed with the original key.
 - **The update-signing key is a single point of failure.** Helpers run as SYSTEM/root and install whatever that key signs, so whoever holds it (or can publish a release through the repository's workflow) could run code on every helper that updates automatically. To limit that: a release installs on its own only after it has been public for **72 hours**, so a bad or forged release can be noticed and pulled first; owners can **pause** automatic updates for 7 days; new Linux installs (servers) start with automatic updates **off**; and installing by hand always shows the version first. The key lives only in this repository's Actions secrets (plus an offline backup); there is no key rotation in the helper yet, so a leaked key means shipping a new public key through a release signed with the old one, before the attacker does.

helper/internal/localui/localui.go

@@ -65,7 +65,7 @@ type Deps struct {
 	Fix         func(ctx context.Context, id string) (check.Result, error)
 	Reports     *power.Reports // presence from the user-session companion (Windows)
 	// AllowLocal, if set, decides whether the program behind a connection (its "ip:port") may use
-	// the window: on Windows, only the person at the screen or an administrator.
+	// the window: on Windows, only the person at the screen or an administrator of the PC.
 	AllowLocal func(remote string) bool
 	Log        *slog.Logger
 	OS         string // default runtime.GOOS (the demo pretends to be Windows)
@@ -119,7 +119,7 @@ func (u *ui) guard(next http.Handler) http.Handler {
 			return
 		}
 		if u.AllowLocal != nil && !u.AllowLocal(r.RemoteAddr) {
-			http.Error(w, "PowerDeck's window is only for the person signed in at this PC's screen. Over Remote Desktop, open PowerDeck as administrator.", http.StatusForbidden)
+			http.Error(w, "PowerDeck's window is only for the person at this PC's screen and its administrators. Sign in with an administrator account, or at the PC itself.", http.StatusForbidden)
 			return
 		}
 		h := w.Header()

helper/internal/setup/setup_windows.go

@@ -6,6 +6,7 @@ package setup
 // run it again.
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"encoding/json"
@@ -336,7 +337,6 @@ func firewallPresent() bool {
 // enableWake turns on magic-packet wake for the adapter with this MAC. Settings are only written
 // when they differ, so re-running (updates) doesn't restart the adapter.
 const wakeScript = `
-$ErrorActionPreference = 'Stop'
 $mac = $args[0]
 $nic = Get-NetAdapter -Physical | Where-Object { $_.MacAddress -eq $mac } | Select-Object -First 1
 if (-not $nic) { throw "No adapter with MAC $mac" }
@@ -347,34 +347,56 @@ if ($pm.WakeOnMagicPacket -ne 'Enabled' -or $pm.WakeOnPattern -ne 'Disabled') {
 Get-NetAdapterAdvancedProperty -Name $nic.Name |
   Where-Object { $_.DisplayName -match 'Magic Packet|Shutdown Wake|Wake.*power off|Wake.*S5' -and $_.ValidDisplayValues -contains 'Enabled' -and $_.DisplayValue -ne 'Enabled' } |
   ForEach-Object { Set-NetAdapterAdvancedProperty -Name $nic.Name -DisplayName $_.DisplayName -DisplayValue 'Enabled' }
-powercfg.exe /deviceenablewake "$($nic.InterfaceDescription)" 2>&1 | Out-Null
+# Best effort, and Windows PowerShell turns a native program's stderr into an error under 'Stop'.
+try { powercfg.exe /deviceenablewake "$($nic.InterfaceDescription)" 2>&1 | Out-Null } catch {}
 $pm = Get-NetAdapterPowerManagement -Name $nic.Name
 Write-Output ("{0}|{1}" -f $pm.WakeOnMagicPacket, $nic.InterfaceDescription)
 `
 
 // powershell runs a script block with single-quoted arguments via -EncodedCommand, which sidesteps
-// Windows command-line quoting entirely.
+// Windows command-line quoting entirely. It returns only what the script writes as output: started
+// this way, PowerShell writes its other streams (progress such as "Preparing modules for first use",
+// warnings, errors) to stderr as CLIXML, which mustn't end up in a check's reading or an error message.
+// So progress is off, any error stops the script, and its message comes back as the error.
 func powershell(script string, args ...string) (string, error) {
-	cmd := "& {" + script + "}"
+	cmd := "$global:ProgressPreference = 'SilentlyContinue'; $ErrorActionPreference = 'Stop'\ntry { & {" + script + "}"
 	for _, a := range args {
 		cmd += " '" + strings.ReplaceAll(a, "'", "''") + "'"
 	}
+	cmd += " } catch { Write-Output $_.Exception.Message; exit 1 }"
+	var stdout bytes.Buffer
+	c := hidden(powershellExe(), "-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-EncodedCommand", encodeCommand(cmd))
+	c.Stdout = &stdout
+	err := c.Run()
+	out := strings.TrimSpace(stdout.String())
+	if err != nil {
+		if msg := lastLine(out); msg != "" {
+			return "", errors.New(msg)
+		}
+		return "", err
+	}
+	return out, nil
+}
+
+func powershellExe() string {
+	return filepath.Join(os.Getenv("SystemRoot"), `System32\WindowsPowerShell\v1.0\powershell.exe`)
+}
+
+// encodeCommand: -EncodedCommand takes base64 of the UTF-16LE script.
+func encodeCommand(cmd string) string {
 	u := windows.StringToUTF16(cmd)
 	b := make([]byte, 0, len(u)*2)
 	for _, c := range u[:len(u)-1] { // drop the trailing NUL
 		b = append(b, byte(c), byte(c>>8))
 	}
-	enc := base64.StdEncoding.EncodeToString(b)
-	out, err := hidden(filepath.Join(os.Getenv("SystemRoot"), `System32\WindowsPowerShell\v1.0\powershell.exe`),
-		"-NoProfile", "-NonInteractive", "-ExecutionPolicy", "Bypass", "-EncodedCommand", enc).CombinedOutput()
-	return strings.TrimSpace(string(out)), err
+	return base64.StdEncoding.EncodeToString(b)
 }
 
 func enableWake(mac string) (string, error) {
 	dashed := strings.ToUpper(strings.ReplaceAll(mac, ":", "-"))
 	out, err := powershell(wakeScript, dashed)
 	if err != nil {
-		return "", fmt.Errorf("%v: %s", err, lastLine(out))
+		return "", err
 	}
 	state, desc, _ := strings.Cut(lastLine(out), "|")
 	if !strings.EqualFold(state, "Enabled") {
@@ -466,7 +488,10 @@ func Checks(e Env) []check.Check {
 					return check.Result{Level: check.Info, Title: "Wake-on-LAN", Detail: "Needs a wired network card."}
 				}
 				out, err := powershell(`(Get-NetAdapterPowerManagement -Name $args[0]).WakeOnMagicPacket`, n.Interface)
-				if err == nil && strings.EqualFold(lastLine(out), "Enabled") {
+				if err != nil {
+					return check.Result{Level: check.Warn, Title: "Couldn't read Wake-on-LAN on " + n.Interface, Detail: err.Error()}
+				}
+				if strings.EqualFold(lastLine(out), "Enabled") {
 					return check.Result{Level: check.OK, Title: "Wake-on-LAN on " + n.Interface, Detail: n.MAC}
 				}
 				return check.Result{Level: check.Warn, Title: "Wake-on-LAN is off on " + n.Interface}

helper/internal/setup/setup_windows_test.go

@@ -0,0 +1,39 @@
+//go:build windows
+
+package setup
+
+import (
+	"strings"
+	"testing"
+)
+
+// Started with -EncodedCommand, PowerShell writes progress ("Preparing modules for first use"),
+// warnings and errors to stderr as CLIXML. A check read that as the adapter's Wake-on-LAN state.
+func TestPowerShellReturnsOnlyOutput(t *testing.T) {
+	// Show that this PowerShell really does write CLIXML here, so the test below means something.
+	raw, _ := hidden(powershellExe(), "-NoProfile", "-NonInteractive", "-EncodedCommand",
+		encodeCommand(`Write-Progress -Activity 'Preparing modules for first use.'; Write-Warning 'careful'; 'Enabled'`)).CombinedOutput()
+	t.Logf("PowerShell's own combined output: %q", raw)
+
+	out, err := powershell(`Write-Progress -Activity 'Preparing modules for first use.'; Write-Warning 'careful'; Write-Output ('Enabled|' + $args[0])`, "Owner's NIC")
+	if err != nil || out != "Enabled|Owner's NIC" {
+		t.Fatalf("got %q, %v; want only the output", out, err)
+	}
+}
+
+func TestPowerShellErrorsAreReadable(t *testing.T) {
+	for _, script := range []string{
+		`throw 'No adapter with MAC 02-00-5E-00-00-01'`,
+		`Write-Error 'No adapter with MAC 02-00-5E-00-00-01'`, // not terminating unless stopped
+		`Write-Progress -Activity 'Preparing modules for first use.'; Get-NetAdapterPowerManagement -Name 'No adapter with MAC 02-00-5E-00-00-01'`,
+	} {
+		out, err := powershell(script)
+		if err == nil {
+			t.Errorf("%s: no error (output %q)", script, out)
+			continue
+		}
+		if msg := err.Error(); strings.Contains(msg, "CLIXML") || strings.Contains(msg, "<Obj") || !strings.Contains(msg, "No adapter with MAC") {
+			t.Errorf("%s: error %q, want PowerShell's message", script, msg)
+		}
+	}
+}

helper/internal/winsvc/caller_windows.go

@@ -14,8 +14,8 @@ var procGetExtendedTcpTable = windows.NewLazySystemDLL("iphlpapi.dll").NewProc("
 
 // LocalCaller reports whether the program behind a loopback connection (remote is the client's
 // "ip:port", server the listening port) runs in the active console session, i.e. belongs to the
-// person at this PC's screen, or is elevated. Other users signed in to the same PC (fast user
-// switching, Remote Desktop) and services are refused.
+// person at this PC's screen, or belongs to an administrator of this PC (over Remote Desktop, say).
+// Standard users signed in to the same PC (fast user switching, Remote Desktop) are refused.
 func LocalCaller(remote string, server uint16) bool {
 	ap, err := netip.ParseAddrPort(remote)
 	if err != nil || !ap.Addr().Is4() {
@@ -29,7 +29,7 @@ func LocalCaller(remote string, server uint16) bool {
 	if windows.ProcessIdToSessionId(pid, &session) == nil && session == windows.WTSGetActiveConsoleSessionId() {
 		return true
 	}
-	return elevated(pid)
+	return administrator(pid)
 }
 
 // owningPID finds the process that owns the client side of a loopback TCP connection.
@@ -63,7 +63,12 @@ func owningPID(client netip.AddrPort, server uint16) (uint32, bool) {
 	return 0, false
 }
 
-func elevated(pid uint32) bool {
+// tokenElevationTypeLimited: an administrator's token with UAC's restrictions (TOKEN_ELEVATION_TYPE).
+const tokenElevationTypeLimited = 3
+
+// administrator: the process runs elevated, or as an administrator who isn't elevated right now
+// (UAC's limited token). UAC doesn't keep an administrator out of anything, so neither does this.
+func administrator(pid uint32) bool {
 	p, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, pid)
 	if err != nil {
 		return false
@@ -74,5 +79,10 @@ func elevated(pid uint32) bool {
 		return false
 	}
 	defer t.Close()
-	return t.IsElevated()
+	if t.IsElevated() {
+		return true
+	}
+	var typ, n uint32
+	err = windows.GetTokenInformation(t, windows.TokenElevationType, (*byte)(unsafe.Pointer(&typ)), uint32(unsafe.Sizeof(typ)), &n)
+	return err == nil && typ == tokenElevationTypeLimited
 }

helper/internal/winsvc/caller_windows_test.go

@@ -0,0 +1,44 @@
+package winsvc
+
+import (
+	"net"
+	"net/netip"
+	"os"
+	"testing"
+)
+
+// The test runs as an administrator (CI), in or outside the console session: either way LocalCaller
+// must find this process behind a real loopback connection and let it in.
+func TestLocalCallerFindsTheProcess(t *testing.T) {
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+	go func() {
+		if c, err := ln.Accept(); err == nil {
+			defer c.Close()
+			_, _ = c.Read(make([]byte, 1))
+		}
+	}()
+	c, err := net.Dial("tcp", ln.Addr().String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer c.Close()
+	server := uint16(ln.Addr().(*net.TCPAddr).Port)
+	client := netip.MustParseAddrPort(c.LocalAddr().String())
+	pid, ok := owningPID(client, server)
+	if !ok || pid != uint32(os.Getpid()) {
+		t.Fatalf("owner %d %v, want %d", pid, ok, os.Getpid())
+	}
+	if !administrator(pid) {
+		t.Skip("not running as an administrator")
+	}
+	if !LocalCaller(client.String(), server) {
+		t.Error("an administrator was refused")
+	}
+	if LocalCaller("127.0.0.1:1", server) {
+		t.Error("a connection nobody owns was let in")
+	}
+}