2 Commits
Author SHA1 Message Date
clawbot 7c6531eaf7 Keep the keychain unlocker passphrase in locked memory (closes #36)
check / check (push) Waiting to run
The passphrase protecting the keychain unlocker's age key was a plain
string passed through encoding/json, leaving copies in ordinary memory
when an unlocker was created and each time one was used.

It is now generated into a locked buffer, and KeychainData, moved to
keychaindata.go, which is not darwin-only so its tests run on Linux,
writes and reads the keychain JSON itself: encode copies the parts
straight into a locked buffer, and decodeKeychainData takes the
passphrase from a json.RawMessage that it wipes. The JSON field names
are unchanged. keychainunlocker.go only calls this code and stores the
item from the locked buffer without a string copy.

Model: opus-5-5
2026-10-03 17:07:56 +02:00
clawbot d52b4f1240 Let a plain docker build pass and stamp the git version (closes #57)
check / check (push) Successful in 1m1s
The size tests skip a case whose secret needs more locked memory than
the process can lock, found by locking a buffer of that size: memguard
panics otherwise, and a plain `docker build .` runs under an 8 MiB
RLIMIT_MEMLOCK. script/cibuild, or any process allowed to lock past the
limit, runs every case.

The build stage stamps the VERSION build argument, else
`git describe --tags --always`, and fails when .git is present but
yields no version. `make build` stamps `git describe` too instead of
the fixed 0.1.0. .dockerignore keeps .git/config out; script/docker is
now the canonical copy.

Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
2026-10-02 14:16:02 +02:00
5 changed files with 284 additions and 65 deletions
+10 -6
View File
@@ -25,6 +25,11 @@ Bring the repo into policy compliance in one commit:
# Completed Steps # Completed Steps
- 2026-10-03: The keychain unlocker's age key passphrase stays in
locked memory: it is generated into a locked buffer, and the
keychain JSON is written and read by `KeychainData` code in
`internal/secret/keychaindata.go` (tested on Linux) without
`encoding/json` holding it; the JSON field names are unchanged.
- 2026-10-02: A plain `docker build .` builds again: the size tests - 2026-10-02: A plain `docker build .` builds again: the size tests
skip a case that needs more locked memory than the process can skip a case that needs more locked memory than the process can
lock, and run every case under `script/cibuild`. The image stamps the lock, and run every case under `script/cibuild`. The image stamps the
@@ -88,12 +93,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: KeychainData stores AgePrivKeyPassphrase as a - Memory security: age identity .String() creates unprotected
plain string (keychainunlocker.go:342,393-396); age identity copies (keychainunlocker.go:356, pgpunlocker.go:256,
.String() creates unprotected copies (keychainunlocker.go:356, version.go:155); age secret key held in a plain string in
pgpunlocker.go:256, version.go:155); age secret key held in a cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
plain string in cli/crypto.go:86,91,113; private keys exposed via to GPGEncryptFunc and EncryptWithPassphrase.
buffer.Bytes() to GPGEncryptFunc and EncryptWithPassphrase.
- Race conditions: no file locking in vault/secrets.go:142-176; - Race conditions: no file locking in vault/secrets.go:142-176;
non-atomic writes can leave the vault inconsistent. non-atomic writes can leave the vault inconsistent.
- Input validation: dots in secret names risk path traversal - Input validation: dots in secret names risk path traversal
-29
View File
@@ -1,29 +0,0 @@
//go:build darwin
package secret
import (
"crypto/rand"
"fmt"
"math/big"
)
// generateRandomString generates a random string of the specified length using the given character set
func generateRandomString(length int, charset string) (string, error) {
if length <= 0 {
return "", fmt.Errorf("length must be positive")
}
result := make([]byte, length)
charsetLen := big.NewInt(int64(len(charset)))
for i := range length {
randomIndex, err := rand.Int(rand.Reader, charsetLen)
if err != nil {
return "", fmt.Errorf("failed to generate random number: %w", err)
}
result[i] = charset[randomIndex.Int64()]
}
return string(result), nil
}
+142
View File
@@ -0,0 +1,142 @@
package secret
import (
"bytes"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"strings"
"github.com/awnumar/memguard"
)
var (
errPassphraseLength = errors.New(
"passphrase length must be a positive even number")
errPassphraseNotHex = errors.New(
"keychain passphrase must be lowercase hex")
errNoKeychainPassphrase = errors.New(
"keychain data has no agePrivKeyPassphrase string")
)
// KeychainData is what a keychain unlocker stores in the macOS keychain.
// It is stored as JSON, but encode and decodeKeychainData keep the
// passphrase out of encoding/json, which would leave copies of it in
// ordinary memory.
type KeychainData struct {
AgePublicKey string
AgePrivKeyPassphrase *memguard.LockedBuffer
EncryptedLongtermKey string
}
// generateRandomPassphrase returns length random lowercase hex characters
// in a locked buffer. The caller must destroy it.
func generateRandomPassphrase(length int) (*memguard.LockedBuffer, error) {
// Each random byte becomes two hex characters.
randomBytes := hex.DecodedLen(length)
if length <= 0 || hex.EncodedLen(randomBytes) != length {
return nil, errPassphraseLength
}
random := memguard.NewBufferRandom(randomBytes)
defer random.Destroy()
passphrase := memguard.NewBuffer(length)
hex.Encode(passphrase.Bytes(), random.Bytes())
passphrase.Freeze()
return passphrase, nil
}
// encode returns d as JSON in a locked buffer:
// {"agePublicKey":"...","agePrivKeyPassphrase":"...","encryptedLongtermKey":"..."}.
// The passphrase is copied straight into the buffer, so it must be hex,
// which JSON does not escape. The caller must destroy the returned buffer.
func (d *KeychainData) encode() (*memguard.LockedBuffer, error) {
if d.AgePrivKeyPassphrase == nil {
return nil, errNilPassphraseBuffer
}
if d.AgePrivKeyPassphrase.Size() == 0 {
return nil, errEmptyPassphrase
}
for _, c := range d.AgePrivKeyPassphrase.Bytes() {
if strings.IndexByte("0123456789abcdef", c) < 0 {
return nil, errPassphraseNotHex
}
}
publicKey, err := json.Marshal(d.AgePublicKey)
if err != nil {
return nil, fmt.Errorf("failed to encode age public key: %w", err)
}
longtermKey, err := json.Marshal(d.EncryptedLongtermKey)
if err != nil {
return nil, fmt.Errorf("failed to encode long-term key: %w", err)
}
parts := [][]byte{
[]byte(`{"agePublicKey":`), publicKey,
[]byte(`,"agePrivKeyPassphrase":"`), d.AgePrivKeyPassphrase.Bytes(),
[]byte(`","encryptedLongtermKey":`), longtermKey,
[]byte(`}`),
}
size := 0
for _, part := range parts {
size += len(part)
}
encoded := memguard.NewBuffer(size)
written := 0
for _, part := range parts {
written += copy(encoded.Bytes()[written:], part)
}
encoded.Freeze()
return encoded, nil
}
// decodeKeychainData parses keychain data written by encode. The caller
// must destroy the returned AgePrivKeyPassphrase.
func decodeKeychainData(data *memguard.LockedBuffer) (*KeychainData, error) {
if data == nil {
return nil, errNilDataBuffer
}
// json.Unmarshal gives a json.RawMessage field the field's JSON text
// unchanged, in the one copy RawMessage makes; it is wiped on return.
var fields struct {
AgePublicKey string `json:"agePublicKey"`
AgePrivKeyPassphrase json.RawMessage `json:"agePrivKeyPassphrase"`
EncryptedLongtermKey string `json:"encryptedLongtermKey"`
}
defer func() { memguard.WipeBytes(fields.AgePrivKeyPassphrase) }()
err := json.Unmarshal(data.Bytes(), &fields)
if err != nil {
return nil, fmt.Errorf("failed to parse keychain data: %w", err)
}
// json.Unmarshal accepted the JSON, so text that starts with a quote is
// a whole string. The passphrase is hex, so it is the text between the
// quotes.
quoted := fields.AgePrivKeyPassphrase
if !bytes.HasPrefix(quoted, []byte(`"`)) {
return nil, errNoKeychainPassphrase
}
return &KeychainData{
AgePublicKey: fields.AgePublicKey,
// NewBufferFromBytes wipes the bytes it copies.
AgePrivKeyPassphrase: memguard.NewBufferFromBytes(
quoted[1 : len(quoted)-1]),
EncryptedLongtermKey: fields.EncryptedLongtermKey,
}, nil
}
+118
View File
@@ -0,0 +1,118 @@
//nolint:testpackage // white-box test of unexported internals
package secret
import (
"encoding/json"
"testing"
"github.com/awnumar/memguard"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestGenerateRandomPassphrase(t *testing.T) {
t.Parallel()
first, err := generateRandomPassphrase(64)
require.NoError(t, err)
defer first.Destroy()
second, err := generateRandomPassphrase(64)
require.NoError(t, err)
defer second.Destroy()
assert.Regexp(t, `^[0-9a-f]{64}$`, first.String())
assert.NotEqual(t, first.String(), second.String())
assert.False(t, first.IsMutable())
for _, length := range []int{0, -2, 63} {
_, err := generateRandomPassphrase(length)
require.ErrorIs(t, err, errPassphraseLength, "length %d", length)
}
}
func TestKeychainDataEncodeDecode(t *testing.T) {
t.Parallel()
passphrase := memguard.NewBufferFromBytes([]byte("0a1b2c3d"))
defer passphrase.Destroy()
data := KeychainData{
AgePublicKey: "age1example",
AgePrivKeyPassphrase: passphrase,
EncryptedLongtermKey: "beef",
}
encoded, err := data.encode()
require.NoError(t, err)
defer encoded.Destroy()
assert.JSONEq(t,
`{"agePublicKey":"age1example",`+
`"agePrivKeyPassphrase":"0a1b2c3d",`+
`"encryptedLongtermKey":"beef"}`,
encoded.String())
assert.False(t, encoded.IsMutable())
decoded, err := decodeKeychainData(encoded)
require.NoError(t, err)
defer decoded.AgePrivKeyPassphrase.Destroy()
assert.Equal(t, "age1example", decoded.AgePublicKey)
assert.Equal(t, "0a1b2c3d", decoded.AgePrivKeyPassphrase.String())
assert.Equal(t, "beef", decoded.EncryptedLongtermKey)
}
func TestKeychainDataEncodeRejectsBadPassphrase(t *testing.T) {
t.Parallel()
tests := []struct {
name string
passphrase *memguard.LockedBuffer
wantErr error
}{
{"nil", nil, errNilPassphraseBuffer},
{"empty", memguard.NewBuffer(0), errEmptyPassphrase},
{
"not hex",
memguard.NewBufferFromBytes([]byte(`abc"def`)),
errPassphraseNotHex,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
data := KeychainData{AgePrivKeyPassphrase: tt.passphrase}
_, err := data.encode()
require.ErrorIs(t, err, tt.wantErr)
})
}
}
func TestDecodeKeychainDataRejectsBadData(t *testing.T) {
t.Parallel()
for _, text := range []string{
`{"agePublicKey":"age1example"}`,
`{"agePrivKeyPassphrase":42}`,
} {
data := memguard.NewBufferFromBytes([]byte(text))
_, err := decodeKeychainData(data)
data.Destroy()
require.ErrorIs(t, err, errNoKeychainPassphrase, text)
}
notJSON := memguard.NewBufferFromBytes([]byte(`{"agePrivKeyPassphrase":`))
defer notJSON.Destroy()
_, err := decodeKeychainData(notJSON)
var syntaxError *json.SyntaxError
require.ErrorAs(t, err, &syntaxError)
}
+14 -30
View File
@@ -45,13 +45,6 @@ type KeychainUnlocker struct {
fs afero.Fs fs afero.Fs
} }
// KeychainData represents the data stored in the macOS keychain
type KeychainData struct {
AgePublicKey string `json:"agePublicKey"`
AgePrivKeyPassphrase string `json:"agePrivKeyPassphrase"`
EncryptedLongtermKey string `json:"encryptedLongtermKey"`
}
// GetIdentity implements Unlocker interface for Keychain-based unlockers // GetIdentity implements Unlocker interface for Keychain-based unlockers
func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
DebugWith("Getting keychain unlocker identity", DebugWith("Getting keychain unlocker identity",
@@ -81,13 +74,18 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
slog.Int("data_length", len(keychainDataBytes)), slog.Int("data_length", len(keychainDataBytes)),
) )
// Move the keychain data into locked memory; this wipes keychainDataBytes
keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
defer keychainDataBuffer.Destroy()
// Step 3: Parse keychain data // Step 3: Parse keychain data
var keychainData KeychainData keychainData, err := decodeKeychainData(keychainDataBuffer)
if err := json.Unmarshal(keychainDataBytes, &keychainData); err != nil { if err != nil {
Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID()) Debug("Failed to parse keychain data", "error", err, "unlocker_id", k.GetID())
return nil, fmt.Errorf("failed to parse keychain data: %w", err) return nil, fmt.Errorf("failed to parse keychain data: %w", err)
} }
defer keychainData.AgePrivKeyPassphrase.Destroy()
Debug("Parsed keychain data successfully", "unlocker_id", k.GetID()) Debug("Parsed keychain data successfully", "unlocker_id", k.GetID())
@@ -109,11 +107,7 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) {
// Step 5: Decrypt the age private key using the passphrase from keychain // Step 5: Decrypt the age private key using the passphrase from keychain
Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID()) Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID())
// Create secure buffer for the keychain passphrase agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase)
passphraseBuffer := memguard.NewBufferFromBytes([]byte(keychainData.AgePrivKeyPassphrase))
defer passphraseBuffer.Destroy()
agePrivKeyBuffer, err := DecryptWithPassphrase(encryptedAgePrivKeyData, passphraseBuffer)
if err != nil { if err != nil {
Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID()) Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID())
@@ -369,6 +363,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to generate age private key passphrase: %w", err) return nil, fmt.Errorf("failed to generate age private key passphrase: %w", err)
} }
defer agePrivKeyPassphrase.Destroy()
// Step 3: Store age recipient as plaintext // Step 3: Store age recipient as plaintext
ageRecipient := ageIdentity.Recipient().String() ageRecipient := ageIdentity.Recipient().String()
@@ -378,15 +373,12 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
} }
// Step 4: Encrypt age private key with the generated passphrase and store on disk // Step 4: Encrypt age private key with the generated passphrase and store on disk
// Create secure buffers for both the private key and passphrase // Create a secure buffer for the private key
agePrivKeyStr := ageIdentity.String() agePrivKeyStr := ageIdentity.String()
agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr)) agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
defer agePrivKeyBuffer.Destroy() defer agePrivKeyBuffer.Destroy()
passphraseBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyPassphrase)) encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, agePrivKeyPassphrase)
defer passphraseBuffer.Destroy()
encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer, passphraseBuffer)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err) return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
} }
@@ -422,13 +414,10 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
EncryptedLongtermKey: hex.EncodeToString(encryptedLtPrivKeyToAge), EncryptedLongtermKey: hex.EncodeToString(encryptedLtPrivKeyToAge),
} }
keychainDataBytes, err := json.Marshal(keychainData) keychainDataBuffer, err := keychainData.encode()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to marshal keychain data: %w", err) return nil, fmt.Errorf("failed to encode keychain data: %w", err)
} }
// Create a secure buffer for keychain data
keychainDataBuffer := memguard.NewBufferFromBytes(keychainDataBytes)
defer keychainDataBuffer.Destroy() defer keychainDataBuffer.Destroy()
// Step 8: Store data in keychain // Step 8: Store data in keychain
@@ -501,7 +490,7 @@ func storeInKeychain(itemName string, data *memguard.LockedBuffer) error {
item.SetAccount(itemName) item.SetAccount(itemName)
item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName)) item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName))
item.SetDescription("Secret vault keychain data") item.SetDescription("Secret vault keychain data")
item.SetData([]byte(data.String())) item.SetData(data.Bytes())
item.SetSynchronizable(keychain.SynchronizableNo) item.SetSynchronizable(keychain.SynchronizableNo)
// Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth // Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth
item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly)
@@ -576,8 +565,3 @@ func deleteFromKeychain(itemName string) error {
return nil return nil
} }
// generateRandomPassphrase generates a random passphrase for encrypting the age private key
func generateRandomPassphrase(length int) (string, error) {
return generateRandomString(length, "0123456789abcdef")
}