diff --git a/TODO.md b/TODO.md index 86f0747..5f9cc13 100644 --- a/TODO.md +++ b/TODO.md @@ -16,11 +16,11 @@ prioritized from most critical (top) to least critical (bottom). - [x] **5. GetValue returns unprotected secret**: `internal/secret/secret.go:93` - `GetValue(unlocker Unlocker) ([]byte, error)` - ✓ FIXED - now returns LockedBuffer internally - [x] **6. DecryptWithIdentity returns unprotected data**: `internal/secret/crypto.go:57` - `DecryptWithIdentity(data []byte, identity age.Identity) ([]byte, error)` - ✓ FIXED - now returns LockedBuffer - [x] **7. DecryptWithPassphrase returns unprotected data**: `internal/secret/crypto.go:94` - `DecryptWithPassphrase(encryptedData []byte, passphrase *memguard.LockedBuffer) ([]byte, error)` - ✓ FIXED - now returns LockedBuffer -- [ ] **8. gpgDecryptDefault returns unprotected data**: `internal/secret/pgpunlocker.go:368` - `gpgDecryptDefault(encryptedData []byte) ([]byte, error)` - returns decrypted data unprotected -- [ ] **9. getSecretValue returns unprotected data**: `internal/cli/crypto.go:269` - `getSecretValue()` returns bare []byte +- [x] **8. gpgDecryptDefault returns unprotected data**: `internal/secret/pgpunlocker.go:368` - `gpgDecryptDefault(encryptedData []byte) ([]byte, error)` - ✓ FIXED - now returns LockedBuffer +- [x] **9. getSecretValue returns unprotected data**: `internal/cli/crypto.go:269` - `getSecretValue()` returns bare []byte - ✓ ALREADY FIXED - returns LockedBuffer ### Intermediate string variables for passphrases -- [ ] **10. Passphrase extracted to string**: `internal/secret/crypto.go:79,100` - `passphraseStr := passphrase.String()` - passphrase extracted to plain string before use with age.NewScryptRecipient +- [x] **10. Passphrase extracted to string**: `internal/secret/crypto.go:79,100` - `passphraseStr := passphrase.String()` - ✓ UNAVOIDABLE - age library requires string parameter - [ ] **11. Age secret key in plain string**: `internal/cli/crypto.go:86,91,113` - Age secret key stored in plain string variable before conversion back to secure buffer ### Unprotected buffer.Bytes() usage diff --git a/internal/secret/crypto.go b/internal/secret/crypto.go index 124edf6..cd60b88 100644 --- a/internal/secret/crypto.go +++ b/internal/secret/crypto.go @@ -81,9 +81,8 @@ func EncryptWithPassphrase(data *memguard.LockedBuffer, passphrase *memguard.Loc return nil, fmt.Errorf("passphrase buffer is nil") } - // Get the passphrase string temporarily - passphraseStr := passphrase.String() - recipient, err := age.NewScryptRecipient(passphraseStr) + // Create recipient directly from passphrase - unavoidable string conversion due to age API + recipient, err := age.NewScryptRecipient(passphrase.String()) if err != nil { return nil, fmt.Errorf("failed to create scrypt recipient: %w", err) } @@ -98,9 +97,8 @@ func DecryptWithPassphrase(encryptedData []byte, passphrase *memguard.LockedBuff return nil, fmt.Errorf("passphrase buffer is nil") } - // Get the passphrase string temporarily - passphraseStr := passphrase.String() - identity, err := age.NewScryptIdentity(passphraseStr) + // Create identity directly from passphrase - unavoidable string conversion due to age API + identity, err := age.NewScryptIdentity(passphrase.String()) if err != nil { return nil, fmt.Errorf("failed to create scrypt identity: %w", err) } diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index 6ba8054..bdf278f 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -72,7 +72,7 @@ pinentry-mode loopback return stdout.Bytes(), nil } - secret.GPGDecryptFunc = func(encryptedData []byte) ([]byte, error) { + secret.GPGDecryptFunc = func(encryptedData []byte) (*memguard.LockedBuffer, error) { cmd := exec.Command("gpg", "--homedir", gnupgHomeDir, "--batch", @@ -91,7 +91,8 @@ pinentry-mode loopback return nil, fmt.Errorf("GPG decryption failed: %w\nStderr: %s", err, stderr.String()) } - return stdout.Bytes(), nil + // Create a secure buffer for the decrypted data + return memguard.NewBufferFromBytes(stdout.Bytes()), nil } // Restore original functions after test diff --git a/internal/secret/pgpunlocker.go b/internal/secret/pgpunlocker.go index 177d8cf..c1281e2 100644 --- a/internal/secret/pgpunlocker.go +++ b/internal/secret/pgpunlocker.go @@ -25,7 +25,8 @@ var ( // GPGDecryptFunc is the function used for GPG decryption // Can be overridden in tests to provide a non-interactive implementation - GPGDecryptFunc = gpgDecryptDefault //nolint:gochecknoglobals // Required for test mocking + //nolint:gochecknoglobals // Required for test mocking + GPGDecryptFunc func(encryptedData []byte) (*memguard.LockedBuffer, error) = gpgDecryptDefault // gpgKeyIDRegex validates GPG key IDs // Allows either: @@ -80,21 +81,22 @@ func (p *PGPUnlocker) GetIdentity() (*age.X25519Identity, error) { // Step 2: Decrypt the age private key using GPG Debug("Decrypting age private key with GPG", "unlocker_id", p.GetID()) - agePrivKeyData, err := GPGDecryptFunc(encryptedAgePrivKeyData) + agePrivKeyBuffer, err := GPGDecryptFunc(encryptedAgePrivKeyData) if err != nil { Debug("Failed to decrypt age private key with GPG", "error", err, "unlocker_id", p.GetID()) return nil, fmt.Errorf("failed to decrypt age private key with GPG: %w", err) } + defer agePrivKeyBuffer.Destroy() DebugWith("Successfully decrypted age private key with GPG", slog.String("unlocker_id", p.GetID()), - slog.Int("decrypted_length", len(agePrivKeyData)), + slog.Int("decrypted_length", agePrivKeyBuffer.Size()), ) // Step 3: Parse the decrypted age private key Debug("Parsing decrypted age private key", "unlocker_id", p.GetID()) - ageIdentity, err := age.ParseX25519Identity(string(agePrivKeyData)) + ageIdentity, err := age.ParseX25519Identity(agePrivKeyBuffer.String()) if err != nil { Debug("Failed to parse age private key", "error", err, "unlocker_id", p.GetID()) @@ -369,7 +371,7 @@ func gpgEncryptDefault(data *memguard.LockedBuffer, keyID string) ([]byte, error } // gpgDecryptDefault is the default implementation of GPG decryption -func gpgDecryptDefault(encryptedData []byte) ([]byte, error) { +func gpgDecryptDefault(encryptedData []byte) (*memguard.LockedBuffer, error) { cmd := exec.Command("gpg", "--quiet", "--decrypt") cmd.Stdin = strings.NewReader(string(encryptedData)) @@ -378,5 +380,8 @@ func gpgDecryptDefault(encryptedData []byte) ([]byte, error) { return nil, fmt.Errorf("GPG decryption failed: %w", err) } - return output, nil + // Create a secure buffer for the decrypted data + outputBuffer := memguard.NewBufferFromBytes(output) + + return outputBuffer, nil }