Skip to content

Commit 3763bfd

Browse files
authored
feat(encryption): Stage 6C-2b — ErrSidecarBehindRaftLog guard primitive (encryption-side) (#782)
## Summary Stage 6C-2b per the [PR #762 plan](#762) and the 6C sub-decomposition landed in [PR #781](#781). Ships the **encryption-side primitive** for the `ErrSidecarBehindRaftLog` §9.1 guard so Stage 6D-and-later code paths (e.g., §7.1 capability fan-out's gap-coverage check) can depend on the predicate without waiting for the raftengine-side scanner helper. The actual raftengine WAL-scan implementation + `main.go` startup-guard phase wiring lands in a follow-up **6C-2c** PR. ## What this PR ships ### One new sentinel | Sentinel | Catches | |---|---| | `ErrSidecarBehindRaftLog` | Partial-write crash: encryption-relevant Raft entry applied to engine but §5.1 sidecar write didn't complete. Recovery: `encryption resync-sidecar`. | Without this guard the encryption package would silently rebuild keystore state from an outdated wrapped-DEK snapshot, and a fail-closed read would fire on the next post-cutover entry with `unknown_key_id` (halting apply). ### Three new exported primitives in `internal/encryption/audit.go` ```go // §5.5 predicate func IsEncryptionRelevantOpcode(opcode byte) bool // Cross-package contract (raftengine implements in 6C-2c) type EncryptionRelevantScanner interface { HasEncryptionRelevantEntryInRange(startExclusive, endInclusive uint64) (bool, error) } // Pure-function guard func GuardSidecarBehindRaftLog(sidecarAppliedIdx, engineAppliedIdx uint64, scanner EncryptionRelevantScanner) error ``` The encryption package owns the **semantic-level** predicate; `fsmwire` owns the **wire-level** constants (`OpEncryptionMin` / `OpEncryptionMax`). Decoupling means a future widening of the encryption opcode range (Stage 6E reserves 0x06 / 0x07) is a single-line change in `fsmwire`, not a cross-package edit. ### Guard contract (5 branches, all tested) | Branch | Behavior | |---|---| | `sidecarAppliedIdx >= engineAppliedIdx` | Caught up. Return nil. **Scanner NEVER called.** | | Non-empty gap, scanner says no hit | Return nil. | | Non-empty gap, scanner says hit | Fire `ErrSidecarBehindRaftLog` with both indices in annotation. | | Scanner error | Propagate wrapped, **NOT** marked with `ErrSidecarBehindRaftLog` (scanner failure ≠ gap-coverage refusal). | | `nil` scanner with non-empty gap | **Fail closed** with `ErrSidecarBehindRaftLog` (cannot prove safety). | | `nil` scanner on caught-up path | Return nil (scanner is irrelevant when there's no gap). | ## Design doc updates - **6C-2b** row rewritten to reflect the primitive-only scope of this PR (encryption package only, no `main.go` wiring). - **6C-2c** row added — raftengine implementation of the scanner + `main.go` startup-guard phase wiring. - Shipping order updated: `6C-1 ✅ → 6C-2 ✅ → 6C-2b (this PR) → 6C-2c → 6D (bundles 6C-3) → 6E (bundles 6C-4)`. - Rationale section updated with the 6C-2b / 6C-2c split: the primitive lands in 6C-2b so Stage 6D-and-later RPC paths that need the same predicate (capability fan-out gap check) can depend on the encryption package without waiting for 6C-2c. ## Why split 6C-2b into "primitive" vs "integration" The §5.5 predicate (`IsEncryptionRelevantOpcode`) is needed by more than just the startup guard. Stage 6E's capability fan-out checks the same predicate when refusing a cutover RPC if any peer's sidecar is behind. Shipping the predicate + sentinel + interface without the raftengine WAL-scan implementation lets 6D / 6E proceed against the encryption package boundary without waiting on the cross-package plumbing. ## Caller audit No public function signatures changed. All four new exports (`IsEncryptionRelevantOpcode`, `EncryptionRelevantScanner`, `GuardSidecarBehindRaftLog`, `ErrSidecarBehindRaftLog`) are **net-additive**. No existing callers to audit. ## Five-lens self-review 1. **Data loss** — net-positive. The primitive enables the §9.1 refusal that catches partial-write crashes between an encryption-relevant Raft commit and the sidecar update. Until 6C-2c wires it into `main.go` the refusal is operator-inert, but the primitive's failure modes (nil scanner → fail closed, scanner error → propagate distinct) are correct. 2. **Concurrency / distributed failures** — pure function, no shared state. Scanner interface is single-call per guard invocation; implementations are free to lock internally. 3. **Performance** — zero hot-path impact. The guard is called once per shard per process start (in 6C-2c); within the guard it's one integer comparison + at most one scanner call. 4. **Data consistency** — `ErrSidecarBehindRaftLog` is the only §9.1 guard that catches the "Raft committed but sidecar missed it" partial-write window. Without it the next post-cutover read would HaltApply with `unknown_key_id`; with it the operator gets a single typed startup refusal pointing at the right recovery runbook. 5. **Test coverage** — 7 new tests cover the predicate (range-member + known-opcode + below/above boundary) AND every branch of the guard (caught-up, gap-not-covered, gap-covered, scanner-error, nil-scanner-fails-closed, nil-scanner-caught-up). The `fakeScanner` exercises the interface contract in isolation from any real raftengine. ## Test plan - [x] `go test -race -timeout=60s ./internal/encryption/...` — PASS - [x] `golangci-lint run ./internal/encryption/...` — 0 issues - [ ] Full Jepsen suite — not run; the primitive has no caller in this PR (operator-inert until 6C-2c) ## Plan Per the design doc updates in this PR, next sub-milestones: - **6C-2c** — raftengine implementation of `EncryptionRelevantScanner` (WAL-based) + `main.go` startup-guard phase wiring - **6C-3** — `ErrNodeIDCollision` + `ErrLocalEpochRollback` (bundles with Stage 6D) - **6D** — `enable-storage-envelope` admin RPC + §7.1 Phase-1 cutover (unblocked once 6C-2c lands) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a safety guard to prevent sidecar lag from silently bypassing encryption-relevant log entries. * Added error handling with operator guidance for sidecar synchronization situations. * **Tests** * Added comprehensive test coverage for encryption relevance detection and sidecar lag validation. <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/bootjp/elastickv/pull/782?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 84b25f5 + 50dbeeb commit 3763bfd

4 files changed

Lines changed: 440 additions & 14 deletions

File tree

docs/design/2026_04_29_partial_data_at_rest_encryption.md

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ Date: 2026-04-29
2222
| 6B | §6.5 KEK plumbing — `--kekFile` / `--kekUri` flags + `internal/encryption/kek` selection at startup + thread `KEKUnwrapper` into the applier so `ApplyBootstrap` / `ApplyRotation` actually KEK-unwrap (replaces the 6A `ErrKEKNotConfigured` stubs) + introduce `--encryption-enabled` flag (default off) + re-enable mutating-RPC wiring in `registerEncryptionAdminServer` gated on (`--encryption-enabled` AND `KEKConfigured()`) | open ||
2323
| 6C-1 | Subset of §9.1 startup refusal guards that depend only on flags + sidecar state: sidecar present without `--encryption-enabled` (`ErrSidecarPresentWithoutFlag`, downgrade prevention), `--encryption-enabled` without `--kekFile` (`ErrKEKRequiredWithFlag`, fail-fast), KEK loaded but sidecar wrapped DEKs do not unwrap (`ErrKEKMismatch`, operator-error catch). `internal/encryption/startup.go`'s `CheckStartupGuards` runs once in `main.go run()` before `buildShardGroups`. | shipped | PR #778 |
2424
| 6C-2 | Process-local §9.1 guards that read only the encryption package's own state: `ErrLocalEpochExhausted` (any active DEK with `local_epoch == 0xFFFF` — refuses to start until rotated, preventing GCM nonce reuse on the next write), `ErrUnsupportedFilesystem` startup probe (`ProbeSidecarFilesystem` exercises the §5.1 write+rename+dir.Sync sequence on a sentinel file in the sidecar directory at startup so filesystem-capability failures surface BEFORE the first encryption-relevant Raft entry, not on the first WriteSidecar call hours into operation). Also includes the gemini-r3 medium parseErr-annotation fix deferred from PR #778 (`ErrKEKMismatch` now wraps both `parseErr` and the unwrap error when the defensive non-decimal-key_id branch fires). The asymmetric `ErrLocalEpochRollback` peer is split into 6C-3 because it requires the writer-registry record (Stage 7). The `ErrSidecarBehindRaftLog` guard moves to 6C-2b because it requires raftengine integration — outside the encryption package boundary. | open | — |
25-
| 6C-2b | `ErrSidecarBehindRaftLog` startup guard — reads the persisted applied-index from raftengine and refuses if the sidecar's `raft_applied_index` is behind by an interval covering any encryption-relevant Raft entry (`isEncryptionRelevant` predicate per §5.5). Plumbing raftengine into the startup-guard path is the load-bearing scope here; the guard itself is a one-screen function. | open ||
25+
| 6C-2b | `ErrSidecarBehindRaftLog` guard PRIMITIVE — encryption-side sentinel, `IsEncryptionRelevantOpcode` predicate (§5.5), `EncryptionRelevantScanner` cross-package interface, and `GuardSidecarBehindRaftLog` pure-function guard that takes both indices + a scanner callback. Tests use a fake scanner to exercise every branch (caught-up / gap-not-covered / gap-covered / scanner-error / nil-scanner-fails-closed). Wires nothing into main.go; the raftengine-side implementation lands in 6C-2c. | shipped-in-this-PR | this PR |
26+
| 6C-2c | Raftengine implementation of `EncryptionRelevantScanner` (WAL-based scan of Raft entries in the supplied index range against the §5.5 predicate) + `main.go` startup-guard phase that runs after `buildShardGroups` for each shard with encryption enabled, before any gRPC server starts serving. Inherits the encryption-side primitive from 6C-2b. | open ||
2627
| 6C-3 | Cluster-wide §9.1 guards that require the Voters ∪ Learners membership view: `ErrNodeIDCollision` (two members hash to the same 16-bit `node_id`), `ErrLocalEpochRollback` (sidecar `local_epoch` ≤ writer-registry record). Naturally bundles with the Stage 6D capability fan-out which already needs that membership view. | open ||
2728
| 6C-4 | Phase-2-specific §9.1 guards: `ErrEnvelopeCutoverDivergence` (sidecar `raft_envelope_cutover_index` disagrees with snapshot header), `ErrEncryptionNotBootstrapped` (`enable-raft-envelope` before bootstrap), `ErrLocalEpochOutOfRange` on the wire side of `GetCapability` / `GetSidecarState`. Bundles with Stage 6E (`enable-raft-envelope` admin RPC + Phase-2 cutover). | open ||
2829
| 6D | §6.6 `enable-storage-envelope` admin RPC + §7.1 Phase-1 storage cutover (§6.2 toggle ON) + Voters ∪ Learners capability gate (depends on 6B for mutator wiring AND 6C-1+6C-2 for §9.1 startup-refusal guards; bundles 6C-3 for the membership-view-dependent collision check — see rationale) | open ||
@@ -186,14 +187,31 @@ production-safe state, and unblocks the next one.
186187
only scope of 6C-2 lets the simpler guards ship without the
187188
raftengine plumbing churn.
188189

189-
* **6C-2b**`ErrSidecarBehindRaftLog`. Plumbs the raftengine's
190-
persisted applied-index into the startup-guard path and refuses
191-
if the sidecar's `raft_applied_index` is behind by an interval
192-
that covers any encryption-relevant entry (`isEncryptionRelevant`
193-
predicate per §5.5: `rotate-dek`, `rewrap-deks`, `bootstrap-
194-
encryption`, `enable-storage-envelope`, `enable-raft-envelope`,
195-
`retire-dek`). The guard itself is a one-screen function; the
196-
load-bearing scope is the cross-package plumbing.
190+
* **6C-2b**`ErrSidecarBehindRaftLog` PRIMITIVE (encryption
191+
package only). Adds the `ErrSidecarBehindRaftLog` sentinel,
192+
`IsEncryptionRelevantOpcode` predicate (§5.5: bytes 0x03
193+
through `OpEncryptionMax` 0x07), `EncryptionRelevantScanner`
194+
cross-package interface, and `GuardSidecarBehindRaftLog`
195+
pure-function guard. The encryption package owns the predicate
196+
and the refusal logic; the raftengine implements the scanner
197+
in 6C-2c. Splitting the primitive from the integration lets
198+
Stage 6D-and-later code paths that need the predicate (e.g.,
199+
the §7.1 capability fan-out's gap-coverage check) depend
200+
ONLY on the encryption package without waiting on the
201+
raftengine-side scan helper.
202+
203+
* **6C-2c** — Raftengine implementation of the
204+
`EncryptionRelevantScanner` interface defined in 6C-2b. Walks
205+
the WAL across the supplied entry-index range, decodes each
206+
entry's opcode byte, and applies the §5.5 predicate. Wires
207+
a new startup-guard phase into `main.go` that runs AFTER
208+
`buildShardGroups` returns and each shard's engine has opened,
209+
but BEFORE any gRPC server starts serving — a different
210+
lifecycle phase from the existing 6C-1 / 6C-2 guards (which
211+
run before engine startup). For each shard with encryption
212+
enabled, the phase calls `GuardSidecarBehindRaftLog` with the
213+
shard's engine `AppliedIndex()` and a scanner backed by the
214+
engine's WAL.
197215

198216
* **6C-3** — Membership-view guards. `ErrNodeIDCollision`
199217
requires a Voters ∪ Learners enumeration of every Raft
@@ -211,11 +229,13 @@ production-safe state, and unblocks the next one.
211229
`local_epoch` values to the §4.1 16-bit field. Bundles with
212230
Stage 6E's `enable-raft-envelope` admin RPC.
213231

214-
**Shipping order:** 6C-1 → 6C-2 → 6C-2b → 6D (bundles 6C-3) →
215-
6E (bundles 6C-4). Stage 6D is unblocked once 6C-1 + 6C-2 +
216-
6C-2b have all shipped (the latter is the last raftengine-
217-
integration piece any cutover RPC depends on); it does not need
218-
to wait for the 6C-3/6C-4 work that bundles into it.
232+
**Shipping order:** 6C-1 → 6C-2 → 6C-2b (primitive) → 6C-2c
233+
(raftengine implementation + main.go wiring) → 6D (bundles
234+
6C-3) → 6E (bundles 6C-4). Stage 6D is unblocked once 6C-1
235+
+ 6C-2 + 6C-2b + 6C-2c have all shipped (the latter pair
236+
is the load-bearing `ErrSidecarBehindRaftLog` work); 6D
237+
does not need to wait for the 6C-3/6C-4 work that bundles
238+
into it.
219239

220240
- **6D and 6E are the actual cutovers** (Phase 1 = storage, Phase
221241
2 = raft). Each touches a different code path (§6.2 storage

internal/encryption/audit.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
package encryption
2+
3+
import (
4+
"github.com/bootjp/elastickv/internal/encryption/fsmwire"
5+
pkgerrors "github.com/cockroachdb/errors"
6+
)
7+
8+
// IsEncryptionRelevantOpcode reports whether the supplied FSM
9+
// opcode byte is one of the §5.5 SIDECAR-MUTATING opcodes —
10+
// the set of entry types whose apply path writes the §5.1
11+
// keys.json sidecar and therefore matters for the
12+
// ErrSidecarBehindRaftLog gap-coverage check:
13+
//
14+
// - 0x04 OpBootstrap (bootstrap-encryption — §5.6)
15+
// - 0x05 OpRotation (rotate-dek + future sub-tags
16+
// rewrap-deks / retire-dek / enable-storage-envelope /
17+
// enable-raft-envelope — §5.2 + §7.1)
18+
// - 0x06 / 0x07 (reserved slots in the fsmwire
19+
// OpEncryption range for Stage 6E wire extensions; whether
20+
// they end up sidecar-mutating is design-stage-defined,
21+
// but conservatively treating them as relevant matches the
22+
// §5.5 enumeration and forces a future ABI extension to be
23+
// explicit about adding a NOT-relevant opcode).
24+
//
25+
// IMPORTANT — 0x03 OpRegistration is EXCLUDED. ApplyRegistration
26+
// in internal/encryption/applier.go only mutates writer-registry
27+
// rows via SetRegistryRow; it never calls WriteSidecar. Treating
28+
// it as sidecar-relevant would fire ErrSidecarBehindRaftLog on
29+
// EVERY startup after the first registration, since the sidecar's
30+
// raft_applied_index is not advanced by registration applies.
31+
// The design's §5.5 enumeration matches this exclusion (rotate-
32+
// dek, rewrap-deks, bootstrap-encryption, enable-storage-envelope,
33+
// enable-raft-envelope, retire-dek — no register-writer).
34+
//
35+
// IMPORTANT — wire-format contract: the opcode byte is `data[0]`
36+
// of the FSM entry payload (the LEADING opcode tag), NOT the
37+
// byte after a wire-version prefix. See kv/fsm_encryption.go
38+
// which dispatches via `wireBytes[0]` and the EncodeBootstrap /
39+
// EncodeRotation / EncodeRegistration helpers in fsmwire/wire.go
40+
// which produce `[opcode, version=0x01, ...payload]`. A scanner
41+
// that misreads the layout and inspects payload bytes (e.g., a
42+
// rotation sub-tag at position 1+) would return false negatives,
43+
// silently letting GuardSidecarBehindRaftLog miss encryption-
44+
// relevant gaps and start with a stale sidecar.
45+
//
46+
// The predicate is the §9.1 ErrSidecarBehindRaftLog guard's
47+
// gap-coverage check: an unapplied entry in the sidecar/engine
48+
// gap matters iff its `data[0]` is in this range. Non-sidecar-
49+
// mutating entries (writes, transactions, control-plane RPCs,
50+
// AND OpRegistration) in the gap do not affect the encryption
51+
// sidecar and are safe to ignore.
52+
//
53+
// Defined here (rather than in fsmwire) so the encryption
54+
// package owns its semantic-level predicate; the fsmwire
55+
// package owns the wire-level constants (OpBootstrap /
56+
// OpEncryptionMax) that this function reads.
57+
func IsEncryptionRelevantOpcode(opcode byte) bool {
58+
return opcode >= fsmwire.OpBootstrap && opcode <= fsmwire.OpEncryptionMax
59+
}
60+
61+
// EncryptionRelevantScanner is the cross-package contract that
62+
// GuardSidecarBehindRaftLog uses to inspect a Raft entry-index
63+
// range for encryption-relevant opcodes WITHOUT importing the
64+
// raftengine into the encryption package.
65+
//
66+
// The raftengine implements this in a follow-up PR (Stage 6C-2c)
67+
// against its WAL + applied-snapshot state, exposing only the
68+
// predicate result. Shipping the encryption-side primitive here
69+
// without the engine-side implementation is intentional: it lets
70+
// Stage 6D-and-later RPC handlers reuse the same predicate
71+
// (`IsEncryptionRelevantOpcode`) without depending on raftengine
72+
// having shipped its scanner first.
73+
//
74+
// HasEncryptionRelevantEntryInRange returns true iff at least
75+
// one Raft entry with index in [startExclusive+1, endInclusive]
76+
// carries a §5.5-relevant opcode. The startExclusive parameter
77+
// is the sidecar's last-applied index (which has already been
78+
// reflected in the sidecar), so the entry AT that index is NOT
79+
// in the gap; the entries that follow it are.
80+
//
81+
// Implementations must handle the empty-range case
82+
// (startExclusive >= endInclusive) by returning (false, nil)
83+
// — the guard precomputes the gap-non-empty branch and only
84+
// calls scan when there's actually a range to inspect, but
85+
// defensive implementations are safer in the face of caller bugs.
86+
type EncryptionRelevantScanner interface {
87+
HasEncryptionRelevantEntryInRange(startExclusive, endInclusive uint64) (bool, error)
88+
}
89+
90+
// GuardSidecarBehindRaftLog implements the §9.1
91+
// ErrSidecarBehindRaftLog refusal logic as a pure function: it
92+
// reads no I/O and depends on nothing beyond the supplied indices
93+
// and the caller-provided scanner.
94+
//
95+
// The contract:
96+
//
97+
// 1. If sidecarAppliedIdx >= engineAppliedIdx, the sidecar is
98+
// caught up (or ahead, which would itself be a bug — but
99+
// this guard's scope is "behind", not "ahead"). Return nil.
100+
// 2. If the gap is non-empty, ask the scanner whether any entry
101+
// in (sidecarAppliedIdx, engineAppliedIdx] is encryption-
102+
// relevant. If yes, fire ErrSidecarBehindRaftLog. If no, the
103+
// gap is harmless and we return nil.
104+
// 3. Propagate any scanner I/O error wrapped with context but
105+
// NOT marked as ErrSidecarBehindRaftLog — scanner failure is
106+
// a different operator problem than the gap-coverage refusal.
107+
//
108+
// The function does not consult flags or encryption state; it is
109+
// the caller's responsibility to skip the call when
110+
// --encryption-enabled is off (the gap is irrelevant in that
111+
// case) or when the engine hasn't yet been opened (no applied
112+
// index to compare against).
113+
func GuardSidecarBehindRaftLog(sidecarAppliedIdx, engineAppliedIdx uint64, scanner EncryptionRelevantScanner) error {
114+
if sidecarAppliedIdx >= engineAppliedIdx {
115+
return nil
116+
}
117+
if scanner == nil {
118+
// A nil scanner cannot inspect the gap, so we cannot
119+
// safely advance past it. Fail closed.
120+
return pkgerrors.Wrapf(ErrSidecarBehindRaftLog,
121+
"sidecar_applied_index=%d engine_applied_index=%d (no scanner provided to inspect gap)",
122+
sidecarAppliedIdx, engineAppliedIdx)
123+
}
124+
hit, err := scanner.HasEncryptionRelevantEntryInRange(sidecarAppliedIdx, engineAppliedIdx)
125+
if err != nil {
126+
return pkgerrors.Wrapf(err,
127+
"encryption: scan raft entries (%d, %d] for encryption-relevant opcodes",
128+
sidecarAppliedIdx, engineAppliedIdx)
129+
}
130+
if !hit {
131+
return nil
132+
}
133+
return pkgerrors.Wrapf(ErrSidecarBehindRaftLog,
134+
"sidecar_applied_index=%d engine_applied_index=%d gap_covers_encryption_relevant_entry",
135+
sidecarAppliedIdx, engineAppliedIdx)
136+
}

0 commit comments

Comments
 (0)