From 2723a8fc440babd599b18dcbb870d6e3716c5da3 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Jun 2023 11:32:54 -0700 Subject: [PATCH 1/8] Add automatic support for p11-kit This commit adds support to automatically use the p11-kit-proxy module on the PKCS#11 KMS if no other module has been specified. Fixes #259 --- kms/pkcs11/pkcs11.go | 41 ++++++++++++++++++++++++++++++++++++++- kms/pkcs11/pkcs11_test.go | 18 ++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 46d0f962..0e9284a5 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -12,11 +12,17 @@ import ( "encoding/hex" "fmt" "math/big" + "os" + "os/exec" + "runtime" "strconv" + "strings" "sync" + "time" "github.com/ThalesIgnite/crypto11" "github.com/pkg/errors" + "go.step.sm/crypto/kms/apiv1" "go.step.sm/crypto/kms/uri" ) @@ -60,7 +66,6 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { } config.Pin = u.Pin() - config.Path = u.Get("module-path") config.TokenLabel = u.Get("token") config.TokenSerial = u.Get("serial") if v := u.Get("slot-id"); v != "" { @@ -70,6 +75,10 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { } config.SlotNumber = &n } + // Get module or default to use p11-kit-proxy.so + if config.Path = u.Get("module-path"); config.Path == "" { + config.Path = findP11KitProxy(ctx) + } } if config.Pin == "" && opts.Pin != "" { config.Pin = opts.Pin @@ -402,4 +411,34 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { return cert, nil } +// findP11KitProxy uses pkg-config to locate p11-kit-proxy.so +func findP11KitProxy(ctx context.Context) string { + var out strings.Builder + + // It should be more than enough even in constraint VMs + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, "pkg-config", "--variable=proxy_module", "p11-kit-1") + cmd.Stdout = &out + if err := cmd.Run(); err != nil { + return "" + } + + path := strings.TrimSpace(out.String()) + if _, err := os.Stat(path); err != nil { + if runtime.GOOS != "darwin" { + return "" + } + + // pkg-config might return an .so file instead of a .dylib on macOs. + path = strings.Replace(path, ".so", ".dylib", 1) + if _, err := os.Stat(path); err != nil { + return "" + } + } + + return path +} + var _ apiv1.CertificateManager = (*PKCS11)(nil) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index e6fcae9f..580806a2 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -41,6 +41,18 @@ func TestNew(t *testing.T) { return k.p11, nil } + var ( + wantMissingModule *PKCS11 + wantErrMissingModule = true + ) + if findP11KitProxy(context.Background()) != "" { + wantMissingModule = k + wantErrMissingModule = false + } + + canceledContext, cancel := context.WithCancel(context.Background()) + cancel() + type args struct { ctx context.Context opts apiv1.Options @@ -68,10 +80,14 @@ func TestNew(t *testing.T) { URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", Pin: "passowrd", }}, k, false}, - {"fail missing module", args{context.Background(), apiv1.Options{ + {"perhaps with missing module", args{context.Background(), apiv1.Options{ Type: "pkcs11", URI: "pkcs11:token=pkcs11-test", Pin: "passowrd", + }}, wantMissingModule, wantErrMissingModule}, + {"fail findP11KitProxy", args{canceledContext, apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:token=pkcs11-test?pin-value=password", }}, nil, true}, {"fail missing pin", args{context.Background(), apiv1.Options{ Type: "pkcs11", From 237a37090fbaa52476451deadbe657168ed680c0 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Jun 2023 14:42:09 -0700 Subject: [PATCH 2/8] Mock findP11KitProxy on New test --- kms/pkcs11/pkcs11.go | 2 +- kms/pkcs11/pkcs11_test.go | 59 +++++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 0e9284a5..991c2650 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -412,7 +412,7 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { } // findP11KitProxy uses pkg-config to locate p11-kit-proxy.so -func findP11KitProxy(ctx context.Context) string { +var findP11KitProxy = func(ctx context.Context) string { var out strings.Builder // It should be more than enough even in constraint VMs diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 580806a2..fb3102d5 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -25,15 +25,18 @@ import ( ) func TestNew(t *testing.T) { - tmp := p11Configure + tmp0 := p11Configure + tmp1 := findP11KitProxy t.Cleanup(func() { - p11Configure = tmp + p11Configure = tmp0 + findP11KitProxy = tmp1 }) k := mustPKCS11(t) t.Cleanup(func() { k.Close() }) + p11Configure = func(config *crypto11.Config) (P11, error) { if strings.Contains(config.Path, "fail") { return nil, errors.New("an error") @@ -41,13 +44,16 @@ func TestNew(t *testing.T) { return k.p11, nil } - var ( - wantMissingModule *PKCS11 - wantErrMissingModule = true - ) - if findP11KitProxy(context.Background()) != "" { - wantMissingModule = k - wantErrMissingModule = false + findP11KitProxy = func(ctx context.Context) string { + select { + case <-ctx.Done(): + return "" + default: + if fail, _ := ctx.Value("fail").(bool); fail { + return "" + } + return "/usr/local/lib/p11-kit-proxy.so" + } } canceledContext, cancel := context.WithCancel(context.Background()) @@ -80,11 +86,16 @@ func TestNew(t *testing.T) { URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", Pin: "passowrd", }}, k, false}, - {"perhaps with missing module", args{context.Background(), apiv1.Options{ + {"ok with missing module", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:token=pkcs11-test", + Pin: "passowrd", + }}, k, false}, + {"fail with missing module", args{context.WithValue(context.Background(), "fail", true), apiv1.Options{ Type: "pkcs11", URI: "pkcs11:token=pkcs11-test", Pin: "passowrd", - }}, wantMissingModule, wantErrMissingModule}, + }}, nil, true}, {"fail findP11KitProxy", args{canceledContext, apiv1.Options{ Type: "pkcs11", URI: "pkcs11:token=pkcs11-test?pin-value=password", @@ -850,3 +861,29 @@ func TestPKCS11_Close(t *testing.T) { }) } } + +func Test_findP11KitProxy(t *testing.T) { + expected := findP11KitProxy(context.Background()) + + canceledContext, cancel := context.WithCancel(context.Background()) + cancel() + + type args struct { + ctx context.Context + } + tests := []struct { + name string + args args + want string + }{ + {"expected", args{context.Background()}, expected}, + {"fail", args{canceledContext}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := findP11KitProxy(tt.args.ctx); got != tt.want { + t.Errorf("findP11KitProxy() = %v, want %v", got, tt.want) + } + }) + } +} From 1e80f6602b1879a5e1c8ee417369583a7c1ed444 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Jun 2023 15:31:53 -0700 Subject: [PATCH 3/8] Rely on dlopen to locate p11-kit-proxy.so --- kms/pkcs11/pkcs11.go | 49 ++++++++++-------------------------- kms/pkcs11/pkcs11_test.go | 53 ++------------------------------------- 2 files changed, 15 insertions(+), 87 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 991c2650..311ebb01 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -12,13 +12,9 @@ import ( "encoding/hex" "fmt" "math/big" - "os" - "os/exec" "runtime" "strconv" - "strings" "sync" - "time" "github.com/ThalesIgnite/crypto11" "github.com/pkg/errors" @@ -75,9 +71,13 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { } config.SlotNumber = &n } - // Get module or default to use p11-kit-proxy.so + // Get module or default to use p11-kit-proxy.so. + // + // pkcs11.New(module string) will use dlopen that will look for the + // given library in the appropriate paths, so there's no need to provide + // the full path. if config.Path = u.Get("module-path"); config.Path == "" { - config.Path = findP11KitProxy(ctx) + config.Path = defaultModule } } if config.Pin == "" && opts.Pin != "" { @@ -109,7 +109,14 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { }, nil } +// defaultModule defines the defaultModule used, in this case is the +// p11-kit-proxy provided by p11-kit. +var defaultModule = "p11-kit-proxy.so" + func init() { + if runtime.GOOS == "darwin" { + defaultModule = "p11-kit-proxy.dylib" + } apiv1.Register(apiv1.PKCS11, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { return New(ctx, opts) }) @@ -411,34 +418,4 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { return cert, nil } -// findP11KitProxy uses pkg-config to locate p11-kit-proxy.so -var findP11KitProxy = func(ctx context.Context) string { - var out strings.Builder - - // It should be more than enough even in constraint VMs - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, "pkg-config", "--variable=proxy_module", "p11-kit-1") - cmd.Stdout = &out - if err := cmd.Run(); err != nil { - return "" - } - - path := strings.TrimSpace(out.String()) - if _, err := os.Stat(path); err != nil { - if runtime.GOOS != "darwin" { - return "" - } - - // pkg-config might return an .so file instead of a .dylib on macOs. - path = strings.Replace(path, ".so", ".dylib", 1) - if _, err := os.Stat(path); err != nil { - return "" - } - } - - return path -} - var _ apiv1.CertificateManager = (*PKCS11)(nil) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index fb3102d5..d6eccf26 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -26,10 +26,8 @@ import ( func TestNew(t *testing.T) { tmp0 := p11Configure - tmp1 := findP11KitProxy t.Cleanup(func() { p11Configure = tmp0 - findP11KitProxy = tmp1 }) k := mustPKCS11(t) @@ -44,21 +42,6 @@ func TestNew(t *testing.T) { return k.p11, nil } - findP11KitProxy = func(ctx context.Context) string { - select { - case <-ctx.Done(): - return "" - default: - if fail, _ := ctx.Value("fail").(bool); fail { - return "" - } - return "/usr/local/lib/p11-kit-proxy.so" - } - } - - canceledContext, cancel := context.WithCancel(context.Background()) - cancel() - type args struct { ctx context.Context opts apiv1.Options @@ -91,15 +74,9 @@ func TestNew(t *testing.T) { URI: "pkcs11:token=pkcs11-test", Pin: "passowrd", }}, k, false}, - {"fail with missing module", args{context.WithValue(context.Background(), "fail", true), apiv1.Options{ + {"fail missing module", args{context.Background(), apiv1.Options{ Type: "pkcs11", - URI: "pkcs11:token=pkcs11-test", - Pin: "passowrd", - }}, nil, true}, - {"fail findP11KitProxy", args{canceledContext, apiv1.Options{ - Type: "pkcs11", - URI: "pkcs11:token=pkcs11-test?pin-value=password", - }}, nil, true}, + }}, k, false}, {"fail missing pin", args{context.Background(), apiv1.Options{ Type: "pkcs11", URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", @@ -861,29 +838,3 @@ func TestPKCS11_Close(t *testing.T) { }) } } - -func Test_findP11KitProxy(t *testing.T) { - expected := findP11KitProxy(context.Background()) - - canceledContext, cancel := context.WithCancel(context.Background()) - cancel() - - type args struct { - ctx context.Context - } - tests := []struct { - name string - args args - want string - }{ - {"expected", args{context.Background()}, expected}, - {"fail", args{canceledContext}, ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := findP11KitProxy(tt.args.ctx); got != tt.want { - t.Errorf("findP11KitProxy() = %v, want %v", got, tt.want) - } - }) - } -} From bc3ba2996b543dca94fd4d754af3be9e347f4646 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 8 Jun 2023 15:44:51 -0700 Subject: [PATCH 4/8] Fix missing-module test --- kms/pkcs11/pkcs11_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index d6eccf26..07ea0939 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -76,7 +76,7 @@ func TestNew(t *testing.T) { }}, k, false}, {"fail missing module", args{context.Background(), apiv1.Options{ Type: "pkcs11", - }}, k, false}, + }}, nil, true}, {"fail missing pin", args{context.Background(), apiv1.Options{ Type: "pkcs11", URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", From a23378d7b9e7bbb11de90ffd9a37fd68540a0ce7 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 9 Jun 2023 11:08:28 -0700 Subject: [PATCH 5/8] Add p11-kit-proxy.dll for windows and validate uri --- kms/pkcs11/pkcs11.go | 53 +++++++++++++++++++++------------------ kms/pkcs11/pkcs11_test.go | 2 +- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 311ebb01..4991c773 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -54,39 +54,41 @@ type PKCS11 struct { // New returns a new PKCS11 KMS. func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { + if opts.URI == "" { + return nil, errors.New("kms uri is required") + } + var config crypto11.Config - if opts.URI != "" { - u, err := uri.ParseWithScheme(Scheme, opts.URI) + u, err := uri.ParseWithScheme(Scheme, opts.URI) + if err != nil { + return nil, err + } + + config.TokenLabel = u.Get("token") + config.TokenSerial = u.Get("serial") + if v := u.Get("slot-id"); v != "" { + n, err := strconv.Atoi(v) if err != nil { - return nil, err + return nil, errors.Wrap(err, "kms uri 'slot-id' is not valid") } + config.SlotNumber = &n + } - config.Pin = u.Pin() - config.TokenLabel = u.Get("token") - config.TokenSerial = u.Get("serial") - if v := u.Get("slot-id"); v != "" { - n, err := strconv.Atoi(v) - if err != nil { - return nil, errors.Wrap(err, "kms uri 'slot-id' is not valid") - } - config.SlotNumber = &n - } - // Get module or default to use p11-kit-proxy.so. - // - // pkcs11.New(module string) will use dlopen that will look for the - // given library in the appropriate paths, so there's no need to provide - // the full path. - if config.Path = u.Get("module-path"); config.Path == "" { - config.Path = defaultModule - } + // Get module or default to use p11-kit-proxy.so. + // + // pkcs11.New(module string) will use dlopen that will look for the + // given library in the appropriate paths, so there's no need to provide + // the full path. + if config.Path = u.Get("module-path"); config.Path == "" { + config.Path = defaultModule } + + config.Pin = u.Pin() if config.Pin == "" && opts.Pin != "" { config.Pin = opts.Pin } switch { - case config.Path == "": - return nil, errors.New("kms uri 'module-path' are required") case config.TokenLabel == "" && config.TokenSerial == "" && config.SlotNumber == nil: return nil, errors.New("kms uri 'token', 'serial' or 'slot-id' are required") case config.Pin == "": @@ -114,8 +116,11 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { var defaultModule = "p11-kit-proxy.so" func init() { - if runtime.GOOS == "darwin" { + switch runtime.GOOS { + case "darwin": defaultModule = "p11-kit-proxy.dylib" + case "windows": + defaultModule = "p11-kit-proxy.dll" } apiv1.Register(apiv1.PKCS11, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { return New(ctx, opts) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 07ea0939..8c50f69e 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -74,7 +74,7 @@ func TestNew(t *testing.T) { URI: "pkcs11:token=pkcs11-test", Pin: "passowrd", }}, k, false}, - {"fail missing module", args{context.Background(), apiv1.Options{ + {"fail missing uri", args{context.Background(), apiv1.Options{ Type: "pkcs11", }}, nil, true}, {"fail missing pin", args{context.Background(), apiv1.Options{ From f840799417103cd6ccbbe300d17307e486939538 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 9 Jun 2023 15:17:11 -0700 Subject: [PATCH 6/8] Add some docs on the pkcs11 kms --- kms/pkcs11/pkcs11.go | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 4991c773..b01215d7 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -52,7 +52,30 @@ type PKCS11 struct { closed sync.Once } -// New returns a new PKCS11 KMS. +// New returns a new PKCS#11 KMS. To initialize it, you need to provide a URI +// with the following format: +// +// - pkcs11:token=smallstep?pin-value=password +// - pkcs11:serial=1a2b3c4d5e6f?pin-source=/path/to/pin.txt +// - pkcs11:slot-id=5?pin-value=password +// - pkcs11:module-path=/path/to/module.so;token=smallstep?pin-value=password +// +// The scheme is "pkcs11"; "token", "serial", or "slot-id" defines the +// cryptographic device to use, "module-path" is the path of the PKCS#11 module +// to use, it will default to the proxy module of the p11-kit project if none is +// specified (p11-kit-proxy.so), "pin-value" provides the user's PIN, and +// "pin-source" defines a file that contains the PIN. +// +// A cryptographic key or object is identified by its "id" or "object" +// attributes. The "id" is the key identifier for the object, it's a hexadecimal +// string, and it will set the CKA_ID attribute of the object. The "object" is +// the name of the object, and it will set the CKA_LABEL attribute. Only one +// attribute is required to identify a key, but this package requires both to +// create a new key. The complete URI for a key looks like this: +// +// - pkcs11:token=smallstep;id=0a10;object=ec-key?pin-value=password +// - pkcs11:token=smallstep;id=0a10?pin-source=/path/to/pin.txt +// - pkcs11:token=smallstep;object=ec=key?pin-value=password func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { if opts.URI == "" { return nil, errors.New("kms uri is required") @@ -127,7 +150,7 @@ func init() { }) } -// GetPublicKey returns the public key .... +// GetPublicKey returns the public key in stored in the given object. func (k *PKCS11) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { if req.Name == "" { return nil, errors.New("getPublicKeyRequest 'name' cannot be empty") From 8d82ef2e269aeb94b715a39902c55fc37221c89d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 9 Jun 2023 16:01:22 -0700 Subject: [PATCH 7/8] Add tests and examples with id with % --- kms/pkcs11/pkcs11.go | 2 +- kms/pkcs11/pkcs11_test.go | 17 +++++++++++++---- kms/pkcs11/setup_test.go | 5 +++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index b01215d7..5c3e096d 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -74,7 +74,7 @@ type PKCS11 struct { // create a new key. The complete URI for a key looks like this: // // - pkcs11:token=smallstep;id=0a10;object=ec-key?pin-value=password -// - pkcs11:token=smallstep;id=0a10?pin-source=/path/to/pin.txt +// - pkcs11:token=smallstep;id=%0a%10?pin-source=/path/to/pin.txt // - pkcs11:token=smallstep;object=ec=key?pin-value=password func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { if opts.URI == "" { diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 8c50f69e..c196f7a3 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -157,7 +157,7 @@ func TestPKCS11_GetPublicKey(t *testing.T) { Name: "pkcs11:id=7373;object=ecdsa-p256-key", }}, &ecdsa.PublicKey{}, false}, {"ECDSA by id", args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=7373", + Name: "pkcs11:id=%73%73", }}, &ecdsa.PublicKey{}, false}, {"ECDSA by label", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:object=ecdsa-p256-key", @@ -223,6 +223,15 @@ func TestPKCS11_CreateKey(t *testing.T) { SigningKey: testObject, }, }, false}, + {"default with percent URI", args{&apiv1.CreateKeyRequest{ + Name: testObjectPercent, + }}, &apiv1.CreateKeyResponse{ + Name: testObjectPercent, + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: testObjectPercent, + }, + }, false}, {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSA, @@ -428,7 +437,7 @@ func TestPKCS11_CreateSigner(t *testing.T) { SigningKey: "pkcs11:id=7371;object=rsa-key", }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, {"RSA PSS", args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7372;object=rsa-pss-key", + SigningKey: "pkcs11:id=%73%72;object=rsa-pss-key", }}, apiv1.SHA256WithRSAPSS, &rsa.PSSOptions{ SaltLength: rsa.PSSSaltLengthEqualsHash, Hash: crypto.SHA256, @@ -437,7 +446,7 @@ func TestPKCS11_CreateSigner(t *testing.T) { SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, {"ECDSA P384", args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", + SigningKey: "pkcs11:id=%73%74;object=ecdsa-p384-key", }}, apiv1.ECDSAWithSHA384, crypto.SHA384, false}, {"ECDSA P521", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", @@ -512,7 +521,7 @@ func TestPKCS11_CreateDecrypter(t *testing.T) { DecryptionKey: "pkcs11:id=7371;object=rsa-key", }}, false}, {"RSA PSS", args{&apiv1.CreateDecrypterRequest{ - DecryptionKey: "pkcs11:id=7372;object=rsa-pss-key", + DecryptionKey: "pkcs11:id=%73%72;object=rsa-pss-key", }}, false}, {"ECDSA P256", args{&apiv1.CreateDecrypterRequest{ DecryptionKey: "pkcs11:id=7373;object=ecdsa-p256-key", diff --git a/kms/pkcs11/setup_test.go b/kms/pkcs11/setup_test.go index 9114d196..3d83c441 100644 --- a/kms/pkcs11/setup_test.go +++ b/kms/pkcs11/setup_test.go @@ -18,6 +18,7 @@ import ( var ( testModule = "" testObject = "pkcs11:id=7370;object=test-name" + testObjectPercent = "pkcs11:id=%73%70;object=test-name" testObjectAlt = "pkcs11:id=7377;object=alt-test-name" testObjectByID = "pkcs11:id=7370" testObjectByLabel = "pkcs11:object=test-name" @@ -27,9 +28,9 @@ var ( Bits int }{ {"pkcs11:id=7371;object=rsa-key", apiv1.SHA256WithRSA, 2048}, - {"pkcs11:id=7372;object=rsa-pss-key", apiv1.SHA256WithRSAPSS, DefaultRSASize}, + {"pkcs11:id=%73%72;object=rsa-pss-key", apiv1.SHA256WithRSAPSS, DefaultRSASize}, {"pkcs11:id=7373;object=ecdsa-p256-key", apiv1.ECDSAWithSHA256, 0}, - {"pkcs11:id=7374;object=ecdsa-p384-key", apiv1.ECDSAWithSHA384, 0}, + {"pkcs11:id=%73%74;object=ecdsa-p384-key", apiv1.ECDSAWithSHA384, 0}, {"pkcs11:id=7375;object=ecdsa-p521-key", apiv1.ECDSAWithSHA512, 0}, } From 572bfb72103a89e3ac71e1ff679bf0149632e168 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 12 Jun 2023 11:20:21 -0700 Subject: [PATCH 8/8] Apply suggestions from code review Co-authored-by: Herman Slatman --- kms/pkcs11/pkcs11.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 5c3e096d..838334de 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -61,9 +61,9 @@ type PKCS11 struct { // - pkcs11:module-path=/path/to/module.so;token=smallstep?pin-value=password // // The scheme is "pkcs11"; "token", "serial", or "slot-id" defines the -// cryptographic device to use, "module-path" is the path of the PKCS#11 module -// to use, it will default to the proxy module of the p11-kit project if none is -// specified (p11-kit-proxy.so), "pin-value" provides the user's PIN, and +// cryptographic device to use. "module-path" is the path of the PKCS#11 module +// to use. It will default to the proxy module of the p11-kit project if none is +// specified (p11-kit-proxy.so). "pin-value" provides the user's PIN, and // "pin-source" defines a file that contains the PIN. // // A cryptographic key or object is identified by its "id" or "object" @@ -75,7 +75,7 @@ type PKCS11 struct { // // - pkcs11:token=smallstep;id=0a10;object=ec-key?pin-value=password // - pkcs11:token=smallstep;id=%0a%10?pin-source=/path/to/pin.txt -// - pkcs11:token=smallstep;object=ec=key?pin-value=password +// - pkcs11:token=smallstep;object=ec-key?pin-value=password func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { if opts.URI == "" { return nil, errors.New("kms uri is required") @@ -150,7 +150,7 @@ func init() { }) } -// GetPublicKey returns the public key in stored in the given object. +// GetPublicKey returns the public key stored in the object identified by the name URI. func (k *PKCS11) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { if req.Name == "" { return nil, errors.New("getPublicKeyRequest 'name' cannot be empty")