Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 148 additions & 7 deletions store/keychain/keychain_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ package keychain
import (
"context"
"errors"
"fmt"
"iter"
"maps"
"strconv"
"strings"

"github.com/danieljoos/wincred"
Expand All @@ -38,6 +40,20 @@ var (
sysErrNoSuchLogonSession = windows.ERROR_NO_SUCH_LOGON_SESSION
)

const (
// maxBlobSize is the maximum size of a Windows Credential Manager blob
// (CRED_MAX_CREDENTIAL_BLOB_SIZE = 5 * 512 bytes).
maxBlobSize = 2560

// chunkCountKey is stored in the primary credential's attributes when a
// secret's encoded blob exceeds maxBlobSize and must be split.
chunkCountKey = "chunk:count"

// chunkIndexKey is stored in each chunk credential's attributes to
// identify it as a chunk and record its position.
chunkIndexKey = "chunk:index"
)

// encodeSecret marshals the secret into a slice of bytes in UTF16 format
func encodeSecret(secret store.Secret) ([]byte, error) {
data, err := secret.Marshal()
Expand Down Expand Up @@ -65,13 +81,72 @@ func decodeSecret(blob []byte, secret store.Secret) error {
return secret.Unmarshal(val)
}

// chunkBlob splits blob into consecutive slices each at most size bytes long.
func chunkBlob(blob []byte, size int) [][]byte {
var chunks [][]byte
for len(blob) > 0 {
n := min(size, len(blob))
chunks = append(chunks, blob[:n])
blob = blob[n:]
}
return chunks
}

// isChunkCredential reports whether the given attributes belong to a chunk
// credential (as opposed to a primary credential).
func isChunkCredential(attrs []wincred.CredentialAttribute) bool {
for _, attr := range attrs {
if attr.Keyword == chunkIndexKey {
return true
}
}
return false
}

type keychainStore[T store.Secret] struct {
serviceGroup string
serviceName string
factory store.Factory[T]
}

// itemChunkLabel returns the target name for the i-th chunk of a secret.
func (k *keychainStore[T]) itemChunkLabel(id store.ID, index int) string {
return fmt.Sprintf("%s:chunk:%d", k.itemLabel(id.String()), index)
}

// readChunks fetches count chunk credentials for id and concatenates their
// raw CredentialBlob bytes in order.
func (k *keychainStore[T]) readChunks(id store.ID, count int) ([]byte, error) {
var blob []byte
for i := range count {
gc, err := wincred.GetGenericCredential(k.itemChunkLabel(id, i))
if err != nil {
return nil, mapWindowsCredentialError(err)
}
blob = append(blob, gc.CredentialBlob...)
}
return blob, nil
}

// deleteChunks removes chunk credentials for id until none remain.
// It is safe to call when no chunks exist.
func (k *keychainStore[T]) deleteChunks(id store.ID) error {
for i := 0; ; i++ {
g := wincred.NewGenericCredential(k.itemChunkLabel(id, i))
err := g.Delete()
if err != nil {
if errors.Is(err, wincred.ErrElementNotFound) {
return nil
}
return mapWindowsCredentialError(err)
}
}
}

func (k *keychainStore[T]) Delete(_ context.Context, id store.ID) error {
if err := k.deleteChunks(id); err != nil {
return err
}
g := wincred.NewGenericCredential(k.itemLabel(id.String()))
err := g.Delete()
if err != nil && !errors.Is(err, wincred.ErrElementNotFound) {
Expand All @@ -87,13 +162,29 @@ func (k *keychainStore[T]) Get(ctx context.Context, id store.ID) (store.Secret,
}

attributes := mapFromWindowsAttributes(gc.Attributes)

// Determine the raw UTF-16 blob before safelyCleanMetadata strips chunkCountKey.
var rawBlob []byte
if countStr, ok := attributes[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
}

safelyCleanMetadata(attributes)

secret := k.factory(ctx, id)
if err := secret.SetMetadata(attributes); err != nil {
return nil, err
}
if err := decodeSecret(gc.CredentialBlob, secret); err != nil {
if err := decodeSecret(rawBlob, secret); err != nil {
return nil, err
}
return secret, nil
Expand Down Expand Up @@ -126,6 +217,9 @@ func isServiceCredential[T store.Secret](k *keychainStore[T], attrs []wincred.Cr
func findServiceCredentials[T store.Secret](k *keychainStore[T], pattern store.Pattern, credentials []*wincred.Credential) iter.Seq[*wincred.Credential] {
return func(yield func(cred *wincred.Credential) bool) {
for _, c := range credentials {
if isChunkCredential(c.Attributes) {
continue
}
if !isServiceCredential(k, c.Attributes) {
continue
}
Expand Down Expand Up @@ -204,10 +298,40 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
safelySetMetadata(k.serviceGroup, k.serviceName, attributes)
safelySetID(id, attributes)

// Always remove stale chunk credentials before writing, so that a
// previously-chunked secret that now fits in a single blob leaves no
// orphaned chunk credentials behind (and vice-versa).
if err := k.deleteChunks(id); err != nil {
return err
}

g := wincred.NewGenericCredential(k.itemLabel(id.String()))
g.UserName = id.String()
g.CredentialBlob = blob
g.Persist = wincred.PersistLocalMachine

// the blob is too large, we will chunk it across multiple entries
if len(blob) > maxBlobSize {
// Write chunk credentials for the oversized blob.
chunks := chunkBlob(blob, maxBlobSize)
for i, chunk := range chunks {
gc := wincred.NewGenericCredential(k.itemChunkLabel(id, i))
gc.UserName = id.String()
gc.CredentialBlob = chunk
gc.Persist = wincred.PersistLocalMachine
gc.Attributes = mapToWindowsAttributes(map[string]string{
chunkIndexKey: strconv.Itoa(i),
})
if err := mapWindowsCredentialError(gc.Write()); err != nil {
return err
}
}
// Write the primary credential with metadata and the chunk count.
// The blob is stored in chunk credentials only.
attributes[chunkCountKey] = strconv.Itoa(len(chunks))
} else {
g.CredentialBlob = blob
}

g.Attributes = mapToWindowsAttributes(attributes)
return mapWindowsCredentialError(g.Write())
}
Expand Down Expand Up @@ -246,19 +370,36 @@ func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (m
return nil, mapWindowsCredentialError(err)
}

decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
blob, _, err := transform.Bytes(decoder, gc.CredentialBlob)
if err != nil {
return nil, 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
Comment on lines +376 to +387
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to dedupe this? Seeing the same code above.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verbosity is best here

}

gcAttributes := mapFromWindowsAttributes(gc.Attributes)
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
}

if err := secret.Unmarshal(blob); err != nil {
return nil, err
}
Expand Down
67 changes: 67 additions & 0 deletions store/keychain/keychain_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,73 @@ import (
"github.com/stretchr/testify/assert"
)

func TestChunkBlob(t *testing.T) {
t.Run("empty blob returns no chunks", func(t *testing.T) {
assert.Empty(t, chunkBlob(nil, 4))
assert.Empty(t, chunkBlob([]byte{}, 4))
})
t.Run("blob smaller than size is a single chunk", func(t *testing.T) {
blob := []byte{1, 2, 3}
chunks := chunkBlob(blob, 4)
assert.Len(t, chunks, 1)
assert.Equal(t, blob, chunks[0])
})
t.Run("blob exactly size is a single chunk", func(t *testing.T) {
blob := []byte{1, 2, 3, 4}
chunks := chunkBlob(blob, 4)
assert.Len(t, chunks, 1)
assert.Equal(t, blob, chunks[0])
})
t.Run("blob splits into equal chunks", func(t *testing.T) {
blob := []byte{1, 2, 3, 4, 5, 6, 7, 8}
chunks := chunkBlob(blob, 4)
assert.Len(t, chunks, 2)
assert.Equal(t, []byte{1, 2, 3, 4}, chunks[0])
assert.Equal(t, []byte{5, 6, 7, 8}, chunks[1])
})
t.Run("blob splits with remainder in last chunk", func(t *testing.T) {
blob := []byte{1, 2, 3, 4, 5}
chunks := chunkBlob(blob, 4)
assert.Len(t, chunks, 2)
assert.Equal(t, []byte{1, 2, 3, 4}, chunks[0])
assert.Equal(t, []byte{5}, chunks[1])
})
t.Run("reassembled chunks equal original blob", func(t *testing.T) {
blob := make([]byte, 2560*3+100)
for i := range blob {
blob[i] = byte(i % 256)
}
chunks := chunkBlob(blob, maxBlobSize)
assert.Len(t, chunks, 4)

var reassembled []byte
for _, c := range chunks {
reassembled = append(reassembled, c...)
}
assert.Equal(t, blob, reassembled)
})
}

func TestIsChunkCredential(t *testing.T) {
t.Run("returns true when chunk:index attribute is present", func(t *testing.T) {
attrs := []wincred.CredentialAttribute{
{Keyword: chunkIndexKey, Value: []byte("0")},
}
assert.True(t, isChunkCredential(attrs))
})
t.Run("returns false when chunk:index attribute is absent", func(t *testing.T) {
attrs := []wincred.CredentialAttribute{
{Keyword: serviceGroupKey, Value: []byte("group")},
{Keyword: serviceNameKey, Value: []byte("name")},
}
assert.False(t, isChunkCredential(attrs))
})
t.Run("returns false for empty attributes", func(t *testing.T) {
assert.False(t, isChunkCredential(nil))
assert.False(t, isChunkCredential([]wincred.CredentialAttribute{}))
})
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want some more tests that verify Save/Filter/Get/Delete on credentials that need to be chunked work as expected?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely, but i think this is good enough to get this fix in quickly

func TestMapWindowsAttributes(t *testing.T) {
t.Run("can map to windows attributes", func(t *testing.T) {
attributes := map[string]string{
Expand Down
Loading