Skip to content

Commit 3717adf

Browse files
BenehikoBenehiko
authored
Merge pull request #492 from docker/zero/out
fix: zero sensitive byte slices after use across all store backends
2 parents 072216a + 1eed258 commit 3717adf

File tree

6 files changed

+103
-66
lines changed

6 files changed

+103
-66
lines changed

‎plugins/pass/commands/set.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func secretMappingFromSTDIN(ctx context.Context, reader io.Reader, id string) (*
7777
if err != nil {
7878
return nil, err
7979
}
80+
defer clear(data)
8081

8182
return &secret{
8283
id: id,

‎store/keychain/keychain_darwin.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
123123
if err != nil {
124124
return nil, err
125125
}
126+
defer clear(result.Data)
126127

127128
attributes, err := convertAttributes(result.Attributes)
128129
if err != nil {
@@ -179,6 +180,7 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
179180
if err != nil {
180181
return err
181182
}
183+
defer clear(data)
182184
item := newKeychainItem(id.String(), k)
183185
item.SetData(data)
184186
// 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
268270
}
269271
safelyCleanMetadata(attr)
270272

271-
i, err := getItemWithData(id.String(), k)
273+
secret, err := k.loadSecret(ctx, id, attr)
272274
if err != nil {
273275
return nil, err
274276
}
275-
276-
secret := k.factory(ctx, id)
277-
if err := secret.SetMetadata(attr); err != nil {
278-
return nil, err
279-
}
280-
if err := secret.Unmarshal(i.Data); err != nil {
281-
return nil, err
282-
}
283277
creds[id] = secret
284278
}
285279

286280
return creds, nil
287281
}
288282

283+
// loadSecret fetches the raw keychain data for id, zeroes it after use,
284+
// and returns a fully populated Secret.
285+
func (k *keychainStore[T]) loadSecret(ctx context.Context, id store.ID, attr map[string]string) (store.Secret, error) {
286+
i, err := getItemWithData(id.String(), k)
287+
if err != nil {
288+
return nil, err
289+
}
290+
defer clear(i.Data)
291+
292+
secret := k.factory(ctx, id)
293+
if err := secret.SetMetadata(attr); err != nil {
294+
return nil, err
295+
}
296+
return secret, secret.Unmarshal(i.Data)
297+
}
298+
289299
func mapKeychainError(err error) error {
290300
if err == nil {
291301
return nil

‎store/keychain/keychain_linux.go‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,24 @@ func (k *keychainStore[T]) Upsert(ctx context.Context, id store.ID, secret store
346346
return k.Save(ctx, id, secret)
347347
}
348348

349+
// loadSecret fetches the raw secret value for itemPath, zeroes it after use,
350+
// and returns a fully populated Secret.
351+
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) {
352+
value, err := svc.GetSecret(itemPath, *session)
353+
if err != nil {
354+
return nil, err
355+
}
356+
defer clear(value)
357+
358+
safelyCleanMetadata(attributes)
359+
360+
secret := k.factory(ctx, id)
361+
if err := secret.SetMetadata(attributes); err != nil {
362+
return nil, err
363+
}
364+
return secret, secret.Unmarshal(value)
365+
}
366+
349367
//gocyclo:ignore
350368
func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
351369
service, err := kc.NewService()
@@ -417,23 +435,10 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
417435
continue
418436
}
419437

420-
value, err := service.GetSecret(itemPath, *session)
438+
secret, err := k.loadSecret(ctx, secretID, service, itemPath, session, attributes)
421439
if err != nil {
422440
return nil, err
423441
}
424-
safelyCleanMetadata(attributes)
425-
426-
secret := k.factory(ctx, secretID)
427-
if err := secret.SetMetadata(attributes); err != nil {
428-
clear(value)
429-
return nil, err
430-
}
431-
if err := secret.Unmarshal(value); err != nil {
432-
clear(value)
433-
return nil, err
434-
}
435-
clear(value)
436-
437442
credentials[secretID] = secret
438443
}
439444

‎store/keychain/keychain_windows.go‎

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
176176
if err != nil {
177177
return nil, err
178178
}
179+
defer clear(rawBlob)
179180
} else {
180181
rawBlob = gc.CredentialBlob
181182
}
@@ -377,45 +378,58 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
377378
return nil, mapWindowsCredentialError(err)
378379
}
379380

380-
gcAttributes := mapFromWindowsAttributes(gc.Attributes)
381-
382-
// Determine the raw UTF-16 blob before safelyCleanMetadata strips chunkCountKey.
383-
var rawBlob []byte
384-
if countStr, ok := gcAttributes[chunkCountKey]; ok {
385-
count, err := strconv.Atoi(countStr)
386-
if err != nil {
387-
return nil, fmt.Errorf("invalid chunk count %q: %w", countStr, err)
388-
}
389-
rawBlob, err = k.readChunks(id, count)
390-
if err != nil {
391-
return nil, err
392-
}
393-
} else {
394-
rawBlob = gc.CredentialBlob
381+
secret, err := k.loadSecret(ctx, id, gc)
382+
if err != nil {
383+
return nil, err
395384
}
385+
secrets[id] = secret
386+
}
396387

