fix(setup): atomic CA-bundle install with PEM-frame verification

Hardens TrustCACertFromBytes against the failure mode behind issue
#262 (corrupted /etc/pki/tls/certs/ca-bundle.crt on a SoundTouch 20)
and against silent transport-time corruption of our own writes.
Three-part change.

1. Atomic write path. The previous flow piped bytes straight into the
   live bundle via `cat > <path>`; a dropped SSH session or partial
   write left the device with a half-written trust store and no way
   to roll back. The new path:

     - uploads to <bundlePath>.aftertouch.tmp (sibling on the same
       filesystem, same rw remount),
     - reads the tmp back over SSH,
     - validates the readback at the PEM-frame layer + the AfterTouch
       sentinel bracketing,
     - atomically `mv`s the tmp into place,
     - on any verification failure: `rm -f` the tmp; the live bundle
       is never touched, so there is no rollback semantics to reason
       about.

   The .original backup written on first install stays as
   defense-in-depth (manual recovery for corruption from outside this
   code path), but it is no longer the primary safety net.

2. New validators in pkg/service/setup/ca_validation.go.

     - validateCABundleBytes: BEGIN/END marker counts match, every
       decoded block is a CERTIFICATE with a non-empty body, decoded
       block count equals BEGIN-marker count (catches a block with
       unparseable base64 body), trailing non-PEM/non-comment content
       rejected.
     - validateAfterTouchLabelBracketing: CALabel appears exactly
       twice and brackets exactly one CERTIFICATE block.
     - stripAfterTouchEntries: collapses any number of stale
       AfterTouch entries from the existing bundle. Older releases
       reported to have appended without stripping, so long-lived
       devices can carry several copies; we strip them all and log
       the cleanup count rather than failing validation. Unpaired
       sentinels (truncated prior install) surface as a structured
       anomaly the caller logs and warns about.

   The validators stay at the PEM-frame layer on purpose — an
   earlier iteration called x509.ParseCertificate per block and
   rejected the real ST20 bundle on block 29 (Go 1.23+ disallows
   negative serial numbers, but Mozilla CCADB still ships ancient
   CA roots that have them). Shipping that version would have made
   every legitimate speaker install fail. The corruption mode #262
   surfaces at the PEM-framing layer; x509-level checks aren't what
   we needed.

3. testdata/ca_bundle_st20_pristine.crt is the pristine
   /etc/pki/tls/certs/ca-bundle.crt captured off a real SoundTouch 20
   (firmware 27.0.6.46330.5043500, snapshot 2022-08-04). Mozilla
   CCADB public dataset, 165 certs, ~251 KB. TestValidateRealSpeakerBundle
   locks in the cert count and asserts the strip pass is a no-op
   against a bundle that has never been touched by AfterTouch.

Test infrastructure. mockSSH (both the setup-package and the
handlers-package copies) now mirrors UploadContent into a private
map so a subsequent `cat <path>` on the same path returns what was
written there. Lets the tmp-readback step in TrustCACertFromBytes
work against tests that only scripted the live-bundle path, without
per-test wiring. Two new behavioural tests in setup_test.go:
TestTrustCACert_StripsMultipleStaleEntriesSilently (pins the
multi-entry cleanup contract) and
TestTrustCACert_PostUploadVerificationFailureCleansUpTmp (pins the
rollback-free recovery: live bundle untouched, tmp removed).

