Conversation
… keys #879 wired signing in both directions but left it unreachable outside a laptop: the ConfigMap shipped `*_VERIFY_KEYSET_PATH` and `*_SIGNING_KID` as empty strings and said seed paths "belong in the Deployment beside the volume" — and no Deployment had one. The only volume in either was `data: emptyDir`, so there was no way to get a key into a pod. That blocks S6/S8/S9, each of which is defined as "reproducible on a local Kind cluster". * `scripts/gen_signing_keys.py` — no key generation existed anywhere, so enabling signing meant hand-rolling `os.urandom(32).hex()` and deriving public keys in a REPL. Seeds are written 0600 under a 0700 directory and **never printed**; only kids and public keys go to stdout, so a pasted terminal transcript cannot leak a private key. Public keys are derived from the seeds rather than typed, so the keyset cannot disagree with the keys it authorizes. Refuses to overwrite an existing seed without `--force`, since that key may already be deployed and approved. * Both Deployments mount a seed Secret at `/etc/ce-signing` and the keyset ConfigMap at `/etc/ce-keyset`, both `optional: true` — without that, every signing-less overlay would block in ContainerCreating waiting for a Secret that will never exist. No `defaultMode`: 0400 would be root-only and there is no `runAsUser`/`fsGroup` (OpenShift assigns the UID), so the default 0644 is what makes the file readable. * `k8s/base/keyset-configmap.yaml` ships **empty** and a test will keep it that way. "There is never a key in this file" is checkable; "only public keys in this file" is not, and a 32-byte seed is indistinguishable from a public key by length. * `k8s/overlays/kind-signed/` flips enforcement on. A separate overlay rather than patching `kind`, because S3/S5 are explicitly unsigned milestones and the before/after contrast is the demo — and because the `test` overlay's "mounts no credential" assertion should keep meaning something. **A per-service Secret, not one shared.** Caught while rendering the overlay: both Deployments initially mounted the same `event-signing-key`, which would have given the two services one seed. That collapses the asymmetry the design rests on — EventBridge would hold the key it verifies runner responses with and could forge them, which is the exact property that made HMAC unacceptable (DESIGN_PHASE2.md §4.3). Now `eventbridge-signing-key` and `eventrunner-signing-key`. This deviates from the single `event-signing-key` name in README_PHASE1.md:704-722; that section is rewritten in a later commit, as it also still describes a symmetric setup. Verified: `data` remains volume index 0 in all three existing overlays, so the demo overlay's `/spec/template/spec/volumes/0` PVC patch still targets the right volume; no overlay renders a `kind: Secret`; `kind` stays fully off while `kind-signed` renders enforcement plus both key paths; the rendered env maps onto the real config fields; and keys from the generator verify through `verify_with_keyset`. 551 passed, 5 skipped — including the three manifest tests this could have broken (`test_no_credential_is_ever_rendered_into_a_manifest`, `test_the_test_overlay_mounts_no_credential_so_mock_mode_is_automatic`, `test_no_manifest_sets_run_as_user`). ruff check and format clean. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
…it on teardown `k8s_deploy.py --signing-keys DIR` applies one seed Secret per service plus the approved-key ConfigMap, modelled on `ensure_ntfy_secret`: via stdin so the seed never reaches `ps` through argv, and a no-op when the flag is absent so a redeploy cannot silently switch enforcement off and strand a signed topic. It validates before applying, because every one of these mistakes produces a cluster that refuses all traffic with no local symptom — the service starts, signs under a kid nobody approved, and the other side rejects everything: * the key directory and each named seed file must exist, * `agents.json` must parse as a keyset and approve at least one kid, * the kid each service will sign under must be IN that keyset. Each failure names the file and the command that fixes it. Seeds are logged as a length only — not even a prefix. 4 hex chars of a 32-byte seed is 16 bits of a private key, and unlike an API token there is nothing to gain from identifying it. The keyset gets the opposite treatment on purpose: kids are logged in full, because "the operator deployed yesterday's key set" is otherwise invisible until it surfaces as an unexplained rejection. `--overlay kind-signed` without `--signing-keys` now aborts unless both Secrets are already deployed. Both volumes are `optional: true`, so without this the pods would start happily with enforcement on and no key, and refuse every event. Teardown: `--purge` now deletes both signing Secrets and the keyset ConfigMap. It also deletes `eventbridge-ntfy`, which was listed in neither the `doomed` preview nor the delete tuple — so a "purged" namespace kept a capability that lets anyone who learns it read and publish your notifications. Leaving private keys behind is worse: a later deploy silently inherits an identity the operator thought they had destroyed. 17 new tests covering generation and application, including that the two services are never given the same seed (a shared one would let EventBridge forge any agent's response — the HMAC property §4.3 rejected), that seeds land 0600 under a 0700 directory, that the CLI prints public keys but never a seed on either stream, and that each malformed-key-directory case is refused before it reaches a cluster. 568 passed, 5 skipped. ruff check and format clean. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — about half the plan. The key material is generated, mountable and applied at deploy time; the preflight check, e2e assertions, the forged-response demo stage and the
README_PHASE1rewrite are still to come (list at the bottom). Opening early because the Secret-handling and manifest decisions are where review is cheapest, and because one of them deviates from a documented convention on purpose.The problem
#879 wired signing in both directions, but none of it could be switched on in a cluster. The ConfigMap shipped
ER_/EB_VERIFY_KEYSET_PATHand*_SIGNING_KIDas empty strings andconfigmap.yaml:54said seed paths "belong in the Deployment beside the volume" — except no Deployment had one. The only volume in either wasdata: emptyDir, andgrep -rn "secretName\|configMap:" k8s/returned nothing. So the feature worked with local file paths on a laptop and was unreachable in Kind or OpenShift.That blocks the demo track: S6, S8 and S9 (#2596/#2598/#2599) are each defined as "reproducible on a local Kind cluster", and S9 is the forged-event teaching moment.
There was also no way to make a key.
shared/signing.pyhaspublic_key(seed)but nothing anywhere generated one — enabling signing meant hand-rollingos.urandom(32).hex()and deriving public keys in a REPL.What's here
scripts/gen_signing_keys.py— seeds at mode 0600 under a 0700 directory, and anagents.jsonwhose public keys are derived from those seeds rather than typed, so the keyset cannot disagree with the keys it authorizes. Never prints a seed on either stream; only kids and public keys, because a pasted terminal transcript is the most likely way a demo key escapes. Refuses to overwrite an existing seed without--force, since that key may already be deployed and in an approved keyset.Both Deployments mount a seed Secret at
/etc/ce-signingand the keyset ConfigMap at/etc/ce-keyset, bothoptional: true— without that, every signing-less overlay would block inContainerCreatingwaiting for a Secret that will never exist.k8s/base/keyset-configmap.yamlships empty. "There is never a key in this file" is checkable; "only public keys in this file" is not, and a 32-byte seed is indistinguishable from a public key by length.k8s/overlays/kind-signed/flips enforcement on. Separate overlay rather than a patch onkind, because S3/S5 are explicitly unsigned milestones and the before/after contrast is the demo.k8s_deploy.py --signing-keys DIRapplies one seed Secret per service plus the keyset ConfigMap, via stdin so the seed never reachespsthrough argv — modelled onensure_ntfy_secret, including the no-op-when-absent behaviour so a redeploy can't silently switch enforcement off and strand a signed topic. It validates first, because every one of these produces a cluster that refuses all traffic with no local symptom: the directory and each named seed must exist,agents.jsonmust parse and approve at least one kid, and the kid each service signs under must be in that keyset.One deliberate deviation, and one bug caught before it shipped
Per-service Secrets, not the documented single one. Both Deployments initially mounted the same
event-signing-key, which would have given the two services one shared seed. That collapses the asymmetry the design rests on — EventBridge would hold the key it verifies runner responses with and could forge them, which is the exact property that made HMAC unacceptable (DESIGN_PHASE2.md§4.3). Noweventbridge-signing-keyandeventrunner-signing-key, with a test asserting they never match.This deviates from
event-signing-keyas documented atREADME_PHASE1.md:704-722. That section needs rewriting regardless — it tells the operator to write a seed to/tmp/seed.hexandrmit, contradicting the stdin-only rule stated 230 lines earlier at:475, and it says "have the publisher sign with the same seed", describing the symmetric setup §4.3 explicitly rejected. The rewrite is in the remaining work; flagging it now so the name change isn't a surprise.Logging asymmetry is intentional. Seeds are logged as a length only — not even a prefix, unlike the API token, because 4 hex chars of a 32-byte seed is 16 bits of a private key and there's nothing to gain from identifying it. Kids are logged in full, because "the operator deployed yesterday's key set" is otherwise invisible until it surfaces as an unexplained rejection.
Also fixed a pre-existing leak:
--purgelistedeventbridge-ntfyin neither thedoomedpreview nor the delete tuple, so a "purged" namespace kept an ntfy topic — a capability that lets anyone who learns it both read and publish your notifications.Testing
568 passed, 5 skipped (5 cluster-gated). 17 new tests.
ruff checkandruff format --checkclean.The three manifest tests this could have broken all still pass, and they're why the design looks the way it does:
test_no_credential_is_ever_rendered_into_a_manifest(so the Secret is applied by the script, never declared — nosecretGenerator),test_the_test_overlay_mounts_no_credential_so_mock_mode_is_automatic(so enforcement stays out of the shared overlays), andtest_no_manifest_sets_run_as_user(so nodefaultMode: 0400, which would be root-only given OpenShift assigns the UID).Verified by rendering rather than assumed:
datais still volume index 0 in all three existing overlays, so the demo overlay'sop: replaceon/spec/template/spec/volumes/0still targets the PVC swap it means to.kind: Secret.kindstays fully off;kind-signedrenders enforcement plus both key paths, and those values map onto the realCfgfields.verify_with_keysetafter ato_kafka_binary/from_kafka_binaryround trip.Not verifiable in my sandbox, and not claimed:
kubectl apply --dry-run=clientneeds a reachable cluster to fetch the OpenAPI schema and fails even with--validate=false, and Kind can't run here (podman has no VM; creating one fails on the proxy's download cap). So schema validity of the new volume stanzas and the actual mount behaviour are unproven — covered bytest_every_object_is_accepted_by_the_api_serverwhen run against a real cluster.Still to do on this branch
k8s_preflight.pycheck 14 — Secret present, keyset parses, every kid the ConfigMap names is in itk8s_e2e_test.py— assert the terminal response carriesce_signatureand the startup banner says signing is ONk8s_demo_flow.py --forge— publish an unsigned event to the responses topic and assert it lands asphase="error"with the payload retained underdata["rejected"]. Mechanism already validated against a live broker:kafka-console-producer.sh --property parse.headers=trueyields a CloudEvent the consumer parses and the verifier rejects (binary mode matters — a structured-mode body givesattrs == {})README_PHASE1.md§"Signed events"Question for review
Is
kind-signedthe right shape? The alternative is a--signedflag on the existingkindoverlay. I went with a separate overlay so the unsigned path keeps working for S3/S5 and so thetestoverlay's no-credential assertion keeps meaning something — but the name is about to get baked into three more files (preflight, e2e, demo flow), so it's cheap to change now and annoying later.Separately: S6 (#2596) specifies ES256 + a SPIFFE-derived
ce-source+ rotation, and this is Ed25519 + a static keyset. The eventing design docs never commit to ES256 — only the issue does, and §4.3 argues Ed25519 on merits with SPIRE as a later swap that leaves verification logic unchanged. Worth an explicit decision rather than a surprise at demo time; I'll comment on #2596.Assisted-By: Claude Code