feat: harden secret unlock and deletion
This commit is contained in:
+20
-1
@@ -1322,8 +1322,13 @@ func (a *App) PluginSecretsStatus(pluginID string) (map[string]interface{}, stri
|
||||
if err != nil {
|
||||
return nil, err.Error()
|
||||
}
|
||||
initialized, err := session.Initialized()
|
||||
if err != nil {
|
||||
return nil, err.Error()
|
||||
}
|
||||
return map[string]interface{}{
|
||||
"unlocked": session.Unlocked(),
|
||||
"initialized": initialized,
|
||||
"unlocked": session.Unlocked(),
|
||||
}, ""
|
||||
}
|
||||
|
||||
@@ -1397,6 +1402,20 @@ func (a *App) PluginSecretsWrite(pluginID string, rawRecord map[string]interface
|
||||
return secretRecordMap(written, false), ""
|
||||
}
|
||||
|
||||
func (a *App) PluginSecretsDelete(pluginID, secretID string) string {
|
||||
if err := a.requirePluginSecretsAccess(pluginID, true); err != nil {
|
||||
return err.Error()
|
||||
}
|
||||
store, err := a.requireUnlockedSecretStore()
|
||||
if err != nil {
|
||||
return err.Error()
|
||||
}
|
||||
if err := store.Delete(secretID); err != nil {
|
||||
return err.Error()
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func (a *App) PluginSecretsCopyLink(pluginID, secretID string) (string, string) {
|
||||
if err := a.requirePluginSecretsAccess(pluginID, false); err != nil {
|
||||
return "", err.Error()
|
||||
|
||||
@@ -1628,6 +1628,9 @@ func TestPluginSecretsRequirePermissionsAndUnlock(t *testing.T) {
|
||||
if status["unlocked"] == true {
|
||||
t.Fatalf("new secret session should be locked: %+v", status)
|
||||
}
|
||||
if status["initialized"] == true {
|
||||
t.Fatalf("new secret session should not be initialized: %+v", status)
|
||||
}
|
||||
|
||||
if errStr := app.PluginSecretsUnlock("no.storage", "master password"); !strings.Contains(errStr, "secrets.read") {
|
||||
t.Fatalf("PluginSecretsUnlock err = %q, want secrets.read permission error", errStr)
|
||||
@@ -1636,6 +1639,10 @@ func TestPluginSecretsRequirePermissionsAndUnlock(t *testing.T) {
|
||||
t.Fatalf("PluginSecretsList before unlock err = %q, want locked", errStr)
|
||||
}
|
||||
|
||||
if errStr := app.PluginSecretsUnlock("secrets.plugin", "123123"); !strings.Contains(errStr, "at least 8 characters") {
|
||||
t.Fatalf("weak PluginSecretsUnlock err = %q, want minimum length error", errStr)
|
||||
}
|
||||
|
||||
if errStr := app.PluginSecretsUnlock("secrets.plugin", "master password"); errStr != "" {
|
||||
t.Fatalf("PluginSecretsUnlock: %s", errStr)
|
||||
}
|
||||
@@ -1646,6 +1653,9 @@ func TestPluginSecretsRequirePermissionsAndUnlock(t *testing.T) {
|
||||
if status["unlocked"] != true {
|
||||
t.Fatalf("secret session not unlocked: %+v", status)
|
||||
}
|
||||
if status["initialized"] != true {
|
||||
t.Fatalf("secret session not initialized: %+v", status)
|
||||
}
|
||||
|
||||
writeResult, errStr := app.PluginSecretsWrite("secrets.plugin", map[string]interface{}{
|
||||
"id": "client-a.database",
|
||||
@@ -1694,6 +1704,17 @@ func TestPluginSecretsRequirePermissionsAndUnlock(t *testing.T) {
|
||||
if link != "[Client A Database](verstak-secret://client-a.database)" {
|
||||
t.Fatalf("link = %q", link)
|
||||
}
|
||||
|
||||
if errStr := app.PluginSecretsDelete("secrets.plugin", "client-a.database"); errStr != "" {
|
||||
t.Fatalf("PluginSecretsDelete: %s", errStr)
|
||||
}
|
||||
list, errStr = app.PluginSecretsList("secrets.plugin")
|
||||
if errStr != "" {
|
||||
t.Fatalf("PluginSecretsList after delete: %s", errStr)
|
||||
}
|
||||
if len(list) != 0 {
|
||||
t.Fatalf("PluginSecretsList after delete = %+v, want empty", list)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPluginSecretsRejectWrongMasterPasswordAcrossSessions(t *testing.T) {
|
||||
|
||||
@@ -25,6 +25,7 @@ const (
|
||||
masterPBKDF2Iterations = 200000
|
||||
masterVerifierPlaintext = "verstak-secret-store:v1"
|
||||
masterMetadataVersion = 1
|
||||
minMasterPasswordLength = 8
|
||||
recordsDirName = "records"
|
||||
masterMetadataFileName = "metadata.json"
|
||||
ScopeGlobal = "global"
|
||||
@@ -184,6 +185,18 @@ func (s *Store) ReadRecord(id string) (SecretRecord, error) {
|
||||
return decoded, nil
|
||||
}
|
||||
|
||||
func (s *Store) Delete(id string) error {
|
||||
if err := validateID(id); err != nil {
|
||||
return err
|
||||
}
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
if err := os.Remove(s.pathForID(id)); err != nil && !os.IsNotExist(err) {
|
||||
return fmt.Errorf("delete secret %q: %w", id, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Store) ListRecords() ([]SecretRecord, error) {
|
||||
s.mu.RLock()
|
||||
entries, err := os.ReadDir(s.root)
|
||||
@@ -350,6 +363,14 @@ func (s *VaultSession) Store() (*Store, error) {
|
||||
return s.store, nil
|
||||
}
|
||||
|
||||
func (s *VaultSession) Initialized() (bool, error) {
|
||||
metadata, err := readMasterMetadata(s.root)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return metadata != nil, nil
|
||||
}
|
||||
|
||||
func (s *VaultSession) Unlock(masterPassword string) (*Store, error) {
|
||||
if strings.TrimSpace(masterPassword) == "" {
|
||||
return nil, fmt.Errorf("master password is empty")
|
||||
@@ -366,6 +387,9 @@ func (s *VaultSession) Unlock(masterPassword string) (*Store, error) {
|
||||
return nil, err
|
||||
}
|
||||
if metadata == nil {
|
||||
if err := validateInitialMasterPassword(masterPassword); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
created, key, err := createMasterMetadata(masterPassword)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -393,6 +417,13 @@ func (s *VaultSession) Unlock(masterPassword string) (*Store, error) {
|
||||
return store, nil
|
||||
}
|
||||
|
||||
func validateInitialMasterPassword(masterPassword string) error {
|
||||
if len([]rune(masterPassword)) < minMasterPasswordLength {
|
||||
return fmt.Errorf("master password must be at least %d characters", minMasterPasswordLength)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func readMasterMetadata(root string) (*masterMetadata, error) {
|
||||
data, err := os.ReadFile(filepath.Join(root, masterMetadataFileName))
|
||||
if err != nil {
|
||||
|
||||
@@ -106,6 +106,30 @@ func TestStoreRejectsUnsafeIDs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestStoreDeletesSecretRecord(t *testing.T) {
|
||||
store, err := NewStore(t.TempDir(), testKey(0x11))
|
||||
if err != nil {
|
||||
t.Fatalf("NewStore: %v", err)
|
||||
}
|
||||
if err := store.Write("server.password", "s3cr3t-value"); err != nil {
|
||||
t.Fatalf("Write: %v", err)
|
||||
}
|
||||
|
||||
if err := store.Delete("server.password"); err != nil {
|
||||
t.Fatalf("Delete: %v", err)
|
||||
}
|
||||
if _, err := store.Read("server.password"); err == nil {
|
||||
t.Fatal("Read after Delete succeeded")
|
||||
}
|
||||
list, err := store.ListRecords()
|
||||
if err != nil {
|
||||
t.Fatalf("ListRecords: %v", err)
|
||||
}
|
||||
if len(list) != 0 {
|
||||
t.Fatalf("ListRecords after Delete = %+v, want empty", list)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStoreListsScopedRecordsWithoutPlaintextOnDisk(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
store, err := NewStore(root, testKey(0x11))
|
||||
@@ -224,3 +248,30 @@ func TestVaultSessionUnlocksWithMasterPasswordOnce(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestVaultSessionReportsInitializationAndRejectsWeakInitialPassword(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
session := NewVaultSession(root)
|
||||
|
||||
initialized, err := session.Initialized()
|
||||
if err != nil {
|
||||
t.Fatalf("Initialized: %v", err)
|
||||
}
|
||||
if initialized {
|
||||
t.Fatal("new secret session is initialized")
|
||||
}
|
||||
if _, err := session.Unlock("123123"); err == nil || !strings.Contains(err.Error(), "at least 8 characters") {
|
||||
t.Fatalf("weak initial Unlock err = %v, want minimum length error", err)
|
||||
}
|
||||
|
||||
if _, err := session.Unlock("strong password"); err != nil {
|
||||
t.Fatalf("Unlock strong initial password: %v", err)
|
||||
}
|
||||
initialized, err = session.Initialized()
|
||||
if err != nil {
|
||||
t.Fatalf("Initialized after unlock: %v", err)
|
||||
}
|
||||
if !initialized {
|
||||
t.Fatal("secret session was not initialized")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user