Harden the token store, macOS signing, the release integrity check and the MSI check

.github/scripts/release-integrity-test.py

@@ -174,6 +174,25 @@ def test_negative_a_certificate_whose_signature_does_not_verify_does_not_chain(s
         self.assertFalse(self.trust.certificate_chains(changed, self.entry["integratedTime"]))
 
 
+class Bundles(unittest.TestCase):
+    SUMS = {"hodeishield-1.0.0-x86_64.tar.gz": "a" * 64, "hodeishield-1.0.0.pkg": "b" * 64}
+
+    def test_the_required_set_ignores_which_bundles_are_attached(self):
+        assets = {"SHA256SUMS", "install.sh", "hodeishield-1.0.0.msi", *self.SUMS}
+        self.assertEqual(ri.must_be_signed(assets, self.SUMS), {"SHA256SUMS", "install.sh", "hodeishield-1.0.0.msi", *self.SUMS})
+
+    def test_negative_a_deleted_bundle_does_not_shrink_the_required_set(self):
+        assets = {"SHA256SUMS", "SHA256SUMS.sigstore.json", *self.SUMS}
+        self.assertEqual(ri.must_be_signed(assets, self.SUMS), {"SHA256SUMS", *self.SUMS})
+
+    def test_a_file_in_sha256sums_is_required_even_if_its_name_matches_no_pattern(self):
+        self.assertIn("other.bin", ri.must_be_signed({"SHA256SUMS"}, {"other.bin": "c" * 64}))
+
+    def test_bundles_and_provenance_are_not_required_to_have_bundles(self):
+        names = {"x.tar.gz.sigstore.json", "multiple.intoto.jsonl"}
+        self.assertEqual(ri.must_be_signed(names, {}), {"SHA256SUMS"})
+
+
 class Parsing(unittest.TestCase):
     def test_sha256sums_in_text_and_binary_mode(self):
         text = "a" * 64 + "  one.tar.gz\n" + "b" * 64 + " *two.zip\n\n"

.github/scripts/release-integrity.py

@@ -17,8 +17,10 @@
                      commit, and from 0.4.0 a successful container-image.yml run;
   files              every release file matches its line in the release's SHA256SUMS, and nothing
                      else is attached;
-  Rekor              every file the release signs (it has a `.sigstore.json` bundle) is looked up in
-                     Rekor by its SHA-256. An entry counts only if its signed entry timestamp
+  bundles            every archive and installer attached, every file in SHA256SUMS and SHA256SUMS
+                     itself must carry a `.sigstore.json` bundle; this set does not depend on which
+                     bundles are attached, so deleting a bundle is reported;
+  Rekor              every one of those files is looked up in Rekor by its SHA-256. An entry counts only if its signed entry timestamp
                      verifies with Rekor's key and its certificate chains to Sigstore's
                      certificate authority at the time it was logged: Rekor accepts any
                      certificate, so one that does not chain could claim any identity. At least
@@ -70,6 +72,10 @@
 ACCEPTED_RUNS: dict[int, str] = {}
 
 VERSION_TAG = re.compile(r"^v(\d+)\.(\d+)\.(\d+)(-[0-9A-Za-z.-]+)?$")
+# The release assets release.yml signs, by name (the "Sign every archive and SHA256SUMS" step).
+SIGNED_ASSET = re.compile(
+    r"hodeishield-.*\.(tar\.gz|zip|msi|pkg|rpm|cdx\.json)|hodeishield[-_].*\.deb|install\.sh"
+)
 IMAGE_SIGNATURE_TAG = re.compile(r"^sha256-[0-9a-f]{64}(\.sig|\.att|\.sbom)?$")
 
 
@@ -232,6 +238,12 @@ def download(url: str) -> bytes:
     return body
 
 
+def must_be_signed(assets, sums) -> set[str]:
+    """The files that must carry a Sigstore bundle, whichever bundles are attached: SHA256SUMS,
+    every file it lists and every attached file that release.yml signs by name."""
+    return {"SHA256SUMS", *sums} | {name for name in assets if SIGNED_ASSET.fullmatch(name)}
+
+
 def parse_sha256sums(text: str) -> dict[str, str]:
     sums = {}
     for line in text.splitlines():
@@ -575,9 +587,12 @@ def problem(text: str):
             ):
                 problem(f"{tag}: {name} is attached but not in SHA256SUMS")
 
