Put age identity keys into locked buffers through one function (closes #38)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
secret.IdentityToLockedBuffer replaces the eight places that converted an age identity's String() to bytes for a locked buffer and left the string, which holds the private key, in ordinary memory. It moves the string's own bytes into the buffer, which overwrites them. The copies age makes while encoding the key remain; the function's comment says so. TODO.md drops these places from the 1.0 memory-security entry, along with its stale version.go reference. Model: opus-5-5
This commit is contained in:
@@ -25,6 +25,18 @@ Bring the repo into policy compliance in one commit:
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
`golangci-lint` in docker on the code as a macOS build compiles it
|
||||||
(`GOOS=darwin`), with cgo off
|
(`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
|
- Command injection: GPG key IDs passed unescaped to exec.Command
|
||||||
(pgpunlocker.go:323-327); data.String() passed unescaped to the
|
(pgpunlocker.go:323-327); data.String() passed unescaped to the
|
||||||
security command (keychainunlocker.go:472-476).
|
security command (keychainunlocker.go:472-476).
|
||||||
- Memory security: age identity .String() creates unprotected
|
- Memory security: age writes an identity's private key out as a
|
||||||
copies (keychainunlocker.go:356, pgpunlocker.go:256,
|
string in ordinary memory, and the copies it makes on the way stay
|
||||||
version.go:155); age secret key held in a plain string in
|
there (`secret.IdentityToLockedBuffer` overwrites only the string
|
||||||
cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
|
itself); private keys exposed via buffer.Bytes() to GPGEncryptFunc
|
||||||
to GPGEncryptFunc and EncryptWithPassphrase.
|
and EncryptWithPassphrase.
|
||||||
- Input validation: no maximum secret size (DoS).
|
- Input validation: no maximum secret size (DoS).
|
||||||
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
||||||
209-216); non-constant-time public key compare (vault.go:95-100).
|
209-216); non-constant-time public key compare (vault.go:95-100).
|
||||||
|
|||||||
@@ -91,8 +91,7 @@ func (cli *Instance) storeNewEncryptionKey(
|
|||||||
return nil, fmt.Errorf("failed to generate age key: %w", err)
|
return nil, fmt.Errorf("failed to generate age key: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Store the generated key directly in a secure buffer
|
secureBuffer := secret.IdentityToLockedBuffer(identity)
|
||||||
secureBuffer := memguard.NewBufferFromBytes([]byte(identity.String()))
|
|
||||||
|
|
||||||
err = vlt.AddSecret(secretName, secureBuffer, false)
|
err = vlt.AddSecret(secretName, secureBuffer, false)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"syscall"
|
"syscall"
|
||||||
|
"unsafe"
|
||||||
|
|
||||||
"filippo.io/age"
|
"filippo.io/age"
|
||||||
"github.com/awnumar/memguard"
|
"github.com/awnumar/memguard"
|
||||||
@@ -102,6 +103,23 @@ func DecryptWithIdentity(
|
|||||||
return resultBuffer, nil
|
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
|
// EncryptWithPassphrase encrypts data using a passphrase with age's
|
||||||
// scrypt-based encryption. Both data and passphrase parameters should
|
// scrypt-based encryption. Both data and passphrase parameters should
|
||||||
// be LockedBuffers for secure memory handling
|
// be LockedBuffers for secure memory handling
|
||||||
|
|||||||
@@ -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())
|
||||||
|
}
|
||||||
@@ -396,8 +396,7 @@ func deriveLongTermPrivateKey(
|
|||||||
"failed to derive long-term key from mnemonic: %w", err)
|
"failed to derive long-term key from mnemonic: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Return the private key in a secure buffer
|
return IdentityToLockedBuffer(ltIdentity), nil
|
||||||
return memguard.NewBufferFromBytes([]byte(ltIdentity.String())), nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// CreateKeychainUnlocker creates a new keychain unlocker and stores it in the
|
// CreateKeychainUnlocker creates a new keychain unlocker and stores it in the
|
||||||
@@ -448,10 +447,7 @@ func CreateKeychainUnlocker(
|
|||||||
defer agePrivKeyPassphrase.Destroy()
|
defer agePrivKeyPassphrase.Destroy()
|
||||||
|
|
||||||
// Step 3: Encrypt age private key with the generated passphrase
|
// Step 3: Encrypt age private key with the generated passphrase
|
||||||
// Create a secure buffer for the private key
|
agePrivKeyBuffer := IdentityToLockedBuffer(ageIdentity)
|
||||||
agePrivKeyStr := ageIdentity.String()
|
|
||||||
|
|
||||||
agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
|
|
||||||
defer agePrivKeyBuffer.Destroy()
|
defer agePrivKeyBuffer.Destroy()
|
||||||
|
|
||||||
encryptedAgePrivKey, err := EncryptWithPassphrase(
|
encryptedAgePrivKey, err := EncryptWithPassphrase(
|
||||||
|
|||||||
@@ -330,7 +330,7 @@ func encryptPGPUnlockerKeys(
|
|||||||
return nil, nil, fmt.Errorf("failed to get long-term key: %w", err)
|
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()
|
defer ltPrivKeyData.Destroy()
|
||||||
|
|
||||||
encryptedLtPrivKey, err := EncryptToRecipient(
|
encryptedLtPrivKey, err := EncryptToRecipient(
|
||||||
@@ -340,8 +340,7 @@ func encryptPGPUnlockerKeys(
|
|||||||
"failed to encrypt long-term private key to age unlocker: %w", err)
|
"failed to encrypt long-term private key to age unlocker: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Use memguard to protect the private key in memory
|
agePrivateKeyBuffer := IdentityToLockedBuffer(ageIdentity)
|
||||||
agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
|
|
||||||
defer agePrivateKeyBuffer.Destroy()
|
defer agePrivateKeyBuffer.Destroy()
|
||||||
|
|
||||||
encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
|
encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
|
||||||
|
|||||||
@@ -175,9 +175,7 @@ func (sv *Version) Save(value *memguard.LockedBuffer) error {
|
|||||||
return fmt.Errorf("failed to generate version keypair: %w", err)
|
return fmt.Errorf("failed to generate version keypair: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Store private key in memguard buffer immediately
|
versionPrivateKeyBuffer := IdentityToLockedBuffer(versionIdentity)
|
||||||
versionPrivateKeyBuffer := memguard.NewBufferFromBytes(
|
|
||||||
[]byte(versionIdentity.String()))
|
|
||||||
defer versionPrivateKeyBuffer.Destroy()
|
defer versionPrivateKeyBuffer.Destroy()
|
||||||
|
|
||||||
DebugWith("Generated version keypair",
|
DebugWith("Generated version keypair",
|
||||||
|
|||||||
@@ -401,7 +401,7 @@ func (v *Vault) CreatePassphraseUnlocker(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Encrypt long-term private key to this unlocker
|
// Encrypt long-term private key to this unlocker
|
||||||
ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
|
ltPrivKeyBuffer := secret.IdentityToLockedBuffer(ltIdentity)
|
||||||
defer ltPrivKeyBuffer.Destroy()
|
defer ltPrivKeyBuffer.Destroy()
|
||||||
|
|
||||||
encryptedLtPrivKey, err := secret.EncryptToRecipient(ltPrivKeyBuffer,
|
encryptedLtPrivKey, err := secret.EncryptToRecipient(ltPrivKeyBuffer,
|
||||||
@@ -530,9 +530,7 @@ func (v *Vault) writeUnlockerFiles(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Encrypt private key with passphrase
|
// Encrypt private key with passphrase
|
||||||
privKeyStr := unlockerIdentity.String()
|
privKeyBuffer := secret.IdentityToLockedBuffer(unlockerIdentity)
|
||||||
|
|
||||||
privKeyBuffer := memguard.NewBufferFromBytes([]byte(privKeyStr))
|
|
||||||
defer privKeyBuffer.Destroy()
|
defer privKeyBuffer.Destroy()
|
||||||
|
|
||||||
encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyBuffer, passphrase)
|
encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyBuffer, passphrase)
|
||||||
|
|||||||
Reference in New Issue
Block a user