diff --git a/plugins/pass/commands/set.go b/plugins/pass/commands/set.go index 3a9d8612..67c7eba4 100644 --- a/plugins/pass/commands/set.go +++ b/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, diff --git a/store/keychain/keychain_darwin.go b/store/keychain/keychain_darwin.go index d59779be..af80ac0c 100644 --- a/store/keychain/keychain_darwin.go +++ b/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 diff --git a/store/keychain/keychain_linux.go b/store/keychain/keychain_linux.go index 39414a17..d127fa0c 100644 --- a/store/keychain/keychain_linux.go +++ b/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 } diff --git a/store/keychain/keychain_windows.go b/store/keychain/keychain_windows.go index 81625df8..f76f0b7f 100644 --- a/store/keychain/keychain_windows.go +++ b/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 { diff --git a/store/posixage/prompt.go b/store/posixage/prompt.go index 6b5e73f1..bc2f031e 100644 --- a/store/posixage/prompt.go +++ b/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 } diff --git a/store/posixage/store.go b/store/posixage/store.go index fbbdee8b..11d44108 100644 --- a/store/posixage/store.go +++ b/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, 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 {