397-
safelyCleanMetadata(gcAttributes)
388+
return secrets, nil
389+
}
398390

399-
secret := k.factory(ctx, id)
400-
if err := secret.SetMetadata(gcAttributes); err != nil {
401-
return nil, err
402-
}
391+
// loadSecret fetches, decodes, and zeroes the raw blob for id, then
392+
// returns a fully populated Secret. rawBlob is zeroed only when it was
393+
// allocated by readChunks (chunked path); gc.CredentialBlob is not ours.
394+
func (k *keychainStore[T]) loadSecret(ctx context.Context, id store.ID, gc *wincred.GenericCredential) (store.Secret, error) {
395+
gcAttributes := mapFromWindowsAttributes(gc.Attributes)
403396

404-
decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
405-
blob, _, err := transform.Bytes(decoder, rawBlob)
397+
var (
398+
rawBlob []byte
399+
chunked bool
400+
)
401+
if countStr, ok := gcAttributes[chunkCountKey]; ok {
402+
count, err := strconv.Atoi(countStr)
406403
if err != nil {
407-
return nil, err
404+
return nil, fmt.Errorf("invalid chunk count %q: %w", countStr, err)
408405
}
409-
410-
if err := secret.Unmarshal(blob); err != nil {
411-
clear(blob)
406+
rawBlob, err = k.readChunks(id, count)
407+
if err != nil {
412408
return nil, err
413409
}
414-
clear(blob)
415-
secrets[id] = secret
410+
chunked = true
411+
} else {
412+
rawBlob = gc.CredentialBlob
413+
}
414+
if chunked {
415+
defer clear(rawBlob)
416416
}
417417

418-
return secrets, nil
418+
safelyCleanMetadata(gcAttributes)
419+
420+
secret := k.factory(ctx, id)
421+
if err := secret.SetMetadata(gcAttributes); err != nil {
422+
return nil, err
423+
}
424+
425+
decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
426+
blob, _, err := transform.Bytes(decoder, rawBlob)
427+
if err != nil {
428+
return nil, err
429+
}
430+
defer clear(blob)
431+
432+
return secret, secret.Unmarshal(blob)
419433
}
420434

421435
func mapWindowsCredentialError(err error) error {

‎store/posixage/prompt.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,12 @@ func promptForEncryptionKeys(ctx context.Context, funcs []promptCaller) (map[sec
105105
return nil, err
106106
}
107107
raw = bytes.TrimSpace(raw)
108-
defer clear(raw)
109108
if len(raw) == 0 {
109+
clear(raw)
110110
return nil, errors.New("empty key provided on registered callback function")
111111
}
112112
m[groupType] = append(m[groupType], string(raw))
113+
clear(raw)
113114
}
114115
return m, nil
115116
}

‎store/posixage/store.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,28 +138,34 @@ func (f *fileStore[T]) decryptSecret(ctx context.Context, encryptedSecrets []sec
138138
if err != nil {
139139
return nil, err
140140
}
141-
defer clear(decryptionKey)
142141

143-
identity, err := secretfile.GetIdentity(keyType, string(decryptionKey))
144-
if err != nil {
145-
return nil, err
146-
}
147-
148-
r, err := age.Decrypt(bytes.NewReader(encryptedSecrets[index].EncryptedData), identity)
142+
plaintext, err := f.tryDecrypt(keyType, decryptionKey, encryptedSecrets[index].EncryptedData)
149143
if err != nil {
150144
f.logger.Errorf("failed to decrypt secret of type :%s", keyType)
151145
continue
152146
}
147+
return plaintext, nil
148+
}
153149

154-
decryptedSecret, err := io.ReadAll(r)
155-
if err != nil {
156-
return nil, err
157-
}
150+
return nil, errors.New("could not decrypt secret with provided decryption keys")
151+
}
152+
153+
// tryDecrypt uses decryptionKey to decrypt encryptedData, zeroing the key after
154+
// use regardless of outcome.
155+
func (f *fileStore[T]) tryDecrypt(keyType secretfile.KeyType, decryptionKey, encryptedData []byte) ([]byte, error) {
156+
defer clear(decryptionKey)
158157

159-
return decryptedSecret, nil
158+
identity, err := secretfile.GetIdentity(keyType, string(decryptionKey))
159+
if err != nil {
160+
return nil, err
160161
}
161162

162-
return nil, errors.New("could not decrypt secret with provided decryption keys")
163+
r, err := age.Decrypt(bytes.NewReader(encryptedData), identity)
164+
if err != nil {
165+
return nil, err
166+
}
167+
168+
return io.ReadAll(r)
163169
}
164170

165171
func (f *fileStore[T]) Delete(ctx context.Context, id store.ID) error {

0 commit comments

Comments
 (0)