From 7df71ed8d4ebe2e0ecb04980ef92ee1eafea5688 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Mon, 7 Sep 2026 14:45:36 +0000 Subject: [PATCH] fix: correct verify's ciphertext field map against the models --- backup/manifest.go | 31 ++++++++++++++++++++++++++++++- backup/manifest_test.go | 24 +++++++++++++++++++++++- backup/verify.go | 24 +++++++++++++++++++----- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/backup/manifest.go b/backup/manifest.go index 91246c0..0251892 100644 --- a/backup/manifest.go +++ b/backup/manifest.go @@ -3,6 +3,7 @@ package backup import ( "errors" "fmt" + "strings" "time" ) @@ -16,6 +17,10 @@ const ManifestName = "manifest.json" // ErrUnknownFormat is returned for an archive this build cannot read. var ErrUnknownFormat = errors.New("unsupported archive format version") +// ErrBadCollectionName is returned for a manifest naming a collection that +// cannot safely be used as a path component. +var ErrBadCollectionName = errors.New("manifest names an unusable collection") + // CollectionEntry describes one collection in the archive. Bytes and SHA256 // cover the uncompressed .bson member, which is what restore verifies before // writing anything. @@ -49,6 +54,27 @@ func (m Manifest) Check() error { return fmt.Errorf("%w: archive is version %d, this build reads version %d", ErrUnknownFormat, m.FormatVersion, FormatVersion) } + // Collection names become path components inside the extraction directory, + // and an archive is operator-supplied input that may not be one we wrote. + for _, c := range m.Collections { + if err := checkCollectionName(c.Name); err != nil { + return err + } + } + return nil +} + +// checkCollectionName refuses a name that could escape a directory when joined +// as a path component. +func checkCollectionName(name string) error { + switch { + case name == "": + return fmt.Errorf("%w: a collection entry has no name", ErrBadCollectionName) + case name == "." || name == "..": + return fmt.Errorf("%w: %q", ErrBadCollectionName, name) + case strings.ContainsAny(name, "/\\"), strings.Contains(name, ".."): + return fmt.Errorf("%w: %q", ErrBadCollectionName, name) + } return nil } @@ -68,6 +94,9 @@ func (m Manifest) Collection(name string) (CollectionEntry, bool) { // match the archive, this is the list of what will be unreadable afterwards, // and an operator deserves to see it before the write rather than discover it // a week later. +// +// settings is not in the list: it holds no encrypted material. Its ESO read +// token is a SHA-256 hash, not ciphertext. func CiphertextCollections() []string { - return []string{"keys", "secrets", "auth_providers", "console_sessions", "settings"} + return []string{"keys", "secrets", "auth_providers", "console_sessions"} } diff --git a/backup/manifest_test.go b/backup/manifest_test.go index b4676cb..1b1bff2 100644 --- a/backup/manifest_test.go +++ b/backup/manifest_test.go @@ -69,7 +69,7 @@ func TestManifestCollectionLookup(t *testing.T) { func TestCiphertextCollections(t *testing.T) { got := CiphertextCollections() - want := []string{"keys", "secrets", "auth_providers", "console_sessions", "settings"} + want := []string{"keys", "secrets", "auth_providers", "console_sessions"} if len(got) != len(want) { t.Fatalf("got %v, want %v", got, want) } @@ -79,3 +79,25 @@ func TestCiphertextCollections(t *testing.T) { } } } + +// TestManifestRefusesUnusableCollectionNames covers names being used as path +// components inside the extraction directory. An archive is operator-supplied +// input and may not be one we wrote. +func TestManifestRefusesUnusableCollectionNames(t *testing.T) { + for _, name := range []string{"", ".", "..", "../etc/passwd", "a/b", "a..b"} { + m := Manifest{ + FormatVersion: FormatVersion, + Collections: []CollectionEntry{{Name: name}}, + } + if err := m.Check(); !errors.Is(err, ErrBadCollectionName) { + t.Fatalf("collection name %q was accepted (err %v)", name, err) + } + } + m := Manifest{ + FormatVersion: FormatVersion, + Collections: []CollectionEntry{{Name: "workflow_log_lines"}}, + } + if err := m.Check(); err != nil { + t.Fatalf("an ordinary collection name was refused: %v", err) + } +} diff --git a/backup/verify.go b/backup/verify.go index 9234d9a..e32bf92 100644 --- a/backup/verify.go +++ b/backup/verify.go @@ -117,12 +117,25 @@ func probe(ctx context.Context, opt VerifyOptions, rep *VerifyReport) error { // ciphertextFields names, per collection, the fields that hold hex ciphertext. // A value is a candidate only if it is a hex string long enough to carry a GCM // nonce and tag, which is what keeps this from probing a plaintext field. +// +// This map MIRRORS BY HAND the bson tags in server/internal/models, which this +// package cannot import: shared/ is a separate module and models is under +// server/internal. It must change in the same commit as any rename of the +// fields below — the same mirrored-constant hazard as web/lib/targets.ts and +// services.MaxWorkloadLogLines. The sources are: +// +// keys — models/key.go: private_key_enc, passphrase_enc +// secrets — models/secret.go: encrypted_value +// auth_providers — models/auth_provider.go: client_secret_enc +// console_sessions — models/console_session.go: rdp_user_enc, rdp_pass_enc +// +// settings is deliberately absent: it holds no ciphertext at all. The ESO read +// token is stored as a SHA-256 hash, which no key opens. var ciphertextFields = map[string][]string{ "keys": {"private_key_enc", "passphrase_enc"}, - "secrets": {"values"}, + "secrets": {"encrypted_value"}, "auth_providers": {"client_secret_enc"}, - "console_sessions": {"rdp_password_enc", "vnc_password_enc"}, - "settings": {"secrets_token_hash_enc"}, + "console_sessions": {"rdp_user_enc", "rdp_pass_enc"}, } func findCiphertext(ctx context.Context, db *mongo.Database, coll string) (string, bool, error) { @@ -150,8 +163,9 @@ func findCiphertext(ctx context.Context, db *mongo.Database, coll string) (strin return "", false, cur.Err() } -// looksLikeCiphertext accepts a hex string long enough to be a sealed value, and -// descends one level into a map so secrets' values sub-document is reachable. +// looksLikeCiphertext accepts a hex string long enough to be a sealed value. It +// descends into a sub-document so a field that holds a map of sealed values is +// still reachable. func looksLikeCiphertext(v any) (string, bool) { switch t := v.(type) { case string: