From 5da51876e3b3ebb01985cd374d9a6fd3a938d58c Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 27 Jun 2026 18:02:41 +0400 Subject: [PATCH 1/2] fix(secrets): seal ed25519 recipients with X25519 ECDH 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 --- docs/docs/guides/secrets-management.md | 13 +- go.mod | 3 +- go.sum | 2 + .../secrets/ciphers/coverage_extra_test.go | 7 +- pkg/api/secrets/ciphers/encryption.go | 62 ++---- pkg/api/secrets/ciphers/x25519.go | 188 ++++++++++++++++ pkg/api/secrets/ciphers/x25519_test.go | 209 ++++++++++++++++++ 7 files changed, 435 insertions(+), 49 deletions(-) create mode 100644 pkg/api/secrets/ciphers/x25519.go create mode 100644 pkg/api/secrets/ciphers/x25519_test.go diff --git a/docs/docs/guides/secrets-management.md b/docs/docs/guides/secrets-management.md index 35d3ae2b..131c8917 100644 --- a/docs/docs/guides/secrets-management.md +++ b/docs/docs/guides/secrets-management.md @@ -1,12 +1,12 @@ # Secrets Management with Simple Container -This comprehensive guide covers how to manage secrets and confidential files using Simple Container's built-in secrets management system. Simple Container uses SSH-RSA encryption to securely store and share secrets within your team while maintaining them in your Git repository. +This comprehensive guide covers how to manage secrets and confidential files using Simple Container's built-in secrets management system. Simple Container encrypts secrets to your team's SSH public keys (RSA and ed25519) so they can be stored and shared safely in your Git repository. ## Overview Simple Container's secrets management provides: -- **SSH-RSA encryption** for secure secret storage +- **Public-key encryption** to SSH recipients (RSA and ed25519) for secure secret storage - **Team-based access control** with public key management - **Git-native workflow** for secret versioning and collaboration - **Built-in commands** for easy secret lifecycle management @@ -24,12 +24,19 @@ Simple Container uses a multi-key encryption approach: This means that when you run `sc secrets reveal`, the system uses your private key to decrypt secrets that were encrypted with your corresponding public key (along with all other team members' public keys). +### Cipher per recipient type + +Each recipient public key gets its own encryption of the secret, using a scheme appropriate to the key type: + +- **`ssh-rsa` recipients** — RSA-OAEP (SHA-256). +- **`ssh-ed25519` recipients** — an ephemeral-static **X25519 ECDH** sealed box (HKDF-SHA256 + ChaCha20-Poly1305): the content key is derived from the ECDH shared secret, so only the holder of the ed25519 private key can decrypt. + ## Prerequisites Before working with secrets, ensure you have: - Simple Container CLI installed -- SSH RSA key pair (2048-bit supported) +- An SSH key pair — RSA (2048-bit) or ed25519 - Access to the project repository ## Configuration Override diff --git a/go.mod b/go.mod index 0230ba7b..997f0ccd 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.4 require ( cloud.google.com/go/storage v1.62.3 + filippo.io/edwards25519 v1.2.0 github.com/MShekow/directory-checksum v1.4.18 github.com/anthonycorbacho/slack-webhook v1.0.1 github.com/antonmedv/expr v1.12.6 @@ -19,6 +20,7 @@ require ( github.com/compose-spec/compose-go v1.20.2 github.com/containerd/platforms v0.2.1 github.com/disgoorg/disgo v0.19.6 + github.com/disgoorg/snowflake/v2 v2.0.3 github.com/fatih/color v1.19.0 github.com/go-delve/delve v1.26.3 github.com/go-git/go-billy/v5 v5.9.0 @@ -182,7 +184,6 @@ require ( github.com/derekparker/trie/v3 v3.2.0 // indirect github.com/disgoorg/json/v2 v2.0.0 // indirect github.com/disgoorg/omit v1.0.0 // indirect - github.com/disgoorg/snowflake/v2 v2.0.3 // indirect github.com/distribution/reference v0.6.0 // indirect github.com/djherbis/times v1.5.0 // indirect github.com/dlclark/regexp2 v1.11.0 // indirect diff --git a/go.sum b/go.sum index 5d89036c..97383b3e 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ cloud.google.com/go/trace v1.11.7 h1:kDNDX8JkaAG3R2nq1lIdkb7FCSi1rCmsEtKVsty7p+U cloud.google.com/go/trace v1.11.7/go.mod h1:TNn9d5V3fQVf6s4SCveVMIBS2LJUqo73GACmq/Tky0s= dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= +filippo.io/edwards25519 v1.2.0 h1:crnVqOiS4jqYleHd9vaKZ+HKtHfllngJIiOpNpoJsjo= +filippo.io/edwards25519 v1.2.0/go.mod h1:xzAOLCNug/yB62zG1bQ8uziwrIqIuxhctzJT18Q77mc= github.com/4meepo/tagalign v1.4.2 h1:0hcLHPGMjDyM1gHG58cS73aQF8J4TdVR96TZViorO9E= github.com/4meepo/tagalign v1.4.2/go.mod h1:+p4aMyFM+ra7nb41CnFG6aSDXqRxU/w1VQqScKqDARI= github.com/Abirdcfly/dupword v0.1.3 h1:9Pa1NuAsZvpFPi9Pqkd93I7LIYRURj+A//dFd5tgBeE= diff --git a/pkg/api/secrets/ciphers/coverage_extra_test.go b/pkg/api/secrets/ciphers/coverage_extra_test.go index 40251a60..38efee69 100644 --- a/pkg/api/secrets/ciphers/coverage_extra_test.go +++ b/pkg/api/secrets/ciphers/coverage_extra_test.go @@ -127,9 +127,10 @@ func TestDecryptLargeStringWithEd25519_TruncatedAfterHeader(t *testing.T) { raw, err := base64.StdEncoding.DecodeString(enc[0]) Expect(err).ToNot(HaveOccurred()) - // Keep salt(32)+nonce(12) and a single body byte: passes the length guard - // (>= 44) but cannot hold a 16-byte AEAD tag, so Open fails. - truncated := base64.StdEncoding.EncodeToString(raw[:45]) + // Keep the X25519 header + a truncated ciphertext: longer than the 69-byte + // minimum (magic+ver+ephPub+nonce+tag) so it passes the length guard, but + // corrupted, so Open fails. + truncated := base64.StdEncoding.EncodeToString(raw[:70]) _, err = DecryptLargeStringWithEd25519(priv, []string{truncated}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to decrypt data")) diff --git a/pkg/api/secrets/ciphers/encryption.go b/pkg/api/secrets/ciphers/encryption.go index ec79ed6a..b9f0b19d 100644 --- a/pkg/api/secrets/ciphers/encryption.go +++ b/pkg/api/secrets/ciphers/encryption.go @@ -160,10 +160,13 @@ func EncryptLargeString(key crypto.PublicKey, s string) ([]string, error) { res[idx] = base64.StdEncoding.EncodeToString(encryptedData) } } else if ed25519Key, ok := key.(ed25519.PublicKey); ok { - // For ed25519, use hybrid encryption with Curve25519 + ChaCha20-Poly1305 - encryptedData, err := encryptWithEd25519(ed25519Key, []byte(s)) + // ed25519 recipients are sealed via ephemeral-static X25519 ECDH + // (see x25519.go): the AEAD key is derived from the ECDH shared secret, + // so only the holder of the private key can decrypt. The previous scheme + // derived the key from the public key alone and is no longer produced. + encryptedData, err := encryptWithX25519(ed25519Key, []byte(s)) if err != nil { - return nil, errors.Wrapf(err, "failed to encrypt secret with ed25519") + return nil, errors.Wrapf(err, "failed to encrypt secret for ed25519 recipient") } res = []string{base64.StdEncoding.EncodeToString(encryptedData)} } else { @@ -199,49 +202,24 @@ func DecryptLargeStringWithEd25519(key ed25519.PrivateKey, chunks []string) ([]b if err != nil { return nil, errors.Wrapf(err, "failed to decode base64 string") } + // New blobs use the X25519 sealed box (see x25519.go). Legacy blobs predate + // it and are read-only here solely to support migration/re-encryption — they + // are NOT confidential (the key was derived from public data); rotate any + // secret ever stored in one. + if isX25519Blob(chunkBytes) { + return decryptWithX25519(key, chunkBytes) + } return decryptWithEd25519(key, chunkBytes) } -// encryptWithEd25519 performs hybrid encryption using HKDF key derivation and ChaCha20-Poly1305 -func encryptWithEd25519(publicKey ed25519.PublicKey, plaintext []byte) ([]byte, error) { - // Generate a random salt for HKDF - salt := make([]byte, 32) - if _, err := rand.Read(salt); err != nil { - return nil, errors.Wrap(err, "failed to generate salt") - } - - // Use HKDF to derive encryption key from ed25519 public key and salt - hkdfReader := hkdf.New(sha256.New, publicKey, salt, []byte("ed25519-chacha20poly1305")) - encryptionKey := make([]byte, 32) - if _, err := hkdfReader.Read(encryptionKey); err != nil { - return nil, errors.Wrap(err, "failed to derive encryption key") - } - - // Create ChaCha20-Poly1305 cipher - cipher, err := chacha20poly1305.New(encryptionKey) - if err != nil { - return nil, errors.Wrap(err, "failed to create cipher") - } - - // Generate nonce - nonce := make([]byte, chacha20poly1305.NonceSize) - if _, err := rand.Read(nonce); err != nil { - return nil, errors.Wrap(err, "failed to generate nonce") - } - - // Encrypt the plaintext - ciphertext := cipher.Seal(nil, nonce, plaintext, nil) - - // Combine salt + nonce + ciphertext - result := make([]byte, 32+len(nonce)+len(ciphertext)) - copy(result[0:32], salt) - copy(result[32:32+len(nonce)], nonce) - copy(result[32+len(nonce):], ciphertext) - - return result, nil -} +// NOTE: the former encryptWithEd25519 was removed. It derived the AEAD key from +// the recipient's PUBLIC key (HKDF(publicKey, salt)) with no key agreement, so +// the ciphertext was decryptable by anyone holding the public key — i.e. zero +// confidentiality. ed25519 recipients are now sealed via X25519 ECDH (x25519.go). -// decryptWithEd25519 performs hybrid decryption using HKDF key derivation and ChaCha20-Poly1305 +// decryptWithEd25519 reads a LEGACY (pre-X25519) ed25519 blob. Kept only so old +// stores can be decrypted for migration/re-encryption. Such blobs are NOT +// confidential — rotate any secret ever stored in one. New blobs use X25519. func decryptWithEd25519(privateKey ed25519.PrivateKey, ciphertext []byte) ([]byte, error) { if len(ciphertext) < 32+chacha20poly1305.NonceSize { return nil, errors.New("ciphertext too short") diff --git a/pkg/api/secrets/ciphers/x25519.go b/pkg/api/secrets/ciphers/x25519.go new file mode 100644 index 00000000..3c2d07bb --- /dev/null +++ b/pkg/api/secrets/ciphers/x25519.go @@ -0,0 +1,188 @@ +package ciphers + +import ( + "bytes" + "crypto/ecdh" + "crypto/ed25519" + "crypto/rand" + "crypto/sha256" + "crypto/sha512" + "io" + + "filippo.io/edwards25519" + "github.com/pkg/errors" + "golang.org/x/crypto/chacha20poly1305" + "golang.org/x/crypto/hkdf" +) + +// x25519Magic marks a sealed-box blob produced by encryptWithX25519, so +// DecryptLargeStringWithEd25519 can route new blobs to the X25519 path while +// still reading legacy (pre-fix) blobs during migration. Legacy blobs began +// with a 32-byte random salt, so this fixed 8-byte prefix collides with a +// legacy blob with probability 2^-64. +var x25519Magic = []byte("scx25519") + +const x25519Version byte = 1 + +// ed25519PublicKeyToX25519 maps an Ed25519 public key (an Edwards point) to the +// equivalent X25519 (Montgomery u-coordinate) public key — the standard +// birational map also used by age/agessh. Conversion is delegated to the vetted +// filippo.io/edwards25519 rather than hand-rolled. +func ed25519PublicKeyToX25519(pub ed25519.PublicKey) ([]byte, error) { + if len(pub) != ed25519.PublicKeySize { + return nil, errors.Errorf("invalid ed25519 public key size: %d", len(pub)) + } + p, err := new(edwards25519.Point).SetBytes(pub) + if err != nil { + return nil, errors.Wrap(err, "failed to decode ed25519 public key as an edwards point") + } + return p.BytesMontgomery(), nil +} + +// ed25519PrivateKeyToX25519 maps an Ed25519 private key to the equivalent X25519 +// private scalar: clamp(SHA-512(seed)[:32]) per RFC 8032 / RFC 7748. Clamping is +// idempotent; crypto/ecdh also clamps internally on use. +func ed25519PrivateKeyToX25519(priv ed25519.PrivateKey) []byte { + h := sha512.Sum512(priv.Seed()) + s := h[:32] + s[0] &= 248 + s[31] &= 127 + s[31] |= 64 + return s +} + +// encryptWithX25519 seals plaintext to an Ed25519 recipient using ephemeral-static +// X25519 ECDH → HKDF-SHA256 → ChaCha20-Poly1305 (a sealed box). The AEAD key is +// derived from the ECDH *shared secret*, so only the recipient's private key can +// decrypt — unlike the removed legacy scheme, whose key was derived from the +// public key alone (and was therefore decryptable by anyone with read access). +// +// Blob layout: magic(8) | version(1) | ephPub(32) | nonce(12) | ciphertext. +func encryptWithX25519(recipientEd25519Pub ed25519.PublicKey, plaintext []byte) ([]byte, error) { + xPub, err := ed25519PublicKeyToX25519(recipientEd25519Pub) + if err != nil { + return nil, err + } + curve := ecdh.X25519() + recipientPub, err := curve.NewPublicKey(xPub) + if err != nil { + return nil, errors.Wrap(err, "failed to build recipient X25519 public key") + } + ephPriv, err := curve.GenerateKey(rand.Reader) + if err != nil { + return nil, errors.Wrap(err, "failed to generate ephemeral X25519 key") + } + ephPub := ephPriv.PublicKey().Bytes() + shared, err := ephPriv.ECDH(recipientPub) + if err != nil { + return nil, errors.Wrap(err, "ephemeral ECDH failed") + } + key, err := deriveX25519Key(shared, ephPub, xPub) + if err != nil { + return nil, err + } + aead, err := chacha20poly1305.New(key) + if err != nil { + return nil, errors.Wrap(err, "failed to create AEAD") + } + nonce := make([]byte, chacha20poly1305.NonceSize) + if _, err := rand.Read(nonce); err != nil { + return nil, errors.Wrap(err, "failed to generate nonce") + } + ct := aead.Seal(nil, nonce, plaintext, x25519AAD(ephPub)) + + blob := make([]byte, 0, len(x25519Magic)+1+len(ephPub)+len(nonce)+len(ct)) + blob = append(blob, x25519Magic...) + blob = append(blob, x25519Version) + blob = append(blob, ephPub...) + blob = append(blob, nonce...) + blob = append(blob, ct...) + return blob, nil +} + +// decryptWithX25519 reverses encryptWithX25519 using the recipient's Ed25519 +// private key (converted to X25519). +func decryptWithX25519(recipientEd25519Priv ed25519.PrivateKey, blob []byte) ([]byte, error) { + // ed25519.PrivateKey.Seed() panics on a wrong-sized key; fail safely instead. + if len(recipientEd25519Priv) != ed25519.PrivateKeySize { + return nil, errors.Errorf("invalid ed25519 private key size: %d", len(recipientEd25519Priv)) + } + // Minimum = magic + version + ephPub + nonce + the 16-byte AEAD tag. Reject + // undersized blobs before doing ECDH/HKDF so trivially-invalid input can't + // burn CPU. + headerLen := len(x25519Magic) + 1 + 32 + chacha20poly1305.NonceSize + chacha20poly1305.Overhead + if len(blob) < headerLen { + return nil, errors.New("x25519 ciphertext too short") + } + off := len(x25519Magic) + if !bytes.Equal(blob[:off], x25519Magic) { + return nil, errors.New("not an x25519 blob") + } + if blob[off] != x25519Version { + return nil, errors.Errorf("unsupported x25519 blob version: %d", blob[off]) + } + off++ + ephPub := blob[off : off+32] + off += 32 + nonce := blob[off : off+chacha20poly1305.NonceSize] + off += chacha20poly1305.NonceSize + ct := blob[off:] + + curve := ecdh.X25519() + xPriv, err := curve.NewPrivateKey(ed25519PrivateKeyToX25519(recipientEd25519Priv)) + if err != nil { + return nil, errors.Wrap(err, "failed to build recipient X25519 private key") + } + ephPubKey, err := curve.NewPublicKey(ephPub) + if err != nil { + return nil, errors.Wrap(err, "failed to parse ephemeral public key") + } + shared, err := xPriv.ECDH(ephPubKey) + if err != nil { + return nil, errors.Wrap(err, "ECDH failed") + } + key, err := deriveX25519Key(shared, ephPub, xPriv.PublicKey().Bytes()) + if err != nil { + return nil, err + } + aead, err := chacha20poly1305.New(key) + if err != nil { + return nil, errors.Wrap(err, "failed to create AEAD") + } + pt, err := aead.Open(nil, nonce, ct, x25519AAD(ephPub)) + if err != nil { + return nil, errors.Wrap(err, "failed to decrypt data") + } + return pt, nil +} + +// deriveX25519Key derives the AEAD key from the ECDH shared secret, binding it to +// both public keys so a wrapped blob cannot be transplanted between recipients. +func deriveX25519Key(shared, ephPub, recipientXPub []byte) ([]byte, error) { + info := make([]byte, 0, len(x25519Magic)+1+len(ephPub)+len(recipientXPub)) + info = append(info, x25519Magic...) + info = append(info, x25519Version) + info = append(info, ephPub...) + info = append(info, recipientXPub...) + r := hkdf.New(sha256.New, shared, nil, info) + key := make([]byte, chacha20poly1305.KeySize) + if _, err := io.ReadFull(r, key); err != nil { + return nil, errors.Wrap(err, "failed to derive key") + } + return key, nil +} + +// x25519AAD binds the format magic, version, and ephemeral public key into the +// AEAD's associated data so none of them can be altered without failing Open. +func x25519AAD(ephPub []byte) []byte { + aad := make([]byte, 0, len(x25519Magic)+1+len(ephPub)) + aad = append(aad, x25519Magic...) + aad = append(aad, x25519Version) + aad = append(aad, ephPub...) + return aad +} + +// isX25519Blob reports whether b was produced by encryptWithX25519. +func isX25519Blob(b []byte) bool { + return len(b) >= len(x25519Magic) && bytes.Equal(b[:len(x25519Magic)], x25519Magic) +} diff --git a/pkg/api/secrets/ciphers/x25519_test.go b/pkg/api/secrets/ciphers/x25519_test.go new file mode 100644 index 00000000..8ebb4fec --- /dev/null +++ b/pkg/api/secrets/ciphers/x25519_test.go @@ -0,0 +1,209 @@ +package ciphers + +import ( + "crypto/ecdh" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "io" + "testing" + + . "github.com/onsi/gomega" + "golang.org/x/crypto/chacha20poly1305" + "golang.org/x/crypto/hkdf" +) + +func TestX25519_RoundTrip(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + msg := []byte("super secret 🔐\nDB_PASSWORD=hunter2\nAPI_KEY=abc") + blob, err := encryptWithX25519(pub, msg) + Expect(err).ToNot(HaveOccurred()) + Expect(isX25519Blob(blob)).To(BeTrue()) + + got, err := decryptWithX25519(priv, blob) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(Equal(msg)) +} + +// TestX25519_KeyConversionConsistency proves the Ed25519->X25519 birational map: +// the X25519 public derived from the converted private must equal the X25519 +// public converted directly from the Ed25519 public. +func TestX25519_KeyConversionConsistency(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + xPub, err := ed25519PublicKeyToX25519(pub) + Expect(err).ToNot(HaveOccurred()) + + xPriv, err := ecdh.X25519().NewPrivateKey(ed25519PrivateKeyToX25519(priv)) + Expect(err).ToNot(HaveOccurred()) + Expect(xPriv.PublicKey().Bytes()).To(Equal(xPub)) +} + +func TestX25519_WrongRecipientCannotDecrypt(t *testing.T) { + RegisterTestingT(t) + _, pubA, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + privB, _, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + blob, err := encryptWithX25519(pubA, []byte("for A only")) + Expect(err).ToNot(HaveOccurred()) + _, err = decryptWithX25519(privB, blob) + Expect(err).To(HaveOccurred()) +} + +func TestX25519_NonDeterministic(t *testing.T) { + RegisterTestingT(t) + _, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + a, err := encryptWithX25519(pub, []byte("same")) + Expect(err).ToNot(HaveOccurred()) + b, err := encryptWithX25519(pub, []byte("same")) + Expect(err).ToNot(HaveOccurred()) + Expect(a).NotTo(Equal(b)) // fresh ephemeral key each time +} + +// --- security regression: the public-key-only decrypt vuln is fixed --- + +// legacyBrokenEncrypt reproduces the REMOVED encryptWithEd25519: the AEAD key was +// derived from the recipient PUBLIC key + salt, with no key agreement. +func legacyBrokenEncrypt(pub []byte, pt []byte) []byte { + salt := make([]byte, 32) + _, _ = rand.Read(salt) + r := hkdf.New(sha256.New, pub, salt, []byte("ed25519-chacha20poly1305")) + key := make([]byte, 32) + _, _ = io.ReadFull(r, key) + aead, _ := chacha20poly1305.New(key) + nonce := make([]byte, chacha20poly1305.NonceSize) + _, _ = rand.Read(nonce) + ct := aead.Seal(nil, nonce, pt, nil) + return append(append(append([]byte{}, salt...), nonce...), ct...) +} + +// publicOnlyDecryptLegacy mounts the original attack: derive the key from the +// PUBLIC key + the salt prefix, then open. Works on legacy blobs, must fail on +// X25519 blobs. +func publicOnlyDecryptLegacy(pub, blob []byte) ([]byte, error) { + salt, nonce, ct := blob[0:32], blob[32:44], blob[44:] + r := hkdf.New(sha256.New, pub, salt, []byte("ed25519-chacha20poly1305")) + key := make([]byte, 32) + _, _ = io.ReadFull(r, key) + aead, _ := chacha20poly1305.New(key) + return aead.Open(nil, nonce, ct, nil) +} + +func TestX25519_FixesPublicKeyDecryptVuln(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + secret := []byte("DB_PASSWORD=hunter2") + + // 1) Legacy scheme leaked: decryptable with the PUBLIC key alone. + legacy := legacyBrokenEncrypt(pub, secret) + got, err := publicOnlyDecryptLegacy(pub, legacy) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(Equal(secret)) + + // 2) X25519 scheme: the same public-only attack must fail. Strip magic+ver so + // the attack "sees" an ephPub|nonce|ct prefix shaped like the old salt|nonce. + fixed, err := encryptWithX25519(pub, secret) + Expect(err).ToNot(HaveOccurred()) + _, err = publicOnlyDecryptLegacy(pub, fixed[len(x25519Magic)+1:]) + Expect(err).To(HaveOccurred()) + + // 3) The real private key still decrypts. + got, err = decryptWithX25519(priv, fixed) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(Equal(secret)) +} + +// --- migration back-compat: legacy blobs still readable through the router --- + +func TestEd25519_LegacyBlobStillDecryptsViaRouter(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + secret := []byte("legacy migration value") + + chunk := base64.StdEncoding.EncodeToString(legacyBrokenEncrypt(pub, secret)) + got, err := DecryptLargeStringWithEd25519(priv, []string{chunk}) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(Equal(secret)) +} + +func TestEd25519_PublicAPIProducesX25519(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + chunks, err := EncryptLargeString(pub, "hello world") + Expect(err).ToNot(HaveOccurred()) + Expect(chunks).To(HaveLen(1)) + + raw, err := base64.StdEncoding.DecodeString(chunks[0]) + Expect(err).ToNot(HaveOccurred()) + Expect(isX25519Blob(raw)).To(BeTrue()) // new format, not the legacy scheme + + got, err := DecryptLargeStringWithEd25519(priv, chunks) + Expect(err).ToNot(HaveOccurred()) + Expect(string(got)).To(Equal("hello world")) +} + +func TestX25519_VersionByteIsRejectedWhenTampered(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + blob, err := encryptWithX25519(pub, []byte("secret")) + Expect(err).ToNot(HaveOccurred()) + // Flip the version byte (immediately after the magic). It is bound into the + // AEAD AAD + HKDF info and explicitly checked, so decrypt must reject it. + blob[len(x25519Magic)] ^= 0xff + _, err = decryptWithX25519(priv, blob) + Expect(err).To(HaveOccurred()) +} + +func TestX25519_InvalidPrivateKeyLengthErrorsNotPanics(t *testing.T) { + RegisterTestingT(t) + priv, _, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + // A wrong-sized key would panic in ed25519.PrivateKey.Seed(); we must return + // a clean error instead. + _, err = decryptWithX25519(priv[:10], []byte("not-a-real-blob")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid ed25519 private key size")) +} + +// TestX25519_TamperedEphemeralKeyFails makes the AAD/HKDF binding load-bearing: +// the ephemeral public key has no explicit equality check, so altering it can +// only be caught cryptographically (ECDH + HKDF info + AEAD AAD all change). +func TestX25519_TamperedEphemeralKeyFails(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + blob, err := encryptWithX25519(pub, []byte("secret value")) + Expect(err).ToNot(HaveOccurred()) + // ephPub begins right after magic(8)+version(1). + blob[len(x25519Magic)+1] ^= 0xff + _, err = decryptWithX25519(priv, blob) + Expect(err).To(HaveOccurred()) +} + +func TestX25519_EmptyPlaintextRoundTrip(t *testing.T) { + RegisterTestingT(t) + priv, pub, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + blob, err := encryptWithX25519(pub, []byte{}) + Expect(err).ToNot(HaveOccurred()) + got, err := decryptWithX25519(priv, blob) + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(BeEmpty()) +} From 8655b6e65ce6fde68adece1791e482ad28519e0e Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 27 Jun 2026 20:02:59 +0400 Subject: [PATCH 2/2] test(secrets): cover x25519 error paths 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 --- pkg/api/secrets/ciphers/x25519_test.go | 53 ++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/pkg/api/secrets/ciphers/x25519_test.go b/pkg/api/secrets/ciphers/x25519_test.go index 8ebb4fec..abfaf2a7 100644 --- a/pkg/api/secrets/ciphers/x25519_test.go +++ b/pkg/api/secrets/ciphers/x25519_test.go @@ -2,6 +2,7 @@ package ciphers import ( "crypto/ecdh" + "crypto/ed25519" "crypto/rand" "crypto/sha256" "encoding/base64" @@ -207,3 +208,55 @@ func TestX25519_EmptyPlaintextRoundTrip(t *testing.T) { Expect(err).ToNot(HaveOccurred()) Expect(got).To(BeEmpty()) } + +func TestX25519_BadRecipientPublicKey(t *testing.T) { + RegisterTestingT(t) + // Wrong-length ed25519 public key. + _, err := encryptWithX25519(ed25519.PublicKey(make([]byte, 10)), []byte("x")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid ed25519 public key size")) + + // 32 bytes that are not a valid Edwards point (roughly half of all strings + // have no curve point); find one and confirm conversion errors. + found := false + b := make([]byte, ed25519.PublicKeySize) + for i := 0; i < 64 && !found; i++ { + _, _ = rand.Read(b) + if _, e := ed25519PublicKeyToX25519(ed25519.PublicKey(append([]byte{}, b...))); e != nil { + found = true + } + } + Expect(found).To(BeTrue(), "expected to find a non-point encoding that fails conversion") +} + +func TestX25519_DecryptRejectsMalformedBlobs(t *testing.T) { + RegisterTestingT(t) + priv, _, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + // Too short for the header. + _, err = decryptWithX25519(priv, []byte("short")) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("too short")) + + // Long enough, but no x25519 magic prefix. + _, err = decryptWithX25519(priv, make([]byte, 80)) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not an x25519 blob")) +} + +func TestX25519_DecryptRejectsLowOrderEphemeralKey(t *testing.T) { + RegisterTestingT(t) + priv, _, err := GenerateEd25519KeyPair() + Expect(err).ToNot(HaveOccurred()) + + // Well-formed header with an all-zero (low-order) ephemeral public key: the + // ECDH must reject it rather than derive an attacker-usable shared secret. + blob := append([]byte{}, x25519Magic...) + blob = append(blob, x25519Version) + blob = append(blob, make([]byte, 32)...) + blob = append(blob, make([]byte, chacha20poly1305.NonceSize)...) + blob = append(blob, make([]byte, 16)...) + _, err = decryptWithX25519(priv, blob) + Expect(err).To(HaveOccurred()) +}