ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Lock the shared fleet key away from the people standing at the bay

Browse Source 
The generated install.ps1 wrote the collector key to HKLM\SOFTWARE\GE\ShopDB and
left it at the DACL the key inherits, which grants BUILTIN\Users read. Any
operator logged on to any bay could read the fleet key, and that key still
authenticates an un-enrolled PC: it claims queued commands and downloads script
bodies and installer packages. The per-PC credential work exists to retire that
key, but it stays until every consumer is off it, so meanwhile what matters is
who can read it.

Protect-HklmShopdbKey breaks inheritance, DROPS the inherited entries rather
than copying them - copying is what silently preserves the Users read - and
states SYSTEM and Administrators by SID in both registry views. By SID, so a
localised Windows still matches.

Everything that legitimately reads this key is SYSTEM (the service, the
GE-Enforce reporters) or an administrator (the adoption tool,
Set-ShopDBServerPins.ps1). The tray reads no registry at all - it talks to the
service over the named pipe - so nothing loses access.

It runs for the key-less package too, because ServerPins live in the same key
and the registry is authoritative for pinning: a standard user must not be able
to rewrite the pins either.

A bay that cannot take the ACL still installs. It says so, loudly, rather than
failing the install or staying quiet, because an agent that is not installed is
a worse outcome than a key that is still readable on that one machine.

Note what this does NOT fix: bays that already carry the key keep their
inherited DACL until they are reinstalled or upgraded through a package built
after this change.
This commit is contained in:
cproudlock
2026-09-18 13:29:36 -04:00
parent 35d4fcd41c
commit fe4f44f855
2 changed files with 118 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
plugins/endpoints/api/siteagent.py
View File

