From f3ae27e2001a52446f6a969c26f74f345f8c8de3 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 14 Jan 2025 12:15:04 -0800 Subject: [PATCH 1/2] support for yubikey management key from a file This commit adds the yubikey attribute management-key-source. This attribute can be used to pass the yubikey management key from a file. Fixes smallstep/step-kms-plugin#207 --- kms/uri/uri.go | 19 +++- kms/uri/uri_test.go | 186 ++++++++++++++++++------------------ kms/yubikey/yubikey.go | 15 ++- kms/yubikey/yubikey_test.go | 21 ++++ 4 files changed, 143 insertions(+), 98 deletions(-) diff --git a/kms/uri/uri.go b/kms/uri/uri.go index e83fb9c1..7700d826 100644 --- a/kms/uri/uri.go +++ b/kms/uri/uri.go @@ -194,10 +194,25 @@ func (u *URI) Pin() string { return "" } +// Read returns the raw content of the file in the given attribute key. This +// method will return nil if the key is missing. +func (u *URI) Read(key string) ([]byte, error) { + path := u.Get(key) + if path == "" { + return nil, nil + } + return readFile(path) +} + func readFile(path string) ([]byte, error) { u, err := url.Parse(path) - if err == nil && (u.Scheme == "" || u.Scheme == "file") && u.Path != "" { - path = u.Path + if err == nil && (u.Scheme == "" || u.Scheme == "file") { + switch { + case u.Path != "": + path = u.Path + case u.Opaque != "": + path = u.Opaque + } } b, err := os.ReadFile(path) if err != nil { diff --git a/kms/uri/uri_test.go b/kms/uri/uri_test.go index db7dde75..5efa0b91 100644 --- a/kms/uri/uri_test.go +++ b/kms/uri/uri_test.go @@ -2,6 +2,8 @@ package uri import ( "net/url" + "os" + "path/filepath" "reflect" "testing" @@ -9,6 +11,13 @@ import ( "github.com/stretchr/testify/require" ) +func mustParse(t *testing.T, s string) *URI { + t.Helper() + u, err := Parse(s) + require.NoError(t, err) + return u +} + func TestNew(t *testing.T) { type args struct { scheme string @@ -154,13 +163,6 @@ func TestParseWithScheme(t *testing.T) { } func TestURI_Has(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } type args struct { key string } @@ -170,15 +172,15 @@ func TestURI_Has(t *testing.T) { args args want bool }{ - {"ok", mustParse("yubikey:slot-id=9a"), args{"slot-id"}, true}, - {"ok empty", mustParse("yubikey:slot-id="), args{"slot-id"}, true}, - {"ok query", mustParse("yubikey:pin=123456?slot-id="), args{"slot-id"}, true}, - {"ok empty no equal", mustParse("yubikey:slot-id"), args{"slot-id"}, true}, - {"ok query no equal", mustParse("yubikey:pin=123456?slot-id"), args{"slot-id"}, true}, - {"ok empty no equal other", mustParse("yubikey:slot-id;pin=123456"), args{"slot-id"}, true}, - {"ok query no equal other", mustParse("yubikey:pin=123456?slot-id&pin=123456"), args{"slot-id"}, true}, - {"fail", mustParse("yubikey:pin=123456"), args{"slot-id"}, false}, - {"fail with query", mustParse("yubikey:pin=123456?slot=9a"), args{"slot-id"}, false}, + {"ok", mustParse(t, "yubikey:slot-id=9a"), args{"slot-id"}, true}, + {"ok empty", mustParse(t, "yubikey:slot-id="), args{"slot-id"}, true}, + {"ok query", mustParse(t, "yubikey:pin=123456?slot-id="), args{"slot-id"}, true}, + {"ok empty no equal", mustParse(t, "yubikey:slot-id"), args{"slot-id"}, true}, + {"ok query no equal", mustParse(t, "yubikey:pin=123456?slot-id"), args{"slot-id"}, true}, + {"ok empty no equal other", mustParse(t, "yubikey:slot-id;pin=123456"), args{"slot-id"}, true}, + {"ok query no equal other", mustParse(t, "yubikey:pin=123456?slot-id&pin=123456"), args{"slot-id"}, true}, + {"fail", mustParse(t, "yubikey:pin=123456"), args{"slot-id"}, false}, + {"fail with query", mustParse(t, "yubikey:pin=123456?slot=9a"), args{"slot-id"}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -190,13 +192,6 @@ func TestURI_Has(t *testing.T) { } func TestURI_Get(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } type args struct { key string } @@ -206,12 +201,12 @@ func TestURI_Get(t *testing.T) { args args want string }{ - {"ok", mustParse("yubikey:slot-id=9a"), args{"slot-id"}, "9a"}, - {"ok first", mustParse("yubikey:slot-id=9a;slot-id=9b"), args{"slot-id"}, "9a"}, - {"ok multiple", mustParse("yubikey:slot-id=9a;foo=bar"), args{"foo"}, "bar"}, - {"ok in query", mustParse("yubikey:slot-id=9a?foo=bar"), args{"foo"}, "bar"}, - {"fail missing", mustParse("yubikey:slot-id=9a"), args{"foo"}, ""}, - {"fail missing query", mustParse("yubikey:slot-id=9a?bar=zar"), args{"foo"}, ""}, + {"ok", mustParse(t, "yubikey:slot-id=9a"), args{"slot-id"}, "9a"}, + {"ok first", mustParse(t, "yubikey:slot-id=9a;slot-id=9b"), args{"slot-id"}, "9a"}, + {"ok multiple", mustParse(t, "yubikey:slot-id=9a;foo=bar"), args{"foo"}, "bar"}, + {"ok in query", mustParse(t, "yubikey:slot-id=9a?foo=bar"), args{"foo"}, "bar"}, + {"fail missing", mustParse(t, "yubikey:slot-id=9a"), args{"foo"}, ""}, + {"fail missing query", mustParse(t, "yubikey:slot-id=9a?bar=zar"), args{"foo"}, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -223,13 +218,6 @@ func TestURI_Get(t *testing.T) { } func TestURI_GetBool(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } type args struct { key string } @@ -239,13 +227,13 @@ func TestURI_GetBool(t *testing.T) { args args want bool }{ - {"true", mustParse("azurekms:name=foo;vault=bar;hsm=true"), args{"hsm"}, true}, - {"TRUE", mustParse("azurekms:name=foo;vault=bar;hsm=TRUE"), args{"hsm"}, true}, - {"tRUe query", mustParse("azurekms:name=foo;vault=bar?hsm=tRUe"), args{"hsm"}, true}, - {"false", mustParse("azurekms:name=foo;vault=bar;hsm=false"), args{"hsm"}, false}, - {"false query", mustParse("azurekms:name=foo;vault=bar?hsm=false"), args{"hsm"}, false}, - {"empty", mustParse("azurekms:name=foo;vault=bar;hsm=?bar=true"), args{"hsm"}, false}, - {"missing", mustParse("azurekms:name=foo;vault=bar"), args{"hsm"}, false}, + {"true", mustParse(t, "azurekms:name=foo;vault=bar;hsm=true"), args{"hsm"}, true}, + {"TRUE", mustParse(t, "azurekms:name=foo;vault=bar;hsm=TRUE"), args{"hsm"}, true}, + {"tRUe query", mustParse(t, "azurekms:name=foo;vault=bar?hsm=tRUe"), args{"hsm"}, true}, + {"false", mustParse(t, "azurekms:name=foo;vault=bar;hsm=false"), args{"hsm"}, false}, + {"false query", mustParse(t, "azurekms:name=foo;vault=bar?hsm=false"), args{"hsm"}, false}, + {"empty", mustParse(t, "azurekms:name=foo;vault=bar;hsm=?bar=true"), args{"hsm"}, false}, + {"missing", mustParse(t, "azurekms:name=foo;vault=bar"), args{"hsm"}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -257,13 +245,6 @@ func TestURI_GetBool(t *testing.T) { } func TestURI_GetEncoded(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } type args struct { key string } @@ -273,15 +254,15 @@ func TestURI_GetEncoded(t *testing.T) { args args want []byte }{ - {"ok", mustParse("yubikey:slot-id=9a"), args{"slot-id"}, []byte{0x9a}}, - {"ok prefix", mustParse("yubikey:slot-id=0x9a"), args{"slot-id"}, []byte{0x9a}}, - {"ok first", mustParse("yubikey:slot-id=9a9b;slot-id=9b"), args{"slot-id"}, []byte{0x9a, 0x9b}}, - {"ok percent", mustParse("yubikey:slot-id=9a;foo=%9a%9b%9c"), args{"foo"}, []byte{0x9a, 0x9b, 0x9c}}, - {"ok in query", mustParse("yubikey:slot-id=9a?foo=9a"), args{"foo"}, []byte{0x9a}}, - {"ok in query percent", mustParse("yubikey:slot-id=9a?foo=%9a"), args{"foo"}, []byte{0x9a}}, - {"ok missing", mustParse("yubikey:slot-id=9a"), args{"foo"}, nil}, - {"ok missing query", mustParse("yubikey:slot-id=9a?bar=zar"), args{"foo"}, nil}, - {"ok no hex", mustParse("yubikey:slot-id=09a?bar=zar"), args{"slot-id"}, []byte{'0', '9', 'a'}}, + {"ok", mustParse(t, "yubikey:slot-id=9a"), args{"slot-id"}, []byte{0x9a}}, + {"ok prefix", mustParse(t, "yubikey:slot-id=0x9a"), args{"slot-id"}, []byte{0x9a}}, + {"ok first", mustParse(t, "yubikey:slot-id=9a9b;slot-id=9b"), args{"slot-id"}, []byte{0x9a, 0x9b}}, + {"ok percent", mustParse(t, "yubikey:slot-id=9a;foo=%9a%9b%9c"), args{"foo"}, []byte{0x9a, 0x9b, 0x9c}}, + {"ok in query", mustParse(t, "yubikey:slot-id=9a?foo=9a"), args{"foo"}, []byte{0x9a}}, + {"ok in query percent", mustParse(t, "yubikey:slot-id=9a?foo=%9a"), args{"foo"}, []byte{0x9a}}, + {"ok missing", mustParse(t, "yubikey:slot-id=9a"), args{"foo"}, nil}, + {"ok missing query", mustParse(t, "yubikey:slot-id=9a?bar=zar"), args{"foo"}, nil}, + {"ok no hex", mustParse(t, "yubikey:slot-id=09a?bar=zar"), args{"slot-id"}, []byte{'0', '9', 'a'}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -294,22 +275,15 @@ func TestURI_GetEncoded(t *testing.T) { } func TestURI_Pin(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } tests := []struct { name string uri *URI want string }{ - {"from value", mustParse("pkcs11:id=%72%73?pin-value=0123456789"), "0123456789"}, - {"from source", mustParse("pkcs11:id=%72%73?pin-source=testdata/pin.txt"), "trim-this-pin"}, - {"from missing", mustParse("pkcs11:id=%72%73"), ""}, - {"from source missing", mustParse("pkcs11:id=%72%73?pin-source=testdata/foo.txt"), ""}, + {"from value", mustParse(t, "pkcs11:id=%72%73?pin-value=0123456789"), "0123456789"}, + {"from source", mustParse(t, "pkcs11:id=%72%73?pin-source=testdata/pin.txt"), "trim-this-pin"}, + {"from missing", mustParse(t, "pkcs11:id=%72%73"), ""}, + {"from source missing", mustParse(t, "pkcs11:id=%72%73?pin-source=testdata/foo.txt"), ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -321,13 +295,6 @@ func TestURI_Pin(t *testing.T) { } func TestURI_String(t *testing.T) { - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } tests := []struct { name string uri *URI @@ -336,7 +303,7 @@ func TestURI_String(t *testing.T) { {"ok new", New("yubikey", url.Values{"slot-id": []string{"9a"}, "foo": []string{"bar"}}), "yubikey:foo=bar;slot-id=9a"}, {"ok newOpaque", NewOpaque("cloudkms", "projects/p/locations/l/keyRings/k/cryptoKeys/c/cryptoKeyVersions/1"), "cloudkms:projects/p/locations/l/keyRings/k/cryptoKeys/c/cryptoKeyVersions/1"}, {"ok newFile", NewFile("/path/to/file.key"), "file:///path/to/file.key"}, - {"ok parse", mustParse("yubikey:slot-id=9a;foo=bar?bar=zar"), "yubikey:foo=bar;slot-id=9a?bar=zar"}, + {"ok parse", mustParse(t, "yubikey:slot-id=9a;foo=bar?bar=zar"), "yubikey:foo=bar;slot-id=9a?bar=zar"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -349,13 +316,6 @@ func TestURI_String(t *testing.T) { func TestURI_GetInt(t *testing.T) { seventy := int64(70) - mustParse := func(s string) *URI { - u, err := Parse(s) - if err != nil { - t.Fatal(err) - } - return u - } type args struct { key string } @@ -365,9 +325,9 @@ func TestURI_GetInt(t *testing.T) { args args want *int64 }{ - {"ok", mustParse("tpmkms:renewal-percentage=70"), args{"renewal-percentage"}, &seventy}, - {"ok empty", mustParse("tpmkms:empty"), args{"renewal-percentage"}, nil}, - {"ok non-integer", mustParse("tpmkms:renewal-percentage=not-an-integer"), args{"renewal-percentage"}, nil}, + {"ok", mustParse(t, "tpmkms:renewal-percentage=70"), args{"renewal-percentage"}, &seventy}, + {"ok empty", mustParse(t, "tpmkms:empty"), args{"renewal-percentage"}, nil}, + {"ok non-integer", mustParse(t, "tpmkms:renewal-percentage=not-an-integer"), args{"renewal-percentage"}, nil}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -382,12 +342,6 @@ func TestURI_GetInt(t *testing.T) { } func TestURI_GetHexEncoded(t *testing.T) { - mustParse := func(t *testing.T, s string) *URI { - t.Helper() - u, err := Parse(s) - require.NoError(t, err) - return u - } type args struct { key string } @@ -418,3 +372,47 @@ func TestURI_GetHexEncoded(t *testing.T) { }) } } + +func TestURI_Read(t *testing.T) { + // Read does not trim the contents of the file + expected := []byte("trim-this-pin \n") + + path := filepath.Join(t.TempDir(), "management.key") + require.NoError(t, os.WriteFile(path, expected, 0600)) + pinURI := &url.URL{ + Scheme: "file", + Path: path, + } + pathURI := &URI{ + URL: &url.URL{Scheme: "yubikey"}, + Values: url.Values{ + "management-key-source": []string{pinURI.String()}, + }, + } + + type args struct { + key string + } + tests := []struct { + name string + uri *URI + args args + want []byte + assertion assert.ErrorAssertionFunc + }{ + {"from attribute", mustParse(t, "yubikey:management-key-source=testdata/pin.txt"), args{"management-key-source"}, expected, assert.NoError}, + {"from query attribute", mustParse(t, "yubikey:?management-key-source=testdata/pin.txt"), args{"management-key-source"}, expected, assert.NoError}, + {"from uri path", pathURI, args{"management-key-source"}, expected, assert.NoError}, + {"from uri opaque", mustParse(t, "yubikey:management-key-source=file:testdata/pin.txt"), args{"management-key-source"}, expected, assert.NoError}, + {"from empty attribute", mustParse(t, "yubikey:management-source-key="), args{"management-key-source"}, nil, assert.NoError}, + {"from missing attribute", mustParse(t, "yubikey:slot-id=82"), args{"management-key-source"}, nil, assert.NoError}, + {"from missing file", mustParse(t, "yubikey:management-key-source=testdata/foo.txt"), args{"management-key-source"}, nil, assert.Error}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.uri.Read(tt.args.key) + tt.assertion(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index 6b05fb15..a6805f12 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -4,6 +4,7 @@ package yubikey import ( + "bytes" "context" "crypto" "crypto/x509" @@ -15,6 +16,7 @@ import ( "strconv" "strings" "sync" + "unicode" "github.com/go-piv/piv-go/v2/piv" "github.com/pkg/errors" @@ -85,6 +87,7 @@ func openCard(card string) (pivKey, error) { // support multiple cards at the same time. // // yubikey:management-key=001122334455667788990011223344556677889900112233?pin-value=123456 +// yubikey:management-key-source=/var/run/management.key?pin-source=/var/run/yubikey.pin // yubikey:serial=112233?pin-source=/var/run/yubikey.pin // // You can also define a slot id, this will be ignored in this method but can be @@ -92,7 +95,7 @@ func openCard(card string) (pivKey, error) { // // yubikey:slot-id=9a?pin-value=123456 // -// If the pin or the management-key are not provided, we will use the default +// If the pin or the management key are not provided, we will use the default // ones. func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { pin := "123456" @@ -109,6 +112,14 @@ func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { } if v := u.Get("management-key"); v != "" { opts.ManagementKey = v + } else if u.Has("management-key-source") { + b, err := u.Read("management-key-source") + if err != nil { + return nil, err + } + if b = bytes.TrimFunc(b, unicode.IsSpace); len(b) > 0 { + opts.ManagementKey = string(b) + } } if v := u.Get("serial"); v != "" { serial = v @@ -119,7 +130,7 @@ func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { if opts.ManagementKey != "" { b, err := hex.DecodeString(opts.ManagementKey) if err != nil { - return nil, errors.Wrap(err, "error decoding managementKey") + return nil, errors.Wrap(err, "error decoding management key") } if len(b) != 24 { return nil, errors.New("invalid managementKey: length is not 24 bytes") diff --git a/kms/yubikey/yubikey_test.go b/kms/yubikey/yubikey_test.go index d5bef176..19b31082 100644 --- a/kms/yubikey/yubikey_test.go +++ b/kms/yubikey/yubikey_test.go @@ -16,8 +16,11 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "encoding/hex" "errors" "fmt" + "os" + "path/filepath" "reflect" "sync" "testing" @@ -28,6 +31,7 @@ import ( "go.step.sm/crypto/kms/apiv1" "go.step.sm/crypto/minica" + "go.step.sm/crypto/randutil" ) type stubPivKey struct { @@ -298,6 +302,11 @@ func TestNew(t *testing.T) { pivCards = pCards }) + managementKey, err := randutil.Salt(24) + require.NoError(t, err) + managementKeyFile := filepath.Join(t.TempDir(), "management.key") + require.NoError(t, os.WriteFile(managementKeyFile, []byte(hex.EncodeToString(managementKey)), 0600)) + yk := newStubPivKey(t, ECDSA) okPivCards := func() ([]string, error) { @@ -359,6 +368,13 @@ func TestNew(t *testing.T) { pivCards = okPivCards pivOpen = okPivOpen }, &YubiKey{yk: yk, pin: "123456", card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: piv.DefaultManagementKey}, false}, + {"ok with management-key-source", args{ctx, apiv1.Options{ + URI: fmt.Sprintf("yubikey:management-key-source=%s?pin-value=123456", managementKeyFile), + }}, func() { + pivMap = sync.Map{} + pivCards = okPivCards + pivOpen = okPivOpen + }, &YubiKey{yk: yk, pin: "123456", card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: managementKey}, false}, {"ok with Pin", args{ctx, apiv1.Options{Pin: "222222"}}, func() { pivMap = sync.Map{} pivCards = okPivCards @@ -384,6 +400,11 @@ func TestNew(t *testing.T) { pivCards = okPivCards pivOpen = okPivOpen }, nil, true}, + {"fail management key source", args{ctx, apiv1.Options{URI: "yubikey:management-key-source=missing.txt"}}, func() { + pivMap = sync.Map{} + pivCards = okPivCards + pivOpen = okPivOpen + }, nil, true}, {"fail pivCards", args{ctx, apiv1.Options{}}, func() { pivMap = sync.Map{} pivCards = failPivCards From 7cf227d1b6c0351733cbc780794eeb7954eb3889 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 14 Jan 2025 14:27:50 -0800 Subject: [PATCH 2/2] Update variable names in test Co-authored-by: Herman Slatman --- kms/uri/uri_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kms/uri/uri_test.go b/kms/uri/uri_test.go index 5efa0b91..b23ae8d2 100644 --- a/kms/uri/uri_test.go +++ b/kms/uri/uri_test.go @@ -379,14 +379,14 @@ func TestURI_Read(t *testing.T) { path := filepath.Join(t.TempDir(), "management.key") require.NoError(t, os.WriteFile(path, expected, 0600)) - pinURI := &url.URL{ + managementKeyURI := &url.URL{ Scheme: "file", Path: path, } pathURI := &URI{ URL: &url.URL{Scheme: "yubikey"}, Values: url.Values{ - "management-key-source": []string{pinURI.String()}, + "management-key-source": []string{managementKeyURI.String()}, }, }