diff --git a/TODO.md b/TODO.md index 40eef4c..2a714f5 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,18 @@ Bring the repo into policy compliance in one commit: # Completed Steps +- 2026-10-04: An age identity's private key goes into a locked buffer + through `secret.IdentityToLockedBuffer` everywhere + (https://git.eeqj.de/sneak/secret/issues/38): the vault's long-term key + when a passphrase, PGP, keychain or Secure Enclave unlocker is created, + the new unlocker's own key, a new secret version's key, and the key + `secret encrypt` generates. Before, each place converted the string age + returns to bytes and left the string in ordinary memory. The function + moves the string's own bytes into the buffer, which overwrites them; the + copies age makes while writing the string remain, as its comment says. + The 1.0 memory-security entry below no longer lists these places, + `internal/cli/crypto.go` among them, nor `version.go:155`, which was + `internal/secret/version.go`, not `internal/cli/version.go`. - 2026-10-04: `script/lint-darwin` (`make lint-darwin`) runs `go vet` and `golangci-lint` in docker on the code as a macOS build compiles it (`GOOS=darwin`), with cgo off @@ -331,11 +343,11 @@ Bring the repo into policy compliance in one commit: - Command injection: GPG key IDs passed unescaped to exec.Command (pgpunlocker.go:323-327); data.String() passed unescaped to the security command (keychainunlocker.go:472-476). - - Memory security: age identity .String() creates unprotected - copies (keychainunlocker.go:356, pgpunlocker.go:256, - version.go:155); age secret key held in a plain string in - cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes() - to GPGEncryptFunc and EncryptWithPassphrase. + - Memory security: age writes an identity's private key out as a + string in ordinary memory, and the copies it makes on the way stay + there (`secret.IdentityToLockedBuffer` overwrites only the string + itself); private keys exposed via buffer.Bytes() to GPGEncryptFunc + and EncryptWithPassphrase. - Input validation: no maximum secret size (DoS). - Timing attacks: bytes.Equal passphrase compare (cli/init.go: 209-216); non-constant-time public key compare (vault.go:95-100). diff --git a/internal/cli/crypto.go b/internal/cli/crypto.go index 12fa0a3..d6da3d6 100644 --- a/internal/cli/crypto.go +++ b/internal/cli/crypto.go @@ -91,8 +91,7 @@ func (cli *Instance) storeNewEncryptionKey( return nil, fmt.Errorf("failed to generate age key: %w", err) } - // Store the generated key directly in a secure buffer - secureBuffer := memguard.NewBufferFromBytes([]byte(identity.String())) + secureBuffer := secret.IdentityToLockedBuffer(identity) err = vlt.AddSecret(secretName, secureBuffer, false) if err != nil { diff --git a/internal/secret/crypto.go b/internal/secret/crypto.go index ad46ab5..f59dcda 100644 --- a/internal/secret/crypto.go +++ b/internal/secret/crypto.go @@ -7,6 +7,7 @@ import ( "io" "os" "syscall" + "unsafe" "filippo.io/age" "github.com/awnumar/memguard" @@ -102,6 +103,23 @@ func DecryptWithIdentity( return resultBuffer, nil } +// IdentityToLockedBuffer returns the private key of id, in age's text form, in +// a new locked buffer. The caller must destroy it. +// +// This is best effort. age gives the key only as a string in ordinary memory. +// The bytes of that string are moved into the buffer, which overwrites them, +// although Go otherwise never changes a string; nothing else holds this one. +// The copies age makes while building the string are left in ordinary memory. +// Avoiding those would mean encoding the key here, straight into the buffer. +func IdentityToLockedBuffer(id *age.X25519Identity) *memguard.LockedBuffer { + key := id.String() + + //nolint:gosec // G103: the string's own bytes, which NewBufferFromBytes wipes + keyBytes := unsafe.Slice(unsafe.StringData(key), len(key)) + + return memguard.NewBufferFromBytes(keyBytes) +} + // EncryptWithPassphrase encrypts data using a passphrase with age's // scrypt-based encryption. Both data and passphrase parameters should // be LockedBuffers for secure memory handling diff --git a/internal/secret/crypto_test.go b/internal/secret/crypto_test.go new file mode 100644 index 0000000..886e135 --- /dev/null +++ b/internal/secret/crypto_test.go @@ -0,0 +1,29 @@ +package secret_test + +import ( + "testing" + + "filippo.io/age" + "git.eeqj.de/sneak/secret/internal/secret" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestIdentityToLockedBuffer checks that the buffer holds the identity's +// private key, and that the identity still gives that key afterwards: the +// helper overwrites the string age returned, so age must not keep it. +func TestIdentityToLockedBuffer(t *testing.T) { + t.Parallel() + + identity, err := age.GenerateX25519Identity() + require.NoError(t, err) + + buffer := secret.IdentityToLockedBuffer(identity) + defer buffer.Destroy() + + parsed, err := age.ParseX25519Identity(buffer.String()) + require.NoError(t, err) + assert.Equal(t, identity.Recipient().String(), parsed.Recipient().String()) + + assert.Equal(t, identity.String(), buffer.String()) +} diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go index 97aebc9..1775dcf 100644 --- a/internal/secret/keychainunlocker.go +++ b/internal/secret/keychainunlocker.go @@ -396,8 +396,7 @@ func deriveLongTermPrivateKey( "failed to derive long-term key from mnemonic: %w", err) } - // Return the private key in a secure buffer - return memguard.NewBufferFromBytes([]byte(ltIdentity.String())), nil + return IdentityToLockedBuffer(ltIdentity), nil } // CreateKeychainUnlocker creates a new keychain unlocker and stores it in the @@ -448,10 +447,7 @@ func CreateKeychainUnlocker( defer agePrivKeyPassphrase.Destroy() // Step 3: Encrypt age private key with the generated passphrase - // Create a secure buffer for the private key - agePrivKeyStr := ageIdentity.String() - - agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr)) + agePrivKeyBuffer := IdentityToLockedBuffer(ageIdentity) defer agePrivKeyBuffer.Destroy() encryptedAgePrivKey, err := EncryptWithPassphrase( diff --git a/internal/secret/pgpunlocker.go b/internal/secret/pgpunlocker.go index f0fad17..f1e17c6 100644 --- a/internal/secret/pgpunlocker.go +++ b/internal/secret/pgpunlocker.go @@ -330,7 +330,7 @@ func encryptPGPUnlockerKeys( return nil, nil, fmt.Errorf("failed to get long-term key: %w", err) } - ltPrivKeyData := memguard.NewBufferFromBytes([]byte(ltIdentity.String())) + ltPrivKeyData := IdentityToLockedBuffer(ltIdentity) defer ltPrivKeyData.Destroy() encryptedLtPrivKey, err := EncryptToRecipient( @@ -340,8 +340,7 @@ func encryptPGPUnlockerKeys( "failed to encrypt long-term private key to age unlocker: %w", err) } - // Use memguard to protect the private key in memory - agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String())) + agePrivateKeyBuffer := IdentityToLockedBuffer(ageIdentity) defer agePrivateKeyBuffer.Destroy() encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID) diff --git a/internal/secret/version.go b/internal/secret/version.go index a002b65..17cca2b 100644 --- a/internal/secret/version.go +++ b/internal/secret/version.go @@ -175,9 +175,7 @@ func (sv *Version) Save(value *memguard.LockedBuffer) error { return fmt.Errorf("failed to generate version keypair: %w", err) } - // Store private key in memguard buffer immediately - versionPrivateKeyBuffer := memguard.NewBufferFromBytes( - []byte(versionIdentity.String())) + versionPrivateKeyBuffer := IdentityToLockedBuffer(versionIdentity) defer versionPrivateKeyBuffer.Destroy() DebugWith("Generated version keypair", diff --git a/internal/vault/unlockers.go b/internal/vault/unlockers.go index 1ca933a..4075066 100644 --- a/internal/vault/unlockers.go +++ b/internal/vault/unlockers.go @@ -401,7 +401,7 @@ func (v *Vault) CreatePassphraseUnlocker( } // Encrypt long-term private key to this unlocker - ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String())) + ltPrivKeyBuffer := secret.IdentityToLockedBuffer(ltIdentity) defer ltPrivKeyBuffer.Destroy() encryptedLtPrivKey, err := secret.EncryptToRecipient(ltPrivKeyBuffer, @@ -530,9 +530,7 @@ func (v *Vault) writeUnlockerFiles( } // Encrypt private key with passphrase - privKeyStr := unlockerIdentity.String() - - privKeyBuffer := memguard.NewBufferFromBytes([]byte(privKeyStr)) + privKeyBuffer := secret.IdentityToLockedBuffer(unlockerIdentity) defer privKeyBuffer.Destroy() encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyBuffer, passphrase)