fix: zero sensitive byte slices after use across all store backends

Replace deferred and manual clear() calls inside loop bodies with
helper methods (loadSecret, tryDecrypt) so that a single defer clear()
covers all exit paths per invocation. Also fixes two bugs in
store/posixage where defer clear() inside a loop body deferred zeroing
until function return rather than end of each iteration.

Affected: keychain (darwin/linux/windows), posixage, plugins/pass.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>

Author: Benehiko

plugins/pass/commands/set.go

@@ -77,6 +77,7 @@ func secretMappingFromSTDIN(ctx context.Context, reader io.Reader, id string) (*
 	if err != nil {
 		return nil, err
 	}
+	defer clear(data)
 
 	return &secret{
 		id:  id,

store/keychain/keychain_darwin.go

@@ -123,6 +123,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
 	if err != nil {
 		return nil, err
 	}
+	defer clear(result.Data)
 
 	attributes, err := convertAttributes(result.Attributes)
 	if err != nil {
@@ -179,6 +180,7 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
 	if err != nil {
 		return err
 	}
+	defer clear(data)
 	item := newKeychainItem(id.String(), k)
 	item.SetData(data)
 	// only creation of a secret needs the label attribute.
@@ -268,24 +270,32 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
 		}
 		safelyCleanMetadata(attr)
 
-		i, err := getItemWithData(id.String(), k)
+		secret, err := k.loadSecret(ctx, id, attr)
 		if err != nil {
 			return nil, err
 		}
-
-		secret := k.factory(ctx, id)
-		if err := secret.SetMetadata(attr); err != nil {
-			return nil, err
-		}
-		if err := secret.Unmarshal(i.Data); err != nil {
-			return nil, err
-		}
 		creds[id] = secret
 	}
 
 	return creds, nil
 }
 
+// loadSecret fetches the raw keychain data for id, zeroes it after use,
+// and returns a fully populated Secret.
+func (k *keychainStore[T]) loadSecret(ctx context.Context, id store.ID, attr map[string]string) (store.Secret, error) {
+	i, err := getItemWithData(id.String(), k)
+	if err != nil {
+		return nil, err
+	}
+	defer clear(i.Data)
+
+	secret := k.factory(ctx, id)
+	if err := secret.SetMetadata(attr); err != nil {
+		return nil, err
+	}
+	return secret, secret.Unmarshal(i.Data)
+}
+
 func mapKeychainError(err error) error {
 	if err == nil {
 		return nil

store/keychain/keychain_linux.go

@@ -346,6 +346,24 @@ func (k *keychainStore[T]) Upsert(ctx context.Context, id store.ID, secret store
 	return k.Save(ctx, id, secret)
 }
 
+// loadSecret fetches the raw secret value for itemPath, zeroes it after use,
+// and returns a fully populated Secret.
+func (k *keychainStore[T]) loadSecret(ctx context.Context, id store.ID, svc *kc.SecretService, itemPath dbus.ObjectPath, session *kc.Session, attributes map[string]string) (store.Secret, error) {
+	value, err := svc.GetSecret(itemPath, *session)
+	if err != nil {
+		return nil, err
+	}
+	defer clear(value)
+
+	safelyCleanMetadata(attributes)
+
+	secret := k.factory(ctx, id)
+	if err := secret.SetMetadata(attributes); err != nil {
+		return nil, err
+	}
+	return secret, secret.Unmarshal(value)
+}
+
 //gocyclo:ignore
 func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	service, err := kc.NewService()
@@ -417,23 +435,10 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
 			continue
 		}
 
-		value, err := service.GetSecret(itemPath, *session)
+		secret, err := k.loadSecret(ctx, secretID, service, itemPath, session, attributes)
 		if err != nil {
 			return nil, err
 		}
-		safelyCleanMetadata(attributes)
-
-		secret := k.factory(ctx, secretID)
-		if err := secret.SetMetadata(attributes); err != nil {
-			clear(value)
-			return nil, err
-		}
-		if err := secret.Unmarshal(value); err != nil {
-			clear(value)
-			return nil, err
-		}
-		clear(value)
-
 		credentials[secretID] = secret
 	}
 

store/keychain/keychain_windows.go

@@ -176,6 +176,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
 		if err != nil {
 			return nil, err
 		}
+		defer clear(rawBlob)
 	} else {
 		rawBlob = gc.CredentialBlob
 	}
@@ -377,45 +378,58 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
 			return nil, mapWindowsCredentialError(err)
 		}
 
