Conversation
The runtime proved current code identity (PCR0) but not the origin of the persisted state it booted on, so a host with AWS-account access could pre-seed or swap the SSM artifacts the runtime owns (KMSKeyID, secret ciphertexts, storage DEK) and the genuine enclave would decrypt and run on attacker-known state. Introduce state-origin receipts: at genesis/migration the enclave emits an NSM-signed attestation whose user_data commits to a state_root over every artifact it owns. Every later boot recomputes the state_root and requires a valid receipt (AWS-root signature, PCR0 == self, user_data covers the recomputed root) before decrypting — otherwise it fails closed. An attacker cannot forge a receipt for the genuine PCR0: only the genuine EIF emits them, and only over state it generated itself. The commitment covers ciphertext hashes, not just KMSKeyID, so swapping a ciphertext under the accepted key (the EC2 role holds un-gated Encrypt/GenerateDataKey) changes the state_root and is rejected. - StateOrigin subsystem (state_origin.go): state_root over runtime-owned SSM artifacts (deterministic CBOR), receipt write/verify, and Establish() — classify -> ensureKey -> verify -> load -> write, owned in one place so "verify before decrypt, commit after load" is structural, not a convention Init must remember. - Migration: the predecessor writes a transition receipt over the successor's state_root (PCR31 -> successor) before flipping KMSKeyID; the successor verifies and adopts it, then writes its own receipt. Rollback-onto-self (predecessor == us) skips the PCR31 check, matching VerifyPredecessorCommitment. - Harden VerifyKeyAuthorization: reject un-gated kms:Decrypt and kms:PutKeyPolicy granted to anyone when locked / a non-root principal when unlocked. - Expose kms_key_locked in /v1/enclave-info. - Tests: state_root determinism, receipt accept/reject, the classification table, the ciphertext-swap fail-closed property, KMS policy posture, and rollback-onto-self; plus an integration tamper -> fail-closed step and a kms_key_locked assertion
Pin the live TLS leaf cert to the fingerprint signed into the enclave's NSM attestation (tlsKeyHash in user_data), closing a MITM gap: the HTTP client, `enclave verify`, and `enclave curl` previously skipped TLS verification, so a host could terminate TLS with its own cert, relay the genuine enclave's attestation, and read/tamper the channel while the client reported "verified". gRPC already pinned; this brings the other three paths to parity. Client: - Split into an unpinned bootstrap client (only GET /enclave/attestation) and a pinned client for everything else. verify() fetches the attestation, activates the pin, then runs PCR + key-binding checks over the pinned client, so a cert mismatch fails the handshake before any app request is sent. - Extract a shared verifyLeafCertPin used by both the HTTP and gRPC transports; reject empty/all-zero hashes (fail closed). - Add Options.StrictTLS (public CA/hostname validation on top of the pin) and Options.SkipKeyBinding (PCR0 + pin only); export PinnedHTTPClient. CLI: - `enclave curl`: attestation-verified by default (--expected-pcr0), routing the user's request over the pinned connection. --insecure opts out (warns on stderr); --strict-tls adds PKI validation; the two are mutually exclusive. - `enclave verify`: pins the live cert to the attested fingerprint after the unpinned bootstrap and runs remaining checks over it. Runtime: - setCertFingerprint fails closed when the chain has no non-CA leaf, instead of publishing an all-zero binding clients would reject. - Drop the unused MockCertFp escape hatch. Refactor: - Rename appKeyHash → signingKeyHash (field + SetSigningKeyHash setter) to parallel its sibling tlsKeyHash. - Move tests to their source-matching files: pin_test.go → client_test.go + verify_test.go, cert_fingerprint_test.go → runtime_test.go. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — PR #135: Pin the live TLS leaf cert
Verdict: APPROVE — this is a high-quality security hardening PR that closes a real MITM gap and adds significant defense-in-depth. Thorough, well-tested, correct fail-closed semantics throughout.
What this PR does (verified against the diff)
TLS cert pinning (#129): Splits the HTTP client into an unpinned bootstrap client (only GET /enclave/attestation) and a pinned client for everything else. The pin is activated after attestation verification, before any app request is sent. A shared verifyLeafCertPin() function covers HTTP, gRPC, and CLI paths — reject empty/all-zero hashes, reject missing peer certs, SHA-256 compare the DER leaf.
State-origin receipts (#131): NSM-signed receipts bind the SSM state (KMSKeyID + all secret ciphertexts + storage DEK) to the enclave's identity. Boot classification (genesis/resume/migration) fails closed on partial or ambiguous state. The canonical CBOR + domain-separated SHA-256 state_root is deterministic and sensitive to any ciphertext swap.
KMS policy posture verification (#131): verifyKeyPolicyPosture goes beyond policyAdmitsPCR0 — it now rejects un-gated kms:Decrypt (no PCR0 condition), rejects kms:PutKeyPolicy to non-root principals (unlocked) or anyone (locked), and treats kms:* / * wildcards as granting both.
SSM overlay hardening: nonOverridableEnv prevents the SSM env overlay from setting ENCLAVE_DEPLOYMENT or ENCLAVE_APP_NAME, closing the attack where an SSM writer sets ENCLAVE_DEPLOYMENT=dev to flip skipCOSEVerification().
Migration ordering fix: storePCR0WithAttestation now writes the attestation doc before the PCR0 commit point, and VerifyPredecessorCommitment fails closed on a recorded PCR0 with a missing attestation (was: return nil).
Security analysis — no issues found
-
Bootstrap/pin split is correct. The unpinned client only fetches
/enclave/attestation(public, no secrets). The pin is set inverify()viac.setPinHash(tlsKeyHash)before any call toverifyKeyBindingorverifyPCR0Chainruns on the pinnedc.httpClient. Cache hits inensureVerifiedskip re-verify but thepinHashpersists from the first verification. Pre-pin requests fail closed (verifyLeafCertPinrejects empty hash). -
setCertFingerprintfail-closed (runtime/runtime.go:978). Previously returnednilon a CA-only chain, leaving an all-zerotlsKeyHashthat clients would accept. Now returns an error. Tested inTestSetCertFingerprint. -
MockCertFpremoved. The test-only escape hatch that let a config value override the real cert fingerprint is gone. Good. -
State-origin
classify()is presence-only, verification is deferred. This is the right design — classification reads SSM cheaply, then the caller runs the expensive NSM-verified check the mode selects before decrypting anything. -
Migration transition receipt ordering (
runtime/migrate.go:261). Written before theKMSKeyIDflip (the atomic commit point), so the successor can verify the handoff on first boot. PCR31 already committed at this point. -
Rollback-onto-self (
state_origin.go:~line 145). WhenpredecessorPCR0Hex == ownPCR0Hex, PCR31 check is skipped (it commits to the failed forward target, not us). PCR0 + state_root coverage is the proof. Well-tested inTestVerifyMigrationTransitionReceipt_RollbackOntoSelf. -
verifyAttestationDocsignature change — removed therootsparameter, reads from the package-levelattestationRootsvar (nil = AWS Nitro root in production, test CA in tests). Fine for single-process runtime use. Tests uset.Cleanupto restore.
Test coverage — excellent
verifyLeafCertPin: match, mismatch, no cert, empty hash, all-zero hashPinnedHTTPClient: real TLS handshake (httptest.NewTLSServer) with matching/mismatching fingerprintssetCertFingerprint: CA-only chain errors, leaf sets fingerprintverifyKeyPolicyPosture: 25+ table-driven cases covering un-gated decrypt, wildcards, locked/unlocked postures, mixed principals- State origin: determinism, sensitivity, missing ciphertext errors
- Receipt verification: genuine, forged, wrong PCR0/purpose/state_root
- Migration transition receipt: genuine, wrong predecessor, wrong PCR31, rollback-onto-self
- Startup classification: genesis, resume, migration, 7 failure modes
nonOverridableEnv: overlay can't flipENCLAVE_DEPLOYMENT- Attestation verification: signed valid, forged rejected, untrusted signer, wrong PCR0, expired cert valid at timestamp
- Integration test extended with
kms_key_lockedcheck
Items to address (non-blocking)
1. Downstream prefix: → deployment: breakage (3 repos)
The config loader defaults Deployment to "dev" when the field is empty. Old enclave.yaml files with prefix: prod will silently ignore the value and use "dev" — a silent misconfiguration. Found in:
ee2e-kv/enclave/enclave.yamlee2e-kv/infra/enclave.yamlintrospector-review/enclave/enclave.yaml
Consider adding a backward-compat shim in loadConfigAt():
if cfg.Deployment == "" && cfg.Prefix != "" {
cfg.Deployment = cfg.Prefix
slog.Warn("enclave.yaml: 'prefix' is deprecated, rename to 'deployment'")
}2. InsecureTLS default semantic change
The old default for client.Options.InsecureTLS was effectively true (skip TLS verification). The new default is nil (pin to attested TLS hash). This is the correct direction, but any consumer calling client.New() without explicitly setting InsecureTLS will get different behavior. No external Go consumers were found in the monorepo, but this should be called out in release notes.
3. attestationRoots global var thread safety (runtime/attestation.go)
The package-level var attestationRoots *x509.CertPool is written by tests via useAttestationRoots() and read by verifyAttestationDoc(). Tests use t.Cleanup to restore, but parallel test execution could race. Consider adding t.Parallel() guards or documenting that attestation tests must not run in parallel. Non-blocking since the runtime is single-process and tests currently pass.
Cross-repo impact
- No external Go consumers of
client.New(),client.Options,SetAttestationKeyHash, orMockCertFpfound across all repos in/root/arkana/repos/. - PR #132 (verify_attestation) and PR #133 (enclave_restart) overlap on
runtime/runtime.go,runtime/attestation.go, andcli/curl.go— expect merge conflicts. - No open PRs on downstream consumer repos.
Not protocol-critical
This PR does not touch VTXOs, transaction signing, forfeit paths, round lifecycle, or exit paths. It is enclave framework security — important, but not protocol-level. No protocol-critical human review flag needed.
🤖 Reviewed by Arkana
The attested tlsKeyHash was sampled once, right after the first ACME cert was issued, then frozen for the life of the enclave. autocert silently renews Let's Encrypt leaves (~30 days before the 90-day expiry) and rewrites the cache, so after a renewal the live cert no longer matched the fingerprint in the attestation document — pinning clients would fail the TLS handshake with a fingerprint mismatch. Replace the one-shot goroutine in configureACME with watchACMECert, which polls the cert cache (fast until the first bind, hourly thereafter) and re-binds e.hashes.tlsKeyHash whenever autocert rotates the leaf. The watcher now also exits cleanly on Stop() instead of leaking. Make AttestationHashes thread-safe with a sync.RWMutex and SetTLSKeyHash/ SetSigningKeyHash setters, since tlsKeyHash is now written concurrently with the per-request reads in Serialize(). This also closes a pre-existing race on signingKeyHash, which the /enclave/hash handler could write while attestation requests read it. Extract the PEM-chain walk into a pure leafFingerprint helper shared by the self-signed and ACME paths, and add TestRebindCertFingerprint covering first issuance, no-op polls, and renewal re-binding.
|
closes #129 |
There was a problem hiding this comment.
Rather than duplicate the sensitive attestation verification and TLS pinning logic in both this file and the client package, we should remove the helpers in this file and delegate to the client package.
e.g.:
c, err := client.New(baseURL, client.Options{
ExpectedPCR0: expectedPCR0,
StrictTLS: strictTLS,
SkipKeyBinding: !doVerifyKey,
})
if err != nil {
return err
}
// runAllChecks is reduced to:
runCheck := verifyPCR0Chain(c)The curl command already does this.
| // InsecureTLS skips TLS certificate verification. Default: true. | ||
| // Nitriding uses self-signed certificates, so this is typically needed. | ||
| // InsecureTLS, when explicitly true, disables TLS cert pinning (raw, | ||
| // unverified TLS). Default pins to the attested tlsKeyHash (#129). |
There was a problem hiding this comment.
Downstream consumers of the client package don't really need to know the issue number (#129) or see it in the doc comments.
Similar with Set by `enclave curl` (#129) for SkipKeyBinding
There was a problem hiding this comment.
Nit: the actual waitSec is always a multiple of the hardcoded interval 5s
e.g.: waitSec = 6 and it doesn't connet/verify, the error will be: verification failed after 6s: ...
but it really waits for 10s
| e.tlsGetCert = mgr.GetCertificate | ||
|
|
||
| // Block on the cert appearing in the cache before computing its | ||
| // fingerprint. autocert populates the cache asynchronously after the | ||
| // first TLS handshake. | ||
| go func() { | ||
| var raw []byte | ||
| for { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) | ||
| got, err := cache.Get(ctx, e.cfg.FQDN) | ||
| cancel() | ||
| if err == nil { | ||
| raw = got | ||
| break | ||
| } | ||
| time.Sleep(5 * time.Second) | ||
| // autocert rewrites the cache on every renewal, so the attested fingerprint | ||
| // must track the cache rather than sample it once. | ||
| go e.watchACMECert(cache) | ||
| return nil |
There was a problem hiding this comment.
Instead of watching the cache, we could wrap mgr.GetCertificates which writes to the cache:
e.tlsGetCert = func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
cert, err := mgr.GetCertificate(hello)
if err == nil && cert != nil && len(cert.Certificate) > 0 {
// update hash if leaf cert changed
}
return cert, err
}https://cs.opensource.google/go/x/crypto/+/refs/tags/v0.48.0:acme/autocert/autocert.go;l=250
…r 0) The S3-backed K/V store sealed every object with nil AES-GCM AAD and a bare nonce||ct blob, so a host that can write S3 could relabel/swap ciphertext between keys or replay it across deployments and it still decrypted cleanly. Bind each object's AAD to "deployment/app/data/<key>" and frame a versioned envelope (0x01 || nonce || ct+tag). A relabeled or cross-deployment blob now fails the GCM tag check on Load. The version byte lets Load reject legacy unversioned objects and leaves room for a future Tier 1 format. Extract the seal/open into pure helpers (sealStorage / openStorage / storageAAD) as a single, S3-free chokepoint, and add the first unit coverage for storage crypto: round-trip, relabel / cross-deployment / cross-app rejection, tamper, short-blob, legacy-format, and wrong-DEK. Dynamic secrets and the ACME cert cache inherit the binding for free since they flow through Store/Load.
…ect-Lock rollback anchor
Replace the S3 `/v1/storage` HTTP API and dynamic secrets with a confidential,
rollback-resistant key/value store: a RESP (Redis-protocol) front-end over
DynamoDB, with values AES-256-GCM-sealed under the in-enclave DEK and AAD-bound
to {deployment, app, key, version, chunk}.
Rollback resistance (issue #134): every committed write is recorded as a
compliance-locked, DEK-sealed S3 object (anchor.go). A boot gate fails closed if
the live store is already rolled back, and a lazy per-read version-floor check
sets a halt flag (/health → 503, RESP refused) on regression. ENCLAVE_ANCHOR_WINDOW
is non-overridable so the operator can't wait out the Object Lock.
Redis surface (redcon, auth = runtime token, attestation-bound TLS):
- strings/keys, hashes, lists, sets, sorted sets, streams (sealed-CBOR blob
per collection)
- incremental SCAN (cursor + MATCH/COUNT/TYPE), INFO, CONFIG
- MULTI/EXEC/DISCARD/WATCH/UNWATCH (optimistic version-WATCH)
- SUBSCRIBE/PSUBSCRIBE/PUBLISH, HELLO (+AUTH) handshake
Infra: tofu provisions the KV table, Object-Lock anchor bucket, IAM, and SSM
params; the RESP listener stays enclave-internal. Storage is demoted to backing
only the ACME cert cache; its per-op metrics are dropped.
Integration: a real go-redis client drives every data type, transactions, SCAN,
and pub/sub end to end across a v1→v2 migration on QEMU + LocalStack.
…ocused files
Replace the monolithic Migrator with methods on *Runtime and introduce an
Nsm session type that owns a single NSM session for batched PCR ops, so
PCR0 is read once in Init and threaded into KMS instead of each subsystem
opening its own session. Deduplicate the SSM read helpers into stateless
readSSMParam/readSSMParamOptional and drop the redundant *Runtime wrappers.
Carve the HTTP plumbing out of runtime.go into cohesive files:
- middleware.go: signing/logging middleware, gatedMux, init-gating helpers,
isGRPCRequest (restored to package scope to fix the broken middleware_test)
- transport.go: protocol-switch reverse-proxy transport + CORS wrapper
- nsm.go: Nsm session type and PCR operations
- acme.go: merge of the former acme_cache.go + ACME directory client
Bind the ACME-served leaf via SetTLSKeyHashIfChanged so the attestation
re-binds (and logs) only on cert rotation rather than on every handshake.
Slim cli/verify.go by delegating attestation verification, TLS pinning, and
key binding to the client package, leaving only the PCR0 upgrade-chain check.
Move verifyPCR31Commitment to attestation.go and reorganize tests
(attestation_test.go, acme_test.go, runtime_test.go) to follow the new layout;
coverage is preserved across the renamed files.
…leware -> responseSignerMiddleware
Refactored Runtime Package.
2 participants
Pin the live TLS leaf cert to the fingerprint signed into the enclave's NSM
attestation (tlsKeyHash in user_data), closing a MITM gap: the HTTP client,
enclave verify, andenclave curlpreviously skipped TLS verification, so ahost could terminate TLS with its own cert, relay the genuine enclave's
attestation, and read/tamper the channel while the client reported "verified".
gRPC already pinned; this brings the other three paths to parity.
Client:
pinned client for everything else. verify() fetches the attestation, activates
the pin, then runs PCR + key-binding checks over the pinned client, so a cert
mismatch fails the handshake before any app request is sent.
reject empty/all-zero hashes (fail closed).
Options.SkipKeyBinding (PCR0 + pin only); export PinnedHTTPClient.
CLI:
enclave curl: attestation-verified by default (--expected-pcr0), routing theuser's request over the pinned connection. --insecure opts out (warns on
stderr); --strict-tls adds PKI validation; the two are mutually exclusive.
enclave verify: pins the live cert to the attested fingerprint after theunpinned bootstrap and runs remaining checks over it.
Runtime:
publishing an all-zero binding clients would reject.
Refactor:
parallel its sibling tlsKeyHash.
verify_test.go, cert_fingerprint_test.go → runtime_test.go.