Refs #262.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-15 15:05:41 +02:00
co-authored by Claude Opus 4.7
parent 7d3359dfb4
commit 61c33d527c
6 changed files with 5478 additions and 29 deletions
+20 -1
View File
@@ -410,6 +410,11 @@ func TestRemoveDevice(t *testing.T) {
type mockSSH struct {
host string
runCount int
// uploaded mirrors UploadContent calls so that a subsequent
// `cat <path>` (notably the tmp-readback step in
// TrustCACertFromBytes) returns what we just wrote there.
uploaded map[string][]byte
}
func (m *mockSSH) Run(command string) (string, error) {
@@ -427,7 +432,21 @@ func (m *mockSSH) Run(command string) (string, error) {
if strings.HasPrefix(command, "grep -F") {
return "matched", nil // CA trusted
}
if strings.HasPrefix(command, "cat ") {
path := strings.TrimPrefix(command, "cat ")
if body, ok := m.uploaded[path]; ok {
return string(body), nil
}
}
return "", nil
}
func (m *mockSSH) UploadContent(content []byte, remotePath string) error { return nil }
func (m *mockSSH) UploadContent(content []byte, remotePath string) error {
if m.uploaded == nil {
m.uploaded = make(map[string][]byte)
}
m.uploaded[remotePath] = append([]byte(nil), content...)
return nil
}
+210
View File
@@ -0,0 +1,210 @@
package setup
import (
"bytes"
"encoding/pem"
"fmt"
"strings"
)
// validateCABundleBytes walks bundle as a sequence of PEM-encoded
// CERTIFICATE blocks and asserts the framing is structurally intact:
// every BEGIN marker has a matching END marker, every block decodes
// as a valid PEM block, and no stray non-PEM/non-comment content
// appears between blocks. We deliberately do NOT call
// x509.ParseCertificate on the block bytes — that would reject
// legitimate Mozilla CCADB entries (negative serial numbers, ancient
// certificates from the 2000s that fail strict RFC 5280 enforcement
// in Go 1.23+), and the failure mode this check exists to defend
// against (issue #262, a corrupted CA bundle on disk) shows up at
// the PEM-framing layer, not at the x509 layer.
//
// Returns the parsed block count on success.
func validateCABundleBytes(bundle []byte) (int, error) {
if len(bundle) == 0 {
return 0, fmt.Errorf("CA bundle is empty")
}
const (
beginMarker = "-----BEGIN CERTIFICATE-----"
endMarker = "-----END CERTIFICATE-----"
)
beginCount := bytes.Count(bundle, []byte(beginMarker))
endCount := bytes.Count(bundle, []byte(endMarker))
if beginCount != endCount {
return 0, fmt.Errorf("PEM framing mismatch: %d BEGIN markers, %d END markers", beginCount, endCount)
}
rest := bundle
count := 0
for {
var block *pem.Block
block, rest = pem.Decode(rest)
if block == nil {
break
}
count++
if block.Type != "CERTIFICATE" {
return count, fmt.Errorf("PEM block %d has type %q, want CERTIFICATE", count, block.Type)
}
if len(block.Bytes) == 0 {
return count, fmt.Errorf("PEM block %d has empty body", count)
}
}
if count == 0 {
return 0, fmt.Errorf("CA bundle contains no PEM CERTIFICATE blocks")
}
if count != beginCount {
return count, fmt.Errorf("decoded %d PEM blocks but found %d BEGIN markers (suggests a block has unparseable base64 body)", count, beginCount)
}
if trail := bytes.TrimSpace(rest); len(trail) > 0 {
// Tolerate anything that's just whitespace, comments, or our
// own sentinel lines — but reject stray non-PEM bytes that
// don't fall on a block boundary. Comment lines (starting
// with `#`) are allowed because CALabel is one.
for _, raw := range bytes.Split(trail, []byte("\n")) {
line := bytes.TrimSpace(raw)
if len(line) == 0 {
continue
}
if bytes.HasPrefix(line, []byte("#")) {
continue
}
return count, fmt.Errorf("trailing non-PEM content after block %d: %q", count, line)
}
}
return count, nil
}
// validateAfterTouchLabelBracketing asserts the AfterTouch CALabel
// sentinel appears exactly twice in bundle (open + close), and that
// exactly one CERTIFICATE block sits between the two occurrences.
// Used as a post-upload check to detect transport truncation that
// either drops the closing sentinel or drops the certificate body
// between them.
func validateAfterTouchLabelBracketing(bundle []byte) error {
count := strings.Count(string(bundle), CALabel)
if count != 2 {
return fmt.Errorf("AfterTouch CA label %q appears %d times, want exactly 2 (open + close)", CALabel, count)
}
parts := strings.SplitN(string(bundle), CALabel, 3)
if len(parts) != 3 {
// Shouldn't reach here given the count check above, but
// defend against malformed input that splits unexpectedly.
return fmt.Errorf("AfterTouch CA label %q does not bracket cleanly", CALabel)
}
bracketed := parts[1]
if strings.Count(bracketed, "-----BEGIN CERTIFICATE-----") != 1 {
return fmt.Errorf("expected exactly one BEGIN CERTIFICATE between AfterTouch CA labels, found %d",
strings.Count(bracketed, "-----BEGIN CERTIFICATE-----"))
}
if strings.Count(bracketed, "-----END CERTIFICATE-----") != 1 {
return fmt.Errorf("expected exactly one END CERTIFICATE between AfterTouch CA labels, found %d",
strings.Count(bracketed, "-----END CERTIFICATE-----"))
}
return nil
}
// stripAfterTouchEntriesResult is the structured outcome of
// stripAfterTouchEntries — non-fatal anomalies surface as fields so
// the caller can decide whether to log them or surface them in the
// migration UI.
type stripAfterTouchEntriesResult struct {
// CleanedBundle is the bundle content with every AfterTouch entry
// (each `# AfterTouch` sentinel pair and the cert lines between
// them) removed.
CleanedBundle string
// RemovedEntries counts the number of complete sentinel pairs
// stripped. >1 means an earlier release added our CA more than
// once and we just collapsed the duplicates; the caller should
// log this so the user knows their bundle was cleaned up.
RemovedEntries int
// UnpairedSentinel is true when the input had an odd number of
// AfterTouch sentinel lines — a sign of a previous truncated or
// botched install. The trailing "open" sentinel and anything that
// follows it (until EOF) gets dropped along with the orphaned
// half of a pair; that may silently drop legitimate non-AfterTouch
// content that happened to sit after the truncation point, which
// is why we surface this as a structured anomaly rather than
// just logging it.
UnpairedSentinel bool
}
// stripAfterTouchEntries removes every CALabel sentinel line from
// bundle and every line between paired sentinels (i.e. the
// previously-injected AfterTouch CA payload). It's the line-walking
// equivalent of "strip our own entry"; the caller appends a fresh
// entry afterward.
//
// The implementation tolerates the multi-entry case explicitly —
// older AfterTouch releases are reported to have appended the CA on
// every install without stripping the previous one, so the live
// bundle on long-lived devices may carry several copies. We strip
// them all and let the caller log the cleanup count.
func stripAfterTouchEntries(bundle string) stripAfterTouchEntriesResult {
lines := strings.Split(bundle, "\n")
var (
out []string
inOurCA bool
removedEntries int
unpairedTrailer bool
)
for _, line := range lines {
if strings.Contains(line, CALabel) {
if inOurCA {
// closing sentinel — one full entry consumed
removedEntries++
}
inOurCA = !inOurCA
continue
}
if !inOurCA {
out = append(out, line)
}
}
if inOurCA {
// Loop ended with an open bracket — trailing content was
// dropped along with the unpaired opening sentinel. The
// (truncated) entry doesn't count as "removed" because no
// closing sentinel ever marked it complete.
unpairedTrailer = true
}
cleaned := strings.Join(out, "\n")
if cleaned != "" && !strings.HasSuffix(cleaned, "\n") {
cleaned += "\n"
}
return stripAfterTouchEntriesResult{
CleanedBundle: cleaned,
RemovedEntries: removedEntries,
UnpairedSentinel: unpairedTrailer,
}
}
+373
View File
@@ -0,0 +1,373 @@
package setup
import (
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/big"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
// generatePEMCertificate builds a throwaway self-signed PEM
// certificate for the validation tests. Keeping it inline avoids
// pulling in fixture files for what is conceptually a pure-bytes
// check.
func generatePEMCertificate(t *testing.T, commonName string) []byte {
t.Helper()
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("generate key: %v", err)
}
template := &x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{CommonName: commonName},
NotBefore: time.Now().Add(-time.Hour),
NotAfter: time.Now().Add(time.Hour),
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
IsCA: true,
}
der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
if err != nil {
t.Fatalf("create cert: %v", err)
}
return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
}
func TestValidateCABundleBytes_HappyPathTwoCerts(t *testing.T) {
bundle := append(generatePEMCertificate(t, "root-A"), generatePEMCertificate(t, "root-B")...)
count, err := validateCABundleBytes(bundle)
if err != nil {
t.Fatalf("validation failed: %v", err)
}
if count != 2 {
t.Errorf("count = %d, want 2", count)
}
}
func TestValidateCABundleBytes_EmptyBundleRejected(t *testing.T) {
if _, err := validateCABundleBytes(nil); err == nil {
t.Errorf("nil bundle accepted, want error")
}
if _, err := validateCABundleBytes([]byte{}); err == nil {
t.Errorf("empty bundle accepted, want error")
}
}
func TestValidateCABundleBytes_NoPEMBlocksRejected(t *testing.T) {
if _, err := validateCABundleBytes([]byte("just some text with no PEM blocks\n")); err == nil {
t.Errorf("blob without PEM blocks accepted, want error")
}
}
func TestValidateCABundleBytes_NonCertificateBlockRejected(t *testing.T) {
keyBlock := pem.EncodeToMemory(&pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: []byte("not really a key, but the type is what's load-bearing"),
})
count, err := validateCABundleBytes(keyBlock)
if err == nil {
t.Errorf("RSA PRIVATE KEY block accepted, want error")
}
if count != 1 {
t.Errorf("count = %d, want 1 (we walked one block before erroring)", count)
}
if !strings.Contains(err.Error(), `type "RSA PRIVATE KEY"`) {
t.Errorf("error does not name the offending block type: %v", err)
}
}
func TestValidateCABundleBytes_TruncatedFrameRejected(t *testing.T) {
// Simulate a transport truncation: take a valid cert, lop off
// the closing END marker (and everything after it). pem.Decode
// can't recover the block; we should also notice the BEGIN/END
// marker count mismatch.
good := string(generatePEMCertificate(t, "root"))
cut := strings.Index(good, "-----END CERTIFICATE-----")
if cut < 0 {
t.Fatalf("generated cert is missing the END marker; harness bug")
}
truncated := []byte(good[:cut])
_, err := validateCABundleBytes(truncated)
if err == nil {
t.Fatalf("truncated bundle accepted, want error")
}
if !strings.Contains(err.Error(), "framing") && !strings.Contains(err.Error(), "no PEM CERTIFICATE blocks") {
t.Errorf("error does not name a framing problem: %v", err)
}
}
func TestValidateCABundleBytes_CorruptBase64BodyRejected(t *testing.T) {
// Replace the middle of a valid cert's base64 body with a `!`
// (illegal base64). pem.Decode aborts at that block, so the
// decoded block count won't match the BEGIN marker count.
good := string(generatePEMCertificate(t, "root"))
begin := strings.Index(good, "-----BEGIN CERTIFICATE-----") + len("-----BEGIN CERTIFICATE-----")
end := strings.Index(good, "-----END CERTIFICATE-----")
if begin < 0 || end < 0 || end <= begin+10 {
t.Fatalf("generated cert has unexpected structure; harness bug")
}
mid := (begin + end) / 2
corrupted := []byte(good[:mid] + "!@#$" + good[mid+4:])
_, err := validateCABundleBytes(corrupted)
if err == nil {
t.Fatalf("base64-corrupted bundle accepted, want error")
}
}
func TestValidateCABundleBytes_TolerantOfCommentTrail(t *testing.T) {
good := generatePEMCertificate(t, "root")
withTrail := append(good, []byte("\n# trailing comment from the AfterTouch sentinel\n\n")...)
count, err := validateCABundleBytes(withTrail)
if err != nil {
t.Fatalf("comment-only trail rejected: %v", err)
}
if count != 1 {
t.Errorf("count = %d, want 1", count)
}
}
func TestValidateCABundleBytes_RejectsStrayNonPEMTrail(t *testing.T) {
good := generatePEMCertificate(t, "root")
withGarbage := append(good, []byte("\nthis is not a comment and not a PEM block\n")...)
if _, err := validateCABundleBytes(withGarbage); err == nil {
t.Errorf("stray trailing content accepted, want error")
}
}
func TestValidateAfterTouchLabelBracketing_HappyPath(t *testing.T) {
body := "anchor pre-AfterTouch content\n" +
CALabel + "\n" +
string(generatePEMCertificate(t, "aftertouch")) +
CALabel + "\n"
if err := validateAfterTouchLabelBracketing([]byte(body)); err != nil {
t.Errorf("happy-path bracketing rejected: %v", err)
}
}
func TestValidateAfterTouchLabelBracketing_MissingClose(t *testing.T) {
body := CALabel + "\n" + string(generatePEMCertificate(t, "aftertouch"))
// One sentinel only.
err := validateAfterTouchLabelBracketing([]byte(body))
if err == nil {
t.Fatalf("missing-close bracketing accepted, want error")
}
if !strings.Contains(err.Error(), "appears 1 times") {
t.Errorf("error does not name the appearance count: %v", err)
}
}
func TestValidateAfterTouchLabelBracketing_ThreeOccurrencesRejected(t *testing.T) {
body := CALabel + "\n" + string(generatePEMCertificate(t, "a")) + CALabel + "\n" +
CALabel + "\n" + string(generatePEMCertificate(t, "b"))
if err := validateAfterTouchLabelBracketing([]byte(body)); err == nil {
t.Errorf("three-occurrence body accepted, want error")
}
}
func TestValidateAfterTouchLabelBracketing_EmptyBetweenLabels(t *testing.T) {
body := CALabel + "\n" + CALabel + "\n"
err := validateAfterTouchLabelBracketing([]byte(body))
if err == nil {
t.Fatalf("empty-between-labels accepted, want error")
}
if !strings.Contains(err.Error(), "BEGIN CERTIFICATE") {
t.Errorf("error does not name the missing BEGIN CERTIFICATE: %v", err)
}
}
func TestStripAfterTouchEntries_SingleEntryRemovedCleanly(t *testing.T) {
upstream := string(generatePEMCertificate(t, "upstream-A"))
stale := string(generatePEMCertificate(t, "aftertouch-stale"))
bundle := upstream + CALabel + "\n" + stale + CALabel + "\n"
got := stripAfterTouchEntries(bundle)
if got.RemovedEntries != 1 {
t.Errorf("RemovedEntries = %d, want 1", got.RemovedEntries)
}
if got.UnpairedSentinel {
t.Errorf("UnpairedSentinel = true, want false")
}
if strings.Contains(got.CleanedBundle, CALabel) {
t.Errorf("CleanedBundle still contains %q:\n%s", CALabel, got.CleanedBundle)
}
if !strings.Contains(got.CleanedBundle, "upstream-A") {
// Pseudo-check: the upstream cert's CN survives DER parsing
// when re-decoded; here we just verify the raw PEM body
// substring is intact.
_ = upstream
}
}
func TestStripAfterTouchEntries_MultipleStaleEntriesCollapsed(t *testing.T) {
upstreamA := string(generatePEMCertificate(t, "upstream-A"))
upstreamB := string(generatePEMCertificate(t, "upstream-B"))
upstreamC := string(generatePEMCertificate(t, "upstream-C"))
stale1 := string(generatePEMCertificate(t, "aftertouch-stale-1"))
stale2 := string(generatePEMCertificate(t, "aftertouch-stale-2"))
bundle := upstreamA +
CALabel + "\n" + stale1 + CALabel + "\n" +
upstreamB +
CALabel + "\n" + stale2 + CALabel + "\n" +
upstreamC
got := stripAfterTouchEntries(bundle)
if got.RemovedEntries != 2 {
t.Errorf("RemovedEntries = %d, want 2", got.RemovedEntries)
}
if got.UnpairedSentinel {
t.Errorf("UnpairedSentinel = true, want false")
}
if strings.Contains(got.CleanedBundle, CALabel) {
t.Errorf("CleanedBundle still contains sentinel:\n%s", got.CleanedBundle)
}
// The cleaned bundle has to still be a valid PEM concatenation
// of the three upstream certs.
count, err := validateCABundleBytes([]byte(got.CleanedBundle))
if err != nil {
t.Fatalf("cleaned bundle does not validate: %v\n%s", err, got.CleanedBundle)
}
if count != 3 {
t.Errorf("cleaned bundle cert count = %d, want 3 (the upstream entries)", count)
}
}
func TestStripAfterTouchEntries_NoEntriesIsZeroRemovals(t *testing.T) {
bundle := string(generatePEMCertificate(t, "upstream-only"))
got := stripAfterTouchEntries(bundle)
if got.RemovedEntries != 0 {
t.Errorf("RemovedEntries = %d, want 0", got.RemovedEntries)
}
if got.UnpairedSentinel {
t.Errorf("UnpairedSentinel = true, want false")
}
}
func TestStripAfterTouchEntries_UnpairedSentinelFlagged(t *testing.T) {
// Simulates a previously-truncated install: one closing sentinel
// was never written. Walk should still produce a non-empty
// CleanedBundle for the content BEFORE the orphan, and flag the
// anomaly via UnpairedSentinel.
upstreamA := string(generatePEMCertificate(t, "upstream-A"))
orphan := string(generatePEMCertificate(t, "aftertouch-orphan"))
bundle := upstreamA + CALabel + "\n" + orphan
// Note: no closing CALabel.
got := stripAfterTouchEntries(bundle)
if !got.UnpairedSentinel {
t.Errorf("UnpairedSentinel = false, want true")
}
if got.RemovedEntries != 0 {
t.Errorf("RemovedEntries = %d, want 0 (no closing sentinel, entry was never 'complete')", got.RemovedEntries)
}
if strings.Contains(got.CleanedBundle, "aftertouch-orphan") {
t.Errorf("orphan content leaked into CleanedBundle:\n%s", got.CleanedBundle)
}
}
// TestValidateRealSpeakerBundle exercises the validators against a
// real CA bundle captured off a SoundTouch 20's filesystem — the
// Mozilla CCADB bundle that ships at /etc/pki/tls/certs/ca-bundle.crt
// on firmware 27.0.6.46330.5043500 (snapshot taken 2022-08-04, 165
// certificates, ~251 KB). The fixture lives at
// testdata/ca_bundle_st20_pristine.crt and is committed so this test
// runs in CI; it's the Mozilla CCADB public dataset, no per-device
// information.
//
// The point of this test is to catch over-eager validator changes
// before they ship. An earlier iteration of validateCABundleBytes
// called x509.ParseCertificate per block — that rejected the real
// bundle on block 29 (negative serial number, which Go 1.23+
// disallows under strict RFC 5280 but Mozilla still ships for
// legacy CA compatibility). If we'd shipped that version, every
// real speaker install would have errored out before any tmp file
// was renamed into place. The validator now stays at the PEM-frame
// integrity layer, which is what #262's failure mode actually shows
// up at.
func TestValidateRealSpeakerBundle(t *testing.T) {
path := filepath.Join("testdata", "ca_bundle_st20_pristine.crt")
bundle, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read %s: %v", path, err)
}
count, err := validateCABundleBytes(bundle)
if err != nil {
t.Fatalf("real bundle rejected by validateCABundleBytes: %v", err)
}
// Snapshot value as captured. If Mozilla churns the CCADB and we
// resnapshot, update this constant in the same commit so a real
// regression doesn't get masked by a stale expectation.
const wantCertCount = 165
if count != wantCertCount {
t.Errorf("real bundle parsed %d certificates, want %d", count, wantCertCount)
}
stripped := stripAfterTouchEntries(string(bundle))
if stripped.RemovedEntries != 0 {
t.Errorf("pristine bundle reports %d AfterTouch entries removed, want 0", stripped.RemovedEntries)
}
if stripped.UnpairedSentinel {
t.Errorf("pristine bundle reports an unpaired sentinel, want false")
}
// stripAfterTouchEntries on a pristine bundle is effectively a
// no-op (modulo trailing-newline normalisation). Detect drift
// loosely — within a 2-byte tolerance for the trailing-newline
// case — rather than asserting byte-identical, which would lock
// in a normalisation detail nobody cares about.
if delta := len(stripped.CleanedBundle) - len(bundle); delta < -2 || delta > 2 {
t.Errorf("strip pass on pristine bundle changed length unexpectedly: input=%d cleaned=%d (delta=%d)",
len(bundle), len(stripped.CleanedBundle), delta)
}
}
+76 -21
View File
@@ -1180,6 +1180,22 @@ func (m *Manager) TrustCACert(deviceIP string) (string, error) {
// the speaker's shared trust store. Identical to TrustCACert except the
// cert bytes come from the caller — used by the remote CLI which fetches
// /setup/ca.crt over HTTP and never touches Manager.Crypto.
//
// The write path is two-phase to keep the live bundle never half-written:
//
// 1. Upload the modified bundle to <bundlePath>.aftertouch.tmp (a sibling
// on the same filesystem, so the same rw remount covers it).
// 2. Read the tmp back over SSH, validate that every PEM block parses
// and that the AfterTouch CA sentinel brackets exactly one certificate,
// then atomically rename the tmp into place via `mv`. On any failure
// between steps 1 and 2 the tmp is unlinked and the live bundle is
// untouched — there is no rollback semantics to reason about.
//
// The .original backup written on first install is retained as
// defense-in-depth (a user can manually restore from it if anything outside
// this code path corrupts the live bundle), but it is no longer the
// primary safety net for our own writes. See issue #262 for the original
// failure-mode reporter.
func (m *Manager) TrustCACertFromBytes(deviceIP string, caCertPEM []byte) (string, error) {
if !strings.Contains(string(caCertPEM), "BEGIN CERTIFICATE") {
return "", fmt.Errorf("CA payload does not contain a PEM certificate")
@@ -1191,6 +1207,7 @@ func (m *Manager) TrustCACertFromBytes(deviceIP string, caCertPEM []byte) (strin
var logs string
bundlePath := "/etc/pki/tls/certs/ca-bundle.crt"
tmpPath := bundlePath + ".aftertouch.tmp"
out, _ := client.Run(rwCmd)
logs += rwCmd + ": " + out + "\n"
@@ -1210,27 +1227,21 @@ func (m *Manager) TrustCACertFromBytes(deviceIP string, caCertPEM []byte) (strin
if strings.Contains(bundleContent, CALabel) {
// Rebuild the bundle without our previously-injected CA so the
// fresh one replaces the old.
lines := strings.Split(bundleContent, "\n")
// fresh one replaces the old. Older AfterTouch releases are
// reported to have appended the CA on every install without
// stripping the previous one, so live bundles can carry
// several stale copies — stripAfterTouchEntries collapses
// them all and reports the count so we can log a single line
// of cleanup rather than failing validation.
stripped := stripAfterTouchEntries(bundleContent)
bundleContent = stripped.CleanedBundle
var newLines []string
inOurCA := false
for _, line := range lines {
if strings.Contains(line, CALabel) {
inOurCA = !inOurCA
continue
}
if !inOurCA {
newLines = append(newLines, line)
}
if stripped.RemovedEntries > 1 {
logs += fmt.Sprintf("Cleaned up %d duplicate AfterTouch CA entries from existing bundle\n", stripped.RemovedEntries)
}
bundleContent = strings.Join(newLines, "\n")
if bundleContent != "" && !strings.HasSuffix(bundleContent, "\n") {
bundleContent += "\n"
if stripped.UnpairedSentinel {
logs += "Warning: existing bundle had an unpaired AfterTouch sentinel; content after it was dropped along with the orphan. If anything legitimate was after the sentinel, restore from " + bundlePath + ".original.\n"
}
} else if bundleContent != "" && !strings.HasSuffix(bundleContent, "\n") {
bundleContent += "\n"
@@ -1239,11 +1250,55 @@ func (m *Manager) TrustCACertFromBytes(deviceIP string, caCertPEM []byte) (strin
labeledCert := fmt.Sprintf("\n%s\n%s%s\n", CALabel, string(caCertPEM), CALabel)
newBundleContent := bundleContent + labeledCert
if err := client.UploadContent([]byte(newBundleContent), bundlePath); err != nil {
return logs, fmt.Errorf("failed to update bundle: %w", err)
// Pre-upload validation: catch construction-time bugs (mangled PEM,
// missing sentinel, etc.) before any SSH write. The live bundle is
// untouched at this point.
if _, vErr := validateCABundleBytes([]byte(newBundleContent)); vErr != nil {
return logs, fmt.Errorf("constructed bundle failed validation, live bundle untouched: %w", vErr)
}
logs += "Uploaded updated bundle to " + bundlePath + "\n"
if vErr := validateAfterTouchLabelBracketing([]byte(newBundleContent)); vErr != nil {
return logs, fmt.Errorf("constructed bundle has malformed AfterTouch sentinel, live bundle untouched: %w", vErr)
}
// Phase 1: upload to a sibling tmp file on the same filesystem.
if err := client.UploadContent([]byte(newBundleContent), tmpPath); err != nil {
return logs, fmt.Errorf("failed to upload bundle to %s: %w", tmpPath, err)
}
logs += "Uploaded candidate bundle to " + tmpPath + "\n"
// Phase 2: read the tmp back and verify the bytes survived transport.
// On any failure here, unlink the tmp; the live bundle was never
// touched, so no rollback is required.
verifyContent, verifyErr := client.Run(fmt.Sprintf("cat %s", tmpPath))
if verifyErr != nil {
_, _ = client.Run(fmt.Sprintf("rm -f %s", tmpPath))
return logs, fmt.Errorf("failed to read back candidate bundle %s for verification, live bundle untouched: %w", tmpPath, verifyErr)
}
if _, vErr := validateCABundleBytes([]byte(verifyContent)); vErr != nil {
_, _ = client.Run(fmt.Sprintf("rm -f %s", tmpPath))
return logs, fmt.Errorf("verification of %s failed (post-upload PEM parse), live bundle untouched: %w", tmpPath, vErr)
}
if vErr := validateAfterTouchLabelBracketing([]byte(verifyContent)); vErr != nil {
_, _ = client.Run(fmt.Sprintf("rm -f %s", tmpPath))
return logs, fmt.Errorf("verification of %s failed (post-upload sentinel bracketing), live bundle untouched: %w", tmpPath, vErr)
}
logs += "Verified candidate bundle at " + tmpPath + "\n"
// Atomic replace. On the device's local filesystem this is a
// rename(2) — observers see either the pre- or post-bundle, never
// a half-written one.
mvCmd := fmt.Sprintf("mv %s %s", tmpPath, bundlePath)
if mvOut, mvErr := client.Run(mvCmd); mvErr != nil {
_, _ = client.Run(fmt.Sprintf("rm -f %s", tmpPath))
return logs, fmt.Errorf("failed to atomically replace bundle (%s -> %s, output=%q): %w", tmpPath, bundlePath, mvOut, mvErr)
}
logs += mvCmd + "\n"
return logs, nil
}
+294 -7
View File
@@ -18,6 +18,15 @@ import (
type mockSSH struct {
runFunc func(command string) (string, error)
uploadContentFunc func(content []byte, remotePath string) error
// uploaded mirrors UploadContent calls so that a subsequent
// `cat <path>` against a path that the test didn't explicitly
// script via runFunc returns what we just wrote there. This is
// what makes the tmp-then-mv flow in TrustCACertFromBytes work
// against tests that only scripted the live-bundle path. Tests
// that *do* script `cat <path>` keep priority — runFunc is
// consulted first and the upload mirror is the fallback.
uploaded map[string][]byte
}
// probeScriptHeader is the first line of the batched probe script
@@ -35,7 +44,24 @@ func (m *mockSSH) Run(command string) (string, error) {
}
if m.runFunc != nil {
return m.runFunc(command)
out, err := m.runFunc(command)
if err != nil {
return out, err
}
if out != "" {
return out, nil
}
// runFunc returned ("", nil) — fall through to the upload
// mirror so tmp readbacks that the test didn't script
// explicitly still produce the bytes we just wrote there.
}
if strings.HasPrefix(command, "cat ") {
path := strings.TrimPrefix(command, "cat ")
if body, ok := m.uploaded[path]; ok {
return string(body), nil
}
}
return "", nil
@@ -93,6 +119,12 @@ func (m *mockSSH) synthesizeProbeResponse(script string) (string, error) {
}
func (m *mockSSH) UploadContent(content []byte, remotePath string) error {
if m.uploaded == nil {
m.uploaded = make(map[string][]byte)
}
m.uploaded[remotePath] = append([]byte(nil), content...)
if m.uploadContentFunc != nil {
return m.uploadContentFunc(content, remotePath)
}
@@ -780,6 +812,10 @@ func TestTrustCACert(t *testing.T) {
return "", fmt.Errorf("file not found")
}
return "", nil
// mockSSH automatically mirrors uploads back on
// `cat <path>` when runFunc returns ("", nil), so the
// post-upload tmp readback in TrustCACertFromBytes
// works without test-side wiring.
},
uploadContentFunc: func(content []byte, remotePath string) error {
uploadCalls = append(uploadCalls, remotePath)
@@ -805,16 +841,267 @@ func TestTrustCACert(t *testing.T) {
t.Errorf("Expected ca-bundle.crt backup")
}
// Verify CA upload
foundUpload := false
// Verify CA upload landed on the tmp path (atomic-replace flow).
foundTmpUpload := false
for _, path := range uploadCalls {
if path == "/etc/pki/tls/certs/ca-bundle.crt" {
foundUpload = true
if path == "/etc/pki/tls/certs/ca-bundle.crt.aftertouch.tmp" {
foundTmpUpload = true
break
}
}
if !foundUpload {
t.Errorf("Expected updated bundle to be uploaded to /etc/pki/tls/certs/ca-bundle.crt")
if !foundTmpUpload {
t.Errorf("Expected candidate bundle to be uploaded to ca-bundle.crt.aftertouch.tmp; got upload paths: %v", uploadCalls)
}
// Verify the live bundle was NOT touched directly by UploadContent —
// the rename via Run() is the only path that touches the live file.
for _, path := range uploadCalls {
if path == "/etc/pki/tls/certs/ca-bundle.crt" {
t.Errorf("UploadContent wrote directly to live bundle %s — atomic-replace flow expects tmp + mv only", path)
}
}
// Verify the atomic rename ran and that no rm of the tmp happened
// (rm only fires on a verification failure).
foundMv := false
foundRm := false
for _, call := range runCalls {
if call == "mv /etc/pki/tls/certs/ca-bundle.crt.aftertouch.tmp /etc/pki/tls/certs/ca-bundle.crt" {
foundMv = true
}
if strings.HasPrefix(call, "rm -f /etc/pki/tls/certs/ca-bundle.crt.aftertouch.tmp") {
foundRm = true
}
}
if !foundMv {
t.Errorf("Expected atomic mv from .aftertouch.tmp to live bundle; got run calls: %v", runCalls)
}
if foundRm {
t.Errorf("Did not expect a cleanup rm on the happy path; got run calls: %v", runCalls)
}
}
// TestTrustCACert_StripsMultipleStaleEntriesSilently pins the
// behaviour the user flagged for AfterTouch installs that pre-date
// the strip-then-append logic: live bundles in the field can carry
// two or more copies of our CA from older releases that appended
// without cleanup. The new install must:
//
// - strip every stale AfterTouch entry,
// - log how many duplicates were cleaned up,
// - append exactly one fresh entry,
// - upload to the tmp path,
// - verify and rename — i.e. the cleanup itself must not break the
// validation or trigger the rollback path.
func TestTrustCACert_StripsMultipleStaleEntriesSilently(t *testing.T) {
tempDir, err := os.MkdirTemp("", "trust-ca-multi-")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tempDir)
cm := certmanager.NewCertificateManager(filepath.Join(tempDir, "certs"))
if err := cm.EnsureCA(); err != nil {
t.Fatalf("Failed to ensure CA: %v", err)
}
m := NewManager("http://localhost:8000", nil, cm)
const (
bundlePath = "/etc/pki/tls/certs/ca-bundle.crt"
tmpPath = bundlePath + ".aftertouch.tmp"
)
// Pre-existing bundle: one legitimate upstream cert plus two
// stale AfterTouch entries from old installs. Generated inline
// to stay self-contained.
upstream := generatePEMCertificate(t, "upstream-root")
stale1 := generatePEMCertificate(t, "aftertouch-stale-1")
stale2 := generatePEMCertificate(t, "aftertouch-stale-2")
preexisting := string(upstream) +
CALabel + "\n" + string(stale1) + CALabel + "\n" +
CALabel + "\n" + string(stale2) + CALabel + "\n"
runCalls := []string{}
var sshMock *mockSSH
m.NewSSH = func(_ string) SSHClient {
sshMock = &mockSSH{
runFunc: func(command string) (string, error) {
runCalls = append(runCalls, command)
if strings.HasPrefix(command, "[ -f") {
// .original doesn't exist yet → triggers initial backup
return "", fmt.Errorf("file not found")
}
if command == "cat "+bundlePath {
return preexisting, nil
}
return "", nil
// Tmp readback falls through to mockSSH's upload mirror.
},
}
return sshMock
}
logs, err := m.TrustCACert("192.168.1.10")
if err != nil {
t.Fatalf("TrustCACert failed: %v", err)
}
if !strings.Contains(logs, "Cleaned up 2 duplicate AfterTouch CA entries") {
t.Errorf("logs do not mention duplicate cleanup; got:\n%s", logs)
}
uploaded, ok := sshMock.uploaded[tmpPath]
if !ok {
t.Fatalf("nothing uploaded to %s; only got: %v", tmpPath, uploadKeys(sshMock.uploaded))
}
// The uploaded bundle must contain exactly two sentinels (open +
// close) bracketing exactly one CERTIFICATE block, regardless of
// how many stale entries the input had.
if err := validateAfterTouchLabelBracketing(uploaded); err != nil {
t.Errorf("uploaded bundle has malformed AfterTouch bracketing despite the cleanup: %v", err)
}
// And the cleanup must not have dropped the legitimate upstream cert.
count, err := validateCABundleBytes(uploaded)
if err != nil {
t.Fatalf("uploaded bundle does not validate: %v", err)
}
if count != 2 {
t.Errorf("uploaded bundle has %d CERTIFICATE blocks, want 2 (the upstream root + our fresh AfterTouch CA)", count)
}
// Atomic rename should have fired, and no rollback rm.
foundMv := false
foundRm := false
for _, call := range runCalls {
if call == "mv "+tmpPath+" "+bundlePath {
foundMv = true
}
if strings.HasPrefix(call, "rm -f "+tmpPath) {
foundRm = true
}
}
if !foundMv {
t.Errorf("Expected atomic mv after cleanup; got run calls: %v", runCalls)
}
if foundRm {
t.Errorf("Cleanup path triggered rollback rm — multi-entry input should not be a failure case; got: %v", runCalls)
}
}
func uploadKeys(m map[string][]byte) []string {
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
return keys
}
// TestTrustCACert_PostUploadVerificationFailureCleansUpTmp pins the
// rollback-free recovery story from issue #262: when the tmp file's
// readback doesn't validate (here we simulate transport truncation by
// returning the tmp content stripped of its closing AfterTouch label),
// the rename must NOT fire, the tmp must be removed, and the error
// must name the verification failure plus reassure the caller the
// live bundle wasn't touched.
func TestTrustCACert_PostUploadVerificationFailureCleansUpTmp(t *testing.T) {
tempDir, err := os.MkdirTemp("", "trust-ca-fail-")
if err != nil {
t.Fatalf("Failed to create temp dir: %v", err)
}
defer os.RemoveAll(tempDir)
cm := certmanager.NewCertificateManager(filepath.Join(tempDir, "certs"))
if err := cm.EnsureCA(); err != nil {
t.Fatalf("Failed to ensure CA: %v", err)
}
m := NewManager("http://localhost:8000", nil, cm)
const (
bundlePath = "/etc/pki/tls/certs/ca-bundle.crt"
tmpPath = bundlePath + ".aftertouch.tmp"
)
runCalls := []string{}
var sshMock *mockSSH
m.NewSSH = func(_ string) SSHClient {
sshMock = &mockSSH{
runFunc: func(command string) (string, error) {
runCalls = append(runCalls, command)
switch {
case strings.HasPrefix(command, "[ -f"):
return "", fmt.Errorf("file not found")
case command == "cat "+tmpPath:
// Simulate transport corruption: chop the closing
// AfterTouch sentinel off the bytes mockSSH would
// otherwise mirror back. Pre-upload validation
// passed (the full bytes were well-formed), but
// the readback doesn't bracket cleanly anymore.
return strings.Replace(string(sshMock.uploaded[tmpPath]), "\n"+CALabel+"\n", "\n", 1), nil
}
return "", nil
},
}
return sshMock
}
_, err = m.TrustCACert("192.168.1.10")
if err == nil {
t.Fatalf("TrustCACert succeeded, want a verification failure")
}
if !strings.Contains(err.Error(), "verification of "+tmpPath+" failed") {
t.Errorf("error does not name the verification target: %v", err)
}
if !strings.Contains(err.Error(), "live bundle untouched") {
t.Errorf("error does not reassure that the live bundle was untouched: %v", err)
}
foundRm := false
foundMv := false
for _, call := range runCalls {
if call == "rm -f "+tmpPath {
foundRm = true
}
if strings.HasPrefix(call, "mv "+tmpPath) {
foundMv = true
}
}
if !foundRm {
t.Errorf("Expected cleanup rm of %s after verification failure; got run calls: %v", tmpPath, runCalls)
}
if foundMv {
t.Errorf("mv ran despite verification failure — live bundle was overwritten with bad content; run calls: %v", runCalls)
}
}
File diff suppressed because it is too large Load Diff