diff --git a/cmd/rds-agent/acknowledge.go b/cmd/rds-agent/acknowledge.go new file mode 100644 index 000000000..a81644c06 --- /dev/null +++ b/cmd/rds-agent/acknowledge.go @@ -0,0 +1,155 @@ +package main + +import ( + "bufio" + "context" + "errors" + "fmt" + "io/fs" + "log/slog" + "os" + "path/filepath" + "strings" + + "github.com/mulgadc/spinifex/internal/rdsgw" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// The completion receipt rds-init writes once the master role is durably +// applied. Deliberately outside PGDATA: clear_unfinished_datadir does an rm -rf +// of PGDATA, so keeping the receipt out of it means no initdb-adjacent path can +// touch it and the datadir stays a byte-for-byte stock one. +const ( + receiptRelDir = ".spinifex-rds/bootstrap" + receiptFile = "receipt.env" + receiptPayload = "RDS_RECEIPT_PAYLOAD_ID" + receiptDBID = "RDS_RECEIPT_DB_INSTANCE_IDENTIFIER" +) + +func (a *Agent) receiptPath() string { + return filepath.Join(a.cfg.DataMount, receiptRelDir, receiptFile) +} + +// Confirms the initial bootstrap, which is what destroys the staged ciphertext. +// A no-op on the attach path, and deliberately synchronous: it inherits the +// cancellation semantics register, bootstrap and the data-mount wait already +// share, and blocking the stateful loops behind it is correct because on a first +// boot the engine is not bootstrapped until rds-init has finished anyway. +// +// It does not wait for the engine to be healthy. The receipt already proves the +// master role was durably applied, and acknowledging early shortens the window +// in which the ciphertext exists. +func (a *Agent) acknowledgeBootstrap(ctx context.Context) error { + pending := a.pending + if pending == nil { + return nil + } + + // Polled rather than waited on: rds-init runs after this agent registered, + // so the file appears some time into the boot. There is no give-up deadline + // here — the control plane's bootstrap timeout already fails the instance, + // and a second one in the guest would only give the two a way to disagree. + if err := retryObserved(ctx, "bootstrap receipt", func(context.Context) error { + return a.checkBootstrapReceipt(pending) + }, func(err error) { + // An absent receipt is the expected state for the whole initdb window, + // so reporting it would put a failure on every healthy create's beat and + // then into the reason a timed-out create records. + if errors.Is(err, fs.ErrNotExist) { + return + } + a.hb.setBootstrapFailure("bootstrap receipt", err) + }); err != nil { + return err + } + a.hb.clearBootstrapFailure() + + if err := retryObserved(ctx, "bootstrap acknowledgement", func(ctx context.Context) error { + return terminalOnDenial(a.cp.AcknowledgeBootstrap(ctx, a.id, pending)) + }, func(err error) { + a.hb.setBootstrapFailure("bootstrap acknowledgement", err) + }); err != nil { + if !errors.Is(err, errRetryTerminal) { + return err + } + // The staged payload will never be delivered to this VM, and blocking the + // command poller and the parameter guard on that would leave a serving + // database no one can administer. The reason rides the heartbeat instead. + slog.ErrorContext(ctx, "rds-agent: the initial bootstrap cannot be acknowledged", + "payloadId", pending.payloadID, "err", err) + return nil + } + a.hb.clearBootstrapFailure() + + // AcknowledgedAt on the record is the durable audit trail, so the receipt is + // removed rather than archived: keeping it would only grow the surface of + // stale receipts riding along in data-volume snapshots. A removal failure is + // harmless, since a later boot fetches attach and never consults one. + if err := os.Remove(a.receiptPath()); err != nil && !os.IsNotExist(err) { + slog.WarnContext(ctx, "rds-agent: could not remove the bootstrap receipt", + "receipt", a.receiptPath(), "err", err) + } + slog.InfoContext(ctx, "rds-agent: initial bootstrap acknowledged", "payloadId", pending.payloadID) + return nil +} + +// The control plane resolved these from its own record — a superseded +// generation, a payload it never staged, a data volume that is not this +// instance's. Asking again cannot change any of them. +func terminalOnDenial(err error) error { + var apiErr *rdsgw.APIError + if !errors.As(err, &apiErr) { + return err + } + switch apiErr.Code { + case awserrors.ErrorAccessDenied, awserrors.ErrorInvalidParameterValue: + return fmt.Errorf("%w: %w", errRetryTerminal, err) + } + return err +} + +// A receipt naming another payload or another DB instance is treated as absent. +// The receipt lives on the data volume, so it rides along in every snapshot of +// it and a restored instance's volume carries the source instance's receipt. +func (a *Agent) checkBootstrapReceipt(pending *pendingBootstrap) error { + path := a.receiptPath() + values, err := readReceipt(path) + if err != nil { + return err + } + if got := values[receiptPayload]; got != pending.payloadID { + return fmt.Errorf("%s names bootstrap payload %q, not %q", path, got, pending.payloadID) + } + if got := values[receiptDBID]; got != a.id.DBInstanceIdentifier { + return fmt.Errorf("%s names DB instance %q, not %q", path, got, a.id.DBInstanceIdentifier) + } + return nil +} + +// KEY=value, the one format both readers can parse: rds-init in POSIX shell +// with no guaranteed jq, and this. +func readReceipt(path string) (map[string]string, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + values := make(map[string]string) + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + key, value, ok := strings.Cut(line, "=") + if !ok { + continue + } + values[strings.TrimSpace(key)] = strings.TrimSpace(value) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("read %s: %w", path, err) + } + return values, nil +} diff --git a/cmd/rds-agent/agent.go b/cmd/rds-agent/agent.go index fa2597f08..84cf8832d 100644 --- a/cmd/rds-agent/agent.go +++ b/cmd/rds-agent/agent.go @@ -3,6 +3,7 @@ package main import ( "bufio" "context" + "errors" "fmt" "log/slog" "net/url" @@ -33,9 +34,12 @@ type identity struct { type controlPlane interface { Register(ctx context.Context, id identity) (*handlers_rds.RegisterDBInstanceOutput, error) SubmitState(ctx context.Context, id identity, health handlers_rds.EngineHealth, message string) (*handlers_rds.SubmitDBStateChangeOutput, error) - // The first call of an instance's life returns Mode=initialize with the - // master password; every later call returns Mode=attach without it. + // Returns Mode=initialize with the master password for as long as a payload + // staged for this VM generation is unacknowledged, and Mode=attach after. GetBootstrapConfig(ctx context.Context, id identity) (*handlers_rds.GetDBBootstrapConfigOutput, error) + // Reports that PostgreSQL durably applied the staged password, which is what + // lets the control plane destroy it. + AcknowledgeBootstrap(ctx context.Context, id identity, pending *pendingBootstrap) error PollCommands(ctx context.Context, id identity, replies []handlers_rds.CommandReply, wait time.Duration) ([]handlers_rds.Command, error) } @@ -115,6 +119,27 @@ func (g *gatewayControlPlane) GetBootstrapConfig(ctx context.Context, id identit return &out, nil } +func (g *gatewayControlPlane) AcknowledgeBootstrap(ctx context.Context, id identity, pending *pendingBootstrap) error { + params := url.Values{ + "PayloadId": {pending.payloadID}, + "VMGeneration": {strconv.FormatInt(pending.vmGeneration, 10)}, + } + setIfPresent(params, "DBInstanceIdentifier", id.DBInstanceIdentifier) + setIfPresent(params, "DataVolumeId", pending.dataVolumeID) + + ctx, cancel := context.WithTimeout(ctx, callTimeout) + defer cancel() + + var out handlers_rds.AcknowledgeDBBootstrapOutput + if err := g.client.Call(ctx, "AcknowledgeDBBootstrap", params, &out); err != nil { + return err + } + if !out.Acknowledged { + return fmt.Errorf("the control plane did not acknowledge bootstrap payload %s", pending.payloadID) + } + return nil +} + // Declared here so the in-guest binary does not link the server side of the API. type pollDBCommandsOutput struct { Commands []handlers_rds.Command `xml:"Commands>member"` @@ -159,6 +184,10 @@ type Agent struct { handoffWriter func(string, *handlers_rds.GetDBBootstrapConfigOutput) error dataMountWaiter func(context.Context, string, string) error + // Set by bootstrap when the fetch replayed a staged payload; nil leaves the + // acknowledgement step a no-op, which is every attach boot. + pending *pendingBootstrap + hb *heartbeater cmd *commander guard *paramGuard @@ -233,6 +262,12 @@ func (a *Agent) Run(ctx context.Context) error { return fmt.Errorf("wait for data mount %s: %w", a.cfg.DataMount, err) } + // The receipt it waits on lives on that mount, so this cannot run any earlier. + // A cancelled wait is a clean stop here too. + if err := a.acknowledgeBootstrap(ctx); err != nil { + return err + } + // Directives are only meaningful against a bootstrapped, mounted engine. go a.cmd.Run(ctx) @@ -308,6 +343,11 @@ const ( retryErrorAttempt = 5 ) +// Wrapping an error in this stops retryObserved. For the failures the control +// plane decides from its own record, no retry can change the answer, and looping +// on one only keeps the caller blocked. +var errRetryTerminal = errors.New("terminal failure") + // Never gives up on its own: an agent that stopped trying would leave the // control plane with no signal at all. func retry(ctx context.Context, what string, fn func(context.Context) error) error { @@ -329,6 +369,10 @@ func retryObserved(ctx context.Context, what string, fn func(context.Context) er if onFailure != nil { onFailure(err) } + if errors.Is(err, errRetryTerminal) { + slog.ErrorContext(ctx, "rds-agent: "+what+" failed terminally, not retrying", "err", err) + return fmt.Errorf("%s: %w", what, err) + } if attempt >= retryErrorAttempt { slog.ErrorContext(ctx, "rds-agent: "+what+" still failing, retrying", "attempt", attempt, "retryIn", delay, "err", err) diff --git a/cmd/rds-agent/agent_test.go b/cmd/rds-agent/agent_test.go index 82a8d9e48..a49d21884 100644 --- a/cmd/rds-agent/agent_test.go +++ b/cmd/rds-agent/agent_test.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "fmt" "net/http" "net/http/httptest" "os" @@ -15,6 +16,7 @@ import ( "github.com/mulgadc/spinifex/internal/gwsign" "github.com/mulgadc/spinifex/internal/rdsgw" + "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_rds "github.com/mulgadc/spinifex/spinifex/gateway/rds" handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" "github.com/mulgadc/spinifex/spinifex/utils" @@ -34,6 +36,12 @@ type fakeControlPlane struct { bootstrapOut *handlers_rds.GetDBBootstrapConfigOutput bootstrapErrs []error bootstrapReqs []identity + // Scripted responses ahead of bootstrapOut, so a test can answer the first + // fetch differently from the ones the agent retries with. + bootstrapQueue []*handlers_rds.GetDBBootstrapConfigOutput + + ackErrs []error + ackReqs []pendingBootstrap states []submittedState @@ -97,9 +105,21 @@ func (f *fakeControlPlane) GetBootstrapConfig(_ context.Context, id identity) (* if err := nextErr(&f.bootstrapErrs); err != nil { return nil, err } + if len(f.bootstrapQueue) > 0 { + out := f.bootstrapQueue[0] + f.bootstrapQueue = f.bootstrapQueue[1:] + return out, nil + } return f.bootstrapOut, nil } +func (f *fakeControlPlane) AcknowledgeBootstrap(_ context.Context, _ identity, pending *pendingBootstrap) error { + f.mu.Lock() + defer f.mu.Unlock() + f.ackReqs = append(f.ackReqs, *pending) + return nextErr(&f.ackErrs) +} + func (f *fakeControlPlane) PollCommands(ctx context.Context, _ identity, replies []handlers_rds.CommandReply, _ time.Duration) ([]handlers_rds.Command, error) { f.mu.Lock() f.pollReplies = append(f.pollReplies, slices.Clone(replies)) @@ -126,6 +146,12 @@ func (f *fakeControlPlane) PollCommands(ctx context.Context, _ identity, replies return nil, ctx.Err() } +func (f *fakeControlPlane) ackRequests() []pendingBootstrap { + f.mu.Lock() + defer f.mu.Unlock() + return slices.Clone(f.ackReqs) +} + func (f *fakeControlPlane) snapshotStates() []submittedState { f.mu.Lock() defer f.mu.Unlock() @@ -176,6 +202,24 @@ func bootstrapOutput(password *string) *handlers_rds.GetDBBootstrapConfigOutput DataVolumeID: "vol-data-01", DataVolumeSerial: "voldata01", VMGeneration: 1, + PayloadID: testPayloadID, + BootstrapPending: password != nil, + } +} + +const testPayloadID = "bp-0123456789abcdef" + +// Writes the completion receipt rds-init leaves behind, so the acknowledgement +// step finds what a finished initialization would have produced. +func writeReceipt(t *testing.T, a *Agent, payloadID, dbInstanceIdentifier string) { + t.Helper() + path := a.receiptPath() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("mkdir receipt dir: %v", err) + } + body := fmt.Sprintf("%s=%s\n%s=%s\n", receiptPayload, payloadID, receiptDBID, dbInstanceIdentifier) + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write receipt: %v", err) } } @@ -293,9 +337,11 @@ func TestRun_RetriesRegisterAndBootstrap(t *testing.T) { } } -func TestBootstrap_RetriesHandoffWithoutRefetchingConsumedPassword(t *testing.T) { +// The fetch mutates nothing, so a handoff that cannot be written is retried +// against the payload already in hand rather than costing a second fetch. +func TestBootstrap_RetriesHandoffWithoutRefetching(t *testing.T) { cp := newFakeControlPlane() - password := "one-shot-secret" + password := "staged-secret" cp.bootstrapOut = bootstrapOutput(&password) cfg := testConfig(t) a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) @@ -321,6 +367,117 @@ func TestBootstrap_RetriesHandoffWithoutRefetchingConsumedPassword(t *testing.T) if writes != 2 { t.Errorf("handoff writes = %d, want 2", writes) } + if a.pending == nil || a.pending.payloadID != testPayloadID { + t.Errorf("pending bootstrap = %+v, want the staged payload %s", a.pending, testPayloadID) + } +} + +// A daemon that predates the encrypted payload answers initialize with nothing +// to initialise with. Writing that handoff would let rds-init run initdb with an +// empty master password, so it is retried until an upgraded node answers. +func TestBootstrap_RetriesInitializeWithNoPassword(t *testing.T) { + cp := newFakeControlPlane() + empty := "" + password := "staged-secret" + cp.bootstrapQueue = []*handlers_rds.GetDBBootstrapConfigOutput{{ + Mode: handlers_rds.BootstrapModeInitialize, DBInstanceIdentifier: "db-resolved", + MasterUsername: "master", MasterUserPassword: &empty, + }} + cp.bootstrapOut = bootstrapOutput(&password) + cfg := testConfig(t) + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + + var handed []*handlers_rds.GetDBBootstrapConfigOutput + a.handoffWriter = func(_ string, got *handlers_rds.GetDBBootstrapConfigOutput) error { + handed = append(handed, got) + return nil + } + + if err := a.bootstrap(t.Context()); err != nil { + t.Fatalf("bootstrap: %v", err) + } + if len(handed) != 1 { + t.Fatalf("handoff writes = %d, want 1 once a usable payload arrived", len(handed)) + } + if handed[0].MasterUserPassword == nil || *handed[0].MasterUserPassword != password { + t.Error("the agent wrote a handoff for a fetch that carried no master password") + } +} + +func TestAcknowledgeBootstrap_WaitsForTheReceiptThenRetiresIt(t *testing.T) { + cp := newFakeControlPlane() + cfg := testConfig(t) + cfg.DataMount = t.TempDir() + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + a.id = identity{DBInstanceIdentifier: "db-resolved"} + a.pending = &pendingBootstrap{payloadID: testPayloadID, vmGeneration: 1, dataVolumeID: "vol-data-01"} + + // rds-init runs after the agent registers, so the receipt is absent when the + // acknowledgement step first looks and appears some way into the boot. + if err := a.checkBootstrapReceipt(a.pending); err == nil { + t.Fatal("a missing receipt must not read as a completed bootstrap") + } + writeReceipt(t, a, testPayloadID, "db-resolved") + + if err := a.acknowledgeBootstrap(t.Context()); err != nil { + t.Fatalf("acknowledgeBootstrap: %v", err) + } + if len(cp.ackRequests()) != 1 { + t.Fatalf("acknowledgements = %d, want 1", len(cp.ackRequests())) + } + if got := cp.ackRequests()[0]; got != *a.pending { + t.Errorf("acknowledged %+v, want %+v", got, *a.pending) + } + if _, err := os.Stat(a.receiptPath()); !os.IsNotExist(err) { + t.Error("the receipt must be removed once the record holds the audit trail") + } +} + +// The receipt rides along in every snapshot of the data volume, so a restored +// instance's volume carries the source instance's receipt. Acknowledging on one +// would destroy the ciphertext this instance still needs. +func TestAcknowledgeBootstrap_IgnoresAForeignReceipt(t *testing.T) { + tests := map[string]struct{ payloadID, dbID string }{ + "another payload": {payloadID: "bp-somebody-else", dbID: "db-resolved"}, + "another instance": {payloadID: testPayloadID, dbID: "db-the-snapshot-source"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + cp := newFakeControlPlane() + cfg := testConfig(t) + cfg.DataMount = t.TempDir() + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + a.id = identity{DBInstanceIdentifier: "db-resolved"} + a.pending = &pendingBootstrap{payloadID: testPayloadID, vmGeneration: 1} + writeReceipt(t, a, tc.payloadID, tc.dbID) + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + if err := a.acknowledgeBootstrap(ctx); err == nil { + t.Fatal("acknowledgeBootstrap returned nil, want it still waiting on a real receipt") + } + if n := len(cp.ackRequests()); n != 0 { + t.Errorf("acknowledgements = %d, want 0", n) + } + }) + } +} + +// An attach boot has nothing staged, so it must not go looking for a receipt the +// datadir will never grow. +func TestAcknowledgeBootstrap_IsANoOpOnAttach(t *testing.T) { + cp := newFakeControlPlane() + cfg := testConfig(t) + cfg.DataMount = t.TempDir() + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + + if err := a.acknowledgeBootstrap(t.Context()); err != nil { + t.Fatalf("acknowledgeBootstrap: %v", err) + } + if n := len(cp.ackRequests()); n != 0 { + t.Errorf("acknowledgements = %d, want 0 when nothing was staged", n) + } } // A configured identifier is asserted on the wire so a mis-provisioned VM is @@ -582,3 +739,69 @@ func readFile(t *testing.T, path string) string { } return string(body) } + +// AccessDenied is decided by the control plane's own record, so no retry can +// change it. Looping on one would leave the command poller and the parameter +// guard unstarted for the life of a VM whose engine is serving normally. +func TestAcknowledgeBootstrap_StopsOnATerminalDenial(t *testing.T) { + cp := newFakeControlPlane() + cp.ackErrs = []error{&rdsgw.APIError{ + Action: "AcknowledgeDBBootstrap", StatusCode: 403, + Code: awserrors.ErrorAccessDenied, + Message: "bootstrap payload bp-x at generation 1 does not match the current state", + }} + cfg := testConfig(t) + cfg.DataMount = t.TempDir() + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + a.id = identity{DBInstanceIdentifier: "db-resolved"} + a.pending = &pendingBootstrap{payloadID: testPayloadID, vmGeneration: 1} + writeReceipt(t, a, testPayloadID, "db-resolved") + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + if err := a.acknowledgeBootstrap(ctx); err != nil { + t.Fatalf("acknowledgeBootstrap = %v, want nil so the stateful loops still start", err) + } + if n := len(cp.ackRequests()); n != 1 { + t.Errorf("acknowledgements = %d, want 1 attempt and no retry", n) + } + if a.hb.bootstrapFailure.Load() == nil { + t.Error("a terminal denial must still reach the operator through the heartbeat") + } + if _, err := os.Stat(a.receiptPath()); err != nil { + t.Error("a denied acknowledgement must leave the receipt for a later attempt") + } +} + +// rds-init runs after this agent, so the receipt is absent for the whole initdb +// window. Reporting that would put a failure on every healthy create's beat, and +// then into the reason a create that timed out for some other cause records. +func TestAcknowledgeBootstrap_DoesNotReportAnAbsentReceiptAsAFailure(t *testing.T) { + cp := newFakeControlPlane() + cfg := testConfig(t) + cfg.DataMount = t.TempDir() + a := newAgent(cfg, cp, newEngineProbe(cfg, staticProbe(0))) + a.id = identity{DBInstanceIdentifier: "db-resolved"} + a.pending = &pendingBootstrap{payloadID: testPayloadID, vmGeneration: 1} + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + if err := a.acknowledgeBootstrap(ctx); err == nil { + t.Fatal("acknowledgeBootstrap returned nil, want it still waiting for rds-init") + } + if got := a.hb.bootstrapFailure.Load(); got != nil { + t.Errorf("bootstrap failure = %q, want none while the receipt has simply not been written", *got) + } + + // A receipt that exists but names another instance is a real mismatch and is + // reported, which is what separates the two cases. + writeReceipt(t, a, testPayloadID, "db-the-snapshot-source") + mismatch, cancelMismatch := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancelMismatch() + if err := a.acknowledgeBootstrap(mismatch); err == nil { + t.Fatal("a foreign receipt must not be acknowledged") + } + if a.hb.bootstrapFailure.Load() == nil { + t.Error("a receipt naming another instance must be reported") + } +} diff --git a/cmd/rds-agent/bootstrap.go b/cmd/rds-agent/bootstrap.go index 10a307aab..e7cfcd885 100644 --- a/cmd/rds-agent/bootstrap.go +++ b/cmd/rds-agent/bootstrap.go @@ -12,6 +12,14 @@ import ( handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" ) +// The staged payload this boot has to acknowledge once PostgreSQL has applied +// it. Nil on an attach boot, which makes the acknowledgement step a no-op. +type pendingBootstrap struct { + payloadID string + vmGeneration int64 + dataVolumeID string +} + // Fetches the boot material and writes the handoff rds-init is blocked waiting // for, then points the health probe at the assigned port. func (a *Agent) bootstrap(ctx context.Context) error { @@ -24,6 +32,14 @@ func (a *Agent) bootstrap(ctx context.Context) error { if fetched == nil { return fmt.Errorf("bootstrap fetch returned no config") } + // A control plane that predates the encrypted payload answers initialize + // with nothing to initialise with. Retrying re-dials the worker queue + // group until an upgraded node answers, so a mixed-version fleet heals + // within one boot rather than needing a reboot. + if fetched.Mode == handlers_rds.BootstrapModeInitialize && + (fetched.MasterUserPassword == nil || *fetched.MasterUserPassword == "") { + return fmt.Errorf("bootstrap fetch returned mode=%s with no master password", fetched.Mode) + } cfg = fetched return nil }, func(err error) { @@ -33,9 +49,8 @@ func (a *Agent) bootstrap(ctx context.Context) error { } a.hb.clearBootstrapFailure() - // Once an initialize response reaches the guest, keep retrying that same - // payload. Re-fetching after a local write failure returns attach because the - // control plane has already consumed the one-shot password. + // The fetch mutates nothing, so a handoff that cannot be written is retried + // against the same staged payload rather than against an attach response. if err := retryObserved(ctx, "bootstrap handoff", func(context.Context) error { return a.handoffWriter(a.cfg.HandoffDir, cfg) }, func(err error) { @@ -48,10 +63,18 @@ func (a *Agent) bootstrap(ctx context.Context) error { a.probe.setPort(int(cfg.Port)) a.engine.setPort(int(cfg.Port)) } + if cfg.BootstrapPending && cfg.PayloadID != "" { + a.pending = &pendingBootstrap{ + payloadID: cfg.PayloadID, + vmGeneration: cfg.VMGeneration, + dataVolumeID: cfg.DataVolumeID, + } + } // Mode is logged but never branched on: rds-init decides whether to initdb // from the state of the datadir. slog.Info("rds-agent: bootstrap config delivered", - "mode", cfg.Mode, "port", cfg.Port, "parameters", len(cfg.Parameters)) + "mode", cfg.Mode, "port", cfg.Port, "parameters", len(cfg.Parameters), + "bootstrapPending", cfg.BootstrapPending) return nil } @@ -107,11 +130,17 @@ func renderBootstrapEnv(cfg *handlers_rds.GetDBBootstrapConfigOutput) string { var b strings.Builder b.WriteString("# Written by rds-agent. Regenerated on every boot; edits are lost.\n") writeEnvLine(&b, "RDS_MODE", cfg.Mode) + writeEnvLine(&b, "RDS_DB_INSTANCE_IDENTIFIER", cfg.DBInstanceIdentifier) writeEnvLine(&b, "RDS_MASTER_USERNAME", cfg.MasterUsername) writeEnvLine(&b, "RDS_DATA_VOLUME_ID", cfg.DataVolumeID) writeEnvLine(&b, "RDS_DATA_VOLUME_SERIAL", cfg.DataVolumeSerial) writeEnvLine(&b, "RDS_VM_GENERATION", strconv.FormatInt(cfg.VMGeneration, 10)) writeEnvLine(&b, "RDS_FORMAT_AUTHORIZED", strconv.FormatBool(cfg.FormatAuthorized)) + // Neither needs volume access, which is why the assertion is made here: the + // agent runs before rds-datadir mounts the volume, so rds-init is the first + // component that can compare it against the completion receipt on disk. + writeEnvLine(&b, "RDS_BOOTSTRAP_PENDING", boolFlag(cfg.BootstrapPending)) + writeEnvLine(&b, "RDS_PAYLOAD_ID", cfg.PayloadID) // Present only in initialize mode: an attach fetch has no password to write // and rds-init must not find a stale one. if cfg.MasterUserPassword != nil { @@ -126,6 +155,15 @@ func renderBootstrapEnv(cfg *handlers_rds.GetDBBootstrapConfigOutput) string { return b.String() } +// rds-init compares against 1 rather than sourcing a Go boolean, so the two +// halves of the guard cannot drift on spelling. +func boolFlag(v bool) string { + if v { + return "1" + } + return "0" +} + func writeEnvLine(b *strings.Builder, key, value string) { b.WriteString(key) b.WriteString("=") diff --git a/scripts/images/rds-postgres/rds-init b/scripts/images/rds-postgres/rds-init index 4089a6d0f..e4b869491 100644 --- a/scripts/images/rds-postgres/rds-init +++ b/scripts/images/rds-postgres/rds-init @@ -9,23 +9,28 @@ # # /run/spinifex-rds/bootstrap.env 0600 KEY=value shell fragment: # RDS_MODE=initialize|attach +# RDS_DB_INSTANCE_IDENTIFIER= # RDS_MASTER_USERNAME= # RDS_MASTER_PASSWORD= # RDS_DB_NAME= # RDS_PORT= # RDS_DATA_VOLUME_ID / RDS_DATA_VOLUME_SERIAL= # RDS_VM_GENERATION / RDS_FORMAT_AUTHORIZED= +# RDS_BOOTSTRAP_PENDING / RDS_PAYLOAD_ID= # /run/spinifex-rds/parameters.conf 0600 resolved parameter-group values in # postgresql.conf syntax # /run/spinifex-rds/server.crt 0600 serving cert, cluster-CA-signed # /run/spinifex-rds/server.key 0600 its private key # -# Nothing there survives a reboot: the next boot re-fetches in `attach` mode and -# the cert is re-minted rather than persisted. +# Nothing there survives a reboot: the next boot re-fetches and the cert is +# re-minted rather than persisted. # # rds-init keys off the datadir, never off RDS_MODE. An initialised datadir # means skip initdb and only refresh config, so a replaced VM comes back on its -# existing data volume without re-bootstrapping. +# existing data volume without re-bootstrapping. RDS_BOOTSTRAP_PENDING is not an +# exception: it is a control-plane-resolved precondition compared against +# on-disk evidence, the same shape as RDS_FORMAT_AUTHORIZED. set -eu PG_VERSION="${RDS_PG_VERSION:-18}" @@ -43,6 +48,12 @@ PGDATA="${RDS_PGDATA:-${DATA_MOUNT}/${PG_VERSION}/data}" # boot attaching to a datadir whose master role was never created. BOOTSTRAP_SENTINEL="${PGDATA}/rds-bootstrap-incomplete" +# Proof that the master role was durably applied, written once initdb and the +# role are both done. Deliberately outside PGDATA, which clear_unfinished_datadir +# rm -rf's: no initdb-adjacent path can touch it and PGDATA stays a stock datadir. +RECEIPT_DIR="${RDS_RECEIPT_DIR:-${DATA_MOUNT}/.spinifex-rds/bootstrap}" +RECEIPT_FILE="${RECEIPT_DIR}/receipt.env" + HANDOFF_DIR="${RDS_HANDOFF_DIR:-/run/spinifex-rds}" ENV_FILE="${HANDOFF_DIR}/bootstrap.env" SOCKET_DIR="${RDS_SOCKET_DIR:-/run/postgresql}" @@ -73,6 +84,11 @@ _bootstrapped=0 clear_unfinished_datadir() { [ "${_created_datadir}" = "1" ] || return 0 [ "${_bootstrapped}" = "1" ] && return 0 + # The receipt is installed before the traps retire, so a sweep in between + # would leave one vouching for a datadir that is about to be deleted. The + # agent would acknowledge it and the control plane would destroy the only + # copy of the password this instance can ever be initialised with. + rm -f -- "${RECEIPT_FILE}" if rm -rf -- "${PGDATA}" && [ ! -e "${PGDATA}" ]; then echo "[rds-init] ERROR: bootstrap did not complete; datadir ${PGDATA} cleared" >&2 return 0 @@ -90,10 +106,16 @@ load_handoff() { . "${ENV_FILE}" RDS_MODE="${RDS_MODE:-}" + RDS_DB_INSTANCE_IDENTIFIER="${RDS_DB_INSTANCE_IDENTIFIER:-}" RDS_MASTER_USERNAME="${RDS_MASTER_USERNAME:-}" RDS_MASTER_PASSWORD="${RDS_MASTER_PASSWORD:-}" RDS_DB_NAME="${RDS_DB_NAME:-}" RDS_PORT="${RDS_PORT:-5432}" + RDS_VM_GENERATION="${RDS_VM_GENERATION:-}" + # Absent from an agent that predates the receipt protocol, which reads as + # "nothing is pending" and leaves every path below unchanged. + RDS_BOOTSTRAP_PENDING="${RDS_BOOTSTRAP_PENDING:-0}" + RDS_PAYLOAD_ID="${RDS_PAYLOAD_ID:-}" [ -n "${RDS_MASTER_USERNAME}" ] || fail "bootstrap config has no RDS_MASTER_USERNAME" @@ -184,8 +206,8 @@ initialise_cluster() { # recorded on the volume itself. Cleared once the master role exists. printf '%s\n' \ 'rds-init created this datadir and did not finish bootstrapping the master role.' \ - 'While this file exists rds-init refuses to start the engine: the one-shot master' \ - 'password is spent, so the cluster cannot be repaired in place. Recover out of band.' \ + 'While this file exists rds-init refuses to start the engine: the datadir carries no' \ + 'usable master role, so it must not be attached to. Recover out of band.' \ > "${BOOTSTRAP_SENTINEL}" chown "${PG_USER}:${PG_USER}" "${BOOTSTRAP_SENTINEL}" chmod 0600 "${BOOTSTRAP_SENTINEL}" @@ -336,6 +358,10 @@ bootstrap_master() { fail "bootstrapping the master role/database failed (rc=${_rc})" fi + # Proof written while the traps are still armed, so a receipt that cannot be + # made durable clears the datadir instead of leaving one it cannot vouch for. + write_completion_receipt + # The master role exists from here, so the datadir is complete and nothing # may clear it — including on the stop failure reported below. mark_bootstrap_complete @@ -347,6 +373,61 @@ bootstrap_master() { fi } +# The value of one receipt key. Never sourced: the receipt is on the data volume, +# so a restored volume can carry one written by another instance, and executing +# it would run whatever that instance's control plane put there. +receipt_value() { + [ -f "${RECEIPT_FILE}" ] || return 0 + sed -n "s/^$1=//p" "${RECEIPT_FILE}" 2>/dev/null | head -n 1 +} + +# Written between the master role and the retirement of the traps, so a receipt +# that cannot be made durable still clears the datadir rather than leaving a +# bootstrapped one nobody can prove. +# +# Fatal on failure, the mirror image of the sentinel removal below: a healthy +# engine whose receipt never landed would come up available while the agent +# blocked forever on a receipt that will never appear, leaving a working database +# that can never accept a password change or a parameter reload. +write_completion_receipt() { + install -d -m 0700 -o "${PG_USER}" -g "${PG_USER}" "${RECEIPT_DIR}" || + fail "the master role exists but the bootstrap receipt directory ${RECEIPT_DIR} could not be created" + + _tmp="${RECEIPT_FILE}.new" + { + echo "# Written by rds-init once the master role was applied. Holds no secrets." + echo "RDS_RECEIPT_PAYLOAD_ID=${RDS_PAYLOAD_ID}" + echo "RDS_RECEIPT_DB_INSTANCE_IDENTIFIER=${RDS_DB_INSTANCE_IDENTIFIER}" + echo "RDS_RECEIPT_VM_GENERATION=${RDS_VM_GENERATION}" + echo "RDS_RECEIPT_COMPLETED_AT=$(date -u +%Y-%m-%dT%H:%M:%SZ)" + } > "${_tmp}" || + fail "the master role exists but the bootstrap receipt ${RECEIPT_FILE} could not be written" + chown "${PG_USER}:${PG_USER}" "${_tmp}" + chmod 0600 "${_tmp}" + + # A receipt still in the page cache is lost by exactly the crash it exists to + # survive, so it is on disk before it is renamed into place. + sync "${_tmp}" 2>/dev/null || sync + mv -f "${_tmp}" "${RECEIPT_FILE}" || + fail "the master role exists but the bootstrap receipt ${RECEIPT_FILE} could not be installed" + sync "${RECEIPT_DIR}" 2>/dev/null || sync +} + +# PG_VERSION with no sentinel and no receipt is ambiguous from the datadir alone: +# either a legacy instance that bootstrapped before receipts existed, which must +# attach, or one that applied the master role and crashed before the receipt was +# durable, which must not. Only the control plane's view of whether the payload +# is still staged separates the two, and that is what RDS_BOOTSTRAP_PENDING says. +require_bootstrap_receipt() { + [ "${RDS_BOOTSTRAP_PENDING}" = "1" ] || return 0 + if [ -n "${RDS_PAYLOAD_ID}" ] && + [ "$(receipt_value RDS_RECEIPT_PAYLOAD_ID)" = "${RDS_PAYLOAD_ID}" ] && + [ "$(receipt_value RDS_RECEIPT_DB_INSTANCE_IDENTIFIER)" = "${RDS_DB_INSTANCE_IDENTIFIER}" ]; then + return 0 + fi + fail "the control plane has no record of a completed bootstrap for this datadir; refusing to attach to a datadir whose master role cannot be proven. Recover the volume out of band, or delete and recreate the instance." +} + # Retires the guards armed at initdb, in an order where no signal in between can # clear a datadir that now carries a usable master role. mark_bootstrap_complete() { @@ -416,8 +497,9 @@ if [ -s "${PGDATA}/PG_VERSION" ]; then # probe with no master role to log in as. The sentinel says the bootstrap did # not finish, not that the volume is empty, so nothing here clears it. if [ -e "${BOOTSTRAP_SENTINEL}" ]; then - fail "datadir ${PGDATA} carries ${BOOTSTRAP_SENTINEL}: an earlier bootstrap was interrupted before the master role existed, and the one-shot master password is spent; recover the data volume out of band rather than starting the engine" + fail "datadir ${PGDATA} carries ${BOOTSTRAP_SENTINEL}: an earlier bootstrap was interrupted before the master role existed; recover the data volume out of band rather than starting the engine" fi + require_bootstrap_receipt log "datadir ${PGDATA} is already initialised — attaching" install_runtime_config else diff --git a/scripts/images/rds-postgres/rds-init_test.sh b/scripts/images/rds-postgres/rds-init_test.sh index 53433c6e3..d2b8f5cc0 100644 --- a/scripts/images/rds-postgres/rds-init_test.sh +++ b/scripts/images/rds-postgres/rds-init_test.sh @@ -35,7 +35,7 @@ done # shellcheck disable=SC2086 set -- ${args} if [ "${dirmode}" = 1 ]; then - mkdir -p "$@" + mkdir -p "$@" || exit 1 [ -n "${mode}" ] && chmod "${mode}" "$@" exit 0 fi @@ -127,6 +127,17 @@ cat > "${STUBBIN}/psql" <<'EOF' exit 0 EOF +# sync stub: a no-op, except that SYNC_KILL_PARENT signals rds-init from the +# directory sync that follows the receipt rename. That is the one window where +# an installed receipt and an armed sweep coexist. +cat > "${STUBBIN}/sync" <<'EOF' +#!/bin/sh +if [ "${SYNC_KILL_PARENT:-0}" = "1" ] && [ -d "${1:-}" ]; then + kill -TERM "$(cat "${KILL_PID_FILE}")" +fi +exit 0 +EOF + chmod +x "${STUBBIN}"/* PATH="${STUBBIN}:${PATH}" export PATH @@ -138,6 +149,8 @@ pass() { echo "ok: $*"; } DATA_MOUNT="${WORK}/data" PGDATA="${DATA_MOUNT}/18/data" SENTINEL="${PGDATA}/rds-bootstrap-incomplete" +RECEIPT_DIR="${DATA_MOUNT}/.spinifex-rds/bootstrap" +RECEIPT="${RECEIPT_DIR}/receipt.env" HANDOFF="${WORK}/run/spinifex-rds" MOUNTS="${WORK}/mounts" KILL_PID_FILE="${WORK}/rds-init.pid" @@ -151,14 +164,22 @@ export INITDB_CALLS PGCTL_CALLS PSQL_CALLS # write_handoff : lay down the bootstrap.env rds-agent # would have written. An empty password omits the field, as `attach` does. # MASTER_USER overrides the master role name, for the reserved-name cases. +# PENDING/PAYLOAD_ID carry the control plane's "this payload is still staged" +# assertion, which an agent predating the receipt protocol omits entirely. write_handoff() { mkdir -p "${HANDOFF}" { echo "RDS_MODE=$1" + echo "RDS_DB_INSTANCE_IDENTIFIER=${DB_ID:-db1}" echo "RDS_MASTER_USERNAME=${MASTER_USER:-mulgamaster}" [ -n "$2" ] && echo "RDS_MASTER_PASSWORD=$2" [ -n "$3" ] && echo "RDS_DB_NAME=$3" + [ -n "${PENDING:-}" ] && echo "RDS_BOOTSTRAP_PENDING=${PENDING}" + [ -n "${PAYLOAD_ID:-}" ] && echo "RDS_PAYLOAD_ID=${PAYLOAD_ID}" + # Last, and unconditional: an optional line failing its test would make + # the whole group's status non-zero and `set -e` would end the run here. echo "RDS_PORT=6543" + echo "RDS_VM_GENERATION=1" } > "${HANDOFF}/bootstrap.env" } @@ -183,6 +204,7 @@ reset_state() { : > "${PGCTL_CALLS}" : > "${PSQL_CALLS}" unset INITDB_FAIL PG_HBA_AS_DIR PSQL_FAIL LS_FAIL RDS_ALLOW_LOCAL_DATADIR MASTER_USER || true + unset PENDING PAYLOAD_ID DB_ID RECEIPT_DIR_OVERRIDE || true } # run_env: invoke a command with every path knob pointed into the temp dir. @@ -194,6 +216,7 @@ run_env() { RDS_BOOTSTRAP_DIR="${WORK}/run/rds-init" \ RDS_LOG_DIR="${WORK}/log" \ RDS_MOUNTS_FILE="${MOUNTS}" \ + RDS_RECEIPT_DIR="${RECEIPT_DIR_OVERRIDE:-${RECEIPT_DIR}}" \ RDS_ALLOW_LOCAL_DATADIR="${RDS_ALLOW_LOCAL_DATADIR:-0}" \ INITDB_FAIL="${INITDB_FAIL:-0}" \ PG_HBA_AS_DIR="${PG_HBA_AS_DIR:-0}" \ @@ -487,6 +510,122 @@ grep -q 'did not stop' "${WORK}/out" \ || pass "stop-fail: sentinel cleared" unset PGCTL_STOP_FAIL +# --- Case 6j: a completed bootstrap leaves a receipt proving it --- +# The receipt is what lets rds-agent tell the control plane the staged password +# was durably applied, so the ciphertext can be destroyed. It lives outside +# PGDATA, which the failure traps rm -rf. +reset_state +PENDING=1 PAYLOAD_ID=bp-alpha DB_ID=db-alpha +export PENDING PAYLOAD_ID DB_ID +write_handoff initialize 's3cr3t' appdb +if run_ok "receipt"; then + [ -f "${RECEIPT}" ] \ + && pass "receipt: written on the initialize path" || fail "receipt: no receipt written" + grep -q '^RDS_RECEIPT_PAYLOAD_ID=bp-alpha$' "${RECEIPT}" \ + && pass "receipt: names the payload it applied" || fail "receipt: wrong or missing payload id" + grep -q '^RDS_RECEIPT_DB_INSTANCE_IDENTIFIER=db-alpha$' "${RECEIPT}" \ + && pass "receipt: names its DB instance" || fail "receipt: wrong or missing DB instance" + grep -q 'PASSWORD\|s3cr3t' "${RECEIPT}" \ + && fail "receipt: carries a secret" || pass "receipt: carries no secrets" + case "${RECEIPT}" in + "${PGDATA}"/*) fail "receipt: written inside the datadir the traps clear" ;; + *) pass "receipt: written outside PGDATA" ;; + esac +fi + +# --- Case 6k: a receipt that cannot be written is fatal --- +# A successful bootstrap whose receipt never landed would bring up a healthy +# engine while the agent blocked forever on a receipt that never appears — +# a working database that can never take a password change or parameter reload. +reset_state +PENDING=1 PAYLOAD_ID=bp-beta +export PENDING PAYLOAD_ID +write_handoff initialize 's3cr3t' appdb +: > "${WORK}/not-a-dir" +RECEIPT_DIR_OVERRIDE="${WORK}/not-a-dir/bootstrap" +export RECEIPT_DIR_OVERRIDE +run_fails "receipt-fail" +[ -e "${PGDATA}/PG_VERSION" ] \ + && fail "receipt-fail: datadir kept although its bootstrap cannot be proven" \ + || pass "receipt-fail: datadir cleared" +grep -q 'bootstrap receipt' "${WORK}/out" \ + && pass "receipt-fail: reported rather than swallowed" || fail "receipt-fail: no failure message" +unset RECEIPT_DIR_OVERRIDE + +# --- Case 6k2: a sweep between the receipt and the retired traps takes both --- +# The receipt is installed while the traps are still armed. A receipt left behind +# by a sweep would be read by rds-agent, acknowledged, and the control plane would +# destroy the only copy of the password this datadir can be initialised with. +reset_state +PENDING=1 PAYLOAD_ID=bp-gamma +export PENDING PAYLOAD_ID +write_handoff initialize 's3cr3t' appdb +SYNC_KILL_PARENT=1 +export SYNC_KILL_PARENT +run_signalled_fails "receipt-swept" +[ -e "${PGDATA}/PG_VERSION" ] \ + && fail "receipt-swept: datadir kept after a signal mid-bootstrap" \ + || pass "receipt-swept: datadir cleared" +[ -e "${RECEIPT}" ] \ + && fail "receipt-swept: receipt survived the datadir it vouches for" \ + || pass "receipt-swept: receipt cleared with the datadir" +unset SYNC_KILL_PARENT + +# --- Case 6l: pending payload, initialised datadir, no receipt -> fail closed --- +# The datadir alone cannot separate a legacy instance that bootstrapped before +# receipts existed from one that applied the master role and crashed before the +# receipt was durable. Only the control plane's pending flag can. +reset_state +PENDING=1 PAYLOAD_ID=bp-gamma DB_ID=db-gamma +export PENDING PAYLOAD_ID DB_ID +write_handoff initialize 's3cr3t' appdb +if run_ok "pending-setup"; then + echo 'customer table data' > "${PGDATA}/base-relation" + rm -f "${RECEIPT}" + run_fails "pending-no-receipt" + [ -f "${PGDATA}/base-relation" ] \ + && pass "pending-no-receipt: datadir preserved" || fail "pending-no-receipt: datadir cleared" + grep -q 'master role cannot be proven' "${WORK}/out" \ + && pass "pending-no-receipt: refusal names the real reason" \ + || fail "pending-no-receipt: no refusal message" + grep -q 'password is spent' "${WORK}/out" \ + && fail "pending-no-receipt: claims the password is spent, which it is not" \ + || pass "pending-no-receipt: does not claim a spent password" + + # --- Case 6m: the same boot with a matching receipt attaches --- + : > "${INITDB_CALLS}"; : > "${PSQL_CALLS}" + mkdir -p "${RECEIPT_DIR}" + printf 'RDS_RECEIPT_PAYLOAD_ID=bp-gamma\nRDS_RECEIPT_DB_INSTANCE_IDENTIFIER=db-gamma\n' > "${RECEIPT}" + if run_ok "pending-with-receipt"; then + [ -s "${INITDB_CALLS}" ] \ + && fail "pending-with-receipt: re-ran initdb" || pass "pending-with-receipt: attached" + fi + + # --- Case 6n: a receipt naming another instance is treated as absent --- + # The receipt is on the data volume, so it rides along in every snapshot of + # it and a restored volume carries the source instance's receipt. + printf 'RDS_RECEIPT_PAYLOAD_ID=bp-gamma\nRDS_RECEIPT_DB_INSTANCE_IDENTIFIER=db-somebody-else\n' > "${RECEIPT}" + run_fails "pending-foreign-receipt" + [ -f "${PGDATA}/base-relation" ] \ + && pass "pending-foreign-receipt: datadir preserved" || fail "pending-foreign-receipt: datadir cleared" +fi +unset PENDING PAYLOAD_ID DB_ID + +# --- Case 6o: no pending payload leaves the attach path unchanged --- +# An agent predating the receipt protocol sends neither flag, and a legacy +# instance carries no receipt. Neither may be turned into a refusal. +reset_state +write_handoff initialize 's3cr3t' appdb +if run_ok "legacy-setup"; then + rm -rf "${RECEIPT_DIR}" + : > "${INITDB_CALLS}" + write_handoff attach '' appdb + if run_ok "legacy-attach"; then + [ -s "${INITDB_CALLS}" ] \ + && fail "legacy-attach: re-ran initdb" || pass "legacy-attach: attached without a receipt" + fi +fi + # --- Case 7: no serving cert -> TLS off rather than a failed start --- reset_state write_handoff initialize 's3cr3t' '' diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index b0798ce41..0573b17f5 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -1059,6 +1059,7 @@ func (d *Daemon) subscribeAll() error { natsSub{handlers_rds.SubjectRegisterWildcard, handleNATSRequest(d.rdsService.RegisterDBInstance), "spinifex-workers"}, natsSub{handlers_rds.SubjectHealthWildcard, handleNATSRequest(d.rdsService.SubmitDBStateChange), "spinifex-workers"}, natsSub{handlers_rds.SubjectGetDBBootstrapConfig, handleNATSRequest(d.rdsService.GetDBBootstrapConfig), "spinifex-workers"}, + natsSub{handlers_rds.SubjectAcknowledgeDBBootstrap, handleNATSRequest(d.rdsService.AcknowledgeDBBootstrap), "spinifex-workers"}, natsSub{handlers_rds.SubjectCreateDBInstance, handleNATSRequest(d.rdsService.CreateDBInstance), "spinifex-workers"}, natsSub{handlers_rds.SubjectDescribeDBInstances, handleNATSRequest(d.rdsService.DescribeDBInstances), "spinifex-workers"}, natsSub{handlers_rds.SubjectRebootDBInstance, handleNATSRequest(d.rdsService.RebootDBInstance), "spinifex-workers"}, @@ -1662,7 +1663,21 @@ func (d *Daemon) startCluster() error { // RDS control plane: KV-backed agent-protocol handlers plus the customer // actions. The cluster CA signs the per-instance serving certs, minted per // bootstrap fetch and never persisted; ca.key sits beside the configured ca.pem. - d.rdsService = handlers_rds.NewService(d.natsConn, d.config.Region).WithDeps(d.buildRDSDeps()) + // + // The master key is mandatory, as it is for ACM and ELBv2 above: the staged + // bootstrap password is encrypted under it, and the fetch is answered by + // whichever node the queue group picks, so a node without it would make the + // first boot of a DB instance fail intermittently rather than not at all. + d.rdsService, err = initServiceWithRetry("RDS service", func() (*handlers_rds.Service, error) { + deps, depsErr := d.buildRDSDeps() + if depsErr != nil { + return nil, depsErr + } + return handlers_rds.NewService(d.natsConn, d.config.Region).WithDeps(deps), nil + }) + if err != nil { + return fmt.Errorf("failed to initialize RDS service: %w", err) + } // One leader across the cluster derives DB instance status from the agent // heartbeat; every node keeps serving the API. diff --git a/spinifex/daemon/rds_deps.go b/spinifex/daemon/rds_deps.go index 313bc4fbf..35aed8c6e 100644 --- a/spinifex/daemon/rds_deps.go +++ b/spinifex/daemon/rds_deps.go @@ -2,12 +2,15 @@ package daemon import ( "context" + "fmt" "log/slog" "os" + "path/filepath" "time" "github.com/aws/aws-sdk-go/service/ec2" gateway_ec2_instance "github.com/mulgadc/spinifex/spinifex/gateway/ec2/instance" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" handlers_systemvpc "github.com/mulgadc/spinifex/spinifex/handlers/systemvpc" ) @@ -37,7 +40,7 @@ func (d *Daemon) buildRDSLaunchDeps() handlers_rds.LaunchDeps { // The control-plane deps on top of the launch primitives: where the endpoint // ENI is placed, the instance role the agent authenticates with, the base zone // the endpoint record lands in, and the VM-state lookup the reconciler needs. -func (d *Daemon) buildRDSDeps() handlers_rds.Deps { +func (d *Daemon) buildRDSDeps() (handlers_rds.Deps, error) { gatewayCA := "" if d.config.NATS.CACert != "" { if caBytes, err := os.ReadFile(d.config.NATS.CACert); err == nil { @@ -48,9 +51,17 @@ func (d *Daemon) buildRDSDeps() handlers_rds.Deps { } } + // No degraded mode: a create cannot stage a master password without it, and + // the fetch that replays one is answered by any node in the queue group. + masterKey, err := handlers_iam.LoadMasterKey(filepath.Join(filepath.Dir(d.configPath), "master.key")) + if err != nil { + return handlers_rds.Deps{}, fmt.Errorf("load RDS master key: %w", err) + } + return handlers_rds.Deps{ CACertPath: d.config.NATS.CACert, CAKeyPath: clusterCAKeyPath(d.config.NATS.CACert), + MasterKey: masterKey, Launch: d.buildRDSLaunchDeps(), Network: d.vpcService, IAM: d.systemRoleEnsurer, @@ -73,7 +84,7 @@ func (d *Daemon) buildRDSDeps() handlers_rds.Deps { MaintenanceWindowBlock: d.config.RDS.MaintenanceWindowBlock, SweepDeleteLimit: d.config.RDS.BackupSweepDeleteLimit, }, - } + }, nil } // Fans out across every host so a DB VM is observed wherever it landed; a diff --git a/spinifex/gateway/rds/agent.go b/spinifex/gateway/rds/agent.go index 0cb1b3b6e..57e185f10 100644 --- a/spinifex/gateway/rds/agent.go +++ b/spinifex/gateway/rds/agent.go @@ -146,7 +146,8 @@ type GetDBBootstrapConfigInput struct { DBInstanceIdentifier string `locationName:"DBInstanceIdentifier"` } -// Serves boot material, including the master password on the first fetch only. +// Serves boot material, replaying the master password for as long as the staged +// payload bound to this VM generation has not been acknowledged. func GetDBBootstrapConfig(ctx context.Context, input *GetDBBootstrapConfigInput, nc *nats.Conn, caller Caller) (any, error) { id, err := authorizeAgent(ctx, nc, caller, input.DBInstanceIdentifier) if err != nil { @@ -159,6 +160,32 @@ func GetDBBootstrapConfig(ctx context.Context, input *GetDBBootstrapConfigInput, }, id.AccountID) } +// PayloadId, VMGeneration and DataVolumeId are the guest's assertions about the +// payload it applied; the control plane checks each against the record. +type AcknowledgeDBBootstrapInput struct { + DBInstanceIdentifier string `locationName:"DBInstanceIdentifier"` + PayloadId string `locationName:"PayloadId"` + VMGeneration int64 `locationName:"VMGeneration"` + DataVolumeId string `locationName:"DataVolumeId"` +} + +// The only agent call whose side effect destroys key material, so it is a +// distinct action rather than a field on the heartbeat: it has to be denied on +// an identity mismatch, which would break liveness reporting if a beat carried it. +func AcknowledgeDBBootstrap(ctx context.Context, input *AcknowledgeDBBootstrapInput, nc *nats.Conn, caller Caller) (any, error) { + id, err := authorizeAgent(ctx, nc, caller, input.DBInstanceIdentifier) + if err != nil { + return nil, err + } + return handlers_rds.NewNATSService(nc).AcknowledgeDBBootstrap(ctx, &handlers_rds.AcknowledgeDBBootstrapInput{ + DBInstanceIdentifier: id.DBInstanceIdentifier, + InstanceID: id.InstanceID, + PayloadID: input.PayloadId, + VMGeneration: input.VMGeneration, + DataVolumeID: input.DataVolumeId, + }, id.AccountID) +} + // Carries results for commands delivered on an earlier poll and asks for the // next one, matching the ECS ack-on-poll shape. type PollDBCommandsInput struct { diff --git a/spinifex/gateway/rds/handler.go b/spinifex/gateway/rds/handler.go index dbdd8a984..de6a4cd1d 100644 --- a/spinifex/gateway/rds/handler.go +++ b/spinifex/gateway/rds/handler.go @@ -121,10 +121,11 @@ var actions = map[string]actionDef{ // Internal agent actions, callable only by the in-guest agent's system role. // They share the namespace because the agent reaches the control plane over // SigV4-on-awsgw like every other in-guest agent. - "RegisterDBInstance": {handler: typed(RegisterDBInstance), internal: true}, - "SubmitDBStateChange": {handler: typed(SubmitDBStateChange), internal: true}, - "PollDBCommands": {handler: typed(PollDBCommands), internal: true}, - "GetDBBootstrapConfig": {handler: typed(GetDBBootstrapConfig), internal: true}, + "RegisterDBInstance": {handler: typed(RegisterDBInstance), internal: true}, + "SubmitDBStateChange": {handler: typed(SubmitDBStateChange), internal: true}, + "PollDBCommands": {handler: typed(PollDBCommands), internal: true}, + "GetDBBootstrapConfig": {handler: typed(GetDBBootstrapConfig), internal: true}, + "AcknowledgeDBBootstrap": {handler: typed(AcknowledgeDBBootstrap), internal: true}, // Recognised but out of scope. Read replicas, Aurora clusters and option // groups are not offered at all; point-in-time restore waits on WAL diff --git a/spinifex/gateway/rds/handler_test.go b/spinifex/gateway/rds/handler_test.go index 81de18b25..99c36caa3 100644 --- a/spinifex/gateway/rds/handler_test.go +++ b/spinifex/gateway/rds/handler_test.go @@ -63,6 +63,7 @@ var v1Actions = []string{ "SubmitDBStateChange", "PollDBCommands", "GetDBBootstrapConfig", + "AcknowledgeDBBootstrap", } // outOfScopeActions are recognised but deliberately not offered in v1. diff --git a/spinifex/handlers/rds/agent.go b/spinifex/handlers/rds/agent.go index a68b62e2e..e9c07a24b 100644 --- a/spinifex/handlers/rds/agent.go +++ b/spinifex/handlers/rds/agent.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "log/slog" "time" "github.com/nats-io/nats.go/jetstream" @@ -12,9 +13,10 @@ import ( "github.com/mulgadc/spinifex/spinifex/vm" ) -// Bootstrap modes returned by GetDBBootstrapConfig. Initialize is the first -// fetch and carries the master password; attach is the same payload minus the -// password, for a fresh VM booting against an existing datadir. +// Bootstrap modes returned by GetDBBootstrapConfig. Initialize carries the +// master password and is replayed for as long as the payload stays staged; +// attach is the same payload minus the password, for a VM booting against an +// existing datadir. const ( BootstrapModeInitialize = "initialize" BootstrapModeAttach = "attach" @@ -83,6 +85,13 @@ type GetDBBootstrapConfigOutput struct { Parameters []Parameter `json:"parameters,omitempty" locationName:"Parameters" locationNameList:"member" xml:"Parameters>member"` + // Names the staged payload this response replayed and asserts that the + // control plane is still waiting for it to be applied. The guest carries both + // into the handoff, where rds-init uses them to refuse a datadir whose master + // role it cannot prove. + PayloadID string `json:"payloadId,omitempty" locationName:"PayloadId" xml:"PayloadId"` + BootstrapPending bool `json:"bootstrapPending" locationName:"BootstrapPending" xml:"BootstrapPending"` + // Empty when no cluster CA is configured; the agent then starts the engine // without TLS rather than failing to boot, since TLS is offered not enforced. ServingCertificate string `json:"servingCertificate,omitempty" locationName:"ServingCertificate"` @@ -90,6 +99,28 @@ type GetDBBootstrapConfigOutput struct { CACertificate string `json:"caCertificate,omitempty" locationName:"CACertificate"` } +// Reports that PostgreSQL durably applied the staged master password, which is +// the only thing that destroys the ciphertext. Distinct from SubmitDBStateChange +// because a heartbeat is best-effort while this must be retried until confirmed +// and denied on an identity mismatch. +// +// DBInstanceIdentifier and InstanceID are set by the gateway from the caller's +// resolved identity; the rest is the guest's own assertion, checked here. +type AcknowledgeDBBootstrapInput struct { + DBInstanceIdentifier string `json:"dbInstanceIdentifier"` + InstanceID string `json:"instanceId"` + PayloadID string `json:"payloadId"` + VMGeneration int64 `json:"vmGeneration"` + DataVolumeID string `json:"dataVolumeId,omitempty"` +} + +// Minimal by design: the agent needs only success to decide it may remove its +// completion receipt. +type AcknowledgeDBBootstrapOutput struct { + Acknowledged bool `json:"acknowledged" locationName:"Acknowledged"` + AcknowledgedAt *time.Time `json:"acknowledgedAt,omitempty" locationName:"AcknowledgedAt" type:"timestamp"` +} + // One directive delivered to an agent on its long poll. Concrete types land // with their owning phases — password apply, parameter reload, grow, quiesce. type Command struct { @@ -270,8 +301,10 @@ func (s *Service) LastSeen(accountID, dbID string) (time.Time, bool) { return live.lastSeen, true } -// Serves boot material and, on the first call, the master password. That call -// clears the cleartext and sets the marker in one CAS, so a replay reads attach. +// Serves boot material and, while a payload is staged for the caller's +// generation, replays the master password. Nothing is mutated on the way, so a +// dropped reply, a gateway restart or a reboot before the guest applied the +// password all recover by simply asking again. func (s *Service) GetDBBootstrapConfig(ctx context.Context, input *GetDBBootstrapConfigInput, accountID string) (*GetDBBootstrapConfigOutput, error) { if input.DBInstanceIdentifier == "" { return nil, errors.New(awserrors.ErrorInvalidParameterValue) @@ -280,96 +313,222 @@ func (s *Service) GetDBBootstrapConfig(ctx context.Context, input *GetDBBootstra if err != nil { return nil, err } - key := DBInstanceKey(input.DBInstanceIdentifier) - for { - var rec DBInstanceRecord - rev, found, err := getJSONRevision(ctx, kv, key, &rec) - if err != nil { - return nil, err - } - if !found { - return nil, errors.New(awserrors.ErrorDBInstanceNotFound) - } - if rec.InstanceID == "" || input.InstanceID != rec.InstanceID { - return nil, errors.New(awserrors.ErrorAccessDenied) - } - - generationMatches := input.VMGeneration > 0 && input.VMGeneration == rec.VMGeneration - // An old gateway may omit the generation for an existing record. It may - // still serve attach material, but it must not consume or expose a live - // create grant until the caller is bound to the current generation. - if rec.FormatAuthorized && !generationMatches { - return nil, errors.New(awserrors.ErrorAccessDenied) - } - serial := rec.DataVolumeSerial - if serial == "" && rec.DataVolumeID != "" { - // Backward-compatible identity for records written before the serial - // field existed. Missing authorization remains false. - serial = vm.VolumeSerial(rec.DataVolumeID) - } - authoritativeSerial := vm.VolumeSerial(rec.DataVolumeID) - grantIdentityValid := generationMatches && rec.DataVolumeID != "" && - rec.DataVolumeSerial != "" && rec.DataVolumeSerial == authoritativeSerial - if rec.FormatAuthorized && !grantIdentityValid { - return nil, errors.New(awserrors.ErrorAccessDenied) - } - - out := &GetDBBootstrapConfigOutput{ - Mode: BootstrapModeAttach, - DBInstanceIdentifier: rec.DBInstanceIdentifier, - Engine: rec.Engine, - EngineVersion: rec.EngineVersion, - DBName: rec.DBName, - MasterUsername: rec.MasterUsername, - Port: rec.Port, - DataVolumeID: rec.DataVolumeID, - DataVolumeSerial: serial, - VMGeneration: rec.VMGeneration, - FormatAuthorized: rec.FormatAuthorized && grantIdentityValid, - Parameters: rec.Bootstrap.ResolvedParameters, - } - - // The cert is minted before the password is consumed, so a mint failure - // leaves the password in KV for the agent's retry. Consuming first would - // leave an instance that can never learn its master password. - cert, err := s.mintServingCert(&rec) - if err != nil { - return nil, err - } - if cert != nil { - out.ServingCertificate = cert.CertificatePEM - out.ServingPrivateKey = cert.PrivateKeyPEM - out.CACertificate = cert.caPEM - } - - if rec.Bootstrap.Consumed { - return out, nil - } - - password := rec.Bootstrap.MasterUserPassword - now := time.Now().UTC() - rec.Bootstrap.MasterUserPassword = "" - rec.Bootstrap.Consumed = true - rec.Bootstrap.ConsumedAt = &now - rec.UpdatedAt = now - if err := updateJSON(ctx, kv, key, rev, &rec); err != nil { - if !errors.Is(err, jetstream.ErrKeyExists) { - return nil, fmt.Errorf("rds bootstrap: consume master password for %s: %w", - input.DBInstanceIdentifier, err) - } - // A revision conflict may be an unrelated record update. Re-read and - // return attach only when the stored consumed marker proves another - // bootstrap fetch won the password. - if err := ctx.Err(); err != nil { - return nil, err - } - continue - } - out.Mode = BootstrapModeInitialize - out.MasterUserPassword = &password - return out, nil + var rec DBInstanceRecord + found, err := getJSON(ctx, kv, DBInstanceKey(input.DBInstanceIdentifier), &rec) + if err != nil { + return nil, err } + if !found { + return nil, errors.New(awserrors.ErrorDBInstanceNotFound) + } + if rec.InstanceID == "" || input.InstanceID != rec.InstanceID { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + + generationMatches := input.VMGeneration > 0 && input.VMGeneration == rec.VMGeneration + // An old gateway may omit the generation for an existing record. It may + // still serve attach material, but it must not expose a live create grant or + // a staged password until the caller is bound to the current generation. + if rec.FormatAuthorized && !generationMatches { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + serial := rec.DataVolumeSerial + if serial == "" && rec.DataVolumeID != "" { + // Backward-compatible identity for records written before the serial + // field existed. Missing authorization remains false. + serial = vm.VolumeSerial(rec.DataVolumeID) + } + authoritativeSerial := vm.VolumeSerial(rec.DataVolumeID) + grantIdentityValid := generationMatches && rec.DataVolumeID != "" && + rec.DataVolumeSerial != "" && rec.DataVolumeSerial == authoritativeSerial + if rec.FormatAuthorized && !grantIdentityValid { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + + out := &GetDBBootstrapConfigOutput{ + Mode: BootstrapModeAttach, + DBInstanceIdentifier: rec.DBInstanceIdentifier, + Engine: rec.Engine, + EngineVersion: rec.EngineVersion, + DBName: rec.DBName, + MasterUsername: rec.MasterUsername, + Port: rec.Port, + DataVolumeID: rec.DataVolumeID, + DataVolumeSerial: serial, + VMGeneration: rec.VMGeneration, + FormatAuthorized: rec.FormatAuthorized && grantIdentityValid, + Parameters: rec.Bootstrap.ResolvedParameters, + } + // Minted per call and never persisted, so every boot — attach included — + // gets fresh serving material. + cert, err := s.mintServingCert(&rec) + if err != nil { + return nil, err + } + if cert != nil { + out.ServingCertificate = cert.CertificatePEM + out.ServingPrivateKey = cert.PrivateKeyPEM + out.CACertificate = cert.caPEM + } + + if err := s.applyStagedBootstrap(ctx, kv, accountID, &rec, generationMatches, out); err != nil { + return nil, err + } + return out, nil +} + +// Fills in the initialize half of the response when a payload is staged for this +// generation, and leaves the attach response untouched otherwise. +// +// A payload this daemon cannot open is reported as unrecoverable and still +// answered with attach: failing the fetch outright would leave a VM retrying +// with no diagnostic channel, whereas an attach lets it boot, register and beat +// so the operator sees a running-but-broken instance. The guest guard fails +// closed on the datadir either way. +// +// Recording that verdict is bookkeeping, so a failure to write it is logged +// rather than returned. Propagating it would discard an attach response this +// call has already built and reinstate the retry loop it exists to avoid; the +// verdict is idempotent, so the next fetch records it. +func (s *Service) applyStagedBootstrap(ctx context.Context, kv jetstream.KeyValue, accountID string, + rec *DBInstanceRecord, generationMatches bool, out *GetDBBootstrapConfigOutput) error { + envelope, _, err := readBootstrapPayload(ctx, kv, rec.DBInstanceIdentifier) + if err != nil { + return err + } + if envelope == nil { + if err := s.scrubLegacyBootstrap(ctx, kv, accountID, rec); err != nil { + slog.ErrorContext(ctx, "rds bootstrap: a legacy plaintext password could not be scrubbed; serving attach regardless", + "dbInstance", rec.DBInstanceIdentifier, "err", err) + } + return nil + } + // A superseded VM gets the same attach material as any other boot but never + // the staged password. + if !generationMatches || envelope.BoundVMGeneration != rec.VMGeneration { + slog.WarnContext(ctx, "rds bootstrap: a staged payload was not served to a caller outside its bound generation", + "dbInstance", rec.DBInstanceIdentifier, "recordGeneration", rec.VMGeneration, + "boundGeneration", envelope.BoundVMGeneration) + return nil + } + + claims, err := s.openBootstrapPayload(envelope, accountID, rec) + if err != nil { + slog.ErrorContext(ctx, "rds bootstrap: the staged payload could not be opened", + "dbInstance", rec.DBInstanceIdentifier, "err", err) + if err := s.markBootstrapUnrecoverable(ctx, kv, accountID, rec.DBInstanceIdentifier, + "the staged bootstrap payload cannot be read by the control plane; delete and recreate the DB instance"); err != nil { + slog.ErrorContext(ctx, "rds bootstrap: the unrecoverable verdict could not be recorded; serving attach regardless", + "dbInstance", rec.DBInstanceIdentifier, "err", err) + } + return nil + } + + password := claims.MasterPassword + out.Mode = BootstrapModeInitialize + out.MasterUsername = claims.MasterUsername + out.MasterUserPassword = &password + out.PayloadID = claims.PayloadID + out.BootstrapPending = true + return nil +} + +// Beta records staged the password as cleartext on the record itself. One that +// was never consumed also never bootstrapped, so there is no datadir to save: +// the password is removed on sight and the instance is marked for recreation +// rather than left holding a secret at rest for an operator who may never act. +func (s *Service) scrubLegacyBootstrap(ctx context.Context, kv jetstream.KeyValue, + accountID string, rec *DBInstanceRecord) error { + if rec.Bootstrap.MasterUserPassword == "" { + return nil + } + rec.Bootstrap.MasterUserPassword = "" + return s.markBootstrapUnrecoverable(ctx, kv, accountID, rec.DBInstanceIdentifier, + "this DB instance staged its master password before the password was encrypted at rest and never completed its initial bootstrap; delete and recreate it") +} + +// Destroys the staged ciphertext once the guest has proven PostgreSQL applied +// it. Deliberately never decrypts: a lost acknowledgement has to remain +// deliverable even after a master.key change made the payload unreadable, so the +// one operation that destroys key material must not depend on reading it. +func (s *Service) AcknowledgeDBBootstrap(ctx context.Context, input *AcknowledgeDBBootstrapInput, + accountID string) (*AcknowledgeDBBootstrapOutput, error) { + if input.DBInstanceIdentifier == "" || input.PayloadID == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + kv, err := s.bucket(ctx, accountID) + if err != nil { + return nil, err + } + rec, _, err := s.getDBInstance(ctx, kv, input.DBInstanceIdentifier) + if err != nil { + return nil, err + } + if rec.InstanceID == "" || input.InstanceID != rec.InstanceID { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + + envelope, rev, err := readBootstrapPayload(ctx, kv, input.DBInstanceIdentifier) + if err != nil { + return nil, err + } + if envelope == nil { + // The true duplicate: the first acknowledgement landed and its response + // was lost. The retained payload ID is the only thing left to answer it + // with, which is why it outlives the key it names. + if rec.Bootstrap.PayloadID != "" && rec.Bootstrap.PayloadID == input.PayloadID { + return &AcknowledgeDBBootstrapOutput{ + Acknowledged: true, AcknowledgedAt: rec.Bootstrap.AcknowledgedAt, + }, nil + } + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "no bootstrap payload %s is or was staged for DB instance %s", input.PayloadID, input.DBInstanceIdentifier) + } + + // A superseded VM must never acknowledge: it would destroy the payload the + // current generation is still waiting to replay. + if envelope.PayloadID != input.PayloadID || envelope.BoundVMGeneration != rec.VMGeneration || + input.VMGeneration != rec.VMGeneration { + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "bootstrap payload %s at generation %d does not match the current state of DB instance %s", + input.PayloadID, input.VMGeneration, input.DBInstanceIdentifier) + } + // An echo of what the fetch handed the guest rather than independent proof: + // the agent does not observe the serial of the device rds-datadir mounted. + // A mismatch is a platform bug worth denying on all the same. + if input.DataVolumeID != "" && input.DataVolumeID != rec.DataVolumeID { + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "DB instance %s is not backed by data volume %s", input.DBInstanceIdentifier, input.DataVolumeID) + } + + // The record first: a failure between the two leaves the payload staged and + // the retry converges. Deleting first would leave a state where neither the + // key nor the retained payload ID can answer the retry. + now := time.Now().UTC() + if err := s.updateInstance(ctx, kv, input.DBInstanceIdentifier, func(stored *DBInstanceRecord) { + stored.Bootstrap.PayloadID = input.PayloadID + stored.Bootstrap.State = BootstrapStateAcknowledged + stored.Bootstrap.AcknowledgedAt = &now + stored.Bootstrap.MasterUserPassword = "" + // A daemon that could not read the payload marked this instance before + // another one served it, so the stale verdict goes with the ciphertext. + stored.Bootstrap.FailureReason = "" + }); err != nil { + return nil, err + } + if err := deleteBootstrapPayload(ctx, kv, input.DBInstanceIdentifier, jetstream.LastRevision(rev)); err != nil { + return nil, err + } + + slog.InfoContext(ctx, "rds: initial bootstrap acknowledged", + "dbInstance", input.DBInstanceIdentifier, "accountId", accountID, + "instanceId", input.InstanceID, "payloadId", input.PayloadID) + s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, input.DBInstanceIdentifier, + "The database engine confirmed the initial master credentials were applied.", + EventCategoryCreation, EventCategoryNotification) + + return &AcknowledgeDBBootstrapOutput{Acknowledged: true, AcknowledgedAt: &now}, nil } type bootstrapCert struct { diff --git a/spinifex/handlers/rds/agent_test.go b/spinifex/handlers/rds/agent_test.go index 1a586a758..debb4a579 100644 --- a/spinifex/handlers/rds/agent_test.go +++ b/spinifex/handlers/rds/agent_test.go @@ -66,11 +66,12 @@ func newTestCA(t *testing.T) CALoader { func newTestService(t *testing.T) *Service { t.Helper() _, nc, _ := testutil.StartTestJetStream(t) - return NewService(nc, testRegion).WithDeps(Deps{LoadCA: newTestCA(t)}) + return NewService(nc, testRegion).WithDeps(Deps{LoadCA: newTestCA(t), MasterKey: testMasterKey}) } -// seedInstance writes a DB instance record in the shape CreateDBInstance will -// leave it: bootstrap config pending, marker unset. +// seedInstance writes a DB instance record without the payload a pending +// bootstrap also needs, which is what an instance past its bootstrap looks like. +// seedPending is the pair for a create that has not been acknowledged yet. func seedInstance(t *testing.T, svc *Service, rec DBInstanceRecord) { t.Helper() kv, err := svc.bucket(context.Background(), testAccountID) @@ -81,6 +82,7 @@ func seedInstance(t *testing.T, svc *Service, rec DBInstanceRecord) { func defaultRecord() DBInstanceRecord { return DBInstanceRecord{ DBInstanceIdentifier: testDBID, + DbiResourceID: testDbiResourceID, AccountID: testAccountID, Status: StatusCreating, Engine: "postgres", @@ -89,15 +91,26 @@ func defaultRecord() DBInstanceRecord { MasterUsername: "postgres", Port: 5432, InstanceID: testInstance, + VMGeneration: firstVMGeneration, ENIPrivateIP: "10.20.30.40", DNSName: "orders-db.123456789012.ap-southeast-2.rds.example.internal", Bootstrap: BootstrapState{ - MasterUserPassword: "s3cr3t-master-pw", + State: BootstrapStatePending, ResolvedParameters: []Parameter{{Name: "shared_buffers", Value: "128MB"}}, }, } } +// The generation-bound fetch a current VM's agent makes. Cases that only need +// attach material build their own input and leave the generation off. +func bootstrapInput() *GetDBBootstrapConfigInput { + return &GetDBBootstrapConfigInput{ + DBInstanceIdentifier: testDBID, + InstanceID: testInstance, + VMGeneration: firstVMGeneration, + } +} + // readRecord returns the raw stored bytes alongside the decoded record, so a // test can assert on what is actually at rest in KV rather than on the decode. func readRecord(t *testing.T, svc *Service) (DBInstanceRecord, string) { @@ -111,41 +124,26 @@ func readRecord(t *testing.T, svc *Service) (DBInstanceRecord, string) { return rec, string(entry.Value()) } -func TestGetDBBootstrapConfig_InitializeThenAttach(t *testing.T) { +func TestGetDBBootstrapConfig_ServesTheFullBootMaterial(t *testing.T) { svc := newTestService(t) rec := defaultRecord() - rec.VMGeneration = 1 rec.DataVolumeID = "vol-data-01" rec.DataVolumeSerial = "voldata01" - seedInstance(t, svc, rec) - ctx := context.Background() - in := &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance, VMGeneration: 1} + seedPending(t, svc, rec) - first, err := svc.GetDBBootstrapConfig(ctx, in, testAccountID) + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.NoError(t, err) - assert.Equal(t, BootstrapModeInitialize, first.Mode) - require.NotNil(t, first.MasterUserPassword) - assert.Equal(t, "s3cr3t-master-pw", *first.MasterUserPassword) + assert.Equal(t, BootstrapModeInitialize, out.Mode) // The rest of the payload has to survive into attach: a fresh VM booting // against an existing datadir gets no password but still needs all of it. - assert.Equal(t, int64(5432), first.Port) - assert.Equal(t, "orders", first.DBName) - assert.Equal(t, []Parameter{{Name: "shared_buffers", Value: "128MB"}}, first.Parameters) - assert.Equal(t, "vol-data-01", first.DataVolumeID) - assert.Equal(t, "voldata01", first.DataVolumeSerial) - assert.Equal(t, int64(1), first.VMGeneration) - assert.False(t, first.FormatAuthorized) - - for i := range 3 { - next, err := svc.GetDBBootstrapConfig(ctx, in, testAccountID) - require.NoError(t, err, "fetch %d", i) - assert.Equal(t, BootstrapModeAttach, next.Mode) - assert.Nil(t, next.MasterUserPassword, "the password must never be re-served") - assert.Equal(t, int64(5432), next.Port) - assert.Equal(t, "orders", next.DBName) - assert.Equal(t, first.Parameters, next.Parameters) - } + assert.Equal(t, int64(5432), out.Port) + assert.Equal(t, "orders", out.DBName) + assert.Equal(t, []Parameter{{Name: "shared_buffers", Value: "128MB"}}, out.Parameters) + assert.Equal(t, "vol-data-01", out.DataVolumeID) + assert.Equal(t, "voldata01", out.DataVolumeSerial) + assert.Equal(t, int64(firstVMGeneration), out.VMGeneration) + assert.False(t, out.FormatAuthorized) } func TestGetDBBootstrapConfig_FormatGrantRequiresExactCurrentIdentity(t *testing.T) { @@ -168,14 +166,13 @@ func TestGetDBBootstrapConfig_FormatGrantRequiresExactCurrentIdentity(t *testing t.Run(tc.name, func(t *testing.T) { svc := newTestService(t) rec := defaultRecord() - rec.VMGeneration = 1 rec.DataVolumeID = "vol-data-01" rec.DataVolumeSerial = "voldata01" rec.FormatAuthorized = true if tc.mutate != nil { tc.mutate(&rec) } - seedInstance(t, svc, rec) + seedPending(t, svc, rec) out, err := svc.GetDBBootstrapConfig(t.Context(), &GetDBBootstrapConfigInput{ DBInstanceIdentifier: testDBID, @@ -184,8 +181,8 @@ func TestGetDBBootstrapConfig_FormatGrantRequiresExactCurrentIdentity(t *testing }, testAccountID) if tc.wantErr { require.Error(t, err) - stored, _ := readRecord(t, svc) - assert.False(t, stored.Bootstrap.Consumed, "a rejected caller must not consume the one-shot password") + envelope, _ := storedPayload(t, svc, testDBID) + assert.NotNil(t, envelope, "a rejected caller must leave the payload staged for the real one") return } require.NoError(t, err) @@ -197,39 +194,6 @@ func TestGetDBBootstrapConfig_FormatGrantRequiresExactCurrentIdentity(t *testing } } -func TestGetDBBootstrapConfig_PasswordGoneFromKVAfterConsumption(t *testing.T) { - svc := newTestService(t) - seedInstance(t, svc, defaultRecord()) - - _, raw := readRecord(t, svc) - require.Contains(t, raw, "s3cr3t-master-pw", "seed must actually contain the cleartext") - - _, err := svc.GetDBBootstrapConfig(context.Background(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) - require.NoError(t, err) - - rec, raw := readRecord(t, svc) - assert.NotContains(t, raw, "s3cr3t-master-pw", "cleartext must not remain anywhere in the stored record") - assert.Empty(t, rec.Bootstrap.MasterUserPassword) - assert.True(t, rec.Bootstrap.Consumed, "only the one-way marker remains") - assert.NotNil(t, rec.Bootstrap.ConsumedAt) -} - -// A restore seeds the marker already flipped, so its very first fetch is an -// attach and no separate restore flag is needed. -func TestGetDBBootstrapConfig_PreConsumedRecordAttachesImmediately(t *testing.T) { - svc := newTestService(t) - rec := defaultRecord() - rec.Bootstrap = BootstrapState{Consumed: true, ResolvedParameters: rec.Bootstrap.ResolvedParameters} - seedInstance(t, svc, rec) - - out, err := svc.GetDBBootstrapConfig(context.Background(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) - require.NoError(t, err) - assert.Equal(t, BootstrapModeAttach, out.Mode) - assert.Nil(t, out.MasterUserPassword) -} - func TestGetDBBootstrapConfig_MintsFreshCertEachFetch(t *testing.T) { svc := newTestService(t) seedInstance(t, svc, defaultRecord()) @@ -291,11 +255,10 @@ func TestGetDBBootstrapConfig_NoDNSNameStillMintsIPOnlyCert(t *testing.T) { // boot a database rather than failing the fetch. func TestGetDBBootstrapConfig_NoCAStillServesConfig(t *testing.T) { _, nc, _ := testutil.StartTestJetStream(t) - svc := NewService(nc, testRegion) - seedInstance(t, svc, defaultRecord()) + svc := NewService(nc, testRegion).WithDeps(Deps{MasterKey: testMasterKey}) + seedPending(t, svc, defaultRecord()) - out, err := svc.GetDBBootstrapConfig(context.Background(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.NoError(t, err) assert.Equal(t, BootstrapModeInitialize, out.Mode) assert.Empty(t, out.ServingCertificate) @@ -306,40 +269,41 @@ func TestGetDBBootstrapConfig_ConfiguredCARequiresENIPrivateIP(t *testing.T) { svc := newTestService(t) rec := defaultRecord() rec.ENIPrivateIP = "" - seedInstance(t, svc, rec) + seedPending(t, svc, rec) - _, err := svc.GetDBBootstrapConfig(t.Context(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) + _, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.Error(t, err) assert.Contains(t, err.Error(), "configured TLS requires an ENI private IP") - stored, raw := readRecord(t, svc) - assert.False(t, stored.Bootstrap.Consumed) - assert.Contains(t, raw, "s3cr3t-master-pw") + envelope, _ := storedPayload(t, svc, testDBID) + assert.NotNil(t, envelope, "a failed fetch must leave the payload replayable") } func TestGetDBBootstrapConfig_PartialCAConfigurationFailsClosed(t *testing.T) { _, nc, _ := testutil.StartTestJetStream(t) - svc := NewService(nc, testRegion).WithDeps(Deps{CACertPath: "/etc/spinifex/ca.pem"}) - seedInstance(t, svc, defaultRecord()) + svc := NewService(nc, testRegion).WithDeps(Deps{ + CACertPath: "/etc/spinifex/ca.pem", + MasterKey: testMasterKey, + }) + seedPending(t, svc, defaultRecord()) - _, err := svc.GetDBBootstrapConfig(t.Context(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) + _, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.Error(t, err) assert.Contains(t, err.Error(), "incomplete cluster CA configuration") - stored, _ := readRecord(t, svc) - assert.False(t, stored.Bootstrap.Consumed) + envelope, _ := storedPayload(t, svc, testDBID) + assert.NotNil(t, envelope) } -// A configured CA that cannot be loaded must fail the fetch *without* consuming -// the password. Leaving the bootstrap state untouched is what makes the agent's +// A configured CA that cannot be loaded fails the fetch before the payload is +// ever read. The staged password is untouched, which is what makes the agent's // retry succeed once the CA is readable again. func TestGetDBBootstrapConfig_UnloadableCALeavesPasswordRecoverable(t *testing.T) { _, nc, _ := testutil.StartTestJetStream(t) loadErr := errors.New("read CA key /etc/spinifex/ca.key: permission denied") broken := true svc := NewService(nc, testRegion).WithDeps(Deps{ + MasterKey: testMasterKey, LoadCA: func() (*x509.Certificate, *rsa.PrivateKey, error) { if broken { return nil, nil, loadErr @@ -347,66 +311,30 @@ func TestGetDBBootstrapConfig_UnloadableCALeavesPasswordRecoverable(t *testing.T return newTestCA(t)() }, }) - seedInstance(t, svc, defaultRecord()) - ctx := context.Background() - in := &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance} + seedPending(t, svc, defaultRecord()) - _, err := svc.GetDBBootstrapConfig(ctx, in, testAccountID) + _, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.Error(t, err) - - rec, raw := readRecord(t, svc) - require.False(t, rec.Bootstrap.Consumed, "a failed mint must not flip the one-way marker") - require.Contains(t, raw, "s3cr3t-master-pw", "the password must still be there to retry against") + envelope, _ := storedPayload(t, svc, testDBID) + require.NotNil(t, envelope, "the password must still be staged to retry against") // Once the CA is readable the retry bootstraps normally. broken = false - out, err := svc.GetDBBootstrapConfig(ctx, in, testAccountID) + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) require.NoError(t, err) assert.Equal(t, BootstrapModeInitialize, out.Mode) require.NotNil(t, out.MasterUserPassword) - assert.Equal(t, "s3cr3t-master-pw", *out.MasterUserPassword) + assert.Equal(t, testMasterPassword, *out.MasterUserPassword) assert.NotEmpty(t, out.ServingCertificate) } -func TestGetDBBootstrapConfig_RetriesCASConflictFromUnrelatedUpdate(t *testing.T) { - _, nc, _ := testutil.StartTestJetStream(t) - baseCA := newTestCA(t) - svc := NewService(nc, testRegion) - var once sync.Once - svc.WithDeps(Deps{LoadCA: func() (*x509.Certificate, *rsa.PrivateKey, error) { - once.Do(func() { - kv, err := svc.bucket(t.Context(), testAccountID) - require.NoError(t, err) - var rec DBInstanceRecord - rev, found, err := getJSONRevision(t.Context(), kv, DBInstanceKey(testDBID), &rec) - require.NoError(t, err) - require.True(t, found) - rec.Agent.Message = "concurrent health update" - require.NoError(t, updateJSON(t.Context(), kv, DBInstanceKey(testDBID), rev, &rec)) - }) - return baseCA() - }}) - seedInstance(t, svc, defaultRecord()) - - out, err := svc.GetDBBootstrapConfig(t.Context(), - &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance}, testAccountID) - require.NoError(t, err) - assert.Equal(t, BootstrapModeInitialize, out.Mode) - require.NotNil(t, out.MasterUserPassword) - assert.Equal(t, "s3cr3t-master-pw", *out.MasterUserPassword) - - rec, _ := readRecord(t, svc) - assert.Equal(t, "concurrent health update", rec.Agent.Message) - assert.True(t, rec.Bootstrap.Consumed) -} - -// Concurrent bootstrap fetches must resolve to exactly one password holder, and -// every caller must still be served TLS material. The CAS provides the first -// half: a plain put would hand the same password to two agents silently. -func TestGetDBBootstrapConfig_ConcurrentFetchesYieldOnePassword(t *testing.T) { +// Under consume-on-fetch, concurrent fetches racing for a one-shot password was +// the whole problem. A replay has no race to lose: every caller bound to the +// generation is served the same password and none of them writes. +func TestGetDBBootstrapConfig_ConcurrentFetchesAllReplayTheSamePassword(t *testing.T) { svc := newTestService(t) - seedInstance(t, svc, defaultRecord()) - in := &GetDBBootstrapConfigInput{DBInstanceIdentifier: testDBID, InstanceID: testInstance} + payloadID := seedPending(t, svc, defaultRecord()) + _, beforeRaw := readRecord(t, svc) const fetches = 4 outs := make([]*GetDBBootstrapConfigOutput, fetches) @@ -416,29 +344,25 @@ func TestGetDBBootstrapConfig_ConcurrentFetchesYieldOnePassword(t *testing.T) { for i := range fetches { wg.Go(func() { <-start - outs[i], errs[i] = svc.GetDBBootstrapConfig(context.Background(), in, testAccountID) + outs[i], errs[i] = svc.GetDBBootstrapConfig(context.Background(), bootstrapInput(), testAccountID) }) } close(start) wg.Wait() - withPassword := 0 for i := range fetches { require.NoError(t, errs[i]) - require.NotNil(t, outs[i]) - if outs[i].MasterUserPassword != nil { - withPassword++ - assert.Equal(t, BootstrapModeInitialize, outs[i].Mode) - } - // A caller that loses the race still boots an engine, and it must not - // boot it with ssl=off while the winner got TLS. - assert.NotEmpty(t, outs[i].ServingCertificate, "every fetch must be served a cert") + require.NotNil(t, outs[i].MasterUserPassword, "fetch %d", i) + assert.Equal(t, testMasterPassword, *outs[i].MasterUserPassword) + assert.Equal(t, payloadID, outs[i].PayloadID) + // Serving material is minted per call, so no caller boots with ssl=off + // while another got TLS. + assert.NotEmpty(t, outs[i].ServingCertificate) assert.NotEmpty(t, outs[i].ServingPrivateKey) } - assert.Equal(t, 1, withPassword, "exactly one fetch may carry the master password") - _, raw := readRecord(t, svc) - assert.NotContains(t, raw, "s3cr3t-master-pw") + _, afterRaw := readRecord(t, svc) + assert.Equal(t, beforeRaw, afterRaw) } func TestGetDBBootstrapConfig_UnknownInstance(t *testing.T) { diff --git a/spinifex/handlers/rds/backup_sweep_test.go b/spinifex/handlers/rds/backup_sweep_test.go index dea2fb59c..58d77302b 100644 --- a/spinifex/handlers/rds/backup_sweep_test.go +++ b/spinifex/handlers/rds/backup_sweep_test.go @@ -238,6 +238,7 @@ func TestSweep_IsBoundedPerPassAndResumes(t *testing.T) { h := newSnapshotHarness(t, false) h.svc = h.svc.WithDeps(Deps{ LoadCA: newTestCA(t), + MasterKey: testMasterKey, Launch: h.launch.deps(), Network: h.network, Snapshots: h.snaps, diff --git a/spinifex/handlers/rds/bootstrap.go b/spinifex/handlers/rds/bootstrap.go new file mode 100644 index 000000000..eb5dbbac1 --- /dev/null +++ b/spinifex/handlers/rds/bootstrap.go @@ -0,0 +1,273 @@ +package handlers_rds + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "time" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go/jetstream" +) + +// The staged master password is encrypted at rest under bootstrap-payloads/{id} +// and replayed to the bound VM generation until the guest proves PostgreSQL +// applied it. The protocol is encrypt → replay → durably apply → acknowledge → +// delete, because no KV mutation can be made atomic with a network delivery. + +const ( + bootstrapEnvelopeVersion = 1 + bootstrapPayloadIDPrefix = "bp" +) + +// The cleartext wrapper, so a stale generation or a payload written under +// another master key is rejected with a legible reason rather than as a GCM +// authentication failure that reads as tampering. +type BootstrapPayloadEnvelope struct { + PayloadID string `json:"payloadId"` + EnvelopeVersion int `json:"envelopeVersion"` + KeyID string `json:"keyId"` + BoundVMGeneration int64 `json:"boundVmGeneration"` + CreatedAt time.Time `json:"createdAt"` + EncryptedPayload string `json:"encryptedPayload"` +} + +// The plaintext inside EncryptedPayload. handlers_iam.EncryptSecret takes no +// additional authenticated data, so every claim is carried here and validated +// after decrypting — ciphertext copied between accounts, instances or +// generations then fails on a claim rather than being accepted. +// +// DbiResourceID is the load-bearing one: deleting db1 and recreating it yields a +// record with the same identifier and the same first generation, and only the +// resource ID tells the two apart. +type bootstrapPayloadClaims struct { + EnvelopeVersion int `json:"envelopeVersion"` + KeyID string `json:"keyId"` + AccountID string `json:"accountId"` + DBInstanceIdentifier string `json:"dbInstanceIdentifier"` + DbiResourceID string `json:"dbiResourceId"` + BoundVMGeneration int64 `json:"boundVmGeneration"` + PayloadID string `json:"payloadId"` + MasterUsername string `json:"masterUsername"` + MasterPassword string `json:"masterPassword"` +} + +// Required at service construction rather than per operation: the fetch is +// answered by whichever daemon the spinifex-workers queue group picks, so a +// per-operation dependency would make a pending fetch succeed or fail depending +// on which node answered. +func (s *Service) masterKey() ([]byte, error) { + if len(s.deps.MasterKey) == 0 { + return nil, errors.New("rds bootstrap: no master key is configured") + } + return s.deps.MasterKey, nil +} + +// Diagnostic only. Key rotation is not supported by any service in the platform; +// this exists so a payload written under a different key reports that rather +// than failing as a corrupt ciphertext. +func bootstrapKeyID(key []byte) string { + sum := sha256.Sum256(key) + return hex.EncodeToString(sum[:8]) +} + +// Stages the master password for rec's current VM generation. The key's presence +// is what makes the bootstrap pending, so it is written once and only ever +// deleted by an acknowledgement, a delete, or a generation bump. +func (s *Service) writeBootstrapPayload(ctx context.Context, kv jetstream.KeyValue, + accountID string, rec *DBInstanceRecord, masterPassword string) (string, error) { + key, err := s.masterKey() + if err != nil { + return "", err + } + keyID := bootstrapKeyID(key) + payloadID := utils.GenerateResourceID(bootstrapPayloadIDPrefix) + + plaintext, err := json.Marshal(bootstrapPayloadClaims{ + EnvelopeVersion: bootstrapEnvelopeVersion, + KeyID: keyID, + AccountID: accountID, + DBInstanceIdentifier: rec.DBInstanceIdentifier, + DbiResourceID: rec.DbiResourceID, + BoundVMGeneration: rec.VMGeneration, + PayloadID: payloadID, + MasterUsername: rec.MasterUsername, + MasterPassword: masterPassword, + }) + if err != nil { + return "", fmt.Errorf("rds bootstrap: marshal the payload for %s: %w", rec.DBInstanceIdentifier, err) + } + ciphertext, err := handlers_iam.EncryptSecret(string(plaintext), key) + if err != nil { + return "", fmt.Errorf("rds bootstrap: encrypt the payload for %s: %w", rec.DBInstanceIdentifier, err) + } + + envelope := BootstrapPayloadEnvelope{ + PayloadID: payloadID, + EnvelopeVersion: bootstrapEnvelopeVersion, + KeyID: keyID, + BoundVMGeneration: rec.VMGeneration, + CreatedAt: time.Now().UTC(), + EncryptedPayload: ciphertext, + } + // Put rather than create: the caller has already won the record reservation + // for this identifier, so a key still sitting here is an orphan from a + // rolled-back create and must not make the identifier unusable forever. + if err := putJSON(ctx, kv, BootstrapPayloadKey(rec.DBInstanceIdentifier), envelope); err != nil { + return "", fmt.Errorf("rds bootstrap: stage the payload for %s: %w", rec.DBInstanceIdentifier, err) + } + return payloadID, nil +} + +// Returns (nil, 0, nil) when nothing is staged, which is what an acknowledged, +// restored or legacy instance reads as. +func readBootstrapPayload(ctx context.Context, kv jetstream.KeyValue, + dbInstanceIdentifier string) (*BootstrapPayloadEnvelope, uint64, error) { + var envelope BootstrapPayloadEnvelope + rev, found, err := getJSONRevision(ctx, kv, BootstrapPayloadKey(dbInstanceIdentifier), &envelope) + if err != nil || !found { + return nil, 0, err + } + return &envelope, rev, nil +} + +// An already-absent key is the state this is trying to reach, so it is not an +// error. +func deleteBootstrapPayload(ctx context.Context, kv jetstream.KeyValue, + dbInstanceIdentifier string, opts ...jetstream.KVDeleteOpt) error { + err := kv.Delete(ctx, BootstrapPayloadKey(dbInstanceIdentifier), opts...) + if err != nil && !errors.Is(err, jetstream.ErrKeyNotFound) { + return fmt.Errorf("rds bootstrap: delete the payload for %s: %w", dbInstanceIdentifier, err) + } + return nil +} + +// Whether a staged payload is still waiting to be applied. Callers that only +// need the yes/no — the modify gate, the reconciler's stale-payload sweep — use +// this rather than decrypting. +func bootstrapPending(ctx context.Context, kv jetstream.KeyValue, dbInstanceIdentifier string) (bool, error) { + envelope, _, err := readBootstrapPayload(ctx, kv, dbInstanceIdentifier) + return envelope != nil, err +} + +// The record's own view, for the read paths that must not pay a second KV round +// trip per instance to answer it. Every writer of the payload key sets State in +// the same operation, so this trails the key only between those two writes; a +// caller deciding whether a payload may be served reads the key via +// bootstrapPending instead. +func resolveBootstrapState(rec *DBInstanceRecord) string { + switch { + case rec.Bootstrap.State != "": + return rec.Bootstrap.State + case rec.Bootstrap.Consumed: + return BootstrapStateLegacyConsumed + default: + return BootstrapStateNone + } +} + +// Decrypts and validates every claim against the record it was staged for. +// Returns an AccessDenied-classed error for a claim mismatch and a plain error +// for a payload this daemon cannot read at all. +func (s *Service) openBootstrapPayload(envelope *BootstrapPayloadEnvelope, accountID string, + rec *DBInstanceRecord) (*bootstrapPayloadClaims, error) { + key, err := s.masterKey() + if err != nil { + return nil, err + } + if envelope.EnvelopeVersion != bootstrapEnvelopeVersion { + return nil, fmt.Errorf("rds bootstrap: payload for %s is envelope version %d, this daemon serves version %d", + rec.DBInstanceIdentifier, envelope.EnvelopeVersion, bootstrapEnvelopeVersion) + } + if keyID := bootstrapKeyID(key); envelope.KeyID != keyID { + return nil, fmt.Errorf("rds bootstrap: payload for %s was encrypted under master key %s, this daemon holds %s; key rotation is not supported", + rec.DBInstanceIdentifier, envelope.KeyID, keyID) + } + + plaintext, err := handlers_iam.DecryptSecret(envelope.EncryptedPayload, key) + if err != nil { + return nil, fmt.Errorf("rds bootstrap: decrypt the payload for %s: %w", rec.DBInstanceIdentifier, err) + } + var claims bootstrapPayloadClaims + if err := json.Unmarshal([]byte(plaintext), &claims); err != nil { + return nil, fmt.Errorf("rds bootstrap: unmarshal the payload for %s: %w", rec.DBInstanceIdentifier, err) + } + + // Every claim, not just the ones the outer envelope already showed: the + // envelope is unauthenticated, so only these prove the ciphertext was + // written for this account, this instance and this generation. + switch { + case claims.EnvelopeVersion != envelope.EnvelopeVersion, + claims.KeyID != envelope.KeyID, + claims.PayloadID != envelope.PayloadID, + claims.BoundVMGeneration != envelope.BoundVMGeneration, + claims.AccountID != accountID, + claims.DBInstanceIdentifier != rec.DBInstanceIdentifier, + claims.DbiResourceID != rec.DbiResourceID, + claims.BoundVMGeneration != rec.VMGeneration: + return nil, awserrors.Errorf(awserrors.ErrorAccessDenied, + "the staged bootstrap payload for %s does not belong to this DB instance generation", rec.DBInstanceIdentifier) + } + if claims.MasterPassword == "" { + return nil, fmt.Errorf("rds bootstrap: payload for %s carries no master password", rec.DBInstanceIdentifier) + } + return &claims, nil +} + +// Records that the staged password can no longer reach this datadir. Written +// rather than inferred so the reason rides DescribeDBInstances, and idempotent +// so a repeated fetch does not churn the record. +func (s *Service) markBootstrapUnrecoverable(ctx context.Context, kv jetstream.KeyValue, + accountID, dbInstanceIdentifier, reason string) error { + changed := false + if err := s.updateInstance(ctx, kv, dbInstanceIdentifier, func(stored *DBInstanceRecord) { + if stored.Bootstrap.State == BootstrapStateUnrecoverable && + stored.Bootstrap.MasterUserPassword == "" && stored.Bootstrap.FailureReason == reason { + return + } + changed = true + stored.Bootstrap.MasterUserPassword = "" + stored.Bootstrap.State = BootstrapStateUnrecoverable + stored.Bootstrap.FailureReason = reason + }); err != nil { + return fmt.Errorf("rds bootstrap: mark %s unrecoverable: %w", dbInstanceIdentifier, err) + } + if changed { + s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, dbInstanceIdentifier, + "The initial bootstrap of this DB instance cannot be completed: "+reason, + EventCategoryFailure, EventCategoryNotification) + } + return nil +} + +// The safety net for a generation bump on an instance that never finished its +// initial bootstrap. The payload is never rebound to the new generation: the +// password is not what stops the replacement working — replaceInstanceVM +// revokes the data-volume format grant, and only the initial create can hold +// one — so rebinding would move the failure rather than fix it. +func (s *Service) discardPendingBootstrap(ctx context.Context, kv jetstream.KeyValue, + accountID string, rec *DBInstanceRecord) error { + pending, err := bootstrapPending(ctx, kv, rec.DBInstanceIdentifier) + if err != nil || !pending { + return err + } + if err := deleteBootstrapPayload(ctx, kv, rec.DBInstanceIdentifier); err != nil { + return err + } + // A serving instance already applied its master role, so its payload was + // staged only because the acknowledgement never landed. Withdrawing it costs + // nothing and must not read as a database the customer has to recreate. + if rec.Status == StatusAvailable { + s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier, + "The staged initial credentials were withdrawn when this DB instance's VM was replaced; the engine is serving and had already applied them.", + EventCategoryNotification) + return nil + } + return s.markBootstrapUnrecoverable(ctx, kv, accountID, rec.DBInstanceIdentifier, + "the staged initial credentials were bound to a VM generation this DB instance no longer has; delete and recreate it") +} diff --git a/spinifex/handlers/rds/bootstrap_test.go b/spinifex/handlers/rds/bootstrap_test.go new file mode 100644 index 000000000..4d85fe1ee --- /dev/null +++ b/spinifex/handlers/rds/bootstrap_test.go @@ -0,0 +1,646 @@ +package handlers_rds + +import ( + "encoding/json" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A fixed 32-byte AES-256 key, so a ciphertext written by one harness is +// readable by another and a wrong-key case is expressible. +var testMasterKey = []byte("0123456789abcdef0123456789abcdef") + +const ( + testDbiResourceID = "db-AAAAAAAAAAAAAAAAA" + testMasterPassword = "s3cr3t-master-pw" + testOtherMasterKeyS = "fedcba9876543210fedcba9876543210" +) + +// stagePayload writes the encrypted payload a create would have staged for rec, +// and returns its ID. +func stagePayload(t *testing.T, svc *Service, rec *DBInstanceRecord) string { + t.Helper() + kv, err := svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + payloadID, err := svc.writeBootstrapPayload(t.Context(), kv, testAccountID, rec, testMasterPassword) + require.NoError(t, err) + return payloadID +} + +// seedPending seeds the record and stages its payload, which together are what a +// finished CreateDBInstance leaves behind. +func seedPending(t *testing.T, svc *Service, rec DBInstanceRecord) string { + t.Helper() + seedInstance(t, svc, rec) + return stagePayload(t, svc, &rec) +} + +func storedPayload(t *testing.T, svc *Service, id string) (*BootstrapPayloadEnvelope, string) { + t.Helper() + kv, err := svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + entry, err := kv.Get(t.Context(), BootstrapPayloadKey(id)) + if err != nil { + return nil, "" + } + var envelope BootstrapPayloadEnvelope + require.NoError(t, json.Unmarshal(entry.Value(), &envelope)) + return &envelope, string(entry.Value()) +} + +func TestBootstrapPayload_RoundTripsAndCarriesNoCleartext(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + seedPending(t, svc, rec) + + envelope, raw := storedPayload(t, svc, testDBID) + require.NotNil(t, envelope) + assert.NotContains(t, raw, testMasterPassword, "the staged password must never be readable at rest") + assert.Equal(t, bootstrapEnvelopeVersion, envelope.EnvelopeVersion) + assert.Equal(t, bootstrapKeyID(testMasterKey), envelope.KeyID) + assert.Equal(t, rec.VMGeneration, envelope.BoundVMGeneration) + + claims, err := svc.openBootstrapPayload(envelope, testAccountID, &rec) + require.NoError(t, err) + assert.Equal(t, testMasterPassword, claims.MasterPassword) + assert.Equal(t, rec.MasterUsername, claims.MasterUsername) + assert.Equal(t, rec.DbiResourceID, claims.DbiResourceID) +} + +// The same password staged twice must not produce the same bytes, or a KV reader +// could tell two instances share a password without decrypting either. +func TestBootstrapPayload_CiphertextIsNotDeterministic(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + seedInstance(t, svc, rec) + + kv, err := svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + first, err := svc.writeBootstrapPayload(t.Context(), kv, testAccountID, &rec, testMasterPassword) + require.NoError(t, err) + one, _ := storedPayload(t, svc, testDBID) + require.NoError(t, deleteBootstrapPayload(t.Context(), kv, testDBID)) + second, err := svc.writeBootstrapPayload(t.Context(), kv, testAccountID, &rec, testMasterPassword) + require.NoError(t, err) + two, _ := storedPayload(t, svc, testDBID) + + assert.NotEqual(t, first, second, "each staging mints its own payload ID") + assert.NotEqual(t, one.EncryptedPayload, two.EncryptedPayload) +} + +func TestBootstrapPayload_TamperedCiphertextIsRejected(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + seedPending(t, svc, rec) + + envelope, _ := storedPayload(t, svc, testDBID) + // Flip one base64 character; GCM authenticates the whole thing, so any edit + // fails rather than yielding a plausible plaintext. + body := []byte(envelope.EncryptedPayload) + if body[10] == 'A' { + body[10] = 'B' + } else { + body[10] = 'A' + } + envelope.EncryptedPayload = string(body) + + _, err := svc.openBootstrapPayload(envelope, testAccountID, &rec) + require.Error(t, err) + assert.Contains(t, err.Error(), "decrypt the payload") +} + +// A payload written under another master key must say so rather than surfacing a +// raw GCM authentication failure, which reads as tampering. +func TestBootstrapPayload_WrongMasterKeyIsReportedLegibly(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + seedPending(t, svc, rec) + envelope, _ := storedPayload(t, svc, testDBID) + + rotated := svc.WithDeps(Deps{MasterKey: []byte(testOtherMasterKeyS)}) + _, err := rotated.openBootstrapPayload(envelope, testAccountID, &rec) + require.Error(t, err) + assert.Contains(t, err.Error(), "key rotation is not supported") + assert.NotContains(t, err.Error(), testMasterPassword) +} + +// Every claim is validated after decrypting, so ciphertext lifted from another +// account, instance, resource or generation is refused rather than replayed. +func TestBootstrapPayload_ClaimsBindItToOneInstanceGeneration(t *testing.T) { + tests := []struct { + name string + accountID string + mutate func(*DBInstanceRecord) + }{ + {name: "another account", accountID: "999999999999"}, + {name: "another DB identifier", mutate: func(rec *DBInstanceRecord) { + rec.DBInstanceIdentifier = "someone-elses-db" + }}, + {name: "a recreate of the same identifier", mutate: func(rec *DBInstanceRecord) { + rec.DbiResourceID = "db-BBBBBBBBBBBBBBBBB" + }}, + {name: "a later VM generation", mutate: func(rec *DBInstanceRecord) { + rec.VMGeneration = 2 + }}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + seedPending(t, svc, rec) + envelope, _ := storedPayload(t, svc, testDBID) + + accountID := testAccountID + if tc.accountID != "" { + accountID = tc.accountID + } + target := rec + if tc.mutate != nil { + tc.mutate(&target) + } + + _, err := svc.openBootstrapPayload(envelope, accountID, &target) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorAccessDenied) + }) + } +} + +// The whole point of the split: a fetch serves the password without writing +// anything, so a lost reply is recovered by asking again. +func TestGetDBBootstrapConfig_ReplaysUntilAcknowledged(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + payloadID := seedPending(t, svc, rec) + in := bootstrapInput() + + _, beforeRaw := readRecord(t, svc) + for i := range 3 { + out, err := svc.GetDBBootstrapConfig(t.Context(), in, testAccountID) + require.NoError(t, err, "fetch %d", i) + assert.Equal(t, BootstrapModeInitialize, out.Mode) + require.NotNil(t, out.MasterUserPassword) + assert.Equal(t, testMasterPassword, *out.MasterUserPassword) + assert.Equal(t, payloadID, out.PayloadID) + assert.True(t, out.BootstrapPending) + } + _, afterRaw := readRecord(t, svc) + assert.Equal(t, beforeRaw, afterRaw, "a replayed fetch must mutate nothing") + + require.NoError(t, acknowledge(t, svc, payloadID)) + out, err := svc.GetDBBootstrapConfig(t.Context(), in, testAccountID) + require.NoError(t, err) + assert.Equal(t, BootstrapModeAttach, out.Mode) + assert.Nil(t, out.MasterUserPassword) + assert.False(t, out.BootstrapPending) +} + +func acknowledge(t *testing.T, svc *Service, payloadID string) error { + t.Helper() + _, err := svc.AcknowledgeDBBootstrap(t.Context(), &AcknowledgeDBBootstrapInput{ + DBInstanceIdentifier: testDBID, + InstanceID: testInstance, + PayloadID: payloadID, + VMGeneration: firstVMGeneration, + }, testAccountID) + return err +} + +func TestAcknowledgeDBBootstrap_Resolution(t *testing.T) { + tests := []struct { + name string + prepare func(t *testing.T, svc *Service, payloadID string) string + wantErr bool + wantWiped bool + }{ + { + name: "the payload is staged and every claim matches", + prepare: func(_ *testing.T, _ *Service, payloadID string) string { return payloadID }, + wantWiped: true, + }, + { + name: "a true duplicate after the key is already gone", + prepare: func(t *testing.T, svc *Service, payloadID string) string { + require.NoError(t, acknowledge(t, svc, payloadID)) + return payloadID + }, + wantWiped: true, + }, + { + name: "no payload is or ever was staged for this ID", + prepare: func(t *testing.T, svc *Service, payloadID string) string { + require.NoError(t, acknowledge(t, svc, payloadID)) + return "bp-somebody-elses" + }, + wantErr: true, + }, + { + name: "the payload ID does not match the staged one", + prepare: func(_ *testing.T, _ *Service, _ string) string { return "bp-stale" }, + wantErr: true, + }, + { + name: "a superseded VM acknowledging at an older generation", + prepare: func(t *testing.T, svc *Service, payloadID string) string { + kv, err := svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + require.NoError(t, svc.updateInstance(t.Context(), kv, testDBID, func(stored *DBInstanceRecord) { + stored.VMGeneration = 2 + })) + return payloadID + }, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := newTestService(t) + payloadID := seedPending(t, svc, defaultRecord()) + requested := tc.prepare(t, svc, payloadID) + + out, err := svc.AcknowledgeDBBootstrap(t.Context(), &AcknowledgeDBBootstrapInput{ + DBInstanceIdentifier: testDBID, + InstanceID: testInstance, + PayloadID: requested, + VMGeneration: firstVMGeneration, + }, testAccountID) + if tc.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorAccessDenied) + return + } + require.NoError(t, err) + assert.True(t, out.Acknowledged) + + if tc.wantWiped { + envelope, _ := storedPayload(t, svc, testDBID) + assert.Nil(t, envelope, "a successful acknowledgement destroys the ciphertext") + stored, _ := readRecord(t, svc) + assert.Equal(t, BootstrapStateAcknowledged, stored.Bootstrap.State) + assert.Equal(t, payloadID, stored.Bootstrap.PayloadID) + require.NotNil(t, stored.Bootstrap.AcknowledgedAt) + } + }) + } +} + +// The operation that destroys key material must not depend on being able to read +// it, or a master.key change would strand a bootstrapped instance forever. +func TestAcknowledgeDBBootstrap_NeverDecrypts(t *testing.T) { + svc := newTestService(t) + payloadID := seedPending(t, svc, defaultRecord()) + + rotated := svc.WithDeps(Deps{LoadCA: newTestCA(t), MasterKey: []byte(testOtherMasterKeyS)}) + _, err := rotated.AcknowledgeDBBootstrap(t.Context(), &AcknowledgeDBBootstrapInput{ + DBInstanceIdentifier: testDBID, + InstanceID: testInstance, + PayloadID: payloadID, + VMGeneration: firstVMGeneration, + }, testAccountID) + require.NoError(t, err) + + envelope, _ := storedPayload(t, svc, testDBID) + assert.Nil(t, envelope) +} + +// The volume ID is an echo of what the fetch handed the guest rather than +// independent proof, but a mismatch is a platform bug worth denying on. +func TestAcknowledgeDBBootstrap_RejectsAForeignDataVolume(t *testing.T) { + svc := newTestService(t) + payloadID := seedPending(t, svc, defaultRecord()) + + _, err := svc.AcknowledgeDBBootstrap(t.Context(), &AcknowledgeDBBootstrapInput{ + DBInstanceIdentifier: testDBID, + InstanceID: testInstance, + PayloadID: payloadID, + VMGeneration: firstVMGeneration, + DataVolumeID: "vol-somebody-else", + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorAccessDenied) + + envelope, _ := storedPayload(t, svc, testDBID) + assert.NotNil(t, envelope, "a denied acknowledgement must leave the payload replayable") +} + +// A beta record whose password was already spent keeps attaching: it is serving +// real data and holds no secret at rest to fix. +func TestGetDBBootstrapConfig_LegacyConsumedRecordAttaches(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + rec.Bootstrap = BootstrapState{Consumed: true, ResolvedParameters: rec.Bootstrap.ResolvedParameters} + seedInstance(t, svc, rec) + + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) + require.NoError(t, err) + assert.Equal(t, BootstrapModeAttach, out.Mode) + assert.Nil(t, out.MasterUserPassword) + + stored, _ := readRecord(t, svc) + assert.Equal(t, BootstrapStateLegacyConsumed, resolveBootstrapState(&stored)) + assert.Empty(t, stored.FailureReason, "an instance that is serving is not a failure") +} + +// A beta record still holding cleartext never bootstrapped, so there is no +// datadir to save. The password is removed on sight rather than left waiting for +// an operator who may never act. +func TestGetDBBootstrapConfig_LegacyPlaintextIsScrubbedIdempotently(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + rec.Bootstrap.MasterUserPassword = "beta-plaintext-pw" + seedInstance(t, svc, rec) + + for i := range 2 { + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) + require.NoError(t, err, "fetch %d", i) + assert.Equal(t, BootstrapModeAttach, out.Mode, "the initialize fetch is denied") + assert.Nil(t, out.MasterUserPassword) + + stored, raw := readRecord(t, svc) + assert.NotContains(t, raw, "beta-plaintext-pw") + assert.Equal(t, BootstrapStateUnrecoverable, resolveBootstrapState(&stored)) + assert.Contains(t, stored.Bootstrap.FailureReason, "delete and recreate") + } +} + +// A payload this daemon cannot open still answers attach, so the VM boots, +// registers and beats and the operator sees a running-but-broken instance +// instead of a VM retrying into silence. +func TestGetDBBootstrapConfig_UnreadablePayloadAttachesAndReportsWhy(t *testing.T) { + svc := newTestService(t) + seedPending(t, svc, defaultRecord()) + + rotated := svc.WithDeps(Deps{LoadCA: newTestCA(t), MasterKey: []byte(testOtherMasterKeyS)}) + out, err := rotated.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) + require.NoError(t, err) + assert.Equal(t, BootstrapModeAttach, out.Mode) + assert.Nil(t, out.MasterUserPassword) + assert.False(t, out.BootstrapPending) + + stored, _ := readRecord(t, svc) + assert.Equal(t, BootstrapStateUnrecoverable, stored.Bootstrap.State) + assert.Contains(t, stored.Bootstrap.FailureReason, "cannot be read by the control plane") +} + +func TestCreateDBInstance_StagesThePasswordEncrypted(t *testing.T) { + h := newCreateHarness(t, "") + _, err := h.svc.CreateDBInstance(t.Context(), validCreateInput(), testAccountID) + require.NoError(t, err) + + kv, err := h.svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + keys, err := bucketKeys(t.Context(), kv) + require.NoError(t, err) + require.Contains(t, keys, BootstrapPayloadKey(testDBInstanceID)) + + // Nothing anywhere in the account bucket may hold the supplied password. + for _, key := range keys { + entry, err := kv.Get(t.Context(), key) + require.NoError(t, err) + assert.NotContains(t, string(entry.Value()), "Sup3rSecret!", "cleartext at rest under %s", key) + } + + var envelope BootstrapPayloadEnvelope + entry, err := kv.Get(t.Context(), BootstrapPayloadKey(testDBInstanceID)) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(entry.Value(), &envelope)) + plaintext, err := handlers_iam.DecryptSecret(envelope.EncryptedPayload, testMasterKey) + require.NoError(t, err) + assert.Contains(t, plaintext, "Sup3rSecret!", + "the payload has to actually carry the password it staged") +} + +func TestCreateDBInstance_WithoutAMasterKeyStagesNothing(t *testing.T) { + h := newCreateHarness(t, "") + h.svc = h.svc.WithDeps(Deps{ + LoadCA: newTestCA(t), + Launch: h.launch.deps(), + Network: h.network, + IAM: testIAMProvider(h.iam), + }) + + _, err := h.svc.CreateDBInstance(t.Context(), validCreateInput(), testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), "no master key is configured") + + kv, err := h.svc.bucket(t.Context(), testAccountID) + require.NoError(t, err) + keys, err := bucketKeys(t.Context(), kv) + require.NoError(t, err) + assert.NotContains(t, keys, DBInstanceKey(testDBInstanceID), "the reservation is withdrawn") + assert.NotContains(t, keys, BootstrapPayloadKey(testDBInstanceID)) +} + +// A restore attaches to a datadir that already has its master role, so nothing +// is staged for it and an acknowledgement against it is denied rather than read +// as a benign duplicate. +func TestRestoredRecord_IsBornWithoutAStagedBootstrap(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + rec.Bootstrap.State = BootstrapStateNone + seedInstance(t, svc, rec) + + out, err := svc.GetDBBootstrapConfig(t.Context(), bootstrapInput(), testAccountID) + require.NoError(t, err) + assert.Equal(t, BootstrapModeAttach, out.Mode) + assert.Nil(t, out.MasterUserPassword) + + stored, _ := readRecord(t, svc) + assert.Equal(t, BootstrapStateNone, resolveBootstrapState(&stored)) + require.Error(t, acknowledge(t, svc, "bp-anything")) +} + +func TestDeleteDBInstance_RemovesTheStagedPayload(t *testing.T) { + h := newLifecycleHarness(t, false) + rec := availableRecord() + seedPending(t, h.svc, rec) + require.NoError(t, h.svc.PutInstanceIndex(t.Context(), testInstance, InstanceIndexEntry{ + AccountID: testAccountID, DBInstanceIdentifier: testDBID, + })) + + _, err := h.svc.DeleteDBInstance(t.Context(), skipFinalSnapshot(), testAccountID) + require.NoError(t, err) + + envelope, _ := storedPayload(t, h.svc, testDBID) + assert.Nil(t, envelope, "the ciphertext must not outlive the instance it belongs to") +} + +// A create that timed out into failed can still be reached by a class change. +// The replacement would lose the format grant only the initial create can hold, +// so it could never come up whatever the password did. +func TestModifyDBInstance_RejectsADisruptiveChangeWhileTheBootstrapIsStaged(t *testing.T) { + h := newModifyHarness(t) + rec := modifiableRecord() + rec.Status = StatusFailed + seedPending(t, h.svc, rec) + + _, err := h.svc.ModifyDBInstance(t.Context(), &rds.ModifyDBInstanceInput{ + DBInstanceIdentifier: aws.String(testDBID), + DBInstanceClass: aws.String("db.t3.large"), + ApplyImmediately: aws.Bool(true), + }, testAccountID) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorDBInstanceInvalidState) + assert.Contains(t, err.Error(), "delete and recreate") + + envelope, _ := storedPayload(t, h.svc, testDBID) + assert.NotNil(t, envelope, "a rejected modify changes nothing") +} + +// The safety net for a generation bump reaching a staged payload through any +// path. The payload is never rebound: rebinding would move where the failure +// surfaces rather than make the replacement work. +// +// Only a VM that never finished its initial bootstrap is a database the customer +// has to recreate. A serving one applied its master role and merely never +// acknowledged, so withdrawing its payload must not read as a failure. +func TestReplaceInstanceVM_DiscardsAStagedBootstrap(t *testing.T) { + tests := []struct { + name string + status Status + wantState string + }{ + {name: "a create that never bootstrapped", status: StatusFailed, + wantState: BootstrapStateUnrecoverable}, + {name: "a serving instance that never acknowledged", status: StatusAvailable, + wantState: BootstrapStatePending}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + h := newModifyHarness(t) + seed := modifiableRecord() + seed.Status = tc.status + rec := seedReplaceable(t, h, seed) + stagePayload(t, h.svc, &rec) + + require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, + replaceInput{Reason: "a class change"})) + + envelope, _ := storedPayload(t, h.svc, testDBID) + assert.Nil(t, envelope, "a bumped generation must not leave a payload nobody can use") + + after, _ := readRecord(t, h.svc) + assert.Equal(t, tc.wantState, after.Bootstrap.State) + if tc.wantState == BootstrapStateUnrecoverable { + assert.Contains(t, after.Bootstrap.FailureReason, "delete and recreate") + return + } + assert.Empty(t, after.Bootstrap.FailureReason, + "a serving database must not be reported as one to recreate") + }) + } +} + +// A rollback that could not remove the payload key must not make the identifier +// unusable: the create that follows owns the reservation, so the orphan it finds +// is its own to overwrite. +func TestCreateDBInstance_StagesOverAnOrphanedPayload(t *testing.T) { + svc := newTestService(t) + rec := defaultRecord() + orphaned := seedPending(t, svc, rec) + + restaged := stagePayload(t, svc, &rec) + + assert.NotEqual(t, orphaned, restaged) + envelope, _ := storedPayload(t, svc, testDBID) + require.NotNil(t, envelope) + assert.Equal(t, restaged, envelope.PayloadID, "the live create's payload must be the one a fetch replays") +} + +// The reason for an unrecoverable bootstrap is owned by this protocol, not by +// the record's FailureReason: the status machine clears that on every +// transition, which is exactly what a timed-out create does next. +func TestMarkBootstrapUnrecoverable_ReasonSurvivesAStatusTransition(t *testing.T) { + h := newModifyHarness(t) + rec := modifiableRecord() + rec.Status = StatusCreating + seedInstance(t, h.svc, rec) + + require.NoError(t, h.svc.markBootstrapUnrecoverable(t.Context(), h.kv(t), testAccountID, testDBID, + "the staged bootstrap payload cannot be read; delete and recreate the DB instance")) + marked, rev, err := h.svc.getDBInstance(t.Context(), h.kv(t), testDBID) + require.NoError(t, err) + require.NoError(t, h.rec.transition(t.Context(), h.kv(t), rev, marked, StatusFailed, + "the database engine did not report healthy within 15m of creation")) + + stored := h.record(t) + assert.Contains(t, stored.Bootstrap.FailureReason, "delete and recreate", + "the only message naming an instance that must be recreated has to outlive the timeout") + assert.Contains(t, stored.FailureReason, "did not report healthy") +} + +// Both reasons reach the customer, and an instance still holding staged +// credentials while it serves is visible without a second call. +func TestProjectDBInstance_ReportsTheBootstrapState(t *testing.T) { + svc := NewService(nil, testRegion) + rec := defaultRecord() + rec.Status = StatusAvailable + rec.Bootstrap.State = BootstrapStateUnrecoverable + rec.Bootstrap.FailureReason = "the staged bootstrap payload cannot be read by the control plane" + + infos := svc.projectDBInstance(&rec).StatusInfos + + require.Len(t, infos, 1) + assert.Equal(t, "bootstrap", aws.StringValue(infos[0].StatusType)) + assert.Equal(t, BootstrapStateUnrecoverable, aws.StringValue(infos[0].Status)) + assert.False(t, aws.BoolValue(infos[0].Normal)) + assert.Equal(t, rec.Bootstrap.FailureReason, aws.StringValue(infos[0].Message)) + + rec.Bootstrap.State = BootstrapStatePending + rec.Bootstrap.FailureReason = "" + pending := svc.projectDBInstance(&rec).StatusInfos + require.Len(t, pending, 1, "a serving engine that never confirmed its credentials is reported") + assert.Equal(t, BootstrapStatePending, aws.StringValue(pending[0].Status)) + + rec.Status = StatusCreating + assert.Empty(t, svc.projectDBInstance(&rec).StatusInfos, + "a create still in flight is pending by definition and needs no status info") +} + +// The gate exists for a create that never bootstrapped, whose replacement VM +// could never format its data volume. A serving instance with a payload staged +// merely never acknowledged, so refusing its class change would be telling the +// customer to destroy a working database. +func TestModifyDBInstance_PendingBootstrapGateIsScopedToFailed(t *testing.T) { + tests := []struct { + name string + status Status + wantErr bool + }{ + {name: "a create that never bootstrapped", status: StatusFailed, wantErr: true}, + {name: "a serving instance that never acknowledged", status: StatusAvailable}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + h := newModifyHarness(t) + rec := modifiableRecord() + rec.Status = tc.status + seedReplaceable(t, h, rec) + stagePayload(t, h.svc, &rec) + + in := modifyInput() + in.DBInstanceClass = aws.String("db.m5.large") + _, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID) + + if tc.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorDBInstanceInvalidState) + assert.Contains(t, err.Error(), "delete and recreate") + return + } + require.NoError(t, err) + envelope, _ := storedPayload(t, h.svc, testDBID) + assert.NotNil(t, envelope, "an accepted modify must leave the payload replayable") + }) + } +} diff --git a/spinifex/handlers/rds/create.go b/spinifex/handlers/rds/create.go index e724f8dc9..d74618094 100644 --- a/spinifex/handlers/rds/create.go +++ b/spinifex/handlers/rds/create.go @@ -78,6 +78,13 @@ func (s *Service) CreateDBInstance(ctx context.Context, input *rds.CreateDBInsta s.rollbackDBInstanceReservation(ctx, kv, key, req.Identifier, rec.DbiResourceID, rollbackRev) }() + // Staged before the launch so no VM can boot ahead of the password it needs, + // and bound to the first generation the record will carry. The rollback above + // removes it with the reservation. + if _, err = s.writeBootstrapPayload(ctx, kv, accountID, &rec, req.MasterPassword); err != nil { + return nil, err + } + launched, err := LaunchDBInstanceVM(ctx, s.deps.Launch, LaunchInput{ DBInstanceIdentifier: req.Identifier, AccountID: accountID, @@ -138,7 +145,8 @@ func (s *Service) CreateDBInstance(ctx context.Context, input *rds.CreateDBInsta } // The record as it stands before any resource exists: everything the request -// determined, plus the staged master password the first bootstrap fetch consumes. +// determined. The master password is never on the record — it is staged +// encrypted under its own key, bound to the generation set here. func newDBInstanceRecord(accountID string, req *validatedCreate, placement *endpointPlacement, parameters []Parameter) DBInstanceRecord { now := time.Now().UTC() return DBInstanceRecord{ @@ -146,6 +154,7 @@ func newDBInstanceRecord(accountID string, req *validatedCreate, placement *endp DbiResourceID: utils.GenerateResourceID(dbiResourceIDPrefix), AccountID: accountID, Status: StatusCreating, + VMGeneration: firstVMGeneration, Engine: req.Engine.Name, EngineVersion: req.EngineVersion, DBInstanceClass: req.InstanceClass, @@ -172,7 +181,7 @@ func newDBInstanceRecord(accountID string, req *validatedCreate, placement *endp Tags: req.Tags, Bootstrap: BootstrapState{ - MasterUserPassword: req.MasterPassword, + State: BootstrapStatePending, ResolvedParameters: parameters, }, CreatedAt: now, @@ -238,6 +247,20 @@ func (s *Service) rollbackDBInstanceReservation(ctx context.Context, kv jetstrea rbCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), rollbackTimeout) defer cancel() + // Before the record, so a failure below cannot leave ciphertext behind with + // nothing naming it. Retried like the record delete: a payload left staged is + // ciphertext at rest that nothing else reclaims. + var payloadErr error + for range rollbackDeleteAttempts { + if payloadErr = deleteBootstrapPayload(rbCtx, kv, identifier); payloadErr == nil { + break + } + } + if payloadErr != nil { + slog.WarnContext(rbCtx, "rds: rollback delete of the staged bootstrap payload failed", + "dbInstance", identifier, "err", payloadErr) + } + var rollbackErr error for range rollbackDeleteAttempts { rollbackErr = kv.Delete(rbCtx, key, jetstream.LastRevision(rev)) diff --git a/spinifex/handlers/rds/create_test.go b/spinifex/handlers/rds/create_test.go index 1c0286995..3f71b3545 100644 --- a/spinifex/handlers/rds/create_test.go +++ b/spinifex/handlers/rds/create_test.go @@ -150,6 +150,7 @@ func newCreateHarness(t *testing.T, baseDomain string) *createHarness { h.svc = NewService(nc, testRegion).WithDeps(Deps{ LoadCA: newTestCA(t), + MasterKey: testMasterKey, Launch: h.launch.deps(), Network: h.network, IAM: testIAMProvider(h.iam), @@ -252,9 +253,10 @@ func TestCreateDBInstance_ProvisionsAndRecordsTheInstance(t *testing.T) { assert.Equal(t, vm.VolumeSerial(rec.DataVolumeID), rec.DataVolumeSerial) assert.True(t, rec.FormatAuthorized, "only the fresh initial-create volume may be formatted") assert.True(t, rec.StorageEncrypted) - // The master password is staged for the one-shot bootstrap fetch, unconsumed. - assert.Equal(t, "Sup3rSecret!", rec.Bootstrap.MasterUserPassword) - assert.False(t, rec.Bootstrap.Consumed) + // The master password is staged under its own encrypted key, never on the + // record; TestCreateDBInstance_StagesThePasswordEncrypted covers the payload. + assert.Equal(t, BootstrapStatePending, rec.Bootstrap.State) + assert.Empty(t, rec.Bootstrap.MasterUserPassword) events := describeEvents(t, h.svc, &rds.DescribeEventsInput{ SourceType: aws.String(EventSourceTypeDBInstance), diff --git a/spinifex/handlers/rds/delete.go b/spinifex/handlers/rds/delete.go index f5c99d516..0338d3683 100644 --- a/spinifex/handlers/rds/delete.go +++ b/spinifex/handlers/rds/delete.go @@ -209,6 +209,11 @@ func (s *Service) teardownDBInstance(ctx context.Context, kv jetstream.KeyValue, return fmt.Errorf("rds: delete the instance index entry for %s: %w", rec.DBInstanceIdentifier, err) } } + // Before the record, so a teardown that stops here leaves no ciphertext + // behind that nothing names. + if err := deleteBootstrapPayload(ctx, kv, rec.DBInstanceIdentifier); err != nil { + return err + } // Last: while it exists the instance is still nameable, and everything above // is reachable only through it. if err := kv.Delete(ctx, DBInstanceKey(rec.DBInstanceIdentifier)); err != nil && !errors.Is(err, jetstream.ErrKeyNotFound) { diff --git a/spinifex/handlers/rds/describe.go b/spinifex/handlers/rds/describe.go index c06739d6e..3205bb907 100644 --- a/spinifex/handlers/rds/describe.go +++ b/spinifex/handlers/rds/describe.go @@ -198,12 +198,28 @@ func (s *Service) projectDBInstance(rec *DBInstanceRecord) *rds.DBInstance { // failed instance carries is reported the one place a human-readable status // message fits. Absent while the instance is healthy, as AWS leaves it. if rec.FailureReason != "" { - out.StatusInfos = []*rds.DBInstanceStatusInfo{{ + out.StatusInfos = append(out.StatusInfos, &rds.DBInstanceStatusInfo{ StatusType: aws.String("instance"), Status: aws.String(string(rec.Status)), Normal: aws.Bool(false), Message: aws.String(rec.FailureReason), - }} + }) + } + // Reported separately because the status machine owns the field above and + // clears it on every transition, which would drop the one message naming an + // instance that has to be recreated. Only the abnormal states: a payload + // still pending on a serving engine, and one that can no longer be applied. + if state := resolveBootstrapState(rec); state == BootstrapStateUnrecoverable || + (state == BootstrapStatePending && rec.Status == StatusAvailable) { + info := &rds.DBInstanceStatusInfo{ + StatusType: aws.String("bootstrap"), + Status: aws.String(state), + Normal: aws.Bool(false), + } + if rec.Bootstrap.FailureReason != "" { + info.Message = aws.String(rec.Bootstrap.FailureReason) + } + out.StatusInfos = append(out.StatusInfos, info) } out.PendingModifiedValues = projectPendingModifiedValues(rec.PendingModifiedValues) out.DBParameterGroups = projectParameterGroup(rec) diff --git a/spinifex/handlers/rds/iam.go b/spinifex/handlers/rds/iam.go index 825b458d0..8f1d4b173 100644 --- a/spinifex/handlers/rds/iam.go +++ b/spinifex/handlers/rds/iam.go @@ -19,13 +19,14 @@ const ( ) // The agent-only actions. The gateway's principal-class gate reserves exactly -// this set, and the role below grants exactly it — one list, so adding a fifth +// this set, and the role below grants exactly it — one list, so adding another // cannot leave the gate and the grant disagreeing. var InternalAgentActions = []string{ "RegisterDBInstance", "SubmitDBStateChange", "PollDBCommands", "GetDBBootstrapConfig", + "AcknowledgeDBBootstrap", } // Never rds:*: granting the customer surface here would let a Postgres RCE on diff --git a/spinifex/handlers/rds/lifecycle_test.go b/spinifex/handlers/rds/lifecycle_test.go index 9a93c6e46..70ffa47a4 100644 --- a/spinifex/handlers/rds/lifecycle_test.go +++ b/spinifex/handlers/rds/lifecycle_test.go @@ -258,6 +258,7 @@ func newLifecycleHarness(t *testing.T, agentFails bool) *lifecycleHarness { h.agent = newStubAgent(t, nc, testAccountID, testDBID, agentFails) h.svc = NewService(nc, testRegion).WithDeps(Deps{ LoadCA: newTestCA(t), + MasterKey: testMasterKey, Instances: h.cmdr, Snapshots: h.snaps, InstanceState: h.vmState, diff --git a/spinifex/handlers/rds/modify.go b/spinifex/handlers/rds/modify.go index d329f7e25..3589afb66 100644 --- a/spinifex/handlers/rds/modify.go +++ b/spinifex/handlers/rds/modify.go @@ -111,6 +111,23 @@ func (s *Service) ModifyDBInstance(ctx context.Context, input *rds.ModifyDBInsta return nil, awserrors.Errorf(awserrors.ErrorDBInstanceInvalidState, "DB instance %s is %s; the requested modification requires it to be %s", id, rec.Status, StatusAvailable) } + // Only from failed. A create that timed out with its bootstrap still staged + // never formatted its data volume, and the replacement VM would lose the + // format grant that only the initial create can hold, so it could never come + // up whatever the password did. An available instance is serving, so a staged + // payload there means only that the acknowledgement never landed — refusing + // its class change would be telling the customer to destroy a working + // database. + if plan.disruptive() && rec.Status == StatusFailed { + pending, err := bootstrapPending(ctx, kv, id) + if err != nil { + return nil, err + } + if pending { + return nil, awserrors.Errorf(awserrors.ErrorDBInstanceInvalidState, + "DB instance %s never completed its initial bootstrap; a class or storage change cannot recover it — delete and recreate the instance", id) + } + } if err := s.applyImmediateModify(ctx, kv, accountID, rec, plan); err != nil { return nil, err diff --git a/spinifex/handlers/rds/modify_test.go b/spinifex/handlers/rds/modify_test.go index 094c1db51..a0a35baeb 100644 --- a/spinifex/handlers/rds/modify_test.go +++ b/spinifex/handlers/rds/modify_test.go @@ -60,6 +60,7 @@ func newModifyHarnessWithAgent(t *testing.T, agentFails bool) *modifyHarness { h.agent = newStubAgent(t, nc, testAccountID, testDBID, agentFails) h.svc = NewService(nc, testRegion).WithDeps(Deps{ LoadCA: newTestCA(t), + MasterKey: testMasterKey, Launch: h.launch.deps(), Network: h.network, IAM: testIAMProvider(h.iam), diff --git a/spinifex/handlers/rds/reconciler.go b/spinifex/handlers/rds/reconciler.go index 8a69b7f58..d78ea9c40 100644 --- a/spinifex/handlers/rds/reconciler.go +++ b/spinifex/handlers/rds/reconciler.go @@ -78,11 +78,17 @@ type Reconciler struct { mu sync.Mutex leader bool + + // The payloads already reported as stuck pending. The condition persists + // until an operator acts, so without this the event would be re-recorded + // every sweep and crowd the bounded ring. + reportedMu sync.Mutex + reportedPending map[string]string } // holder identifies this daemon in the lease. func NewReconciler(svc *Service, holder string) *Reconciler { - return &Reconciler{svc: svc, holder: holder} + return &Reconciler{svc: svc, holder: holder, reportedPending: make(map[string]string)} } // Drives the leadership and reconcile loop until ctx is cancelled. Intended as @@ -242,6 +248,9 @@ func (r *Reconciler) reconcileInstance(ctx context.Context, kv jetstream.KeyValu if err != nil || !found { return err } + if err := r.reportStalePendingBootstrap(ctx, kv, accountID, &rec); err != nil { + return err + } switch rec.Status { case StatusCreating: return r.reconcileCreating(ctx, kv, rev, accountID, &rec) @@ -262,6 +271,56 @@ func (r *Reconciler) reconcileInstance(ctx context.Context, kv jetstream.KeyValu } } +// An available instance whose payload is still staged bootstrapped against an +// agent too old to acknowledge, so the ciphertext will sit there until an +// operator acts. Only reported: deleting the payload on the grounds that a +// healthy engine implies a completed bootstrap is exactly the inference this +// protocol refuses to make. +func (r *Reconciler) reportStalePendingBootstrap(ctx context.Context, kv jetstream.KeyValue, + accountID string, rec *DBInstanceRecord) error { + key := accountID + "/" + rec.DBInstanceIdentifier + stale := rec.Status == StatusAvailable && time.Since(rec.CreatedAt) > r.svc.bootstrapTimeout() + if !stale { + r.forgetPendingReport(key) + return nil + } + envelope, _, err := readBootstrapPayload(ctx, kv, rec.DBInstanceIdentifier) + if err != nil { + return err + } + if envelope == nil { + r.forgetPendingReport(key) + return nil + } + if !r.notePendingReport(key, envelope.PayloadID) { + return nil + } + slog.WarnContext(ctx, "rds: a staged bootstrap payload is still pending on an available DB instance", + "dbInstance", rec.DBInstanceIdentifier, "accountId", accountID, "payloadId", envelope.PayloadID) + r.svc.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier, + "The database engine is serving but has not confirmed its initial master credentials were applied; the staged credentials remain stored until it does.", + EventCategoryNotification) + return nil +} + +// Reports whether this payload is newly stuck, so the event is recorded once per +// payload for as long as this node holds the lease. +func (r *Reconciler) notePendingReport(key, payloadID string) bool { + r.reportedMu.Lock() + defer r.reportedMu.Unlock() + if r.reportedPending[key] == payloadID { + return false + } + r.reportedPending[key] = payloadID + return true +} + +func (r *Reconciler) forgetPendingReport(key string) { + r.reportedMu.Lock() + defer r.reportedMu.Unlock() + delete(r.reportedPending, key) +} + func (r *Reconciler) reconcileCreating(ctx context.Context, kv jetstream.KeyValue, rev uint64, accountID string, rec *DBInstanceRecord) error { // No lower bound on the heartbeat: the VM is new, so any beat naming it is // necessarily this instance's. diff --git a/spinifex/handlers/rds/records.go b/spinifex/handlers/rds/records.go index f6815b192..aa25f041a 100644 --- a/spinifex/handlers/rds/records.go +++ b/spinifex/handlers/rds/records.go @@ -209,18 +209,46 @@ func (r *DBInstanceRecord) GetTags() map[string]string { return r.Tags } func (r *DBInstanceRecord) SetTags(tags map[string]string) { r.Tags = tags } -// The Consumed marker scopes the master password to a single fetch, not the -// action: replace, recovery and restore all re-fetch without one. +// How far the initial bootstrap got. The payload key's existence is the +// authoritative meaning of pending; these are resolved from it and are +// diagnostics on the record. +const ( + BootstrapStatePending = "pending" + BootstrapStateAcknowledged = "acknowledged" + // A beta record whose password was already spent by the consume-on-fetch + // protocol. A read-time interpretation, never a stored value. + BootstrapStateLegacyConsumed = "legacy-consumed" + // The master password can no longer be delivered to this datadir, so the + // instance has to be deleted and recreated. + BootstrapStateUnrecoverable = "unrecoverable" + // What a restored record is born as: no payload was ever staged for it. + BootstrapStateNone = "none" +) + +// The record's view of the initial bootstrap. The master password itself lives +// encrypted under bootstrap-payloads/{id}, never here, and that key is what a +// fetch replays until the guest proves PostgreSQL applied it. type BootstrapState struct { - // Cleared by the same CAS that sets Consumed, so the cleartext outlives - // only the boot it serves. - MasterUserPassword string `json:"masterUserPassword,omitempty"` - // Only ever flips forward. - Consumed bool `json:"consumed"` - ConsumedAt *time.Time `json:"consumedAt,omitempty"` + // Kept after acknowledgement so a duplicate acknowledgement, whose payload + // key is already gone, is still answerable. + PayloadID string `json:"payloadId,omitempty"` + // One of the BootstrapState* values above. + State string `json:"state,omitempty"` + AcknowledgedAt *time.Time `json:"acknowledgedAt,omitempty"` + // Why the initial bootstrap cannot complete. Owned by this protocol rather + // than by the record's FailureReason, which the status machine clears on + // every transition and would overwrite exactly when the create times out. + FailureReason string `json:"failureReason,omitempty"` // Already evaluated against the instance class, so the agent receives // literals and never a formula. ResolvedParameters []Parameter `json:"resolvedParameters,omitempty"` + + // Beta consume-on-fetch fields, decoded so an existing record keeps reading + // as legacy-consumed and never written by this protocol. The password is + // scrubbed the first time a fetch sees one. + Consumed bool `json:"consumed,omitempty"` + ConsumedAt *time.Time `json:"consumedAt,omitempty"` + MasterUserPassword string `json:"masterUserPassword,omitempty"` } // Snapshot types and statuses, matching AWS. A final snapshot is manual: the diff --git a/spinifex/handlers/rds/recovery_test.go b/spinifex/handlers/rds/recovery_test.go index 9784b39a9..f39151ce4 100644 --- a/spinifex/handlers/rds/recovery_test.go +++ b/spinifex/handlers/rds/recovery_test.go @@ -386,5 +386,6 @@ func TestProjectDBInstance_ReportsTheFailureReason(t *testing.T) { rec.Status = StatusAvailable rec.FailureReason = "" + rec.Bootstrap.State = BootstrapStateAcknowledged assert.Empty(t, svc.projectDBInstance(&rec).StatusInfos, "a healthy instance carries no status info, as AWS leaves it") } diff --git a/spinifex/handlers/rds/replace.go b/spinifex/handlers/rds/replace.go index 34661388e..023330bb6 100644 --- a/spinifex/handlers/rds/replace.go +++ b/spinifex/handlers/rds/replace.go @@ -67,13 +67,19 @@ func (s *Service) replaceInstanceVM(ctx context.Context, kv jetstream.KeyValue, } rec.FormatAuthorized = false } + if err := context.Cause(ctx); err != nil { + return err + } + // The generation this replace bumps is what the staged payload is bound to, + // and it is never rebound. Withdrawn before the VM is touched, so the + // ciphertext is gone even if the replace fails half-way. + if err := s.discardPendingBootstrap(ctx, kv, accountID, rec); err != nil { + return err + } // Checkpointed first so the replacement boots on a clean datadir rather than // one it has to replay a WAL over. A wedged agent degrades this rather than // blocking the replace, exactly as a stop does. - if err := context.Cause(ctx); err != nil { - return err - } s.stopEngineOrRecordFallback(ctx, accountID, rec, "replacing its VM") oldInstanceID := rec.InstanceID diff --git a/spinifex/handlers/rds/replace_test.go b/spinifex/handlers/rds/replace_test.go index f418f5a6b..0ed1fe83f 100644 --- a/spinifex/handlers/rds/replace_test.go +++ b/spinifex/handlers/rds/replace_test.go @@ -132,7 +132,7 @@ func TestReplaceInstanceVM_RewritesTheInstanceIndex(t *testing.T) { require.NotNil(t, current) assert.Equal(t, testDBID, current.DBInstanceIdentifier) assert.Equal(t, testAccountID, current.AccountID) - assert.Equal(t, int64(1), current.VMGeneration) + assert.Equal(t, int64(2), current.VMGeneration) } // The record names the new VM and forgets the old one's health, while every @@ -149,7 +149,7 @@ func TestReplaceInstanceVM_RecordsTheNewVMAndKeepsTheEndpoint(t *testing.T) { stored := h.record(t) assert.Equal(t, testReplacementInstance, stored.InstanceID) - assert.Equal(t, int64(1), stored.VMGeneration) + assert.Equal(t, int64(2), stored.VMGeneration) assert.NotEqual(t, "eni-sys01", stored.SystemENIID) // The old VM's health must not read as the new one's, or the reconciler // calls the replace finished before the replacement has said anything. @@ -166,7 +166,7 @@ func TestReplaceInstanceVM_RecordsTheNewVMAndKeepsTheEndpoint(t *testing.T) { // The caller's copy is kept in step, so its own record write cannot // resurrect the old VM's identity on top of this one. assert.Equal(t, testReplacementInstance, rec.InstanceID) - assert.Equal(t, int64(1), rec.VMGeneration) + assert.Equal(t, int64(2), rec.VMGeneration) assert.False(t, rec.FormatAuthorized) } @@ -267,7 +267,7 @@ func TestReplaceInstanceVM_LeavesTheRecordRecoverableOnEveryFailure(t *testing.T stored := h.record(t) assert.Equal(t, testInstance, stored.InstanceID) - assert.Zero(t, stored.VMGeneration) + assert.Equal(t, int64(firstVMGeneration), stored.VMGeneration) assert.Equal(t, testEndpointENI, stored.ENIID) assert.Equal(t, testDataVolume, stored.DataVolumeID) diff --git a/spinifex/handlers/rds/restore.go b/spinifex/handlers/rds/restore.go index 2bc14bb79..fa6dd1d96 100644 --- a/spinifex/handlers/rds/restore.go +++ b/spinifex/handlers/rds/restore.go @@ -6,7 +6,6 @@ import ( "fmt" "log/slog" "strings" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -344,14 +343,14 @@ func rejectUnimplementedRestore(input *rds.RestoreDBInstanceFromDBSnapshotInput) } // The reserved record for a restored instance: everything the request and the -// snapshot decided, with the bootstrap marked consumed so the agent's first -// fetch returns attach rather than a password it would use to run initdb. +// snapshot decided. No bootstrap payload is ever staged for it, so the agent's +// first fetch returns attach rather than a password it would use to run initdb. +// Born none rather than acknowledged, so an acknowledgement arriving against it +// is denied with a legible reason instead of read as a benign duplicate. func newRestoredDBInstanceRecord(accountID string, req *validatedCreate, placement *endpointPlacement, parameters []Parameter, snapshot *DBSnapshotRecord) DBInstanceRecord { rec := newDBInstanceRecord(accountID, req, placement, parameters) - now := time.Now().UTC() - rec.Bootstrap.Consumed = true - rec.Bootstrap.ConsumedAt = &now + rec.Bootstrap.State = BootstrapStateNone // Carried over so a later ModifyDBInstance --master-user-password still reads // as a rotation of the credentials the datadir was written with. rec.MasterPasswordUpdatedAt = snapshot.MasterPasswordUpdatedAt diff --git a/spinifex/handlers/rds/restore_test.go b/spinifex/handlers/rds/restore_test.go index e3559caf3..630bd5a42 100644 --- a/spinifex/handlers/rds/restore_test.go +++ b/spinifex/handlers/rds/restore_test.go @@ -67,8 +67,7 @@ func TestRestoreDBInstanceFromDBSnapshot_BuildsANewInstanceOnTheSnapshotsData(t // The datadir already holds the master role and its password hash, so // the agent's first bootstrap fetch has to attach rather than run initdb. - assert.True(t, stored.Bootstrap.Consumed) - require.NotNil(t, stored.Bootstrap.ConsumedAt) + assert.Equal(t, BootstrapStateNone, stored.Bootstrap.State) assert.Empty(t, stored.Bootstrap.MasterUserPassword) require.NotNil(t, h.launch.launcher.input) diff --git a/spinifex/handlers/rds/service.go b/spinifex/handlers/rds/service.go index d8c95e3ac..bc12e2dcb 100644 --- a/spinifex/handlers/rds/service.go +++ b/spinifex/handlers/rds/service.go @@ -23,7 +23,11 @@ type Deps struct { CAKeyPath string // Overrides the file-backed CA loader in tests. LoadCA CALoader - Launch LaunchDeps + // The cluster master key the staged bootstrap password is encrypted under. + // Mandatory: without it a create cannot stage a password at all, so it is + // resolved at construction rather than per operation. + MasterKey []byte + Launch LaunchDeps // Resolves the customer subnet and validates the security groups the // customer ENI is created with. Network networkResolver diff --git a/spinifex/handlers/rds/service_nats.go b/spinifex/handlers/rds/service_nats.go index d41e6413f..54a5a65e7 100644 --- a/spinifex/handlers/rds/service_nats.go +++ b/spinifex/handlers/rds/service_nats.go @@ -182,3 +182,8 @@ func (s *NATSService) GetDBBootstrapConfig(ctx context.Context, input *GetDBBoot return utils.NATSRequest[GetDBBootstrapConfigOutput](ctx, s.nc, SubjectGetDBBootstrapConfig, input, defaultTimeout, accountID) } + +func (s *NATSService) AcknowledgeDBBootstrap(ctx context.Context, input *AcknowledgeDBBootstrapInput, accountID string) (*AcknowledgeDBBootstrapOutput, error) { + return utils.NATSRequest[AcknowledgeDBBootstrapOutput](ctx, s.nc, + SubjectAcknowledgeDBBootstrap, input, defaultTimeout, accountID) +} diff --git a/spinifex/handlers/rds/snapshot_test.go b/spinifex/handlers/rds/snapshot_test.go index 043f9a9ca..44b6e0852 100644 --- a/spinifex/handlers/rds/snapshot_test.go +++ b/spinifex/handlers/rds/snapshot_test.go @@ -49,6 +49,7 @@ func newSnapshotHarness(t *testing.T, agentFails bool) *snapshotHarness { h.svc = NewService(nc, testRegion).WithDeps(Deps{ LoadCA: newTestCA(t), + MasterKey: testMasterKey, Launch: h.launch.deps(), Network: h.network, IAM: testIAMProvider(h.iam), diff --git a/spinifex/handlers/rds/store.go b/spinifex/handlers/rds/store.go index 433f857d0..1e1944698 100644 --- a/spinifex/handlers/rds/store.go +++ b/spinifex/handlers/rds/store.go @@ -40,6 +40,7 @@ const ( // Key-path helpers for the per-account bucket: // // db-instances/{dbInstanceIdentifier} +// bootstrap-payloads/{dbInstanceIdentifier} // db-snapshots/{dbSnapshotIdentifier} // db-subnet-groups/{name} // db-parameter-groups/{name}/meta @@ -59,6 +60,17 @@ func DBInstanceKey(dbInstanceIdentifier string) string { return DBInstancesPrefix() + dbInstanceIdentifier } +// The encrypted bootstrap payload, kept out of the instance record so it has its +// own CAS domain and so a daemon that predates it cannot drop it through a +// read-modify-marshal of the record. +func BootstrapPayloadsPrefix() string { + return "bootstrap-payloads/" +} + +func BootstrapPayloadKey(dbInstanceIdentifier string) string { + return BootstrapPayloadsPrefix() + dbInstanceIdentifier +} + // Manual and automated snapshots share this space, distinguished by the // snapshot type on the record. func DBSnapshotsPrefix() string { diff --git a/spinifex/handlers/rds/subjects.go b/spinifex/handlers/rds/subjects.go index 540757201..45696f510 100644 --- a/spinifex/handlers/rds/subjects.go +++ b/spinifex/handlers/rds/subjects.go @@ -13,9 +13,11 @@ const ( SubjectRegisterWildcard = busPrefix + "*.*.register" SubjectHealthWildcard = busPrefix + "*.*.health" - // A Layer-1 subject, not a bus one: serving bootstrap config is a - // control-plane read rather than reconciler↔agent traffic. - SubjectGetDBBootstrapConfig = "rds.GetDBBootstrapConfig" + // Layer-1 subjects, not bus ones: serving bootstrap config is a control-plane + // read, and the acknowledgement that ends it is a control-plane write, rather + // than reconciler↔agent traffic. + SubjectGetDBBootstrapConfig = "rds.GetDBBootstrapConfig" + SubjectAcknowledgeDBBootstrap = "rds.AcknowledgeDBBootstrap" // Layer-1 customer action subjects, answered by whichever daemon the queue // group picks — a create does its own orchestration inline.