From c9774e89e09200fcba82063c40534681846de764 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 15 Jul 2025 07:22:41 +0200 Subject: [PATCH] WIP: refactor to use memguard for secure memory handling - Add memguard dependency - Update ReadPassphrase to return LockedBuffer - Update EncryptWithPassphrase/DecryptWithPassphrase to accept LockedBuffer - Remove string wrapper functions - Update all callers to create LockedBuffers at entry points - Update interfaces and mock implementations --- .claude/settings.local.json | 3 +- go.mod | 2 ++ go.sum | 5 +++ internal/cli/crypto.go | 39 +++++++++++++++----- internal/cli/init.go | 30 +++++++++------- internal/cli/unlockers.go | 10 +++--- internal/cli/vault.go | 7 +++- internal/secret/crypto.go | 45 +++++++++++++++++------ internal/secret/keychainunlocker.go | 30 +++++++++++++--- internal/secret/passphrase_test.go | 5 ++- internal/secret/passphraseunlocker.go | 51 +++++++++++++++++++-------- internal/secret/pgpunlock_test.go | 5 ++- internal/secret/secret.go | 3 +- internal/secret/secret_test.go | 3 +- internal/secret/version_test.go | 3 +- internal/vault/unlockers.go | 11 ++++-- internal/vault/vault_test.go | 5 ++- pkg/bip85/bip85.go | 2 +- 18 files changed, 194 insertions(+), 65 deletions(-) diff --git a/.claude/settings.local.json b/.claude/settings.local.json index ed217fc..c59f668 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -22,7 +22,8 @@ "Bash(git checkout:*)", "Bash(ls:*)", "WebFetch(domain:golangci-lint.run)", - "Bash(go:*)" + "Bash(go:*)", + "WebFetch(domain:pkg.go.dev)" ], "deny": [] } diff --git a/go.mod b/go.mod index e4fe87f..4fcc0ce 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.24.1 require ( filippo.io/age v1.2.1 + github.com/awnumar/memguard v0.22.5 github.com/btcsuite/btcd v0.24.2 github.com/btcsuite/btcd/btcec/v2 v2.1.3 github.com/btcsuite/btcd/btcutil v1.1.6 @@ -18,6 +19,7 @@ require ( ) require ( + github.com/awnumar/memcall v0.2.0 // indirect github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 // indirect diff --git a/go.sum b/go.sum index ca00620..bac509d 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,10 @@ c2sp.org/CCTV/age v0.0.0-20240306222714-3ec4d716e805/go.mod h1:FomMrUJ2Lxt5jCLmZ filippo.io/age v1.2.1 h1:X0TZjehAZylOIj4DubWYU1vWQxv9bJpo+Uu2/LGhi1o= filippo.io/age v1.2.1/go.mod h1:JL9ew2lTN+Pyft4RiNGguFfOpewKwSHm5ayKD/A4004= github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII= +github.com/awnumar/memcall v0.2.0 h1:sRaogqExTOOkkNwO9pzJsL8jrOV29UuUW7teRMfbqtI= +github.com/awnumar/memcall v0.2.0/go.mod h1:S911igBPR9CThzd/hYQQmTc9SWNu3ZHIlCGaWsWsoJo= +github.com/awnumar/memguard v0.22.5 h1:PH7sbUVERS5DdXh3+mLo8FDcl1eIeVjJVYMnyuYpvuI= +github.com/awnumar/memguard v0.22.5/go.mod h1:+APmZGThMBWjnMlKiSM1X7MVpbIVewen2MTkqWkA/zE= github.com/btcsuite/btcd v0.20.1-beta/go.mod h1:wVuoA8VJLEcwgqHBwHmzLRazpKxTv13Px/pDuV7OomQ= github.com/btcsuite/btcd v0.22.0-beta.0.20220111032746-97732e52810c/go.mod h1:tjmYdS6MLJ5/s0Fj4DbLgSbDHbEqLJrtnHecBFkdz5M= github.com/btcsuite/btcd v0.23.5-0.20231215221805-96c9fd8078fd/go.mod h1:nm3Bko6zh6bWP60UxwoT5LzdGJsQJaPo6HjduXq9p6A= @@ -107,6 +111,7 @@ golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/internal/cli/crypto.go b/internal/cli/crypto.go index 691f326..560e165 100644 --- a/internal/cli/crypto.go +++ b/internal/cli/crypto.go @@ -8,6 +8,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" + "github.com/awnumar/memguard" "github.com/spf13/cobra" ) @@ -83,8 +84,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error { ageSecretKey = identity.String() - // Store the generated key as a secret - err = vlt.AddSecret(secretName, []byte(ageSecretKey), false) + // Store the generated key as a secret using secure buffer + secureBuffer := memguard.NewBufferFromBytes([]byte(ageSecretKey)) + defer secureBuffer.Destroy() + + err = vlt.AddSecret(secretName, secureBuffer.Bytes(), false) if err != nil { return fmt.Errorf("failed to store age key: %w", err) } @@ -95,7 +99,16 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error { return fmt.Errorf("failed to get secret value: %w", err) } - ageSecretKey = string(secretValue) + // Create secure buffer for the secret value + secureBuffer := memguard.NewBufferFromBytes(secretValue) + defer secureBuffer.Destroy() + + // Clear the original secret value + for i := range secretValue { + secretValue[i] = 0 + } + + ageSecretKey = secureBuffer.String() // Validate that it's a valid age secret key if !isValidAgeSecretKey(ageSecretKey) { @@ -103,8 +116,11 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error { } } - // Parse the secret key - identity, err := age.ParseX25519Identity(ageSecretKey) + // Parse the secret key using secure buffer + finalSecureBuffer := memguard.NewBufferFromBytes([]byte(ageSecretKey)) + defer finalSecureBuffer.Destroy() + + identity, err := age.ParseX25519Identity(finalSecureBuffer.String()) if err != nil { return fmt.Errorf("failed to parse age secret key: %w", err) } @@ -185,15 +201,22 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error { return fmt.Errorf("failed to get secret value: %w", err) } - ageSecretKey := string(secretValue) + // Create secure buffer for the secret value + secureBuffer := memguard.NewBufferFromBytes(secretValue) + defer secureBuffer.Destroy() + + // Clear the original secret value + for i := range secretValue { + secretValue[i] = 0 + } // Validate that it's a valid age secret key - if !isValidAgeSecretKey(ageSecretKey) { + if !isValidAgeSecretKey(secureBuffer.String()) { return fmt.Errorf("secret '%s' does not contain a valid age secret key", secretName) } // Parse the age secret key to get the identity - identity, err := age.ParseX25519Identity(ageSecretKey) + identity, err := age.ParseX25519Identity(secureBuffer.String()) if err != nil { return fmt.Errorf("failed to parse age secret key: %w", err) } diff --git a/internal/cli/init.go b/internal/cli/init.go index ef51820..91e04ee 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -11,6 +11,7 @@ import ( "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/tyler-smith/go-bip39" @@ -125,24 +126,25 @@ func (cli *Instance) Init(cmd *cobra.Command) error { vlt.Unlock(ltIdentity) // Prompt for passphrase for unlocker - var passphraseStr string + var passphraseBuffer *memguard.LockedBuffer if envPassphrase := os.Getenv(secret.EnvUnlockPassphrase); envPassphrase != "" { secret.Debug("Using unlock passphrase from environment variable") - passphraseStr = envPassphrase + passphraseBuffer = memguard.NewBufferFromBytes([]byte(envPassphrase)) } else { secret.Debug("Prompting user for unlock passphrase") // Use secure passphrase input with confirmation - passphraseStr, err = readSecurePassphrase("Enter passphrase for unlocker: ") + passphraseBuffer, err = readSecurePassphrase("Enter passphrase for unlocker: ") if err != nil { secret.Debug("Failed to read unlock passphrase", "error", err) return fmt.Errorf("failed to read passphrase: %w", err) } } + defer passphraseBuffer.Destroy() // Create passphrase-protected unlocker secret.Debug("Creating passphrase-protected unlocker") - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseStr) + passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) if err != nil { secret.Debug("Failed to create unlocker", "error", err) @@ -190,23 +192,27 @@ func (cli *Instance) Init(cmd *cobra.Command) error { // readSecurePassphrase reads a passphrase securely from the terminal without echoing // This version adds confirmation (read twice) for creating new unlockers -func readSecurePassphrase(prompt string) (string, error) { +// Returns a LockedBuffer containing the passphrase +func readSecurePassphrase(prompt string) (*memguard.LockedBuffer, error) { // Get the first passphrase - passphrase1, err := secret.ReadPassphrase(prompt) + passphraseBuffer1, err := secret.ReadPassphrase(prompt) if err != nil { - return "", err + return nil, err } + defer passphraseBuffer1.Destroy() // Read confirmation passphrase - passphrase2, err := secret.ReadPassphrase("Confirm passphrase: ") + passphraseBuffer2, err := secret.ReadPassphrase("Confirm passphrase: ") if err != nil { - return "", fmt.Errorf("failed to read passphrase confirmation: %w", err) + return nil, fmt.Errorf("failed to read passphrase confirmation: %w", err) } + defer passphraseBuffer2.Destroy() // Compare passphrases - if passphrase1 != passphrase2 { - return "", fmt.Errorf("passphrases do not match") + if passphraseBuffer1.String() != passphraseBuffer2.String() { + return nil, fmt.Errorf("passphrases do not match") } - return passphrase1, nil + // Create a new buffer with the confirmed passphrase + return memguard.NewBufferFromBytes(passphraseBuffer1.Bytes()), nil } diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index e407c75..db2b859 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -10,6 +10,7 @@ import ( "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" + "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -254,18 +255,19 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error // The CreatePassphraseUnlocker method will handle getting the long-term key // Check if passphrase is set in environment variable - var passphraseStr string + var passphraseBuffer *memguard.LockedBuffer if envPassphrase := os.Getenv(secret.EnvUnlockPassphrase); envPassphrase != "" { - passphraseStr = envPassphrase + passphraseBuffer = memguard.NewBufferFromBytes([]byte(envPassphrase)) } else { // Use secure passphrase input with confirmation - passphraseStr, err = readSecurePassphrase("Enter passphrase for unlocker: ") + passphraseBuffer, err = readSecurePassphrase("Enter passphrase for unlocker: ") if err != nil { return fmt.Errorf("failed to read passphrase: %w", err) } } + defer passphraseBuffer.Destroy() - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseStr) + passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) if err != nil { return err } diff --git a/internal/cli/vault.go b/internal/cli/vault.go index 30d9e7b..1b51ee5 100644 --- a/internal/cli/vault.go +++ b/internal/cli/vault.go @@ -10,6 +10,7 @@ import ( "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/tyler-smith/go-bip39" @@ -272,12 +273,16 @@ func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error { secret.Debug("Using unlock passphrase from environment variable") + // Create secure buffer for passphrase + passphraseBuffer := memguard.NewBufferFromBytes([]byte(passphraseStr)) + defer passphraseBuffer.Destroy() + // Unlock the vault with the derived long-term key vlt.Unlock(ltIdentity) // Create passphrase-protected unlocker secret.Debug("Creating passphrase-protected unlocker") - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseStr) + passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) if err != nil { secret.Debug("Failed to create unlocker", "error", err) diff --git a/internal/secret/crypto.go b/internal/secret/crypto.go index 38b505e..5fcf3e2 100644 --- a/internal/secret/crypto.go +++ b/internal/secret/crypto.go @@ -8,6 +8,7 @@ import ( "syscall" "filippo.io/age" + "github.com/awnumar/memguard" "golang.org/x/term" ) @@ -63,8 +64,15 @@ func DecryptWithIdentity(data []byte, identity age.Identity) ([]byte, error) { } // EncryptWithPassphrase encrypts data using a passphrase with age's scrypt-based encryption -func EncryptWithPassphrase(data []byte, passphrase string) ([]byte, error) { - recipient, err := age.NewScryptRecipient(passphrase) +// The passphrase parameter should be a LockedBuffer for secure memory handling +func EncryptWithPassphrase(data []byte, passphrase *memguard.LockedBuffer) ([]byte, error) { + if passphrase == nil { + return nil, fmt.Errorf("passphrase buffer is nil") + } + + // Get the passphrase string temporarily + passphraseStr := passphrase.String() + recipient, err := age.NewScryptRecipient(passphraseStr) if err != nil { return nil, fmt.Errorf("failed to create scrypt recipient: %w", err) } @@ -73,8 +81,15 @@ func EncryptWithPassphrase(data []byte, passphrase string) ([]byte, error) { } // DecryptWithPassphrase decrypts data using a passphrase with age's scrypt-based decryption -func DecryptWithPassphrase(encryptedData []byte, passphrase string) ([]byte, error) { - identity, err := age.NewScryptIdentity(passphrase) +// The passphrase parameter should be a LockedBuffer for secure memory handling +func DecryptWithPassphrase(encryptedData []byte, passphrase *memguard.LockedBuffer) ([]byte, error) { + if passphrase == nil { + return nil, fmt.Errorf("passphrase buffer is nil") + } + + // Get the passphrase string temporarily + passphraseStr := passphrase.String() + identity, err := age.NewScryptIdentity(passphraseStr) if err != nil { return nil, fmt.Errorf("failed to create scrypt identity: %w", err) } @@ -84,18 +99,19 @@ func DecryptWithPassphrase(encryptedData []byte, passphrase string) ([]byte, err // ReadPassphrase reads a passphrase securely from the terminal without echoing // This version is for unlocking and doesn't require confirmation -func ReadPassphrase(prompt string) (string, error) { +// Returns a LockedBuffer containing the passphrase for secure memory handling +func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) { // Check if stdin is a terminal if !term.IsTerminal(syscall.Stdin) { // Not a terminal - never read passphrases from piped input for security reasons - return "", fmt.Errorf("cannot read passphrase from non-terminal stdin " + + return nil, fmt.Errorf("cannot read passphrase from non-terminal stdin " + "(piped input or script). Please set the SB_UNLOCK_PASSPHRASE " + "environment variable or run interactively") } // stdin is a terminal, check if stderr is also a terminal for interactive prompting if !term.IsTerminal(syscall.Stderr) { - return "", fmt.Errorf("cannot prompt for passphrase: stderr is not a terminal " + + return nil, fmt.Errorf("cannot prompt for passphrase: stderr is not a terminal " + "(running in non-interactive mode). Please set the SB_UNLOCK_PASSPHRASE " + "environment variable") } @@ -104,13 +120,22 @@ func ReadPassphrase(prompt string) (string, error) { fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout passphrase, err := term.ReadPassword(syscall.Stdin) if err != nil { - return "", fmt.Errorf("failed to read passphrase: %w", err) + return nil, fmt.Errorf("failed to read passphrase: %w", err) } fmt.Fprintln(os.Stderr) // Print newline to stderr since ReadPassword doesn't echo if len(passphrase) == 0 { - return "", fmt.Errorf("passphrase cannot be empty") + return nil, fmt.Errorf("passphrase cannot be empty") } - return string(passphrase), nil + // Create a secure buffer and copy the passphrase + secureBuffer := memguard.NewBufferFromBytes(passphrase) + + // Clear the original passphrase slice + for i := range passphrase { + passphrase[i] = 0 + } + + return secureBuffer, nil } + diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go index 4402739..1f48bf6 100644 --- a/internal/secret/keychainunlocker.go +++ b/internal/secret/keychainunlocker.go @@ -13,6 +13,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -102,7 +103,11 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { // Step 5: Decrypt the age private key using the passphrase from keychain Debug("Decrypting age private key with keychain passphrase", "unlocker_id", k.GetID()) - agePrivKeyData, err := DecryptWithPassphrase(encryptedAgePrivKeyData, keychainData.AgePrivKeyPassphrase) + // Create secure buffer for the keychain passphrase + passphraseBuffer := memguard.NewBufferFromBytes([]byte(keychainData.AgePrivKeyPassphrase)) + defer passphraseBuffer.Destroy() + + agePrivKeyData, err := DecryptWithPassphrase(encryptedAgePrivKeyData, passphraseBuffer) if err != nil { Debug("Failed to decrypt age private key with keychain passphrase", "error", err, "unlocker_id", k.GetID()) @@ -116,7 +121,17 @@ func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { // Step 6: Parse the decrypted age private key Debug("Parsing decrypted age private key", "unlocker_id", k.GetID()) - ageIdentity, err := age.ParseX25519Identity(string(agePrivKeyData)) + + // Create a secure buffer for the private key data + agePrivKeyBuffer := memguard.NewBufferFromBytes(agePrivKeyData) + defer agePrivKeyBuffer.Destroy() + + // Clear the original private key data + for i := range agePrivKeyData { + agePrivKeyData[i] = 0 + } + + ageIdentity, err := age.ParseX25519Identity(agePrivKeyBuffer.String()) if err != nil { Debug("Failed to parse age private key", "error", err, "unlocker_id", k.GetID()) @@ -342,8 +357,15 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er } // Step 4: Encrypt age private key with the generated passphrase and store on disk - agePrivateKeyBytes := []byte(ageIdentity.String()) - encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivateKeyBytes, agePrivKeyPassphrase) + // Create secure buffers for both the private key and passphrase + agePrivKeyStr := ageIdentity.String() + agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr)) + defer agePrivKeyBuffer.Destroy() + + passphraseBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyPassphrase)) + defer passphraseBuffer.Destroy() + + encryptedAgePrivKey, err := EncryptWithPassphrase(agePrivKeyBuffer.Bytes(), passphraseBuffer) if err != nil { return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err) } diff --git a/internal/secret/passphrase_test.go b/internal/secret/passphrase_test.go index a756d2a..d58257b 100644 --- a/internal/secret/passphrase_test.go +++ b/internal/secret/passphrase_test.go @@ -9,6 +9,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -76,7 +77,9 @@ func TestPassphraseUnlockerWithRealFS(t *testing.T) { // Test encrypting private key with passphrase t.Run("EncryptPrivateKey", func(t *testing.T) { privKeyData := []byte(agePrivateKey) - encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyData, testPassphrase) + passphraseBuffer := memguard.NewBufferFromBytes([]byte(testPassphrase)) + defer passphraseBuffer.Destroy() + encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyData, passphraseBuffer) if err != nil { t.Fatalf("Failed to encrypt private key: %v", err) } diff --git a/internal/secret/passphraseunlocker.go b/internal/secret/passphraseunlocker.go index d09603b..37a269b 100644 --- a/internal/secret/passphraseunlocker.go +++ b/internal/secret/passphraseunlocker.go @@ -7,6 +7,7 @@ import ( "path/filepath" "filippo.io/age" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -15,16 +16,17 @@ type PassphraseUnlocker struct { Directory string Metadata UnlockerMetadata fs afero.Fs - Passphrase string + Passphrase *memguard.LockedBuffer // Secure buffer for passphrase } // getPassphrase retrieves the passphrase from memory, environment, or user input -func (p *PassphraseUnlocker) getPassphrase() (string, error) { +// Returns a LockedBuffer for secure memory handling +func (p *PassphraseUnlocker) getPassphrase() (*memguard.LockedBuffer, error) { // First check if we already have the passphrase - if p.Passphrase != "" { + if p.Passphrase != nil && p.Passphrase.IsAlive() { Debug("Using in-memory passphrase", "unlocker_id", p.GetID()) - - return p.Passphrase, nil + // Return a copy of the passphrase buffer + return memguard.NewBufferFromBytes(p.Passphrase.Bytes()), nil } Debug("No passphrase in memory, checking environment") @@ -32,20 +34,22 @@ func (p *PassphraseUnlocker) getPassphrase() (string, error) { passphraseStr := os.Getenv(EnvUnlockPassphrase) if passphraseStr != "" { Debug("Using passphrase from environment", "unlocker_id", p.GetID()) - - return passphraseStr, nil + // Convert to secure buffer + secureBuffer := memguard.NewBufferFromBytes([]byte(passphraseStr)) + + return secureBuffer, nil } Debug("No passphrase in environment, prompting user") // Prompt for passphrase - passphraseStr, err := ReadPassphrase("Enter unlock passphrase: ") + secureBuffer, err := ReadPassphrase("Enter unlock passphrase: ") if err != nil { Debug("Failed to read passphrase", "error", err, "unlocker_id", p.GetID()) - - return "", fmt.Errorf("failed to read passphrase: %w", err) + + return nil, fmt.Errorf("failed to read passphrase: %w", err) } - return passphraseStr, nil + return secureBuffer, nil } // GetIdentity implements Unlocker interface for passphrase-based unlockers @@ -55,10 +59,11 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) { slog.String("unlocker_type", p.GetType()), ) - passphraseStr, err := p.getPassphrase() + passphraseBuffer, err := p.getPassphrase() if err != nil { return nil, err } + defer passphraseBuffer.Destroy() // Read encrypted private key of unlocker unlockerPrivPath := filepath.Join(p.Directory, "priv.age") @@ -79,7 +84,7 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) { Debug("Decrypting unlocker private key with passphrase", "unlocker_id", p.GetID()) // Decrypt the unlocker private key with passphrase - privKeyData, err := DecryptWithPassphrase(encryptedPrivKeyData, passphraseStr) + privKeyData, err := DecryptWithPassphrase(encryptedPrivKeyData, passphraseBuffer) if err != nil { Debug("Failed to decrypt unlocker private key", "error", err, "unlocker_id", p.GetID()) @@ -93,7 +98,17 @@ func (p *PassphraseUnlocker) GetIdentity() (*age.X25519Identity, error) { // Parse the decrypted private key Debug("Parsing decrypted unlocker identity", "unlocker_id", p.GetID()) - identity, err := age.ParseX25519Identity(string(privKeyData)) + + // Create a secure buffer for the private key data + privKeyBuffer := memguard.NewBufferFromBytes(privKeyData) + defer privKeyBuffer.Destroy() + + // Clear the original private key data + for i := range privKeyData { + privKeyData[i] = 0 + } + + identity, err := age.ParseX25519Identity(privKeyBuffer.String()) if err != nil { Debug("Failed to parse unlocker private key", "error", err, "unlocker_id", p.GetID()) @@ -133,6 +148,11 @@ func (p *PassphraseUnlocker) GetID() string { // Remove implements Unlocker interface - removes the passphrase unlocker func (p *PassphraseUnlocker) Remove() error { + // Clean up the passphrase from memory if it exists + if p.Passphrase != nil && p.Passphrase.IsAlive() { + p.Passphrase.Destroy() + } + // For passphrase unlockers, we just need to remove the directory // No external resources (like keychain items) to clean up if err := p.fs.RemoveAll(p.Directory); err != nil { @@ -152,7 +172,8 @@ func NewPassphraseUnlocker(fs afero.Fs, directory string, metadata UnlockerMetad } // CreatePassphraseUnlocker creates a new passphrase-protected unlocker -func CreatePassphraseUnlocker(fs afero.Fs, stateDir string, passphrase string) (*PassphraseUnlocker, error) { +// The passphrase must be provided as a LockedBuffer for security +func CreatePassphraseUnlocker(fs afero.Fs, stateDir string, passphrase *memguard.LockedBuffer) (*PassphraseUnlocker, error) { // Get current vault currentVault, err := GetCurrentVault(fs, stateDir) if err != nil { diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index a7942e1..e0ad3ee 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -16,6 +16,7 @@ import ( "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -270,7 +271,9 @@ Passphrase: ` + testPassphrase + ` vlt.Unlock(ltIdentity) // Create a passphrase unlocker first (to have current unlocker) - passUnlocker, err := vlt.CreatePassphraseUnlocker("test-passphrase") + passphraseBuffer := memguard.NewBufferFromBytes([]byte("test-passphrase")) + defer passphraseBuffer.Destroy() + passUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) if err != nil { t.Fatalf("Failed to create passphrase unlocker: %v", err) } diff --git a/internal/secret/secret.go b/internal/secret/secret.go index 3171a54..66ce9ff 100644 --- a/internal/secret/secret.go +++ b/internal/secret/secret.go @@ -11,6 +11,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -21,7 +22,7 @@ type VaultInterface interface { GetName() string GetFilesystem() afero.Fs GetCurrentUnlocker() (Unlocker, error) - CreatePassphraseUnlocker(passphrase string) (*PassphraseUnlocker, error) + CreatePassphraseUnlocker(passphrase *memguard.LockedBuffer) (*PassphraseUnlocker, error) } // Secret represents a secret in a vault diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go index 7872047..742e3bb 100644 --- a/internal/secret/secret_test.go +++ b/internal/secret/secret_test.go @@ -9,6 +9,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/stretchr/testify/require" ) @@ -120,7 +121,7 @@ func (m *MockVault) GetCurrentUnlocker() (Unlocker, error) { return nil, nil } -func (m *MockVault) CreatePassphraseUnlocker(_ string) (*PassphraseUnlocker, error) { +func (m *MockVault) CreatePassphraseUnlocker(_ *memguard.LockedBuffer) (*PassphraseUnlocker, error) { return nil, nil } diff --git a/internal/secret/version_test.go b/internal/secret/version_test.go index 46020c6..182e703 100644 --- a/internal/secret/version_test.go +++ b/internal/secret/version_test.go @@ -41,6 +41,7 @@ import ( "time" "filippo.io/age" + "github.com/awnumar/memguard" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -74,7 +75,7 @@ func (m *MockVersionVault) GetCurrentUnlocker() (Unlocker, error) { return nil, fmt.Errorf("not implemented in mock") } -func (m *MockVersionVault) CreatePassphraseUnlocker(_ string) (*PassphraseUnlocker, error) { +func (m *MockVersionVault) CreatePassphraseUnlocker(_ *memguard.LockedBuffer) (*PassphraseUnlocker, error) { return nil, fmt.Errorf("not implemented in mock") } diff --git a/internal/vault/unlockers.go b/internal/vault/unlockers.go index af8d3de..2deab9e 100644 --- a/internal/vault/unlockers.go +++ b/internal/vault/unlockers.go @@ -10,6 +10,7 @@ import ( "filippo.io/age" "git.eeqj.de/sneak/secret/internal/secret" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -316,7 +317,8 @@ func (v *Vault) SelectUnlocker(unlockerID string) error { } // CreatePassphraseUnlocker creates a new passphrase-protected unlocker -func (v *Vault) CreatePassphraseUnlocker(passphrase string) (*secret.PassphraseUnlocker, error) { +// The passphrase must be provided as a LockedBuffer for security +func (v *Vault) CreatePassphraseUnlocker(passphrase *memguard.LockedBuffer) (*secret.PassphraseUnlocker, error) { vaultDir, err := v.GetDirectory() if err != nil { return nil, fmt.Errorf("failed to get vault directory: %w", err) @@ -343,8 +345,11 @@ func (v *Vault) CreatePassphraseUnlocker(passphrase string) (*secret.PassphraseU } // Encrypt private key with passphrase - privKeyData := []byte(unlockerIdentity.String()) - encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyData, passphrase) + privKeyStr := unlockerIdentity.String() + privKeyBuffer := memguard.NewBufferFromBytes([]byte(privKeyStr)) + defer privKeyBuffer.Destroy() + + encryptedPrivKey, err := secret.EncryptWithPassphrase(privKeyBuffer.Bytes(), passphrase) if err != nil { return nil, fmt.Errorf("failed to encrypt unlocker private key: %w", err) } diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index 0d2d310..be6ed9e 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -6,6 +6,7 @@ import ( "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/pkg/agehd" + "github.com/awnumar/memguard" "github.com/spf13/afero" ) @@ -172,7 +173,9 @@ func TestVaultOperations(t *testing.T) { } // Create a passphrase unlocker - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker("test-passphrase") + passphraseBuffer := memguard.NewBufferFromBytes([]byte("test-passphrase")) + defer passphraseBuffer.Destroy() + passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) if err != nil { t.Fatalf("Failed to create passphrase unlocker: %v", err) } diff --git a/pkg/bip85/bip85.go b/pkg/bip85/bip85.go index 7911dc9..6c31650 100644 --- a/pkg/bip85/bip85.go +++ b/pkg/bip85/bip85.go @@ -33,7 +33,7 @@ const ( // AppHDWIF is the application number for WIF (Wallet Import Format) for Bitcoin Core AppHDWIF = 2 // AppXPRV is the application number for extended private key - AppXPRV = 32 + AppXPRV = 32 APP_HEX = 128169 //nolint:revive // ALL_CAPS used for BIP85 constants APP_PWD64 = 707764 // Base64 passwords //nolint:revive // ALL_CAPS used for BIP85 constants AppPWD85 = 707785 // Base85 passwords