-        signed = [name for name in digests if f"{name}.sigstore.json" in assets]
-        if not signed:
-            problem(f"{tag}: no file has a Sigstore bundle")
+        signed = []
+        for name in sorted(must_be_signed(assets, sums)):
+            if f"{name}.sigstore.json" not in assets:
+                problem(f"{tag}: {name} has no Sigstore bundle")
+            elif name in digests:
+                signed.append(name)
         for name in signed:
             matched, wrong = judge_rekor(
                 rekor_entries(digests[name], trust), repository=repository, tag=tag, releases=signers

.github/workflows/macos-signing-check.yml

@@ -16,6 +16,12 @@ on:
 
 permissions: {}
 
+# The Apple Team ID the Developer ID certificates must belong to: repository policy, changed only
+# through a reviewed pull request. Empty until a maintainer sets it, so signing fails closed. The
+# same value is in release.yml; keep the two in step.
+env:
+  MACOS_TEAM_ID: ""
+
 concurrency:
   group: macos-signing-check
   cancel-in-progress: false
@@ -42,6 +48,12 @@ jobs:
           APPLE_NOTARIZATION_ISSUER_ID: ${{ secrets.APPLE_NOTARIZATION_ISSUER_ID }}
         run: |
           set -euo pipefail
+          # The expected Apple Team ID is repository policy (MACOS_TEAM_ID above), not a secret: the
+          # certificates in the environment must belong to it. Empty or malformed fails closed.
+          if ! [[ "$MACOS_TEAM_ID" =~ ^[A-Z0-9]{10}$ ]]; then
+            echo "::error::MACOS_TEAM_ID is not set to a 10-character Apple Team ID in this workflow."
+            exit 1
+          fi
           missing=""
           for name in MACOS_CERTIFICATE MACOS_CERTIFICATE_PASSWORD MACOS_SIGNING_IDENTITY \
                       MACOS_INSTALLER_CERTIFICATE MACOS_INSTALLER_CERTIFICATE_PASSWORD \
@@ -116,8 +128,8 @@ jobs:
               exit 1
             fi
           done
-          if [ -z "$team" ] || [ "$team" = "not set" ]; then
-            echo "::error::The signature carries no TeamIdentifier."
+          if [ "$team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The signature's TeamIdentifier is not the expected MACOS_TEAM_ID."
             exit 1
           fi
 
@@ -171,8 +183,8 @@ jobs:
           bin="$RUNNER_TEMP/check/signing-check"
           pkg="$RUNNER_TEMP/signing-check.pkg"
           team="$(codesign --display -vv "$bin" 2>&1 | sed -n 's/^TeamIdentifier=//p')"
-          if [ -z "$team" ] || [ "$team" = "not set" ]; then
-            echo "::error::The signature carries no TeamIdentifier."
+          if [ "$team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The signature's TeamIdentifier is not the expected MACOS_TEAM_ID."
             exit 1
           fi
           accepted=false
@@ -192,6 +204,12 @@ jobs:
             echo "::error::The package is not signed with a Developer ID Installer certificate."
             exit 1
           fi
+          # The installer certificate's Team ID is the last parenthesis of its name.
+          pkg_team="$(sed -n 's/^ *[0-9]*\. Developer ID Installer: .* (\([A-Z0-9]\{10\}\))$/\1/p' <<< "$sig")"
+          if [ "$pkg_team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The installer certificate's Team ID is not the expected MACOS_TEAM_ID."
+            exit 1
+          fi
           xcrun stapler validate "$pkg"
           spctl --assess --type install --verbose=2 "$pkg"
           {

.github/workflows/release.yml

@@ -276,6 +276,10 @@ jobs:
       VERSION: ${{ needs.version.outputs.version }}
       TARGET: universal-apple-darwin
       SIGN: ${{ needs.version.outputs.sign_macos }}
+      # The Apple Team ID the Developer ID certificates must belong to: repository policy, changed
+      # only through a reviewed pull request. Empty until a maintainer sets it, so signing fails
+      # closed. The same value is in macos-signing-check.yml; keep the two in step.
+      MACOS_TEAM_ID: ""
     steps:
       - name: Download the unsigned binary
         uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
@@ -338,6 +342,12 @@ jobs:
           APPLE_NOTARIZATION_ISSUER_ID: ${{ secrets.APPLE_NOTARIZATION_ISSUER_ID }}
         run: |
           set -euo pipefail
+          # The expected Apple Team ID is repository policy (MACOS_TEAM_ID above), not a secret: the
+          # certificates in the environment must belong to it. Empty or malformed fails closed.
+          if ! [[ "$MACOS_TEAM_ID" =~ ^[A-Z0-9]{10}$ ]]; then
+            echo "::error::MACOS_TEAM_ID is not set to a 10-character Apple Team ID in this workflow."
+            exit 1
+          fi
           missing=0
           for name in MACOS_CERTIFICATE MACOS_CERTIFICATE_PASSWORD MACOS_SIGNING_IDENTITY \
                       MACOS_INSTALLER_CERTIFICATE MACOS_INSTALLER_CERTIFICATE_PASSWORD \
@@ -397,8 +407,8 @@ jobs:
               exit 1
             fi
           done
-          if [ -z "$team" ] || [ "$team" = "not set" ]; then
-            echo "::error::The signature carries no TeamIdentifier."
+          if [ "$team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The signature's TeamIdentifier is not the expected MACOS_TEAM_ID."
             exit 1
           fi
 
@@ -461,8 +471,8 @@ jobs:
           bin="stage/${BIN_NAME}"
           pkg="dist/hodeishield-${VERSION}-${TARGET}.pkg"
           team="$(codesign --display -vv "$bin" 2>&1 | sed -n 's/^TeamIdentifier=//p')"
-          if [ -z "$team" ] || [ "$team" = "not set" ]; then
-            echo "::error::The signature carries no TeamIdentifier."
+          if [ "$team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The signature's TeamIdentifier is not the expected MACOS_TEAM_ID."
             exit 1
           fi
           accepted=false
@@ -482,6 +492,12 @@ jobs:
             echo "::error::The package is not signed with a Developer ID Installer certificate."
             exit 1
           fi
+          # The installer certificate's Team ID is the last parenthesis of its name.
+          pkg_team="$(sed -n 's/^ *[0-9]*\. Developer ID Installer: .* (\([A-Z0-9]\{10\}\))$/\1/p' <<< "$sig")"
+          if [ "$pkg_team" != "$MACOS_TEAM_ID" ]; then
+            echo "::error::The installer certificate's Team ID is not the expected MACOS_TEAM_ID."
+            exit 1
+          fi
           xcrun stapler validate "$pkg"
           spctl --assess --type install --verbose=2 "$pkg"
           {
@@ -848,7 +864,8 @@ jobs:
           retention-days: 1
 
   # Signs the MSI with the same Azure Artifact Signing profile as the exe. Like `sign-windows`, it
-  # checks nothing out and runs no repository or WiX code. The MSI is published only when signed.
+  # runs no repository or WiX code: the checkout is only the reference copy of LICENSE, NOTICE and
+  # user-information.md that the extracted payload is compared with. The MSI is published only when signed.
   sign-msi:
     name: msi (sign)
     needs: [version, sign-windows, windows-msi]
@@ -862,6 +879,11 @@ jobs:
       VERSION: ${{ needs.version.outputs.version }}
       SIGN: ${{ needs.version.outputs.sign_windows }}
     steps:
+      - name: Checkout
+        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
       - name: Download the MSI
         uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
         with:
@@ -880,9 +902,11 @@ jobs:
 
       # The MSI was built by a job that ran WiX, third-party code: do not sign it on trust. Using
       # only Windows' own Installer API, check that it carries no custom actions or embedded
-      # binaries (so nothing runs at install time but standard actions), then extract it with an
-      # administrative install and check its payload is exactly the exe sign-windows signed, plus
-      # LICENSE, NOTICE and USER-INFORMATION.md.
+      # binaries (so nothing runs at install time but standard actions), and that its administrative
+      # sequences hold only passive standard actions: `msiexec /a` runs AdminExecuteSequence, and
+      # actions such as SelfRegModules would load a DLL shipped as a payload file. Then extract it
+      # with an administrative install and check its payload is exactly the exe sign-windows signed,
+      # plus LICENSE, NOTICE and USER-INFORMATION.md byte for byte as in the checked-out commit.
       - name: Check the MSI before signing it
         shell: powershell
         env:
@@ -893,21 +917,57 @@ jobs:
           $installer = New-Object -ComObject WindowsInstaller.Installer
           $db = $installer.OpenDatabase($msi, 0)
           # The COM calls return values PowerShell would add to the function's output: discard them.
-          function Count-Rows([string]$table) {
-            try { $view = $db.OpenView("SELECT * FROM ``$table``") } catch { return [int]0 }
+          # Fails closed: only a table that is not in _Tables is empty; any other error stops the job.
+          function Get-Rows([string]$table, [string[]]$columns) {
+            $list = ($columns | ForEach-Object { "``$_``" }) -join ", "
+            $view = $db.OpenView("SELECT $list FROM ``$table``")
             [void]$view.Execute()
-            $n = 0
-            while ($null -ne $view.Fetch()) { $n++ }
+            $rows = @()
+            while ($null -ne ($record = $view.Fetch())) {
+              $rows += ,((1..$columns.Count | ForEach-Object { $record.StringData($_) }) -join "|")
+            }
             [void]$view.Close()
-            return [int]$n
+            return $rows
           }
           $bad = @()
-          foreach ($table in @("CustomAction", "Binary", "MsiEmbeddedUI", "MsiEmbeddedChainer")) {
-            $n = [int](Count-Rows $table)
-            Write-Host "$table rows: $n"
-            if ($n -ne 0) { $bad += $table }
+          # Every table must be one of those WiX writes for this package (those of the published
+          # 0.5.1 MSI). Anything else, CustomAction, Binary, SelfReg, Class, TypeLib, MsiAssembly,
+          # ServiceInstall, RemoveFile, Shortcut, IniFile... included, is refused by not being listed.
+          $tables = @(Get-Rows "_Tables" @("Name"))
+          Write-Host "Tables: $($tables -join ', ')"
+          $knownTables = @("Property", "Upgrade", "LaunchCondition", "Directory", "File", "Component", "Environment", "Registry", "Feature", "FeatureComponents", "AdminUISequence", "AdminExecuteSequence", "AdvtExecuteSequence", "InstallUISequence", "InstallExecuteSequence", "Media", "MsiFileHash", "_Validation")
+          $extraTables = @($tables | Where-Object { $knownTables -notcontains $_ })
+          if ($extraTables.Count -gt 0) { $bad += "tables ($($extraTables -join ', '))" }
+          # Only these standard actions may appear in a sequence (a subset: a WiX that omits one
+          # still passes). `msiexec /a` runs AdminExecuteSequence, so a custom or payload-loading
+          # action there would run code on this runner.
+          $allowedActions = @{
+            AdminExecuteSequence   = @("CostInitialize", "FileCost", "CostFinalize", "InstallValidate", "InstallInitialize", "InstallAdminPackage", "InstallFiles", "InstallFinalize")
+            AdminUISequence        = @("CostInitialize", "FileCost", "CostFinalize", "ExecuteAction")
+            AdvtExecuteSequence    = @("CostInitialize", "CostFinalize", "InstallValidate", "InstallInitialize", "InstallFinalize", "PublishFeatures", "PublishProduct")
+            InstallUISequence      = @("CostInitialize", "FileCost", "CostFinalize", "ExecuteAction", "FindRelatedProducts", "LaunchConditions", "ValidateProductID", "MigrateFeatureStates")
+            InstallExecuteSequence = @("CostInitialize", "FileCost", "CostFinalize", "InstallValidate", "InstallInitialize", "InstallFiles", "InstallFinalize", "PublishFeatures", "PublishProduct", "FindRelatedProducts", "LaunchConditions", "ValidateProductID", "MigrateFeatureStates", "ProcessComponents", "UnpublishFeatures", "RemoveRegistryValues", "RemoveEnvironmentStrings", "RemoveFiles", "RemoveFolders", "CreateFolders", "WriteRegistryValues", "WriteEnvironmentStrings", "RegisterUser", "RegisterProduct", "RemoveExistingProducts")
+          }
+          foreach ($sequence in $allowedActions.Keys) {
+            if ($tables -notcontains $sequence) { continue }
+            $found = @(Get-Rows $sequence @("Action"))
+            Write-Host "$sequence actions: $($found -join ', ')"
+            $extra = @($found | Where-Object { $allowedActions[$sequence] -notcontains $_ })
+            if ($extra.Count -gt 0) { $bad += "$sequence ($($extra -join ', '))" }
           }
-          $files = [int](Count-Rows "File")
+          # The only state the package changes: its InstallDir value and the machine PATH entry.
+          $pinned = @{
+            Registry    = @{ Columns = @("Root", "Key", "Name", "Value", "Component_"); Rows = @("2|Software\Hodeitek\HodeiShield CLI|InstallDir|[INSTALLFOLDER]|SystemPath") }
+            Environment = @{ Columns = @("Environment", "Name", "Value", "Component_"); Rows = @("PathEntry|=-*PATH|[~];[INSTALLFOLDER]|SystemPath") }
+          }
+          foreach ($table in $pinned.Keys) {
+            if ($tables -notcontains $table) { continue }
+            $rows = @(Get-Rows $table $pinned[$table].Columns)
+            Write-Host "$table rows: $($rows -join ' ; ')"
+            $extra = @($rows | Where-Object { $pinned[$table].Rows -notcontains $_ })
+            if ($extra.Count -gt 0) { $bad += "$table ($($extra -join ' ; '))" }
+          }
+          $files = @(Get-Rows "File" @("File")).Count
           Write-Host "File rows: $files"
           if ($files -ne 4) { $bad += "File ($files rows, expected 4)" }
           [System.Runtime.InteropServices.Marshal]::ReleaseComObject($db) | Out-Null
@@ -922,8 +982,15 @@ jobs:
           $exe = $payload | Where-Object { $_.Name -eq "hodeishield.exe" }
           $hash = (Get-FileHash -LiteralPath $exe.FullName -Algorithm SHA256).Hash.ToLower()
           if ($hash -ne $env:EXE_SHA256) { Write-Host "::error::the exe in the MSI is $hash, not the signed $env:EXE_SHA256"; exit 1 }
+          # The texts must be the ones in the repository at this commit, which windows-msi packaged.
+          $reference = @{ "LICENSE" = "LICENSE"; "NOTICE" = "NOTICE"; "USER-INFORMATION.md" = "docs\user-information.md" }
+          foreach ($name in $reference.Keys) {
+            $inMsi = (Get-FileHash -LiteralPath ($payload | Where-Object { $_.Name -eq $name }).FullName -Algorithm SHA256).Hash
+            $inRepo = (Get-FileHash -LiteralPath $reference[$name] -Algorithm SHA256).Hash
+            if ($inMsi -ne $inRepo) { Write-Host "::error::$name in the MSI ($inMsi) differs from the repository's ($inRepo)"; exit 1 }
+          }
           Remove-Item -Recurse -Force $target
-          Write-Host "MSI content: OK (no custom actions or binaries; payload is the signed exe, LICENSE, NOTICE, USER-INFORMATION.md)"
+          Write-Host "MSI content: OK (no custom actions or binaries; only passive admin actions; payload is the signed exe and the repository's LICENSE, NOTICE, USER-INFORMATION.md)"
 
       - name: Azure login (OIDC)
         if: env.SIGN == 'true'

.gitignore

@@ -19,3 +19,4 @@ target
 #  and can be added to the global gitignore or merged into this file.  For a more nuclear
 #  option (not recommended) you can uncomment the following to ignore the entire idea folder.
 #.idea/
+__pycache__/