sneak/secret
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove deprecated Secret.Save function

Browse Source 
- Removed unused deprecated Save(value []byte, force bool) function
- This function accepted unprotected secret data which was a security issue
- All code now uses vault.AddSecret directly with LockedBuffer
- Updated TODO.md to reflect completion of this security fix
This commit is contained in:
Jeffrey Paul 2025-07-15 08:40:35 +02:00
parent 9cbe055791
commit e82d428b05
2 changed files with 23 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
TODO.md
View File

@ -4,6 +4,29 @@ This document outlines the bugs, issues, and improvements that need to be
addressed before the 1.0 release of the secret manager. Items are
prioritized from most critical (top) to least critical (bottom).
## CRITICAL MEMORY SECURITY ISSUES
### Functions accepting bare []byte for sensitive data
- [x] **1. Secret.Save accepts unprotected data**: `internal/secret/secret.go:67` - `Save(value []byte, force bool)` - ✓ REMOVED - deprecated function deleted
- [ ] **2. EncryptWithPassphrase accepts unprotected data**: `internal/secret/crypto.go:73` - `EncryptWithPassphrase(data []byte, passphrase *memguard.LockedBuffer)` - the data parameter should be LockedBuffer
- [ ] **3. storeInKeychain accepts unprotected data**: `internal/secret/keychainunlocker.go:469` - `storeInKeychain(itemName string, data []byte)` - stores secrets in keychain with unprotected data
- [ ] **4. gpgEncryptDefault accepts unprotected data**: `internal/secret/pgpunlocker.go:351` - `gpgEncryptDefault(data []byte, keyID string)` - encrypts unprotected data
### Functions returning unprotected secrets
- [ ] **5. GetValue returns unprotected secret**: `internal/secret/secret.go:93` - `GetValue(unlocker Unlocker) ([]byte, error)` - returns decrypted secret as bare []byte
- [ ] **6. DecryptWithIdentity returns unprotected data**: `internal/secret/crypto.go:57` - `DecryptWithIdentity(data []byte, identity age.Identity) ([]byte, error)` - returns decrypted data unprotected
- [ ] **7. DecryptWithPassphrase returns unprotected data**: `internal/secret/crypto.go:94` - `DecryptWithPassphrase(encryptedData []byte, passphrase *memguard.LockedBuffer) ([]byte, error)` - returns decrypted data unprotected
- [ ] **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
### 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
- [ ] **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
- [ ] **12. GPG encrypt exposes private key**: `internal/secret/pgpunlocker.go:256` - `GPGEncryptFunc(agePrivateKeyBuffer.Bytes(), gpgKeyID)` - private key exposed to external function
- [ ] **13. Keychain encrypt exposes private key**: `internal/secret/keychainunlocker.go:371` - `EncryptWithPassphrase(agePrivKeyBuffer.Bytes(), passphraseBuffer)` - private key passed as bare bytes
## Code Cleanups
* we shouldn't be passing around a statedir, it should be read from the

26
internal/secret/secret.go
View File

@ -62,32 +62,6 @@ func NewSecret(vault VaultInterface, name string) *Secret {
}
}
// Save is deprecated - use vault.AddSecret directly which creates versions
// Kept for backward compatibility
func (s *Secret) Save(value []byte, force bool) error {
DebugWith("Saving secret (deprecated method)",
slog.String("secret_name", s.Name),
slog.String("vault_name", s.vault.GetName()),
slog.Int("value_length", len(value)),
slog.Bool("force", force),
)
// Create a secure buffer for the value - note that the caller
// should ideally pass a LockedBuffer directly to vault.AddSecret
valueBuffer := memguard.NewBufferFromBytes(value)
defer valueBuffer.Destroy()
err := s.vault.AddSecret(s.Name, valueBuffer, force)
if err != nil {
Debug("Failed to save secret", "error", err, "secret_name", s.Name)
return err
}
Debug("Successfully saved secret", "secret_name", s.Name)
return nil
}
// GetValue retrieves and decrypts the current version's value using the provided unlocker
func (s *Secret) GetValue(unlocker Unlocker) ([]byte, error) {