From ea17c373131883a1498511804e4e57de9cc89948 Mon Sep 17 00:00:00 2001 From: Remylus Losius Date: Wed, 17 Jun 2026 22:51:42 -0400 Subject: [PATCH] fix(packaging): preserve operator TLS cert across the cert-shipping -> rc.10 upgrade The #596 generate-if-absent fix protects rc.10->onward upgrades, but the one-time transition FROM a release that shipped the cert in its payload (<= rc.9) removed the operator's cert entirely: rc.9 owned /etc/openwatch/tls/cert.pem, rc.10 does not, so the package manager reclaimed the orphaned file AFTER rc.10's provisioning had already run and skipped it. RPM left it missing (service can't start TLS); DEB regenerated a demo (operator cert lost). Found via the Stage-3 gap-closure RPM/DEB install-transition tests. Fix: - RPM: declare the cert/key paths %ghost so rpm tracks them with no payload content and does not reclaim the operator's file on the transition. - DEB: preinst stashes an existing cert/key to .dpkg-bak on upgrade (before dpkg removes the orphan); postinst restores it before provisioning. - AC-22 updated: assert %ghost (flag 'g', no content) on RPM and the preinst/postinst preserve dance on DEB; upgrade-container-test.sh now asserts an operator cert survives the rpm -U upgrade. - release-package-build spec v1.3.0 (C-05 + AC-22 extended). Verified in containers: genuine published rc.9 -> rc.10 preserves the operator cert+key on both RPM (%ghost) and DEB (preinst/postinst); fresh install still generates a demo; steady-state reinstall preserves. --- CHANGELOG.md | 4 ++- packaging/deb/postinst | 10 ++++++ packaging/deb/preinst | 14 ++++++++ packaging/rpm/openwatch.spec | 5 +++ packaging/tests/package_test.go | 44 +++++++++++++++++++---- packaging/tests/upgrade-container-test.sh | 13 ++++++- specs/release/package-build.spec.yaml | 21 +++++++---- 7 files changed, 96 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b21cc378..e18da14f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,9 @@ overwriting an operator's TLS certificate. certificate is now generated at install time only when the TLS files are absent (the same generate-if-absent model already used for the server's identity keys), so a certificate you put in place survives upgrades untouched - (#596). + (#596). This also covers the one-time upgrade from an earlier build that did + ship the demo certificate (rc.9 and before): your certificate is preserved + rather than removed during that transition too (#598). --- diff --git a/packaging/deb/postinst b/packaging/deb/postinst index fe9ac671..99289a3b 100755 --- a/packaging/deb/postinst +++ b/packaging/deb/postinst @@ -16,6 +16,16 @@ if [ "$1" = "configure" ]; then # failure should abort configure rather than leave an unbootable service. /usr/lib/openwatch/provision-identity-keys.sh + # Restore an operator TLS cert/key that preinst stashed before dpkg removed + # the orphaned demo cert during an upgrade from a cert-shipping release + # (<= rc.9). The restore runs BEFORE provisioning so the operator's file + # wins over a freshly generated demo. + for f in cert.pem key.pem; do + if [ -f "/etc/openwatch/tls/.$f.dpkg-bak" ]; then + mv -f "/etc/openwatch/tls/.$f.dpkg-bak" "/etc/openwatch/tls/$f" + fi + done + # Provision a demo TLS cert + key (generate-if-absent). Not shipped in the # payload so an upgrade never overwrites an operator's replacement cert. /usr/lib/openwatch/provision-tls-cert.sh diff --git a/packaging/deb/preinst b/packaging/deb/preinst index 31d31c92..ef5387db 100755 --- a/packaging/deb/preinst +++ b/packaging/deb/preinst @@ -11,6 +11,20 @@ set -e +# On upgrade FROM a release that shipped the demo TLS cert in its payload +# (<= rc.9), dpkg removes that now-orphaned cert/key during unpack because +# rc.10+ no longer own those paths. Stash a copy first so postinst can restore +# an operator's replacement certificate. rc.10+ generate the cert in postinst +# and never ship it, so a steady-state upgrade has nothing to remove and this +# is a harmless round-trip. (The RPM uses %ghost to the same end.) +if [ "$1" = "upgrade" ]; then + for f in cert.pem key.pem; do + if [ -f "/etc/openwatch/tls/$f" ]; then + cp -p "/etc/openwatch/tls/$f" "/etc/openwatch/tls/.$f.dpkg-bak" 2>/dev/null || : + fi + done +fi + getent group openwatch >/dev/null || groupadd --system openwatch getent passwd openwatch >/dev/null || \ useradd --system --gid openwatch \ diff --git a/packaging/rpm/openwatch.spec b/packaging/rpm/openwatch.spec index 864dd303..10c43257 100644 --- a/packaging/rpm/openwatch.spec +++ b/packaging/rpm/openwatch.spec @@ -145,7 +145,12 @@ systemctl daemon-reload || : # TLS dir ships empty (0750); %post generates the demo cert/key into it # generate-if-absent. The cert/key files are intentionally NOT packaged, so # a package upgrade cannot revert an operator's replacement certificate. +# They are declared %ghost — rpm tracks the paths (no payload content, nothing +# laid down or verified) so that on an upgrade FROM a release that DID ship the +# cert (<= rc.9), rpm does not reclaim/erase the operator's file as an orphan. %dir %attr(0750, root, openwatch) /etc/openwatch/tls +%ghost %attr(0644, root, openwatch) /etc/openwatch/tls/cert.pem +%ghost %attr(0600, openwatch, openwatch) /etc/openwatch/tls/key.pem %config(noreplace) %attr(0640, root, openwatch) /etc/openwatch/openwatch.toml %config(noreplace) %attr(0640, root, openwatch) /etc/openwatch/upgrade.conf %attr(0644, root, root) /etc/systemd/system/openwatch.service diff --git a/packaging/tests/package_test.go b/packaging/tests/package_test.go index 145e2a12..0a8e5d02 100644 --- a/packaging/tests/package_test.go +++ b/packaging/tests/package_test.go @@ -659,20 +659,30 @@ func TestTLS_PostInstallProvisions(t *testing.T) { } } - // Payload ships the empty tls dir + the helper, but NEVER the cert/key - // files themselves (that is what made upgrades clobber operator certs). + // Payload ships the empty tls dir + the helper, and carries NO real + // cert/key content. On RPM the cert/key paths are declared %ghost so + // rpm tracks them (flag 'g') without laying down content — this stops a + // package upgrade FROM a cert-shipping release (<= rc.9) from reclaiming + // the operator's file as an orphan, while still never shipping a cert. const tlsDir = "/etc/openwatch/tls" - rpmFiles := rpmQuery(t, rpm, "[%{FILENAMES}\n]") - if !strings.Contains(rpmFiles, helperPath) { + if !strings.Contains(rpmQuery(t, rpm, "[%{FILENAMES}\n]"), helperPath) { t.Errorf("RPM payload missing the TLS helper %s", helperPath) } - if !strings.Contains(rpmFiles, tlsDir) { + rpmGhost := rpmQuery(t, rpm, "[%{FILENAMES} %{FILEFLAGS:fflags}\n]") + if !strings.Contains(rpmGhost, tlsDir) { t.Errorf("RPM payload missing the %s directory", tlsDir) } - if strings.Contains(rpmFiles, "tls/cert.pem") || strings.Contains(rpmFiles, "tls/key.pem") { - t.Errorf("RPM payload MUST NOT ship the TLS cert/key; got:\n%s", rpmFiles) + for _, p := range []string{"cert.pem", "key.pem"} { + re := regexp.MustCompile(`/etc/openwatch/tls/` + regexp.QuoteMeta(p) + `\s+\S*g\S*`) + if !re.MatchString(rpmGhost) { + t.Errorf("RPM tls/%s MUST be declared %%ghost (flag 'g', no payload content); file list:\n%s", p, rpmGhost) + } } + // DEB has no %ghost: the cert/key are not in the payload, and preinst + // stashes an operator's cert before dpkg removes the orphan on upgrade, + // while postinst restores it. So the cert/key are NEVER real payload + // files in either format. debFiles := debContents(t, deb) if !strings.Contains(debFiles, helperPath) { t.Errorf("DEB payload missing the TLS helper %s", helperPath) @@ -684,6 +694,16 @@ func TestTLS_PostInstallProvisions(t *testing.T) { t.Errorf("DEB payload MUST NOT ship the TLS cert/key; got:\n%s", debFiles) } + // DEB preinst backs up an operator cert on upgrade; postinst restores it. + preinst := readPackagingFile(t, dir, "deb", "preinst") + if !strings.Contains(preinst, ".dpkg-bak") || !strings.Contains(preinst, `"$1" = "upgrade"`) { + t.Errorf("deb/preinst must back up the TLS cert/key (.dpkg-bak) on upgrade") + } + postinst := readPackagingFile(t, dir, "deb", "postinst") + if !strings.Contains(postinst, ".dpkg-bak") || !strings.Contains(postinst, "mv -f") { + t.Errorf("deb/postinst must restore the preserved TLS cert/key (.dpkg-bak)") + } + // The build scripts no longer stage a demo cert into the payload. for _, bs := range []string{"rpm/build-rpm.sh", "deb/build-deb.sh"} { b, err := os.ReadFile(filepath.Join(dir, "packaging", bs)) @@ -697,6 +717,16 @@ func TestTLS_PostInstallProvisions(t *testing.T) { }) } +// readPackagingFile reads packaging// as a string. +func readPackagingFile(t *testing.T, appDir, sub, name string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(appDir, "packaging", sub, name)) + if err != nil { + t.Fatalf("read packaging/%s/%s: %v", sub, name, err) + } + return string(b) +} + // readDebControlScript extracts a named maintainer script from a DEB // using `dpkg-deb --ctrl-tarfile` piped to `tar`. Returns the script body // as a string. diff --git a/packaging/tests/upgrade-container-test.sh b/packaging/tests/upgrade-container-test.sh index ff9f0697..8f5c6737 100755 --- a/packaging/tests/upgrade-container-test.sh +++ b/packaging/tests/upgrade-container-test.sh @@ -48,6 +48,13 @@ echo "### install OLD package (release 1)" rpm -i --nodeps /rpms/old/openwatch-*.rpm echo "OPENWATCH_DATABASE_DSN=$DSN" > /etc/openwatch/secrets.env +# Simulate an operator who replaced the demo TLS cert with their own. It MUST +# survive the upgrade — including the transition from a release that shipped +# the cert in its payload (the cert/key are %ghost in current packages, so rpm +# must not reclaim the operator's file). release-package-build C-05 / AC-22. +echo "OPERATOR-TLS-CERT-SENTINEL-DO-NOT-CLOBBER" > /etc/openwatch/tls/cert.pem +echo "OPERATOR-TLS-KEY-SENTINEL" > /etc/openwatch/tls/key.pem + echo "### bring DB to head, then roll back the head migration ($HEAD_VER) to simulate the prior version" set -a; . /etc/openwatch/secrets.env; set +a openwatch migrate >/dev/null @@ -76,8 +83,12 @@ fail=0 ls /var/lib/openwatch/backups/openwatch-pre-upgrade-*.sql >/dev/null 2>&1 || { echo "FAIL: no pre-upgrade backup"; fail=1; } grep -q "stop openwatch.service" /tmp/systemctl.log || { echo "FAIL: service not stopped"; fail=1; } grep -q "start openwatch.service" /tmp/systemctl.log || { echo "FAIL: service not restarted"; fail=1; } +grep -q "OPERATOR-TLS-CERT-SENTINEL-DO-NOT-CLOBBER" /etc/openwatch/tls/cert.pem 2>/dev/null \ + || { echo "FAIL: operator TLS cert was NOT preserved across the upgrade (cert.pem=$(head -1 /etc/openwatch/tls/cert.pem 2>/dev/null || echo MISSING))"; fail=1; } +grep -q "OPERATOR-TLS-KEY-SENTINEL" /etc/openwatch/tls/key.pem 2>/dev/null \ + || { echo "FAIL: operator TLS key was NOT preserved across the upgrade"; fail=1; } if [ "$fail" -eq 0 ]; then - echo "RESULT: PASS - the package upgrade migrated $PREV_VER -> $HEAD_VER with a backup and a stop/start" + echo "RESULT: PASS - the package upgrade migrated $PREV_VER -> $HEAD_VER with a backup, a stop/start, and preserved the operator TLS cert" else echo "RESULT: FAIL" exit 1 diff --git a/specs/release/package-build.spec.yaml b/specs/release/package-build.spec.yaml index f6742ad8..aea1a428 100644 --- a/specs/release/package-build.spec.yaml +++ b/specs/release/package-build.spec.yaml @@ -1,7 +1,7 @@ spec: id: release-package-build title: Native RPM and DEB packaging - version: "1.2.0" + version: "1.3.0" status: approved tier: 2 @@ -73,7 +73,12 @@ spec: time by a post-install scriptlet, generate-if-absent (never overwrite). The packages ship only the empty /etc/openwatch/tls directory plus the provisioning helper. openssl MUST be a package dependency (shared with - C-11). + C-11). Additionally, an operator's certificate MUST survive the one-time + upgrade FROM a release that DID ship the cert in its payload (<= rc.9): + the RPM MUST declare the cert/key paths %ghost (rpm tracks the path with + no payload content, so it does not reclaim the operator's file as an + orphan), and the DEB MUST stash an existing cert/key in preinst (before + dpkg removes the orphaned file) and restore it in postinst. type: security enforcement: error - id: C-06 @@ -232,9 +237,13 @@ spec: root:openwatch, key.pem 0600 openwatch:openwatch) only when both files are absent. The package payloads ship the empty /etc/openwatch/tls directory and the helper at /usr/lib/openwatch/provision-tls-cert.sh, - but do NOT contain cert.pem or key.pem (verified via rpm -qpl and - dpkg-deb -c), and the build scripts no longer stage a cert into the - payload — so a package upgrade never reverts an operator's replacement - certificate. + but do NOT contain cert.pem or key.pem as real payload content (verified + via rpm -qpl and dpkg-deb -c), and the build scripts no longer stage a + cert into the payload. For the one-time upgrade from a cert-shipping + release (<= rc.9): the RPM declares the cert/key paths %ghost (file flag + 'g', no content) so rpm does not reclaim the operator's orphaned file, + and the DEB preinst stashes the cert/key to a .dpkg-bak on upgrade while + the postinst restores them — so a package upgrade never reverts (or + removes) an operator's replacement certificate, including the transition. priority: critical references_constraints: [C-05]