@@ -410,6 +410,57 @@ function Set-HklmValueBothViews {{
}}
}}
# Lock SOFTWARE\\GE\\ShopDB to SYSTEM and Administrators, in both views.
#
# WHY. The shared fleet key is written here and stays until every consumer is
# on a per-PC credential, and the DACL this key inherits by default grants
# BUILTIN\\Users read - so any logged-on user on any bay could read the fleet
# key, which is the exact exposure the per-PC credential migration exists to
# close. Everything that legitimately reads this key runs as SYSTEM (the agent
# service, the GE-Enforce reporters) or as an admin (the adoption tool,
# Set-ShopDBServerPins.ps1). The tray reads no registry at all - it talks to the
# service over the named pipe - so nothing loses access here.
#
# It also protects ServerPins from a standard user, which is what makes the
# registry safe to treat as authoritative for pinning.
function Protect-HklmShopdbKey {{
param([Parameter(Mandatory)] [string] $SubKey)
$rights = [System.Security.AccessControl.RegistryRights]
foreach ($view in @([Microsoft.Win32.RegistryView]::Registry64,
[Microsoft.Win32.RegistryView]::Registry32)) {{
$base = [Microsoft.Win32.RegistryKey]::OpenBaseKey(
[Microsoft.Win32.RegistryHive]::LocalMachine, $view)
try {{
$key = $base.OpenSubKey(
$SubKey,
[Microsoft.Win32.RegistryKeyPermissionCheck]::ReadWriteSubTree,
$rights::ChangePermissions -bor $rights::ReadPermissions)
if (-not $key) {{ continue }}
try {{
$acl = $key.GetAccessControl(
[System.Security.AccessControl.AccessControlSections]::Access)
# Break inheritance and DROP the inherited entries rather than
# copying them in: copying is what silently preserves the
# BUILTIN\\Users read this function exists to remove.
$acl.SetAccessRuleProtection($true, $false)
foreach ($rule in @($acl.Access)) {{ $null = $acl.RemoveAccessRule($rule) }}
foreach ($sid in @("S-1-5-18", "S-1-5-32-544")) {{
$who = New-Object System.Security.Principal.SecurityIdentifier($sid)
$acl.AddAccessRule((New-Object System.Security.AccessControl.RegistryAccessRule(
$who,
$rights::FullControl,
[System.Security.AccessControl.InheritanceFlags]::ContainerInherit,
[System.Security.AccessControl.PropagationFlags]::None,
[System.Security.AccessControl.AccessControlType]::Allow)))
}}
$key.SetAccessControl($acl)
}}
finally {{ $key.Dispose() }}
}}
finally {{ $base.Dispose() }}
}}
}}
# REG_MULTI_SZ in both views, compare-before-write so a reinstall with the same
# pins leaves the value (and its last-write time) alone.
function Set-HklmMultiStringBothViews {{
@@ -492,6 +543,17 @@ else {{
Write-Warning "no server pin in this package; ServerPins left as is."
}}
# Harden the key AFTER the writes: the DACL is replaced, not merged, so writing
# first and locking second keeps this correct whether or not the key pre-existed.
try {{
Protect-HklmShopdbKey -SubKey "SOFTWARE\\GE\\ShopDB"
Write-Host " HKLM\\SOFTWARE\\GE\\ShopDB restricted to SYSTEM and Administrators."
}}
catch {{
Write-Warning "could not restrict HKLM\\SOFTWARE\\GE\\ShopDB: $($_.Exception.Message)"
Write-Warning "the shared key on this PC stays readable by any logged-on user until that is fixed."
}}
# 1a. Trust the bundled server certificate, if one shipped. The agent's outbound
# TLS to a self-signed or internal-CA server fails closed until the machine
# trusts the cert. Importing into LocalMachine\\Root covers the SYSTEM service.

56
tests/test_plugins/test_endpoints_siteagent_build.py
View File

@@ -289,6 +289,62 @@ def test_leaf_certificate_is_imported_into_root(client, db, auth_headers):
assert json.loads(files['appsettings.json'])['serverpins'] == [leafpin]
def test_installer_restricts_the_registry_key_to_system_and_admins(
client, db, auth_headers, servercert):
"""The shared key stays in HKLM until every consumer is on a per-PC
credential, so what matters meanwhile is who can read it. The key inherits a
DACL that grants BUILTIN\\Users read, which is the exposure the per-PC
migration exists to close - the installer must not leave it that way."""
files = _files(_build(client, auth_headers, collectorkey=SECRET,
includesharedkey='true'))
script = files['install.ps1'].decode('ascii')
assert 'function Protect-HklmShopdbKey' in script
assert 'Protect-HklmShopdbKey -SubKey "SOFTWARE\\GE\\ShopDB"' in script
# SYSTEM and Administrators by SID, so a localised Windows still matches.
assert '"S-1-5-18", "S-1-5-32-544"' in script
# Inheritance broken WITHOUT copying the inherited entries: copying them is
# what silently preserves the Users read.
assert 'SetAccessRuleProtection($true, $false)' in script
assert 'RemoveAccessRule($rule)' in script
def test_the_key_is_written_before_it_is_locked(client, db, auth_headers, servercert):
"""SetAccessControl replaces the DACL rather than merging, so a lock that ran
first would be undone by nothing - but a write that ran after a lock the
installer no longer holds rights for would fail. Order is the guarantee."""
script = _files(_build(client, auth_headers, collectorkey=SECRET,
includesharedkey='true'))['install.ps1'].decode('ascii')
write = script.index('-Name "CollectorKey" -Value $CollectorKey')
lock = script.index('Protect-HklmShopdbKey -SubKey')
assert write < lock
def test_the_registry_lock_failing_does_not_fail_the_install(
client, db, auth_headers, servercert):
"""A bay that cannot take the ACL still needs its agent. It must say plainly
that the key is readable there, rather than dying or staying quiet."""
script = _files(_build(client, auth_headers, collectorkey=SECRET,
includesharedkey='true'))['install.ps1'].decode('ascii')
lock = script.index('Protect-HklmShopdbKey -SubKey')
tail = script[lock:lock + 600]
assert 'catch' in tail
assert 'readable by any logged-on user' in tail
def test_the_keyless_package_still_locks_the_key(client, db, auth_headers, servercert):
"""ServerPins live in the same key and the registry is authoritative for
pinning, so a standard user must not be able to rewrite them either."""
files = _files(_build(client, auth_headers, collectorkey=SECRET,
includesharedkey=False))
script = files['install.ps1'].decode('ascii')
assert b'CollectorKey' not in files['install.ps1']
assert 'Protect-HklmShopdbKey -SubKey "SOFTWARE\\GE\\ShopDB"' in script
def test_install_script_helper_without_pins_or_key():
script = siteagent._install_script('https://shopdb.test', '', 900,
serverpins=[], includesharedkey=False)