From 5db93c115630472232270bb42e5c423b3ddeac09 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Wed, 29 Jul 2026 01:27:49 +0200 Subject: [PATCH] fix(packaging/windows): a release must not sign itself with a throwaway cert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both pack scripts fell back to a fresh `New-SelfSignedCertificate` whenever MSIX_CERT_PFX_B64 was absent, and said nothing about it beyond one Write-Host. That fallback is right for canary and dev builds. On a tag it is not: an ephemeral cert is regenerated per build, so nobody can pin it, and a release signed with one is indistinguishable from a release signed by whoever else got to the artifact. A secret that got renamed, rotated away, or simply wasn't inherited by a new workflow would have downgraded a real release silently and shipped it. pack-msix.ps1 and pack-host-installer.ps1 now resolve -RequireSignedCert (default 'auto' = true iff GITHUB_REF is refs/tags/v*) and throw instead of falling back; pack-host-installer.ps1 also refuses -NoSign on the same condition. Reading GITHUB_REF inside the scripts rather than taking a workflow flag means a future packaging workflow inherits the guard instead of having to remember it. No effect on today's releases: MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD are both set as repo secrets, so the supplied-cert branch is the one tag builds take. This only closes the trapdoor underneath it. Verified on the windows-amd64 runner: both files parse, and the guard resolves true for refs/tags/v0.21.0, false for refs/heads/main and for an unset ref, with 'true'/'false' overriding as intended. Not touched: the pf-vdisplay / gamepad driver builds use the same pattern with DRIVER_CERT_PFX_B64, which has no secret behind it — those need attestation signing (a Partner Center action), and a guard there would just fail every tag. Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/windows-host.yml | 4 +++- .gitea/workflows/windows-msix.yml | 13 +++++++----- clients/windows/packaging/pack-msix.ps1 | 18 ++++++++++++++++- packaging/windows/pack-host-installer.ps1 | 24 +++++++++++++++++++++-- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/.gitea/workflows/windows-host.yml b/.gitea/workflows/windows-host.yml index f5f12b53..6cf48855 100644 --- a/.gitea/workflows/windows-host.yml +++ b/.gitea/workflows/windows-host.yml @@ -22,7 +22,9 @@ # # Signing reuses the client's MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD secrets (CN=unom). Without them # an ephemeral self-signed cert is generated and its public .cer published next to the installer -# (import once to LocalMachine\TrustedPublisher). See packaging/windows/pack-host-installer.ps1. +# (import once to LocalMachine\TrustedPublisher). That fallback is for canary/CI ONLY — on a v* tag +# the pack script FAILS CLOSED rather than ship a release signed by a per-build throwaway cert. +# See packaging/windows/pack-host-installer.ps1. # # GPU backends: the host builds with --features nvenc,amf-qsv,qsv = all three vendors in one installer. # - NVENC (NVIDIA, direct SDK): nothing needed at build time — the entry points are resolved at diff --git a/.gitea/workflows/windows-msix.yml b/.gitea/workflows/windows-msix.yml index 5368c1ab..2532377f 100644 --- a/.gitea/workflows/windows-msix.yml +++ b/.gitea/workflows/windows-msix.yml @@ -21,11 +21,14 @@ # Published to the generic registry + the `canary/` alias. # Both arches share the version; artifacts are arch-suffixed (..._x64.msix / ..._arm64.msix). # -# Signing (packaging/pack-msix.ps1): if the MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD Actions secrets -# are set (a real or shared code-signing .pfx whose subject DN == Publisher), the package is signed -# with them. Otherwise an ephemeral self-signed cert is generated and its public .cer is published -# next to the .msix (users import it to Trusted People before install). Drop in a real cert later -# with no workflow change — just add the secrets (+ pass -Publisher if its subject differs). +# Signing (clients/windows/packaging/pack-msix.ps1): if the MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD +# Actions secrets are set (a real or shared code-signing .pfx whose subject DN == Publisher), the +# package is signed with them. Otherwise an ephemeral self-signed cert is generated and its public +# .cer is published next to the .msix (users import it to Trusted People before install). +# +# That fallback is for canary/CI ONLY. On a v* tag the pack script FAILS CLOSED — a missing secret +# aborts the build instead of quietly shipping a release signed by a per-build throwaway cert that +# no one can pin. Nothing to opt into here: the script reads GITHUB_REF itself. name: windows-msix # One pending run per workflow+ref: a newer push supersedes the queued/running one and cancels # it (a canary only needs the latest commit; each release tag is its own ref so tag runs never diff --git a/clients/windows/packaging/pack-msix.ps1 b/clients/windows/packaging/pack-msix.ps1 index bc82ebeb..8fccc866 100644 --- a/clients/windows/packaging/pack-msix.ps1 +++ b/clients/windows/packaging/pack-msix.ps1 @@ -14,6 +14,8 @@ in-process. The package installs only where that cert is trusted, so the matching public .cer is exported next to the .msix for the user to import (Trusted People) before install. Swap in a real cert later with zero manifest changes — just pass -PfxBase64/-Publisher. + This fallback is for canary/CI/dev ONLY: on a v* tag build a missing cert is a hard failure + (-RequireSignedCert), never a silent downgrade to a throwaway cert. Run on the Windows runner (or the dev VM) with the MSVC/Windows SDK present. @@ -33,7 +35,9 @@ param( [string]$OutDir = (Join-Path $TargetDir 'msix'), [string]$Publisher = 'CN=unom', # MUST equal the signing cert subject DN [string]$PfxBase64 = $env:MSIX_CERT_PFX_B64, # optional: base64 of a code-signing .pfx - [string]$PfxPassword = $env:MSIX_CERT_PASSWORD + [string]$PfxPassword = $env:MSIX_CERT_PASSWORD, + # 'auto' (default) = required iff this is a v* tag build; 'true'/'false' to force. See below. + [ValidateSet('auto', 'true', 'false')][string]$RequireSignedCert = 'auto' ) $ErrorActionPreference = 'Stop' $ProgressPreference = 'SilentlyContinue' @@ -138,11 +142,23 @@ $msix = Join-Path $OutDir "punktfunk-client-windows_${Version}_${Arch}.msix" if ($LASTEXITCODE -ne 0) { throw "makeappx pack failed ($LASTEXITCODE)" } # --- signing cert (supplied stable pfx OR ephemeral self-signed) --- +# FAIL CLOSED on a real release. The ephemeral fallback below exists so canary/CI/dev builds keep +# working without the secret, but it is a per-build throwaway cert: nobody can pin it, and a package +# signed with one is indistinguishable from a package signed by an attacker's. Silently falling back +# on a tag build would ship exactly that to users under the release's name — so on refs/tags/v* an +# absent MSIX_CERT_PFX_B64 is a build failure, not a downgrade. ('auto' resolves from GITHUB_REF, so +# a workflow can't forget to opt in; -RequireSignedCert true/false overrides for local testing.) +$requireCert = if ($RequireSignedCert -eq 'auto') { $env:GITHUB_REF -like 'refs/tags/v*' } + else { [Convert]::ToBoolean($RequireSignedCert) } $pfxPath = Join-Path $OutDir 'signing.pfx' $cerPath = Join-Path $OutDir "punktfunk-client-windows_${Version}_${Arch}.cer" if ($PfxBase64) { Write-Host "signing with supplied code-signing cert (MSIX_CERT_PFX_B64)" [IO.File]::WriteAllBytes($pfxPath, [Convert]::FromBase64String($PfxBase64)) +} elseif ($requireCert) { + throw ("release build ($env:GITHUB_REF) with no MSIX_CERT_PFX_B64 — refusing to fall back to an " + + "ephemeral self-signed cert. Restore the MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD repo " + + "secrets, or pass -RequireSignedCert false if this really is a test build.") } else { Write-Host "no MSIX_CERT_PFX_B64 -> generating an ephemeral self-signed cert (subject $Publisher)" if (-not $PfxPassword) { $PfxPassword = 'punktfunk' } diff --git a/packaging/windows/pack-host-installer.ps1 b/packaging/windows/pack-host-installer.ps1 index ae31b884..58e4100e 100644 --- a/packaging/windows/pack-host-installer.ps1 +++ b/packaging/windows/pack-host-installer.ps1 @@ -5,7 +5,9 @@ .DESCRIPTION From a release `cargo build -p punktfunk-host --features nvenc` output (the exe), this: 1. resolves a code-signing cert (supplied stable .pfx from CI secrets OR an ephemeral self-signed - CN=unom - same scheme as the client's pack-msix.ps1) and exports the public .cer, + CN=unom - same scheme as the client's pack-msix.ps1) and exports the public .cer. The + ephemeral fallback is for canary/CI/dev ONLY: on a v* tag build a missing cert (or -NoSign) + is a hard failure, never a silent downgrade to a throwaway cert - see -RequireSignedCert, 2. signs the inner punktfunk-host.exe, 3. stages the pf-vdisplay virtual-display driver bundle (unless -NoDriver), 4. runs ISCC to build punktfunk-host-setup-.exe, @@ -31,7 +33,9 @@ param( [string]$BunExe = $env:BUN_EXE, # portable bun.exe runtime for the console + runner [string]$VbCableDir = $env:VBCABLE_DIR, # official base VB-CABLE package -> bundle the virtual mic [switch]$NoDriver, # build without the bundled pf-vdisplay driver - [switch]$NoSign # skip signing (local debug) + [switch]$NoSign, # skip signing (local debug) + # 'auto' (default) = required iff this is a v* tag build; 'true'/'false' to force. See below. + [ValidateSet('auto', 'true', 'false')][string]$RequireSignedCert = 'auto' ) $ErrorActionPreference = 'Stop' $ProgressPreference = 'SilentlyContinue' @@ -71,6 +75,17 @@ $iscc = Find-Iscc Write-Host "ISCC: $iscc" # --- signing cert (supplied stable pfx OR ephemeral self-signed) ----------------------------- +# FAIL CLOSED on a real release. The ephemeral fallback below exists so canary/CI/dev builds keep +# working without the secret, but it is a per-build throwaway cert: nobody can pin it, and an +# installer signed with one is indistinguishable from one signed by an attacker. Silently falling +# back on a tag build would ship exactly that to users under the release's name — so on refs/tags/v* +# a missing MSIX_CERT_PFX_B64 (or -NoSign) is a build failure, not a downgrade. ('auto' resolves from +# GITHUB_REF so a workflow can't forget to opt in; -RequireSignedCert true/false overrides.) +$requireCert = if ($RequireSignedCert -eq 'auto') { $env:GITHUB_REF -like 'refs/tags/v*' } + else { [Convert]::ToBoolean($RequireSignedCert) } +if ($NoSign -and $requireCert) { + throw "release build ($env:GITHUB_REF) with -NoSign — refusing to publish an unsigned installer." +} $pfxPath = Join-Path $OutDir 'signing.pfx' $cerPath = Join-Path $OutDir "punktfunk-host-windows_${Version}.cer" $signtool = $null @@ -81,6 +96,11 @@ if (-not $NoSign) { Write-Host "signing with supplied code-signing cert (MSIX_CERT_PFX_B64)" [IO.File]::WriteAllBytes($pfxPath, [Convert]::FromBase64String($PfxBase64)) } + elseif ($requireCert) { + throw ("release build ($env:GITHUB_REF) with no MSIX_CERT_PFX_B64 — refusing to fall back to " + + "an ephemeral self-signed cert. Restore the MSIX_CERT_PFX_B64 / MSIX_CERT_PASSWORD " + + "repo secrets, or pass -RequireSignedCert false if this really is a test build.") + } else { Write-Host "no MSIX_CERT_PFX_B64 -> generating an ephemeral self-signed cert (subject $Publisher)" if (-not $PfxPassword) { $PfxPassword = 'punktfunk' }