Skip to content

Separate the seal key from the key that encrypts node volumes - #56

Merged
sethbergman merged 2 commits into
mainfrom
fix/separate-seal-and-data-keys
Aug 30, 2026
Merged

Separate the seal key from the key that encrypts node volumes#56
sethbergman merged 2 commits into
mainfrom
fix/separate-seal-and-data-keys

Conversation

@sethbergman

Copy link
Copy Markdown
Owner

Closes #55.

They were one key, and terraform destroy scheduled it.

storage.tf warns that a snapshot is encrypted under the auto-unseal key and that it "must not be scheduled for deletion when a cluster is torn down". The root volume used the same key — so a teardown put it on a seven-day timer, and with it every snapshot ever taken with that key, including from clusters that no longer existed. The document said one thing and the code did the other.

Two keys

key window holds
<cluster>-vault-autounseal 30 days Vault's seal, and the snapshot bucket's SSE
<cluster>-vault-data 7 days root volumes only

The bucket stays on the seal key deliberately. Reading a snapshot needs this key for the object and this key for the keyring Vault sealed inside it — so exactly one thing has to outlive a teardown. One rule an operator can follow; two is a rule they will half-follow.

teardown-cloud.sh now names both keys and says which one to cancel, since the whole hazard is that they look alike in the console.

The assertions are split by what each layer can answer

terraform test checks configuration — two keys, distinct descriptions, the windows that make the split worth having.

It does not compare arns, because the mock gives every aws_kms_key the same one:

mock_resource "aws_kms_key" {
  defaults = { arn = "arn:aws:kms:...:key/12345678-..." }
}

So != can never hold and == passes trivially. My first version asserted exactly that and failed — tests/README.md warns about it in as many words, and I ignored it.

The arn comparison lives in tests/cloud-apply-emulated instead, where the API mints real ones: the two keys differ, and the root volume is not on the seal key.

Verified

terraform fmt clean, 28 passed / 0 failed in WSL.

Still not settled by any of this

Whether the ASG service-linked role needs explicit key-policy grants once a restrictive policy replaces the default. moto does not enforce IAM, so that half still waits for a real apply — as noted in #55.

🤖 Generated with Claude Code

sethbergman and others added 2 commits August 29, 2026 18:55
They were one key, and `terraform destroy` scheduled it.

storage.tf warns that a snapshot is encrypted under the auto-unseal key
and that the key "must not be scheduled for deletion when a cluster is
torn down". The root volume used the same key, so a teardown put it on a
seven-day timer -- and with it every snapshot ever taken with that key,
including snapshots from clusters that no longer existed. The document
said one thing and the code did the other.

Now two keys:

  <cluster>-vault-autounseal   30 day window
    Vault's seal, and the snapshot bucket's SSE. Deliberately the same
    key for both, because reading a snapshot needs this key for the
    object and this key for the keyring sealed inside it. One thing to
    keep alive is a rule an operator can follow; two is a rule they will
    half-follow.

  <cluster>-vault-data          7 day window
    Root volumes only. They go when the instance goes, so losing the key
    costs nothing that was not already leaving.

teardown-cloud.sh now names both and says which one to cancel, since the
whole hazard is that they look alike in the console.

The assertions are split by what each layer can actually answer.
terraform test checks configuration -- two keys, distinct descriptions,
the windows that make the split worth having -- because its mock gives
every aws_kms_key the same arn, so comparing arns there passes or fails
for both at once and proves nothing. tests/README.md says exactly that
about mocked values and the first version of this test ignored it. The
arn comparison lives in tests/cloud-apply-emulated, where the API mints
real ones.

Verified in WSL: fmt clean, 28 assertions pass.

Closes #55.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The emulated apply asserted a resource count written when the profile had
one KMS key. Splitting the seal key from the node volume key made it two,
and the count was not updated with them.

Both of the assertions that matter passed against the real API in the
same run -- the two keys are distinct, and the root volume is not on the
seal key -- so this is the expectation being stale, not the change being
wrong. Second time a hardcoded count in this file has gone out of date
after a deliberate change; the counts are there to catch resources
appearing or vanishing unnoticed, and that is worth the occasional edit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sethbergman

Copy link
Copy Markdown
Owner Author

Green, and one commit landed after the description above

27/27, MERGEABLE / CLEAN.

aws_kms_key: expected 1

The first run failed on a resource count in the emulated apply — mine,
written when the profile had one KMS key. Splitting it made two and I did
not update the count with them.

What passed in that same run is the part that matters. The profile
applied end to end with both keys and destroyed cleanly, and both new
assertions held against real arns:

PASS  the AWS profile applies end to end
PASS  the seal key and the volume key are different keys
PASS  the root volume is not encrypted with the seal key
PASS  terraform destroy removes everything it made

Those two are exactly what terraform test structurally cannot check,
since its mock hands every aws_kms_key the same arn. So the split is
verified by the only layer that can verify it.

On the stale count

Second time a hardcoded count in that file has gone out of date after a
deliberate change — the IAM roles did the same when network.tf's
flow-logs role turned out to exist.

Worth keeping rather than loosening. The counts earn their place by
catching a resource appearing or vanishing unnoticed, and the price is
that a deliberate change has to update them. That is the trade working.

What this does not settle

The other half of #55: whether the ASG service-linked role needs explicit
key-policy grants once a restrictive policy replaces the default. moto
does not enforce IAM, so a real apply is the only thing that answers it.
Recorded in the issue rather than closed with the rest.

@sethbergman
sethbergman merged commit 87105d4 into main Aug 30, 2026
27 checks passed
@sethbergman
sethbergman deleted the fix/separate-seal-and-data-keys branch August 30, 2026 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Root volume and the seal share one KMS key, so destroy can strand every snapshot

1 participant