Horizontal scaling - #151
Conversation
…ave deploys Two paths are unsafe as soon as more than one enclave boots against a deployment, and both fail silently. establishLoadedState commits genesis with an unconditional ssm.Set(kmsKeyIDParam(), ...). Every artifact written before it is key-scoped in its SSM path, so N concurrent genesis boots each mint a KMS key, a DEK and a set of static secrets without colliding, then race on that one unscoped slot. Last writer wins and the losers get no error: they keep serving with a DEK no later boot can derive, sealing KV values, freshness anchors and the ACME cache under it, and their static secrets differ, so their PCR16+ extensions no longer match their peers'. Guard it with an S3 conditional-write lease. The lease object carries only an expiry — every transition is an If-None-Match or If-Match write, so the ETag returned by the last successful write is the entire claim. There is deliberately no holder field: it would invite a `holder == me` guard, which is correct on DynamoDB (no per-item ETag) and wrong here, and especially wrong with a reboot-stable ID, since a restarted enclave would conclude it owns a lease held by its own dead predecessor. There is no fencing token either; one only works if the downstream resource validates it, and S3 does not know what one is. Genesis takes the lease withoutSteal. A half-finished genesis must never be taken over: the KMSKeyID commit cannot be made conditional, because SSM has no compare-and-swap on value and the parameter is pre-created with an "UNSET" sentinel that rules out create-only. A holder that stalled past its lease and later resumed would clobber whoever took over and orphan that enclave's DEK. Genesis runs once, at deploy time, with an operator present, so wedging the fleet with an actionable error beats diverging it silently. Waiters watch KMSKeyID rather than queueing on the lock — they have no work to serialize — so they resume together within a poll interval of the commit. The freshness-anchor boot gate has the second bug. Establish compares Object-Locked anchors against a live version map built by a DynamoDB scan that never set ConsistentRead, so a peer committing between the scan and the anchor listing is indistinguishable from a rollback and halts the booting enclave. Make the scan strongly consistent and re-read the offending key with a consistent GetItem before halting. A re-read failure does not halt: an unreachable table is not proof of rollback.
Two changes, both needed before a fleet can sit behind a load balancer. tls-alpn-01 cannot survive one. The CA dials :443 and gets hashed to an arbitrary target, but only the enclave that created the order holds the challenge certificate, so validation succeeds about 1/N of the time. autocert cannot help: it implements only http-01 and tls-alpn-01, and its Manager is process-local, so N enclaves would issue N simultaneous orders for one FQDN and exhaust the CA's duplicate-certificate limit. Replace it with DNS-01 driven directly against x/crypto/acme, and give the fleet one certificate instead of one each. Certificate and key live in a single DEK-sealed S3 object — one object so a write cannot tear them apart, DEK-sealed because every enclave of the same PCR0 derives the same key and can therefore open it. That shared certificate is what lets every enclave attest the same tlsKeyHash, which is the whole reason a load balancer can route freely. Renewal is serialized by the S3 lease and is deliberately not lazy: renewing inside the handshake would be a thundering herd, and the first client after expiry would wait out a multi-minute order. A background refresher polls the object's ETag, adopts a peer's renewal when it moves, and orders only when the certificate is inside renewBefore and it wins the lease. Three orderings in that path are load-bearing and easy to break silently. The ETag is captured before the order and never refreshed, so the conditional write means "nothing happened while I was gone" — re-reading it immediately before writing would let an issuer whose lease lapsed while it was descheduled overwrite the peer that took over, and the code would still look correct. S3 is written before memory, so a rejected write cannot leave an enclave serving a leaf no peer has with its attestation bound to it. And a 412 adopts the peer's certificate rather than re-running the order, which would burn another duplicate-certificate slot for a write that would also fail. Route53 is the only DNS provider. INSYNC is waited for because that means every Route53 authoritative server serves the change, which is what the CA queries; no resolver is vendored to poll nameservers directly, since that would add a dependency to a measured EIF for no extra guarantee. The grant is scoped to _acme-challenge TXT records in one zone. It does let the host mint certificates for the FQDN — the enclave holds no credentials the host lacks — which is acceptable because client trust comes from pinning the attested tlsKeyHash, not from the CA. Separately, the freshness-anchor boot gate compared Object-Locked anchors against a DynamoDB scan that never set ConsistentRead, so a peer committing between the scan and the anchor listing was indistinguishable from a rollback and halted the booting enclave. Make the scan strongly consistent and re-read the offending key before halting. A re-read failure does not halt: an unreachable table is not proof of rollback.
- Updated `awaitGenesis` to improve lease acquisition logic and error handling. - Modified `EstablishState` to pass the genesis lease to `establishLoadedState`. - Enhanced `establishLoadedState` to verify the lease before committing genesis. - Added tests to ensure proper handling of lapsed locks and live holders during genesis. - Refactored `acme-test.sh` for DNS-01 challenge handling, including hosted zone creation and TXT record mirroring. - Updated Docker Compose configuration to streamline the testing environment for DNS-01 challenges.
Nothing so far demonstrated N>1 end to end. The unit tests cover the genesis lease, the cert manager and the boot gate in isolation, and `make test-acme` covers a single enclave's issuance and its reuse across a reboot — but a reboot cannot stand in for a second node. Same EIF, same PCR0: from AWS's point of view a restart and a joining peer are indistinguishable. What was missing is concurrency — one enclave alive, holding state and serving, while another boots and navigates around it. Two enclaves cannot share the current vsock fabric. The guest dials a fixed vsock://3:1024 (runtime/nitriding/proxy.go) and only one host process can hold that port; VMADDR_CID_HOST is not bindable and CID_ANY cannot coexist with CID_LOCAL on one port, so per-CID separation is out. Putting both guests on one gvproxy does not work either — they present the same MAC and collide on the static lease 192.168.127.2, and the L2 switch delivers to whichever link registered last. Port separation is the way through, and it needs a per-instance channel that is upstream of everything. Add runtime/devcmdline.go, ported from the threshold scaling work on `dkg`: a whitelisted kernel command-line channel read before networking, config or the SSM overlay, carrying deployment, gvproxy_port and imds_out. Baking those would change PCRs and SSM is downstream of the ports themselves, so the command line is the only place left. It is a strict no-op unless IsDev(), which reads the baked, measured ENCLAVE_DEV flag, so an untrusted host cannot inject config into a production enclave this way. The gate moved inside the function rather than sitting at the call site as it does on `dkg`, where the accompanying test passed only because the host's /proc/cmdline happened to be clean. This is safe for the measurement the KMS policy gates on: QEMU computes PCR0 as SHA384 of the EIF and only then concatenates -append onto the EIF's own cmdline. Both nodes measure identically, which the test asserts explicitly rather than assumes. test/run-fleet.sh boots node A cold and node B into the live fleet, each with its own supervisor on its own vsock ports, sharing one deployment. It owns QEMU directly instead of going through ENCLAVE_START_CMD: the supervisor's watchdog restarts an enclave that misses twelve polls, about sixty seconds, and a cold ACME boot takes minutes.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Review — Horizontal scaling (#151)
Scope: genesis lease, S3 conditional-write mutex, shared certificate lifecycle (certmanager/certstore/acme_issuer/acme_dns01), Schnorr signing removal, attestation UserData format change, anchor boot-gate re-read, dev-cmdline overrides.
The overall design is sound. The genesis serialization story is clean, the DNS-01 flow is correct, and the test matrix (unit + fleet + ACME end-to-end) is reasonable for the size of the change. The findings below are what needs resolving before merge.
M1 — storeBundle does not atomically update served cert and attested fingerprint
runtime/certmanager.go:91–97
m.hashes.SetTLSKeyHash(bundle.leafHash) // (1) attestation now says hash B
m.current.Store(bundle) // (2) served cert now IS leaf BThese are two distinct operations on two distinct synchronisation primitives (sync.RWMutex inside attestationHashes and sync.atomic.Pointer). Between (1) and (2) any NSM attestation request returns tlsKeyHash = B, while GetCertificate still returns leaf A. A client that fetches attestation in that window, then immediately opens a connection, will fail the TLS pin.
The PR body says "The certificate and attested hash are installed together through one atomic.Pointer" — that description does not match the code. The invariant the PR relies on (tlsKeyHash and served leaf are always consistent) is not enforced.
Fix options:
- Store both pieces in one struct and swap them with a single
atomic.Pointer.Store. - Or reverse the order: call
m.current.Store(bundle)first, thenm.hashes.SetTLSKeyHash(bundle.leafHash). Clients see the new leaf before attestation updates — they get the old hash, fail the pin, retry, and succeed. The current order (attest first, serve second) is strictly worse: clients see a hash for a leaf that doesn't exist yet, and that pattern is indistinguishable from an active MITM.
M2 — certstore.go:putConditional silently drops the retryable S3 409
runtime/certstore.go:176–200
The lease code carefully distinguishes S3 412 (genuine contention) from 409 (ConditionalRequestConflict, a retryable S3-internal race). putConditional does not: a 409 during a cert write returns a raw non-errCertChanged error. renew() propagates this as a hard failure rather than adopting the peer's cert, and the caller would treat it as a broken issuance rather than retrying. The next poll (5–15 min) recovers, but under a burst of concurrent first-boots this could fail every node that isn't the winner of the conditional write.
Fix: wrap 409 as errCertChanged (or add the same retry loop used in claimLease) so the caller adopts and does not restart the ACME order.
M3 — time.After goroutine accumulation in blocking acquire loops
runtime/s3lease.go (AcquireLease inner select) and runtime/state.go (awaitGenesis inner select)
Both loops use:
case <-time.After(leasePollInterval):time.After allocates a timer that is not GC'd until it fires (up to leasePollInterval = 5 s). In a boot storm with N enclaves queuing on genesis, each enclave accumulates un-stopped time.After timers for the full wait. Prefer time.NewTimer + defer t.Stop() with t.Reset on reuse.
M4 — Route53API declares GetHostedZone but never calls it
runtime/aws_clients.go:113–120
GetHostedZone appears in the interface but has zero call sites (acme_dns01.go only uses ChangeResourceRecordSets and GetChange). Dead interface surface that every mock must implement. Remove it.
M5 — tick() nil-bundle branch after ETag change is untested and silent
runtime/certmanager.go:160–176
if etag != "" && etag != m.currentETag() {
bundle, err := m.store.loadCert(ctx)
if err != nil { return err }
if bundle != nil {
m.storeBundle(bundle)
}
// falls through with stale in-memory bundle — no log, no error
}If a peer's new ETag is observed but loadCert returns (nil, nil) — object deleted between HeadObject and GetObject — tick falls through holding the stale bundle. The fleet continues serving whatever it last had while the ETag mismatch keeps firing every poll. Add a test for this edge and either return an error (force the next poll to re-check) or log a warning.
Minor / informational
acme_dns01.go:RemoveChallenge does not wait for propagation.
PublishChallenge calls waitInSync before telling the CA to validate. RemoveChallenge fires DELETE without waiting. This is safe in practice — ACME servers do not re-query DNS after Accept — but a short comment explaining why the asymmetry is intentional would help future readers.
acme_issuer.go:loadOrCreateAccountKey 2-iteration loop error message.
The final errors.New("object kept disappearing between load and create") is returned at a point that can only be reached if S3 serialises two load/create cycles and the object vanished both times. A slog.Error here (not just an errors.New) would surface this in ops dashboards if it ever fires.
vhost-device-vsock forward-cid=1 → 2 (test/acme-test.sh:95).
CID 2 is the QEMU host, CID 1 is the hypervisor. The old value looks like a pre-existing bug; the fix is correct.
UserData format change (39 bytes, was 79).
Old clients check len(userData) < sigKeyEnd and return ("", nil), silently skipping signing-key verification rather than erroring — so old clients against new enclaves are backward-safe. New clients against old enclaves read bytes 7:39 as tlsKeyHash, which is the same field at those offsets in the old format. Both directions are safe. A migration note in OPERATIONS.md for operators running mixed-version fleets would be useful.
Lease.Verify TOCTOU (acknowledged in PR body).
The ETag comparison correctly catches displaced leases (the peer who steals must write a new object, changing the ETag). The analysis in the PR body is accurate.
Test coverage gap
The new genesis serialization path is well-exercised. What's missing: a renew() call that receives a 409 (not 412) from saveCert and asserts that the cert-changed adoption path triggers rather than a hard error. The fakeS3.putConflicts knob already exists — a test for M2 is straightforward.
Summary. M1 (storeBundle atomicity) and M2 (missing 409 handling in putConditional) are correctness bugs; M3 (time.After leak) and M4 (dead interface method) are clean-up items that should go in this PR. M5 is a test gap.
…lastic IP - Refactored curl command to retrieve the endpoint from tofu outputs, replacing Elastic IP with load balancer DNS name. - Updated framework files to reflect the change from Elastic IP to endpoint in deployment outputs. - Modified status command to display the endpoint instead of Elastic IP. - Adjusted Terraform configuration to create a load balancer and associated resources, replacing Elastic IP with endpoint outputs. - Enhanced fleet test scripts to utilize Moto for EC2 and ELBv2 mocks, ensuring proper infrastructure assertions. - Updated enclave harness to redirect EC2 and ELBv2 calls to Moto, facilitating local testing.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (1f07a0f): networking layer — Elastic IP → NLB, instance moved to private subnet, NAT gateway added, ssmmessages/ec2messages VPC endpoints, R0 fleet test assertions against Moto. 0 of 5 prior issues fixed; all remain open. No runtime files were touched.
Prior-issue status
| ID | Status |
|---|---|
M1 — storeBundle atomicity (certmanager.go:91–97) |
Still open — certmanager.go not in this diff. |
M2 — putConditional drops retryable S3 409 (certstore.go:176–200) |
Still open — certstore.go not in this diff. |
M3 — time.After goroutine accumulation (s3lease.go, state.go) |
Still open — neither file changed. |
M4 — GetHostedZone dead interface method (aws_clients.go) |
Still open — aws_clients.go not in this diff. |
M5 — tick() nil-bundle branch untested (certmanager.go:160–176) |
Still open — no new coverage. |
The minor informational items (RemoveChallenge propagation comment, loadOrCreateAccountKey error log, UserData migration note) are similarly unchanged.
New findings — 1f07a0f
N1 — Missing port 6379 NLB listener (Medium)
cli/tofu_files.go — aws_lb_listener.https and aws_lb_target_group.https
The PR body lists :6379 as a required NLB listener alongside :443:
"Add an NLB with TCP-only listeners on :443 and :6379."
The implementation adds only the :443 listener and one target group. The NLB SG (nlb_https_ingress, nlb_egress_to_instances) and the nitro SG (https_ingress) are all scoped to port 443. There are no corresponding rules or listeners for 6379.
If the runtime serves on 6379 (the RESP port referenced in the DynamoDB comment at cli/tofu_files.go:987), clients behind the NLB cannot reach it — the connection would hit the NLB with no listener and be refused. The infrastructure gating (create_network) being the same for both resources means a 6379 listener and target group can be added in a later commit without restructuring, but the current state silently drops that traffic.
Please either: (a) add the 6379 listener + target group + SG rules in this commit, or (b) confirm the runtime does not serve on 6379 behind the LB and update the PR body to remove it from the requirements list.
N2 — Moto service_started vs service_healthy (Low)
test/docker-compose.yml:152–157, test/run-fleet.sh:270–302
The fleet-runner's depends_on for moto uses condition: service_started, which fires as soon as Docker starts the container — before Moto's HTTP server is listening. run-fleet.sh then calls tofu_init_apply -var 'local_network=true', which immediately makes ELBv2 API calls against Moto.
In practice the window is narrow because tofu init takes several seconds. But under load or on a slow runner, Moto might not have bound its port yet and tofu apply will fail with a connection-refused rather than a clear error message.
Fix: add a healthcheck to the moto service (e.g., test: ["CMD", "curl", "-sf", "http://127.0.0.1:5001/"]) and change the dependency condition to service_healthy.
N3 — deregistration_delay = 30 may cut long connections (Informational)
cli/tofu_files.go — aws_lb_target_group.https
The default NLB deregistration delay is 300 s. 30 s is aggressive: any in-flight request that takes longer (e.g., a slow client upload or a long-poll) will be forcibly closed when a node is cycled. This is fine for the current short-lived request/response pattern, but should be documented so a future operator does not wonder why connections are dropped during rolling restarts.
Correct decisions worth noting
preserve_client_ip = false: The comment correctly explains that instance-target NLBs preserve client IPs by default, which breaks SG-to-SG matching. Disabling it is the right call for this security model; the trade (NLB node addresses in logs rather than client IPs) is acceptable because the runtime does not use the client address.- HTTPS health check on
/health: A TCP health check would mark an enclave healthy as soon as the listener binds, before genesis and ACME certificate bootstrap complete. The HTTPS/healthcheck is the correct signal. - Route53 ALIAS with
evaluate_target_health = true: Correct — Route53 will stop resolving the name if the NLB's health check fails, preventing traffic from reaching a cold or dead fleet. - Single-AZ NLB (
subnets = [aws_subnet.public[0].id]): Acceptable for the current count-1 deployment. Multi-AZ expansion should add subnet IDs here rather than restructuring.
Summary. M1 and M2 are the correctness blockers I flagged last pass; they still need to be addressed. N1 (missing 6379 listener) needs either an implementation or an explicit statement that 6379 is not served through the LB. N2 is a test reliability fix. The networking architecture itself — private instance, NAT, TCP-passthrough NLB, HTTPS /health gate — is sound.
Replace aws_instance.nitro with a launch template plus an ASG, which registers
and drains load-balancer targets itself and replaces an enclave that stops
serving (health_check_type = "ELB").
health_check_grace_period is 1800s, not the 300s default: a waiter blocks up to
genesisLeaseWait (10m) on the lease and the genesis holder then runs a DNS-01
order bounded by certIssueTimeout (10m), so at the default a cold fleet would
terminate enclaves mid-boot and never converge. No instance_refresh, which keeps
the old ignore_changes = [ami, user_data] behaviour — applies never disturb a
running enclave. min = max = desired; the group is for replacement, not scaling.
The destroy path loses its ${self.id}. Since it schedules KMS key deletion and
is best-effort, it moves to terraform_data.enclave_shutdown, which discovers
instances by the propagated ASG tag and logs the IDs it found.
instance_id becomes asg_name: the group replaces instances, so IDs are resolved
at runtime via DescribeInstances with a tag filter. Migration still targets one
host, as it always has.
Tests needed an autoscaling endpoint override beside ec2 and elbv2 — the three
share one mock, hence COMPUTE_ENDPOINT. A missing key reaches real AWS and fails
as InvalidClientTokenId rather than an unknown service.
…imize network traffic
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (9116d0c): aws_instance → Launch Template + Auto Scaling Group. 0 of 8 prior issues fixed; all remain open. No runtime files touched; this diff is infrastructure and CLI tooling only.
Prior-issue status
| ID | Status |
|---|---|
M1 — storeBundle atomicity (certmanager.go:91–97) |
Still open — not in this diff. |
M2 — putConditional drops retryable S3 409 (certstore.go:176–200) |
Still open — not in this diff. |
M3 — time.After goroutine accumulation (s3lease.go, state.go) |
Still open — not in this diff. |
M4 — GetHostedZone dead interface method (aws_clients.go) |
Still open — not in this diff. |
M5 — tick() nil-bundle branch untested (certmanager.go:160–176) |
Still open — not in this diff. |
N1 — Missing port 6379 NLB listener (tofu_files.go) |
Still open — not addressed. |
N2 — Moto service_started vs service_healthy (docker-compose.yml) |
Still open — not in this diff. |
N3 — deregistration_delay = 30 documentation gap (informational) |
Still open — not addressed. |
New findings — 9116d0c
P1 — Applying this diff to an existing deployment causes downtime (Medium)
cli/tofu_files.go — aws_instance.nitro → aws_launch_template.nitro + aws_autoscaling_group.nitro
tofu apply on an existing deployment destroys aws_instance.nitro[0] (which triggers the old destroy provisioner — SSM stop + key deletion schedule), then brings up the ASG, then waits up to 30 m for the new instance to pass the NLB health check. During that window the fleet is serving 0 instances. This is unavoidable with a resource-type change, but it needs to be called out clearly in the PR body (currently the status table still marks infrastructure as "Not done", which is no longer accurate). Operators need a planned maintenance window for this apply.
P2 — wait_for_capacity_timeout equals health_check_grace_period — no buffer (Low)
cli/tofu_files.go — aws_autoscaling_group.nitro
If an enclave uses the full grace period and then fails the NLB health check, the ASG starts its replacement at exactly the moment tofu apply gives up and returns an error. The apply exits with "waiting for capacity timed out" while the ASG is mid-replacement, leaving the operator with a partial state and a confusing error. Setting wait_for_capacity_timeout = "45m" (or longer) gives the ASG one replacement cycle of headroom beyond the grace period and produces a clearer signal — either the replacement is healthy within the extended window, or the apply fails with a genuine unhealthy instance.
The comment correctly identifies the 30 m floor (genesis 10 m + DNS-01 10 m + margin); the issue is that the timeout should be the floor plus one replacement cycle, not equal to the floor.
P3 — instance_id output removed without a deprecation path (Medium)
cli/tofu_files.go:202–204 (root module) and cli/tofu_files.go:1799–1801 (enclave module)
The instance_id output has been renamed to asg_name. Any external consumer of tofu output instance_id — CI pipelines, monitoring dashboards, enclave migrate invocations with cached outputs — will silently receive an empty string on the next run. The CLI already handles the empty-string case gracefully (falls through to the ASG lookup), but external scripts won't. A deprecated alias output (returning "" with a description explaining it was removed) would let external consumers detect the break explicitly rather than silently getting nothing.
If existing deployments will be destroyed and re-created rather than migrated in-place (which is the least-risk approach given P1), this is lower priority — but it should still be documented.
P4 — set -u in enclave_shutdown provisioner with word-split loop is fragile (Low)
cli/tofu_files.go — terraform_data.enclave_shutdown destroy provisioner
IDS= always binds IDS, so set -u won't abort on an empty result. However, --output text with --query 'Reservations[].Instances[].InstanceId' returns instance IDs separated by tabs or newlines depending on the AWS CLI version. An unquoted `` with IFS-split works for space/tab/newline delimiters, but if any instance ID ever contains whitespace (it won't — AWS IDs are alphanumeric), this would split incorrectly. More concretely: the existing SSM send-command call takes a single `--instance-ids` with one ID per loop iteration, which is correct. This pattern is safe for the current output format but would silently break if the query format or CLI version changes output quoting. A `while IFS= read -r ID; do ... done <<< "$IDS"` form is more robust, but this is cosmetic.
Correct decisions worth noting
version = "$Latest"on the ASG launch template: Candidate applies update the template (new version), but since there's noinstance_refreshand capacity is fixed, running instances are not replaced. The comment correctly documents the deliberate rollout path (aws autoscaling start-instance-refresh). The absence oflifecycle { ignore_changes = [ami, user_data] }on the template is intentional and explained.health_check_type = "ELB": Correct — ASG-level health checks alone would mark an enclave healthy as soon as the host is reachable via EC2 health, before genesis and ACME bootstrap. ELB health ensures the full boot path completes before traffic is accepted and before a sick enclave is replaced.- ASG
depends_oncoveringcandidate_upload: Preserved — prevents the ASG from launching instances before the EIF and supervisor binary are in S3. instancesInASGdiscovery via tag rather than autoscaling SDK: Reusing the existing EC2 client is pragmatic and the tag (aws:autoscaling:groupName) is reliably propagated by the ASG.status.gohandles the error gracefully.- Fleet test additions (
fleet-test.sh:133–161): CheckingHealthCheckType = ELB, target group wiring, and Nitro Enclave enablement on the launch template are the right R0 assertions. The guard[ -n "$LT_NAME" ]prevents a false-pass if Moto doesn't populateLaunchTemplate.
Summary. M1 and M2 are the correctness blockers; they remain open. P1 (downtime on apply) and P3 (output rename) need documentation before merge. P2 (timeout equals grace period) needs a numeric fix — 45m or 60m for wait_for_capacity_timeout. The ASG architecture itself is sound.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (9116d0c): aws_instance → Launch Template + Auto Scaling Group. 0 of 8 prior issues fixed; all remain open. No runtime files touched; this diff is infrastructure and CLI tooling only.
Prior-issue status
| ID | Status |
|---|---|
M1 — storeBundle atomicity (certmanager.go:91–97) |
Still open — not in this diff. |
M2 — putConditional drops retryable S3 409 (certstore.go:176–200) |
Still open — not in this diff. |
M3 — time.After goroutine accumulation (s3lease.go, state.go) |
Still open — not in this diff. |
M4 — GetHostedZone dead interface method (aws_clients.go) |
Still open — not in this diff. |
M5 — tick() nil-bundle branch untested (certmanager.go:160–176) |
Still open — not in this diff. |
N1 — Missing port 6379 NLB listener (tofu_files.go) |
Still open — not addressed. |
N2 — Moto service_started vs service_healthy (docker-compose.yml) |
Still open — not in this diff. |
N3 — deregistration_delay = 30 documentation gap (informational) |
Still open — not addressed. |
New findings — 9116d0c
P1 — Applying this diff to an existing deployment causes downtime (Medium)
cli/tofu_files.go — aws_instance.nitro → aws_launch_template.nitro + aws_autoscaling_group.nitro
tofu apply on an existing deployment destroys aws_instance.nitro[0] (which triggers the old destroy provisioner — SSM stop + key deletion schedule), then brings up the ASG, then waits up to 30 m for the new instance to pass the NLB health check. During that window the fleet is serving 0 instances. This is unavoidable with a resource-type change, but it needs to be called out explicitly in the PR body (currently the status table still marks infrastructure as "Not done", which is no longer accurate after this commit). Operators need a planned maintenance window for this apply.
P2 — wait_for_capacity_timeout equals health_check_grace_period — no buffer (Low)
cli/tofu_files.go — aws_autoscaling_group.nitro
health_check_grace_period = 1800 # 30 m
wait_for_capacity_timeout = "30m" # also 30 m
If an enclave uses the full grace period before failing the NLB health check, the ASG begins replacement at exactly the moment tofu apply gives up and returns an error. The apply exits with "waiting for capacity timed out" while the ASG is mid-cycle, leaving the operator with partial state and a confusing error message. Setting wait_for_capacity_timeout = "45m" (or "60m") gives the ASG one replacement cycle of headroom beyond the grace period. The comment correctly identifies the 30 m floor (genesis 10 m + DNS-01 10 m + margin); the timeout should be the floor plus one replacement cycle, not equal to it.
P3 — instance_id output removed without a deprecation path (Medium)
cli/tofu_files.go:202–204 (root module) and cli/tofu_files.go:1799–1801 (enclave module)
The instance_id output has been renamed to asg_name. Any external consumer of tofu output instance_id — CI pipelines, monitoring dashboards, enclave migrate invocations with a cached outputs file — will silently receive an empty string on the next run. The CLI already handles this gracefully (falls through to the ASG lookup), but external scripts won't. A deprecated alias output that returns "" with a description noting the removal would let external consumers detect the break explicitly. If existing deployments are being torn down and re-created (the lowest-risk path given P1), this is lower priority — but it should be documented.
P4 — Shell word-split loop in enclave_shutdown destroy provisioner (Low)
cli/tofu_files.go — terraform_data.enclave_shutdown
IDS=$(aws ec2 describe-instances ... --output text || true)
for ID in $IDS; do
--output text with --query 'Reservations[].Instances[].InstanceId' emits IDs separated by tabs/newlines depending on the result structure. The unquoted $IDS relies on IFS word-splitting. AWS instance IDs are alphanumeric so this won't mis-split today, but the pattern is fragile if the query or CLI version changes quoting. A while IFS= read -r ID; do ... done <<< "$IDS" form is safer. This is cosmetic — the current output format is stable — but worth tidying.
Correct decisions worth noting
version = "$Latest"on the ASG launch template: Candidate applies update the template (new version) but don't trigger replacement because there's noinstance_refreshand capacity is fixed. The comment correctly documents the deliberate rollout path (aws autoscaling start-instance-refresh). The absence oflifecycle { ignore_changes = [ami, user_data] }on the template is intentional and explained.health_check_type = "ELB": Correct. EC2-level ASG health checks would mark an enclave healthy as soon as the host responds via EC2 health, before genesis and ACME bootstrap complete. ELB health ensures the full boot path completes before traffic is accepted and before a sick enclave is replaced.- ASG
depends_oncoveringcandidate_upload: Preserved — prevents the ASG from launching instances before the EIF and supervisor binary are in S3. instancesInASGdiscovery via EC2 tag rather than the autoscaling SDK: Reusing the existing EC2 client is pragmatic. The tag (aws:autoscaling:groupName) is reliably propagated by the ASG.status.gohandles the discovery error gracefully.- Fleet test additions (
test/fleet-test.sh:133–161): CheckingHealthCheckType = ELB, target group wiring, and Nitro Enclave enablement on the launch template are the right R0 assertions. The guard[ -n "$LT_NAME" ]prevents a false-pass if Moto doesn't populateLaunchTemplate.
Summary. M1 and M2 are the correctness blockers from the original review; they remain open. P1 (downtime on apply) and P3 (output rename without deprecation) need documentation before merge. P2 (timeout equal to grace period, no buffer) needs a numeric fix — "45m" or "60m" for wait_for_capacity_timeout. The ASG architecture itself — fixed-size group, ELB health check, NLB target registration via target_group_arns, enclave_shutdown via tag-based instance discovery — is sound.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (`b507159`): three new VPC endpoints — STS (Interface), CloudWatch Logs (Interface), DynamoDB (Gateway) — plus expanded fleet-test assertions. 0 of 12 prior issues fixed; all remain open. No runtime files touched.
Prior-issue status
| ID | Status |
|---|---|
| M1 — `storeBundle` atomicity (`certmanager.go:91–97`) | Still open — not in this diff. |
| M2 — `putConditional` drops retryable S3 409 (`certstore.go:176–200`) | Still open — not in this diff. |
| M3 — `time.After` goroutine accumulation (`s3lease.go`, `state.go`) | Still open — not in this diff. |
| M4 — `GetHostedZone` dead interface method (`aws_clients.go`) | Still open — not in this diff. |
| M5 — `tick()` nil-bundle branch untested (`certmanager.go:160–176`) | Still open — not in this diff. |
| N1 — Missing port 6379 NLB listener (`tofu_files.go`) | Still open — not addressed. |
| N2 — Moto `service_started` vs `service_healthy` (`docker-compose.yml`) | Still open — not in this diff. |
| N3 — `deregistration_delay = 30` documentation gap (informational) | Still open — not addressed. |
| P1 — Apply-to-existing-deployment causes downtime (documentation gap) | Still open — not addressed. |
| P2 — `wait_for_capacity_timeout = "30m"` equals grace period, no buffer | Still open — not changed. |
| P3 — `instance_id` output renamed without deprecation path | Still open — not addressed. |
| P4 — Shell word-split loop in `enclave_shutdown` provisioner (cosmetic) | Still open — not changed. |
New findings — `b507159`
Q1 — fleet-test asserts endpoints Moto may not have created (Low)
test/fleet-test.sh:164
The loop previously checked only `ssmmessages` and `ec2messages`. It now also checks `kms ssm dynamodb s3`. Those four have been present in `tofu_files.go` since earlier commits and are gated by `local.create_network`, so they exist in Moto after `tofu apply -var 'local_network=true'`. This expansion is correct and a welcome coverage improvement.
One caveat: the filter is a suffix glob (Name=service-name,Values=*.${svc}). For `svc=ssm` this produces `*.ssm`, which matches `com.amazonaws..ssm` but not `com.amazonaws..ssmmessages` — so there is no false-pass from the two overlapping names. Verified clean.
Q2 — STS and Logs interface endpoints use nitro SG — correct but worth confirming (Informational)
cli/tofu_files.go:1420–1442
Both new interface endpoints are assigned `security_group_ids = [aws_security_group.nitro[0].id]`, the same SG as the existing `kms`, `ssm`, `ssmmessages`, and `ec2messages` endpoints. The instance (also in the nitro SG) reaches the endpoint ENIs via the self-referencing TCP-443 ingress rule (`self_tcp`). This is correct: the self-referencing rule explicitly permits instance-to-endpoint-ENI traffic when both are in the same SG. Pattern is consistent with prior endpoints.
Q3 — DynamoDB gateway endpoint includes public route table (Informational)
cli/tofu_files.go:1456
route_table_ids = [aws_route_table.public[0].id, aws_route_table.private[0].id]Instances are in the private subnet, so only `private[0]` matters functionally. The public route table inclusion is harmless (gateway endpoints are VPC-internal, not internet-exposed) and mirrors the identical pattern in `aws_vpc_endpoint.s3`. No action needed; noted for operator awareness that DynamoDB traffic from any future public-subnet resource also routes through the endpoint.
Correct decisions in this commit
- STS endpoint comment: correctly identifies `GetCallerIdentity` (called during KMS bootstrap) as a boot-path dependency — not a nicety.
- DynamoDB as Gateway, not Interface: Gateway endpoints are free and route-table-based; using Interface here would cost ~$7/month per AZ and add an ENI to manage. Gateway is the right choice.
- `logs` service name: `com.amazonaws.${region}.logs` is the canonical AWS service name for CloudWatch Logs. The fleet-test filter `*.logs` is unambiguous and correct.
- Subnet consistency: all three new endpoints are confined to `private[0]` (Interface) or both route tables (Gateway) — matches existing endpoints exactly.
- `private_dns_enabled = true` on STS and Logs: required for the SDK's default endpoint resolver to use the VPC endpoint without custom configuration. Correct.
Summary. This commit cleanly rounds out the VPC endpoint inventory. The three additions are architecturally correct and the fleet-test expansion is the right coverage move. No new protocol or security issues. M1 and M2 remain the correctness blockers; P2 (`wait_for_capacity_timeout`) remains the infra numeric fix needed before merge.
- Updated method names in certmanager_test.go and certstore.go for consistency with Go naming conventions (e.g., `issue` to `Issue`, `saveCert` to `SaveCert`). - Introduced a new Lease struct to manage S3 leases, including methods for acquiring, releasing, and renewing leases. - Implemented lease acquisition logic with contention handling and heartbeat renewal. - Added tests for lease functionality, including scenarios for uncontended and contended lease acquisition, lease theft detection, and heartbeat renewal. - Refactored TLS configuration to utilize the new lease management and account key loading methods. - Enhanced fake S3 implementation to simulate missing objects and conditional request conflicts for testing.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (c7a2a4f): runtime refactor — consolidates DNS-01 into acme_issuer.go, fixes M1/M2/M4/M5, partially fixes M3, exports certStore/certManager methods, replaces storeBundle with SetTLSKeyHashSource. 4 of 13 prior issues fixed; 9 remain open. Infrastructure issues (N1–N3, P1–P4) are untouched — not in this diff.
Prior-issue status
| ID | Status |
|---|---|
M1 — storeBundle atomicity (certmanager.go) |
Fixed in c7a2a4f. SetTLSKeyHashSource registers a closure that reads m.currentCert directly; both Serialize and GetCertificate now load the same atomic.Pointer[certBundle]. The two-operation window is gone. TestAttestedHashAlwaysMatchesServedLeaf covers the invariant. |
M2 — putConditional drops S3 409 (certstore.go) |
Fixed in c7a2a4f. isConditionalConflict(err) now maps 409 to errCertChanged alongside 412. TestCertStoreConditionalConflictIsCertChanged covers it. |
M3 — time.After goroutine accumulation |
Partially fixed. The heartbeat goroutine now uses time.NewTicker(l.ttl/3) with defer ticker.Stop() — correct. Two time.After calls remain: lease.go:101 (AcquireLease blocking poll) and state.go:89 (awaitGenesis inner select). Both leak up to leasePollInterval (5 s) per waiting iteration. Fix both the same way the heartbeat was fixed: time.NewTimer + reset + stop, or a ticker. |
M4 — GetHostedZone dead interface method |
Fixed in c7a2a4f. Removed from Route53API and from fakeRoute53. |
M5 — tick() nil-bundle branch untested |
Fixed in c7a2a4f. tick now calls LoadCert directly instead of HeadObject+GetObject, eliminating the TOCTOU. TestCertManagerTickSurvivesVanishedCert covers the vanished-object path with the getMissing knob. |
| N1 — Missing port 6379 NLB listener | Still open — not in this diff. |
N2 — Moto service_started vs service_healthy |
Still open — not in this diff. |
N3 — deregistration_delay = 30 documentation gap |
Still open — not in this diff. |
| P1 — Apply-to-existing-deployment causes downtime (doc gap) | Still open — not in this diff. |
P2 — wait_for_capacity_timeout equals grace period |
Still open — not in this diff. |
P3 — instance_id output renamed without deprecation path |
Still open — not in this diff. |
P4 — Shell word-split in enclave_shutdown provisioner |
Still open — not in this diff. |
New findings — c7a2a4f
R1 — tick() nil dereference if currentCert is unset (Low)
runtime/certmanager.go — tick
bundle := m.currentCert.Load()
if time.Until(bundle.notAfter) > renewBefore { // panics if bundle == nil
Bootstrap guarantees currentCert is non-nil before Run is called, so this is unreachable on the normal path. But there is no type-level enforcement: tick is also called directly from tests, and a future refactor that reorders the call sequence produces a nil dereference at runtime rather than a compile error. Add an explicit nil guard:
if bundle == nil {
return errors.New("tick: no certificate installed — Bootstrap must run first")
}R2 — Certificate installation is now silent (Informational)
runtime/certmanager.go — all m.currentCert.Store(bundle) call sites
storeBundle logged the leaf hash and expiry on every install. All five replacement m.currentCert.Store(bundle) call sites are silent. Operators can no longer see in logs when a certificate rotated or what leaf is currently live. Restore the log at each installation point, or wrap currentCert.Store in a thin helper that logs.
R3 — Invariant-critical comments stripped (Informational)
runtime/certstore.go, runtime/certmanager.go, runtime/lease.go
The commit removes several comments that explained non-obvious protocol invariants. Examples:
certstore.go / SaveCert: "This condition is the only thing containing a zombie issuer … Never make this write unconditional."certmanager.go / renew: "certETag is captured by the caller before the order begins and must not be refreshed here … Re-reading it immediately before the write would let a zombie issuer overwrite the peer that took over."certmanager.go / renew: "S3 first, memory second: installing a certificate whose write was rejected would leave this enclave serving a leaf no peer has."lease.go / leaseTTL: the heartbeat-at-ttl/3 clock-skew analysis.
The code still enforces these invariants correctly. The problem is future maintainers: without the explanations, the write-order constraint and the conditional-write guard look like choices that can be simplified when they are load-bearing protocol correctness. Restore the invariant comments, or move them to ARCHITECTURE.md with back-references from code.
Correct decisions worth noting
SetTLSKeyHashSourcearchitecture: registering a read-through closure rather than pushing a value is the right pattern. Both paths (self-signed viaSetTLSKeyHash, fleet ACME viaSetTLSKeyHashSource) are cleanly separated. Thesrcfallback (zero hash before the first cert) is safe — a client cannot reach attestation before the server is listening.acme_dns01.goremoval: inlining intoacme_issuer.goremoves thednsProviderinterface layer, which served no testing purpose —fakeRoute53already mocked at the AWS SDK boundary. Simpler.LoadOrCreateAccountKeyrefactor: the old 2-iteration loop with the cryptic "object kept disappearing" error is replaced with a cleaner single-retry. The 409-wrapping from M2 also flows throughsaveAccountKey→putConditionalcorrectly.TestAttestedHashAlwaysMatchesServedLeaf: exercises the right invariant — checks thathashes.Serialize()andGetCertificatereturn consistent values before and after a cert swap.TestCertManagerTickSurvivesVanishedCert:require.Sameon the pointer correctly verifies the previous bundle stays in service rather than being replaced by nil.
Summary. M1, M2, M4, and M5 are closed; the heartbeat timer leak (part of M3) is fixed. Two time.After calls remain in AcquireLease and awaitGenesis (M3 partial). R1 (nil guard) is a low-complexity fix. R3 (stripped comments) should be addressed before merge — they are the primary defence against a future change accidentally breaking the ordering invariants that prevent zombie issuers and cert/hash splits. The remaining open blocker from prior passes: M3 partial, N1 (6379 listener), and P2 (timeout equals grace period) require code changes.
- replace EstablishState with NewBoot / plan / finalise - determineMode picks genesis/resume/migration; each mode verifies its own preconditions - snapshot secrets become a map keyed by metadata; state_root hashes them name-sorted - decrypt secrets in config order — slice position picks the PCR register - rename state.go to boot.go
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
One commit since last pass (b53ccf50): refactor state.go → boot.go (explicit genesisBoot/resumeBoot/migrationBoot mode types). 0 of 16 prior issues fixed; all remain open. No certmanager/certstore/lease/infra files touched.
Prior-issue status
| ID | Status |
|---|---|
M1 — storeBundle atomicity |
Fixed (c7a2a4f) — no change this pass |
M2 — putConditional drops S3 409 |
Fixed (c7a2a4f) — no change this pass |
M3 — time.After leak (lease.go:101, state.go:89) |
Still open — boot.go:375 is exactly the code that was state.go:89, moved without being fixed; lease.go:101 unchanged |
M4 — GetHostedZone dead interface |
Fixed (c7a2a4f) — no change this pass |
M5 — tick() nil-bundle branch untested |
Fixed (c7a2a4f) — no change this pass |
| N1 — Missing 6379 NLB listener | Still open |
N2 — Moto service_started vs service_healthy |
Still open |
N3 — deregistration_delay = 30 doc gap |
Still open |
| P1 — Apply downtime documentation | Still open |
P2 — wait_for_capacity_timeout equals grace period |
Still open |
P3 — instance_id output removed without deprecation |
Still open |
P4 — Shell word-split in enclave_shutdown |
Still open |
R1 — tick() nil dereference guard (certmanager.go) |
Still open |
R2 — Certificate installation silent (certmanager.go) |
Still open |
R3 — Invariant comments stripped (certstore.go, certmanager.go, lease.go) |
Still open |
New findings — b53ccf50
S1 — time.After in awaitGenesis moved without being fixed (M3 is now worse to describe)
runtime/boot.go:375
case <-time.After(leasePollInterval):The code at state.go:89 was copied verbatim to boot.go:375. M3 called out two sites; this refactor was a natural moment to fix both (especially awaitGenesis, which now has a cleaner shape for the fix). With leasePollInterval = 5s and genesisLeaseWait = 10m, up to 120 un-stopped timers accumulate per waiting enclave. Fix: replace with time.NewTimer(leasePollInterval) + defer t.Stop() and reset inside the loop, matching what the heartbeat already does at lease.go:163.
S2 — awaitGenesis returns a live Lease when a peer commits between the lease win and the second plan() call (Low)
runtime/boot.go:361–378
lease, err := TryAcquireLease(waitCtx, b.s3, bucket, genesisLeaseName, leaseTTL)
if lease != nil {
if err := b.plan(waitCtx); err != nil {
_ = lease.Release(context.WithoutCancel(ctx))
return nil, ...
}
if genesis, stillGenesis := b.mode.(*genesisBoot); stillGenesis {
genesis.lease = lease
}
return lease, nil // non-nil even when b.mode is now *resumeBoot
}When TryAcquireLease wins but the second plan() finds the peer already committed (mode flips to *resumeBoot), awaitGenesis returns (lease, nil) with b.mode == *resumeBoot. finalise defers lease.Release, so there is no leak — the lease is correctly cleaned up. But it is held for the full duration of the resume-boot finalise path (NSM attestation verify, DEK decrypt, secret decrypt), blocking other potential waiters unnecessarily.
Fix: release the lease immediately when the second plan reveals the work is done:
if genesis, stillGenesis := b.mode.(*genesisBoot); stillGenesis {
genesis.lease = lease
return lease, nil
}
_ = lease.Release(context.WithoutCancel(ctx))
slog.Info("genesis completed by peer after lease won, resuming")
return nil, nilThere is also no test for this specific interleaving. TestAwaitGenesisSkipsLeaseWhenPeerCommitted covers the case where the peer committed before TryAcquireLease is called; the case above (commit after winning) is unexercised.
Correct decisions worth noting
- Explicit mode types (
genesisBoot,resumeBoot,migrationBoot) are a clear improvement over the old flag-matrix instate.go. The interface —verify,buildSnapshot,verifySnapshot,commitSnapshot— makes the per-mode invariants hard to violate accidentally. plan()/finalise()split: separating the SSM reads (plan) from the side-effecting work (finalise) makes each testable independently. The existing test suite exploits this well.TestEstablishLoadedStateRefusesCommitWithoutTheLease: directly tests the zombie-prevention path (lease lost mid-genesis → KMSKeyID not committed). This is the most important correctness invariant in genesis and it is now explicitly covered.TestBootResumesAfterPeerGenesis: verifies identical DEK across two calls toestablishon the same SSM/KMS — the right end-to-end test for the fleet-shared-key property.runBootPlanhelper: the comment// Keep it in step with finaliseis the right instruction; the divergence (noverifyPredecessorCommitment, noFetchOrCreatePrimaryKMS) is intentional and well-documented.migrate.gotype change ([]persistedSecret→map[StaticSecretMetadata]string): the old slice-of-structs was an internal type that leaked through theWriteTransitionReceiptboundary; using the same map type asbootSnapshot.staticSecretsremoves the impedance mismatch cleanly.
Summary. The refactor is architecturally sound and significantly clearer than the old code. The only new code-level items are S1 (M3 carried over unchanged at the new location) and S2 (lease held too long on a narrow race path). The correctness blockers from prior passes that still need addressing before merge: M3 (time.After in both boot.go:375 and lease.go:101), R1 (nil guard in certmanager), R3 (invariant comments), and P2 (wait_for_capacity_timeout numeric fix).
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Review — enclave#151 Horizontal scaling
What it does
Replaces per-response Schnorr signing (X-Attestation-Signature / X-Attestation-Pubkey headers) with attested TLS leaf pinning. The attestation document's UserData now carries "sha256:" || SHA256(TLS leaf DER). Clients verify once at attestation time, then trust the pinned connection.
Architectural reasoning (sound)
The stated rationale is correct: a per-boot Schnorr key cannot be shared across a fleet behind a load balancer. A client that verifies an attestation document from node A cannot validate a response signature produced by node B. A shared TLS certificate with a hash embedded in the attestation document has no such problem.
What to verify before merging
- Client breakage: Any client or SDK that currently reads
X-Attestation-SignatureorX-Attestation-Pubkeywill stop receiving those headers. Confirm all consumers (SDK,client/client.go, external integrators) are updated or don't depend on them. - Attestation UserData layout: The PR shifts
tlsKeyHashto bytes[7:39](replacing the priorsigningKeyHashat[36:68]). Any tooling or operator scripts that read UserData at the old offsets will silently read the wrong bytes. Verify the offset change is reflected everywhere. - Fleet test: The new
test-fleetmake target andfleetcompose profile are a good addition. Confirm this test runs in CI for this PR. - Pre-init responses: Previously responses before
initDonewere unsigned (explicitly noted). Confirm the TLS binding is present from boot, not only after init completes — a fleet member that crashes before init wouldn't have the leaf pinned in the attestation yet.
Verdict
The architectural direction is correct and well-reasoned. The implementation change to UserData layout and the removal of the signing middleware are potentially breaking for existing clients. Human review should confirm the rollout scope and client compatibility.
|
Changes were requested on this PR. @aruokhai need any help addressing the feedback? |
1 similar comment
|
Changes were requested on this PR. @aruokhai need any help addressing the feedback? |
|
M3 — won't fix. The finding assumes The proposed accumulation also can't occur here. The only The "boot storm with N enclaves" doesn't compound this: each enclave is a separate process/VM, and Using |
2 participants
Horizontal Scaling
Run N identical-PCR0 enclaves as a shared fleet, using the same IAM role, KMS key, SSM tree, storage bucket, KV table, and TLS certificate.
Status
The runtime is safe at N>1 today;
make test-fleetproves this with two concurrent enclaves. Real AWS deployment still requires the infrastructure work below.Shared State
Same-PCR0 enclaves derive the same DEK from the same KMS key, allowing any node to decrypt data sealed by another.
RecipientAttestation:PCR0, not instance identity./{deployment}/{app}/…tree, including the genesisKMSKeyID.Genesis
Genesis is serialized with an S3 lease before
CreateKey. This prevents concurrent cold boots from creating multiple keys and committing the wrong one.The lease uses S3 conditional writes and an in-memory ETag as the claim. Heartbeats run at TTL/3;
412means the lease was lost.409is a retryable S3 race, not contention.Because the SSM commit is unconditional,
lease.Verify()runs immediately before it to prevent an expired/descheduled holder from clobbering a newer owner.The lease is not a security boundary: the instance role can manipulate it. Compromise can cause denial of renewal, but cannot mint or read a certificate without the required external permissions and PCR0-bound decryption.
Fleet Certificate
tls-alpn-01cannot work reliably behind a load balancer, so the fleet uses DNS-01. ACME-enabled enclaves require a Route53 hosted zone.The certificate and key are stored together in one DEK-sealed S3 object. Renewal is serialized by an
acme-renewallease and performed proactively:Three invariants are critical:
412, reload the peer's certificate; never retry the ACME order.The certificate and attested hash are installed together through one
atomic.Pointer, preventing the node from serving one leaf while attesting another.Boot Gate
freshnessAnchor.Establishnow re-reads a stale KV key with a strongly consistentGetItembefore halting. This avoids bricking a joining node when a peer commits between the initial version-vector scan and anchor listing. Failed re-reads do not halt boot.Response Signing
The per-boot response-signing key is removed. It caused verification failures behind a load balancer because each enclave had a different key.
Authenticity now comes from attested TLS: clients pin the live certificate against
tlsKeyHash.user_datais now 39 bytes:"sha256:"plus the raw 32-byte digest.Infrastructure Still Required
cli/tofu_files.gostill deploys a single instance with an Elastic IP. A real fleet needs:count = var.instance_countonaws_instance.nitro; not an ASG, which would break existing migration/instance targeting and lifecycle behavior.:443and:6379. No ALB or TLS listener, which would terminate TLS and break attestation pinning.:443access to the NLB security group. CIDR rules cannot be used because instance-target NLBs preserve client source IPs.sts,logs,ssmmessages,ec2messages, and a DynamoDB gateway endpoint.DNS-01 means the host can technically mint certificates for the FQDN, but that does not bypass client trust: clients require the certificate to match an attested
tlsKeyHashbacked by PCR0-matching code.Testing
make test-fleetboots two enclaves from the same EIF, with distinct vsock ports supplied through the kernel command line.It verifies:
The strongest checks are B reading data sealed by A, proving shared DEK derivation, and
/test/pcr-secretsreturning the same value through both nodes, proving both decrypted the same ciphertext with the same KMS key.test-acmeandtest-fleetsharetest/app/tofuand must not run concurrently because teardown can delete.terraformduring the other test.Not Covered
Large-N single issuance and renewal contention are covered at unit level with
fakeS3, not end-to-end. Validate both on a two-instance staging deployment before merging the infrastructure changes.