fix(secrets): seal ed25519 recipients with X25519 ECDH#345
Merged
Conversation
The ed25519 recipient path derived the AEAD key from the recipient PUBLIC key (HKDF(publicKey, salt)) with no key agreement, so any party holding the public key (committed in .sc/secrets.yaml) could decrypt. Replace with an ephemeral-static X25519 sealed box: the key comes from the ECDH shared secret, so only the private-key holder can decrypt. Ed25519->X25519 conversion via the vetted filippo.io/edwards25519. - New encryptWithX25519/decryptWithX25519 (x25519.go); versioned blob with a magic prefix; the format magic + version + ephemeral pubkey are bound into the HKDF info and AEAD AAD. - EncryptLargeString routes ed25519 recipients to X25519; the broken encryptWithEd25519 is removed (legacy decrypt kept, read-only, for migration). - Private-key length guarded to avoid a Seed() panic; undersized blobs (below header+AEAD tag) rejected before ECDH to avoid wasted work. - Tests: round-trip (incl. empty plaintext), key-conversion consistency, wrong-recipient, non-determinism, version-tamper + ephemeral-key-tamper rejection, invalid-key handling, a regression proving public-key-only decrypt no longer works, and legacy-blob back-compat. - docs/guides/secrets-management: document RSA-OAEP vs ed25519 X25519 schemes. Reviewed by a multi-model panel over two rounds (Codex gpt-5.5 + Gemini + Claude crypto lens). Secrets ever encrypted to an ed25519 recipient must be rotated; git history retains the legacy ciphertext. Follow-up: remove the legacy decrypt path after migration. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Cre-eD
requested review from
Laboratory,
smecsia and
universe-ops
as code owners
Semgrep Scan ResultsRepository:
Scanned at 2026-06-27 16:03 UTC |
Security Scan ResultsRepository:
Scanned at 2026-06-27 16:04 UTC |
📊 Statement coverageMeasured on the documented included set (see
Baseline: |
Add coverage for the input-validation / malformed-input branches surfaced by the statement-coverage report: wrong-length and non-point recipient keys, too-short and non-magic blobs, and a low-order ephemeral key rejected by ECDH. Raises ed25519PublicKeyToX25519 to 100%, decryptWithX25519 to ~89%, encryptWithX25519 to ~81%; remaining gaps are unreachable infra-failure wraps (rand / AEAD constructor / HKDF read). Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
smecsia
approved these changes
Jun 27, 2026
universe-ops
approved these changes
Jun 27, 2026
This comment has been minimized.
This comment has been minimized.
Cre-eD
added a commit
that referenced
this pull request
Jun 28, 2026
Consolidates the keyless-secrets **design** and its **Phase-1 implementation** into one PR (supersedes #344, which was doc-only). ## 1. RFC — keyless secret backend (`docs/design/keyless-secrets/README.md`) Design for getting the master key out of GitHub: KMS-held recipients + OIDC, no behaviour change. Records the decision: **inline values + per-scope keys, via SOPS, file-per-scope** (`.sc/secrets.<scope>.yaml`) — avoids the multiplexed "partial-MAC paradox", gives per-scope key isolation. ## 2. Phase-1 — fail-closed store version guard (code + tests) Prerequisite for any future on-disk format change (incl. the SOPS migration above): - Adds `version` to the `secrets.yaml` schema (`EncryptedSecretFiles.Version`); absent/`0` = the original format (full back-compat). - `CurrentSecretsFileVersion = 0` — the highest schema version this build understands. - The reader **refuses** any store whose `version` exceeds what the build supports, instead of silently dropping the fields it can't model (which would corrupt the store on the next write). - Tests: rejects a newer version, accepts current/absent. **Why it ships first:** this reader must roll out fleet-wide *before* any higher-versioned store is ever written, or older binaries clobber it. It is a no-op for every existing (version-0) store today. Branch is synced with `main`, so history includes the already-merged #345 (X25519) — net new here = the guard + RFC + tests. Validated via a preview build (see comments). --------- Signed-off-by: Dmitrii Creed <creeed22@gmail.com> Co-authored-by: Ilya <smecsia@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
3 participants
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.
Summary
ssh-ed25519secret recipients were not confidential. The AEAD key was derived asHKDF(recipientPublicKey, salt)with no key agreement — both inputs are public (the recipient public key is committed in.sc/secrets.yaml; the salt is stored in the ciphertext), so anyone with read access to a repository could decrypt any value encrypted to anssh-ed25519recipient without a private key.ssh-rsarecipients (RSA-OAEP) are unaffected.This replaces the ed25519 path with an ephemeral-static X25519 sealed box: ed25519→X25519 conversion via the vetted
filippo.io/edwards25519, then ECDH shared secret → HKDF-SHA256 → ChaCha20-Poly1305. The key now derives from the ECDH shared secret, so only the holder of the ed25519 private key can decrypt.Changes
ciphers/x25519.go:encryptWithX25519/decryptWithX25519; versioned blob (magic | version | ephPub | nonce | ciphertext); magic + version + ephemeral pubkey bound into the HKDFinfoand the AEAD AAD.EncryptLargeStringroutesssh-ed25519recipients to X25519; the brokenencryptWithEd25519is removed. Legacy blobs stay decryptable (read-only) so existing stores can be migrated.Seed()panic); undersized blobs rejected before ECDH.docs/guides/secrets-management.mddocuments the RSA-OAEP vs ed25519-X25519 schemes.Action required for consumers
Fixing the code does not un-expose ciphertext already committed to git history. Rotate the credential values of any secret ever encrypted to an
ssh-ed25519recipient, then re-encrypt tossh-rsa/X25519 recipients and remove the ed25519 recipient.Follow-up
Remove the legacy ed25519 decrypt path once consumers have migrated.
Reviewed over two adversarial review rounds.