-		gcAttributes := mapFromWindowsAttributes(gc.Attributes)
-
-		// Determine the raw UTF-16 blob before safelyCleanMetadata strips chunkCountKey.
-		var rawBlob []byte
-		if countStr, ok := gcAttributes[chunkCountKey]; ok {
-			count, err := strconv.Atoi(countStr)
-			if err != nil {
-				return nil, fmt.Errorf("invalid chunk count %q: %w", countStr, err)
-			}
-			rawBlob, err = k.readChunks(id, count)
-			if err != nil {
-				return nil, err
-			}
-		} else {
-			rawBlob = gc.CredentialBlob
+		secret, err := k.loadSecret(ctx, id, gc)
+		if err != nil {
+			return nil, err
 		}
+		secrets[id] = secret
+	}
 
-		safelyCleanMetadata(gcAttributes)
+	return secrets, nil
+}
 
-		secret := k.factory(ctx, id)
-		if err := secret.SetMetadata(gcAttributes); err != nil {
-			return nil, err
-		}
+// loadSecret fetches, decodes, and zeroes the raw blob for id, then
+// returns a fully populated Secret. rawBlob is zeroed only when it was
+// allocated by readChunks (chunked path); gc.CredentialBlob is not ours.
+func (k *keychainStore[T]) loadSecret(ctx context.Context, id store.ID, gc *wincred.GenericCredential) (store.Secret, error) {
+	gcAttributes := mapFromWindowsAttributes(gc.Attributes)
 
-		decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
-		blob, _, err := transform.Bytes(decoder, rawBlob)
+	var (
+		rawBlob []byte
+		chunked bool
+	)
+	if countStr, ok := gcAttributes[chunkCountKey]; ok {
+		count, err := strconv.Atoi(countStr)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("invalid chunk count %q: %w", countStr, err)
 		}
-
-		if err := secret.Unmarshal(blob); err != nil {
-			clear(blob)
+		rawBlob, err = k.readChunks(id, count)
+		if err != nil {
 			return nil, err
 		}
-		clear(blob)
-		secrets[id] = secret
+		chunked = true
+	} else {
+		rawBlob = gc.CredentialBlob
+	}
+	if chunked {
+		defer clear(rawBlob)
 	}
 
-	return secrets, nil
+	safelyCleanMetadata(gcAttributes)
+
+	secret := k.factory(ctx, id)
+	if err := secret.SetMetadata(gcAttributes); err != nil {
+		return nil, err
+	}
+
+	decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
+	blob, _, err := transform.Bytes(decoder, rawBlob)
+	if err != nil {
+		return nil, err
+	}
+	defer clear(blob)
+
+	return secret, secret.Unmarshal(blob)
 }
 
 func mapWindowsCredentialError(err error) error {

store/posixage/prompt.go

@@ -105,11 +105,12 @@ func promptForEncryptionKeys(ctx context.Context, funcs []promptCaller) (map[sec
 			return nil, err
 		}
 		raw = bytes.TrimSpace(raw)
-		defer clear(raw)
 		if len(raw) == 0 {
+			clear(raw)
 			return nil, errors.New("empty key provided on registered callback function")
 		}
 		m[groupType] = append(m[groupType], string(raw))
+		clear(raw)
 	}
 	return m, nil
 }

store/posixage/store.go

@@ -138,28 +138,34 @@ func (f *fileStore[T]) decryptSecret(ctx context.Context, encryptedSecrets []sec
 		if err != nil {
 			return nil, err
 		}
-		defer clear(decryptionKey)
 
-		identity, err := secretfile.GetIdentity(keyType, string(decryptionKey))
-		if err != nil {
-			return nil, err
-		}
-
-		r, err := age.Decrypt(bytes.NewReader(encryptedSecrets[index].EncryptedData), identity)
+		plaintext, err := f.tryDecrypt(keyType, decryptionKey, encryptedSecrets[index].EncryptedData)
 		if err != nil {
 			f.logger.Errorf("failed to decrypt secret of type :%s", keyType)
 			continue
 		}
+		return plaintext, nil
+	}
 
-		decryptedSecret, err := io.ReadAll(r)
-		if err != nil {
-			return nil, err
-		}
+	return nil, errors.New("could not decrypt secret with provided decryption keys")
+}
+
+// tryDecrypt uses decryptionKey to decrypt encryptedData, zeroing the key after
+// use regardless of outcome.
+func (f *fileStore[T]) tryDecrypt(keyType secretfile.KeyType, decryptionKey []byte, encryptedData []byte) ([]byte, error) {
+	defer clear(decryptionKey)
 
-		return decryptedSecret, nil
+	identity, err := secretfile.GetIdentity(keyType, string(decryptionKey))
+	if err != nil {
+		return nil, err
 	}
 
-	return nil, errors.New("could not decrypt secret with provided decryption keys")
+	r, err := age.Decrypt(bytes.NewReader(encryptedData), identity)
+	if err != nil {
+		return nil, err
+	}
+
+	return io.ReadAll(r)
 }
 
 func (f *fileStore[T]) Delete(ctx context.Context, id store.ID) error {