D5: an app restore works from the drive alone (v0.188.0)
The recovery unit on the customer's drive now carries the PORTABLE secret class, so Tier-1/Tier-2 restore no longer depends on the whole-guest tier. A customer needs the drive and nothing else. Part 0's rulings overturned the brief's recommendation, on evidence: - the data_key flag is untrustworthy (4+ encryption keys the catalog itself labels as such are unflagged) -> R-127 - a DB password is not resettable in practice: POSTGRES_PASSWORD is ignored once PGDATA is non-empty, so a regenerated value leaves the app unable to authenticate against its own restored rows while the dump replay still reports success (proven on a throwaway postgres:16-alpine) Ruling (operator): type:secret travels, type:password never does, minus the nonPortableSecrets code register. Plaintext -- withholding the internet- reachable class is what licenses that, and the two are coupled. Precedence: the UNIT WINS over the guest -- the unit's secrets were captured in the same run as the dumps beside them, so they match the data being restored. The fail-closed data-key gate is unchanged. Secret values are never logged; the manifest records NAMES only.
This commit is contained in:
@@ -11,27 +11,53 @@ import (
|
||||
)
|
||||
|
||||
// reconcileRestoreSecrets merges the recovery unit's non-secret env with the secrets recovered from
|
||||
// the guest's own app.yaml, and applies the FAIL-CLOSED data-key gate. It is the safety-critical heart
|
||||
// of Phase 2b and is deliberately a pure function (no I/O) so it can be exhaustively unit-tested.
|
||||
// the unit itself (D5) and from the guest's own app.yaml, and applies the FAIL-CLOSED data-key gate.
|
||||
// It is the safety-critical heart of Phase 2b and is deliberately a pure function (no I/O) so it can
|
||||
// be exhaustively unit-tested — the D5 source arrives as an ARGUMENT, not as a read.
|
||||
//
|
||||
// Policy (per the Phase 2 design — see REPORT/CHANGELOG):
|
||||
// - Regenerate NOTHING. Every secret comes from the guest (live rootfs, or PBS whole-guest restore).
|
||||
// Policy:
|
||||
// - Regenerate NOTHING here. Secrets come from the unit (portable class) or the guest (the rest).
|
||||
// - A missing DATA-ENCRYPTING key (`dataKeyNames`) is FATAL: regenerating it would render the
|
||||
// restored data unreadable, so we refuse and tell the operator to do a PBS whole-guest restore.
|
||||
// - A missing resettable secret (DB password, admin password) is NON-fatal: it's returned in
|
||||
// `missing` so the caller can warn; the app may simply need a credential reset, no data is lost.
|
||||
func reconcileRestoreSecrets(nonSecretEnv, recoveredSecrets map[string]string, secretNames, dataKeyNames []string) (fullEnv map[string]string, missing []string, err error) {
|
||||
// D5 means the key is normally IN the unit — but "normally" is not a reason to soften the gate.
|
||||
// - A missing resettable secret is NON-fatal: returned in `missing` so the caller can warn or
|
||||
// regenerate it (O4). No data is lost.
|
||||
//
|
||||
// PRECEDENCE — the UNIT WINS over the guest when both hold a value for the same name.
|
||||
//
|
||||
// This is not arbitrary and it is not "newest wins". The unit's secrets are captured in the SAME run
|
||||
// as the dumps beside them (runVolumeDumps → captureAllRecoveryUnits, backup.go), so the unit's value
|
||||
// is the one that MATCHES THE DATA ABOUT TO BE RESTORED, whereas the guest's value is merely the most
|
||||
// recent. Where they disagree the guest's has been rotated since the capture, and preferring it is
|
||||
// precisely the data-loss bug:
|
||||
// - a rotated data-encrypting key does not decrypt data encrypted with the old one;
|
||||
// - a rotated DB password does not match the scram/mysql hash inside the restored data directory
|
||||
// (POSTGRES_PASSWORD is ignored once PGDATA is non-empty), so the app cannot reach its own rows.
|
||||
//
|
||||
// The restore persists fullEnv back to the guest's app.yaml (RecreateStackDefinitionFromUnit), so
|
||||
// unit-wins also leaves the guest consistent with the data now on disk.
|
||||
func reconcileRestoreSecrets(nonSecretEnv, unitSecrets, guestSecrets map[string]string, secretNames, dataKeyNames []string) (fullEnv map[string]string, missing []string, err error) {
|
||||
fullEnv = make(map[string]string, len(nonSecretEnv)+len(secretNames))
|
||||
for k, v := range nonSecretEnv {
|
||||
fullEnv[k] = v
|
||||
}
|
||||
// resolve applies the precedence: unit first, guest only as a fallback.
|
||||
resolve := func(n string) (string, bool) {
|
||||
if v, ok := unitSecrets[n]; ok && v != "" {
|
||||
return v, true
|
||||
}
|
||||
if v, ok := guestSecrets[n]; ok && v != "" {
|
||||
return v, true
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
have := func(n string) bool {
|
||||
v, ok := recoveredSecrets[n]
|
||||
return ok && v != ""
|
||||
_, ok := resolve(n)
|
||||
return ok
|
||||
}
|
||||
for _, n := range secretNames {
|
||||
if have(n) {
|
||||
fullEnv[n] = recoveredSecrets[n]
|
||||
if v, ok := resolve(n); ok {
|
||||
fullEnv[n] = v
|
||||
} else {
|
||||
missing = append(missing, n)
|
||||
}
|
||||
@@ -45,24 +71,42 @@ func reconcileRestoreSecrets(nonSecretEnv, recoveredSecrets map[string]string, s
|
||||
}
|
||||
if len(missingDataKeys) > 0 {
|
||||
return nil, missing, fmt.Errorf(
|
||||
"refusing to restore: data-encrypting key(s) %v could not be recovered from the guest's app.yaml — "+
|
||||
"refusing to restore: data-encrypting key(s) %v are in NEITHER the recovery unit nor the guest's app.yaml — "+
|
||||
"a PBS whole-guest restore is required first (regenerating the key would render stored data unreadable)",
|
||||
missingDataKeys)
|
||||
}
|
||||
return fullEnv, missing, nil
|
||||
}
|
||||
|
||||
// readStrippedEnv parses the non-secret env from a recovery unit's secret-stripped app.yaml.
|
||||
func readStrippedEnv(path string) map[string]string {
|
||||
// readUnitEnv parses a recovery unit's app.yaml and SPLITS it into the plain config env and the
|
||||
// secrets the unit carries (D5), using the manifest's portable-secret names as the discriminator.
|
||||
//
|
||||
// The split is driven by the MANIFEST, not by guessing from key names: the manifest and the app.yaml
|
||||
// are captured together and checksummed together, so they cannot disagree about which entries are
|
||||
// secrets. A schema-1 unit has no portable names, so everything lands in nonSecret — exactly the
|
||||
// pre-D5 behaviour, which is what makes an old unit still restorable.
|
||||
func readUnitEnv(path string, portableNames []string) (nonSecret, unitSecrets map[string]string) {
|
||||
nonSecret, unitSecrets = map[string]string{}, map[string]string{}
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return map[string]string{}
|
||||
return nonSecret, unitSecrets
|
||||
}
|
||||
var s strippedAppYaml
|
||||
if yaml.Unmarshal(data, &s) != nil || s.Env == nil {
|
||||
return map[string]string{}
|
||||
return nonSecret, unitSecrets
|
||||
}
|
||||
return s.Env
|
||||
isPortable := make(map[string]bool, len(portableNames))
|
||||
for _, n := range portableNames {
|
||||
isPortable[n] = true
|
||||
}
|
||||
for k, v := range s.Env {
|
||||
if isPortable[k] {
|
||||
unitSecrets[k] = v
|
||||
continue
|
||||
}
|
||||
nonSecret[k] = v
|
||||
}
|
||||
return nonSecret, unitSecrets
|
||||
}
|
||||
|
||||
// hasReplayableDump reports whether dumpDir holds a .sql dump that the replay could actually use.
|
||||
@@ -85,13 +129,17 @@ func hasReplayableDump(dumpDir string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// RestoreFromRecoveryUnit recreates an app from its on-drive recovery unit + the guest's own secrets.
|
||||
// RestoreFromRecoveryUnit recreates an app from its on-drive recovery unit.
|
||||
//
|
||||
// It reads the unit manifest, recovers the secret values from the guest's live app.yaml, applies the
|
||||
// fail-closed data-key gate, restores the named-volume data from the unit's tars, then restores the
|
||||
// app's definition from the unit and redeploys it with the reconstructed env (re-pulling the pinned
|
||||
// image). No secret is ever regenerated, and no secret is read from the unit. If no unit exists it
|
||||
// falls back to the legacy volume-only RestoreApp.
|
||||
// It reads the unit manifest, takes the portable secrets from the UNIT and the rest from the guest's
|
||||
// live app.yaml (unit wins — see reconcileRestoreSecrets), applies the fail-closed data-key gate,
|
||||
// restores the named-volume data from the unit's tars, then restores the app's definition from the unit
|
||||
// and redeploys it with the reconstructed env (re-pulling the pinned image). If no unit exists it falls
|
||||
// back to the legacy volume-only RestoreApp.
|
||||
//
|
||||
// D5: this no longer needs the guest. A restore with the guest's app.yaml absent succeeds, which is
|
||||
// pinned by TestRestoreFromRecoveryUnitWithGuestAbsent — the withheld class is regenerated (O4) and
|
||||
// only a data key missing from BOTH sources still refuses.
|
||||
func (m *Manager) RestoreFromRecoveryUnit(stackName string) error {
|
||||
if m.stackProvider == nil {
|
||||
return fmt.Errorf("stack provider not configured")
|
||||
@@ -126,11 +174,15 @@ func (m *Manager) RestoreFromRecoveryUnit(stackName string) error {
|
||||
}
|
||||
|
||||
composeDir := RecoveryUnitComposePath(nsRoot, stackName)
|
||||
nonSecretEnv := readStrippedEnv(filepath.Join(composeDir, "app.yaml"))
|
||||
nonSecretEnv, unitSecrets := readUnitEnv(filepath.Join(composeDir, "app.yaml"), manifest.PortableSecretEnvVars)
|
||||
|
||||
// Recover secrets from the GUEST (never the unit), then apply the fail-closed gate.
|
||||
recovered := m.stackProvider.RecoverStackSecrets(stackName, manifest.SecretEnvVars)
|
||||
fullEnv, missing, err := reconcileRestoreSecrets(nonSecretEnv, recovered, manifest.SecretEnvVars, manifest.DataKeyEnvVars)
|
||||
// D5: the unit carries the portable class, so this is the leg that no longer needs the guest. The
|
||||
// guest is still consulted for the WITHHELD class (internet-reachable admin logins) and as the
|
||||
// fallback for a schema-1 unit — it returns an empty map when the guest is gone, which is the whole
|
||||
// point: a Tier-1/2 restore must survive that. Precedence is unit-over-guest (see
|
||||
// reconcileRestoreSecrets), then the fail-closed gate.
|
||||
guestSecrets := m.stackProvider.RecoverStackSecrets(stackName, manifest.SecretEnvVars)
|
||||
fullEnv, missing, err := reconcileRestoreSecrets(nonSecretEnv, unitSecrets, guestSecrets, manifest.SecretEnvVars, manifest.DataKeyEnvVars)
|
||||
if err != nil {
|
||||
m.logger.Printf("[ERROR] [backup] Restore REFUSED for %s: %v", stackName, err)
|
||||
return err
|
||||
@@ -141,6 +193,16 @@ func (m *Manager) RestoreFromRecoveryUnit(stackName string) error {
|
||||
// encrypted in the guest app.yaml and round-trips on the next backup/restore. Data-keys are
|
||||
// never generated: the fail-closed gate above already refused if one was missing, and the
|
||||
// generator itself refuses data-key fields (defense-in-depth). Values are never logged.
|
||||
//
|
||||
// D5 shrinks this path to the rare case: the portable class now comes from the unit, so a
|
||||
// generator run means the secret was empty at capture AND absent from the guest.
|
||||
//
|
||||
// It does NOT claim the reset is harmless. R-127: for a DB password it is not — a restored data
|
||||
// directory keeps the OLD role hash (POSTGRES_PASSWORD is ignored once PGDATA is non-empty), so a
|
||||
// regenerated value leaves the app unable to authenticate against its own restored rows while the
|
||||
// dump replay, which uses the container's local trust socket, still reports success. The old wording
|
||||
// here asserted "stored data is unaffected" for every non-data-key secret; that is false for the 18
|
||||
// DB/root-password fields and is now scoped to what is actually true.
|
||||
if len(missing) > 0 {
|
||||
dataKeySet := make(map[string]bool, len(manifest.DataKeyEnvVars))
|
||||
for _, dk := range manifest.DataKeyEnvVars {
|
||||
@@ -158,7 +220,7 @@ func (m *Manager) RestoreFromRecoveryUnit(stackName string) error {
|
||||
unresolved = append(unresolved, name)
|
||||
}
|
||||
if len(generated) > 0 {
|
||||
m.logger.Printf("[WARN] [backup] Restore %s: generated replacement for %v — the credential was reset (old value unrecoverable); stored data is unaffected (no data-key involved)",
|
||||
m.logger.Printf("[WARN] [backup] Restore %s: generated replacement for %v — the credential was reset (old value unrecoverable); no data-encrypting key was involved, but a regenerated DATABASE password will not match the restored data directory's stored hash (R-127) — check the app can reach its data",
|
||||
stackName, generated)
|
||||
}
|
||||
if len(unresolved) > 0 {
|
||||
|
||||
Reference in New Issue
Block a user