diff --git a/config/config.go b/config/config.go index cc77230a..10317f6b 100644 --- a/config/config.go +++ b/config/config.go @@ -118,24 +118,43 @@ func (c *Config) validate() error { } c.TLSServerName = strings.TrimSpace(c.TLSServerName) // Do a basic validation on Keys and KeyUsages. + identifierMap := make(map[string]*KeyConfig, len(c.Keys)) + slotMap := make(map[uint]string, len(c.Keys)) + for idx, key := range c.Keys { + if _, exist := identifierMap[key.Identifier]; key.Identifier == "" || exist { + return fmt.Errorf("key %q: require a unique name for Identifier field", key.Identifier) + } + identifierMap[key.Identifier] = &c.Keys[idx] + + if key.UserPinPath == "" { + return fmt.Errorf("key %q: require the pin code file path for slot number #%d", key.Identifier, key.SlotNumber) + } + if cachedPinPath, exist := slotMap[key.SlotNumber]; exist && key.UserPinPath != cachedPinPath { + return fmt.Errorf("key %q: unmatched pin code path for slot number #%d", key.Identifier, key.SlotNumber) + } + slotMap[key.SlotNumber] = key.UserPinPath + + if key.CreateCACertIfNotExist && key.X509CACertLocation == "" { + return fmt.Errorf("key %q: CA cert is supposed to be created if it doesn't exist but X509CACertLocation is not specified", key.Identifier) + } + + if key.KeyType < crypki.RSA || key.KeyType > crypki.ECDSA { + return fmt.Errorf("key %q: invalid Key type specified", key.Identifier) + } + } + for _, ku := range c.KeyUsages { if _, ok := endpoints[ku.Endpoint]; !ok { return fmt.Errorf("unknown endpoint %q", ku.Endpoint) } // Check that all key identifiers are defined in Keys, // and all keys used for "/sig/x509-cert" have x509 CA cert configured. - next: for _, id := range ku.Identifiers { - for _, key := range c.Keys { - if key.KeyType < crypki.RSA || key.KeyType > crypki.ECDSA { - return fmt.Errorf("key %q: invalid KeyType specified", key.Identifier) - } - if key.Identifier == id { - if ku.Endpoint == X509CertEndpoint && key.X509CACertLocation == "" { - return fmt.Errorf("key %q is used for signing x509 certs, but X509CACertLocation is not specified", id) - } - continue next + if key, exist := identifierMap[id]; exist { + if ku.Endpoint == X509CertEndpoint && key.X509CACertLocation == "" { + return fmt.Errorf("key %q: key is used to sign x509 certs, but X509CACertLocation is not specified", id) } + continue } return fmt.Errorf("key identifier %q not found for endpoint %q", id, ku.Endpoint) } diff --git a/config/config_test.go b/config/config_test.go index 5b430d7e..8ac7f119 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -40,14 +40,38 @@ func TestParse(t *testing.T) { config: cfg, expectError: false, }, - "bad-config-unknown-endpoint": { - filePath: "testdata/testconf-bad-unknown-endpoint.json", - expectError: true, - }, "bad-config-unknown-identifier": { filePath: "testdata/testconf-bad-unknown-identifier.json", expectError: true, }, + "bad-duplicate-identifier-json": { + filePath: "testdata/testconf-bad-duplicate-identifier.json", + expectError: true, + }, + "bad-config-bad-non-specify-identifier": { + filePath: "testdata/testconf-bad-non-specify-identifier.json", + expectError: true, + }, + "bad-config-bad-non-specify-pin-path": { + filePath: "testdata/testconf-bad-non-specify-pin-path.json", + expectError: true, + }, + "bad-config-bad-non-specify-x509-ca-cert-path": { + filePath: "testdata/testconf-bad-non-specify-x509-ca-cert-path.json", + expectError: true, + }, + "bad-config-bad-non-x509-cert-for-x509-endpoint": { + filePath: "testdata/testconf-bad-non-x509-cert-for-x509-endpoint.json", + expectError: true, + }, + "bad-config-bad-same-slot-different-pin-path": { + filePath: "testdata/testconf-bad-same-slot-different-pin-path.json", + expectError: true, + }, + "bad-config-bad-unsupported-key-type": { + filePath: "testdata/testconf-bad-unsupported-key-type.json", + expectError: true, + }, "bad-config-bad-json": { filePath: "testdata/testconf-bad-json.json", expectError: true, @@ -67,6 +91,7 @@ func TestParse(t *testing.T) { if !tt.expectError { t.Errorf("unexpected err: %v", err) } + t.Log(err) return } if tt.expectError { diff --git a/config/testdata/testconf-bad-duplicate-identifier.json b/config/testdata/testconf-bad-duplicate-identifier.json new file mode 100644 index 00000000..c8ab0de2 --- /dev/null +++ b/config/testdata/testconf-bad-duplicate-identifier.json @@ -0,0 +1,16 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"Identifier": "key1", "KeyLabel": "foo", "SlotNumber": 1, "UserPinPath" : "/path/1", "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key1", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/config/testdata/testconf-bad-non-specify-identifier.json b/config/testdata/testconf-bad-non-specify-identifier.json new file mode 100644 index 00000000..b692c982 --- /dev/null +++ b/config/testdata/testconf-bad-non-specify-identifier.json @@ -0,0 +1,16 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"KeyLabel": "foo", "SlotNumber": 1, "UserPinPath" : "/path/1", "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key2", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/config/testdata/testconf-bad-non-specify-pin-path.json b/config/testdata/testconf-bad-non-specify-pin-path.json new file mode 100644 index 00000000..334f10c5 --- /dev/null +++ b/config/testdata/testconf-bad-non-specify-pin-path.json @@ -0,0 +1,16 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"Identifier": "key1", "KeyLabel": "foo", "SlotNumber": 1, "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key2", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/config/testdata/testconf-bad-unknown-endpoint.json b/config/testdata/testconf-bad-non-specify-x509-ca-cert-path.json similarity index 66% rename from config/testdata/testconf-bad-unknown-endpoint.json rename to config/testdata/testconf-bad-non-specify-x509-ca-cert-path.json index 42772884..806acd5f 100644 --- a/config/testdata/testconf-bad-unknown-endpoint.json +++ b/config/testdata/testconf-bad-non-specify-x509-ca-cert-path.json @@ -1,6 +1,6 @@ { - "TLSClientAuthMode": 4, "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, "X509CACertLocation":"testdata/cacert.pem", "Keys": [ {"Identifier": "key1", "KeyLabel": "foo", "SlotNumber": 1, "UserPinPath" : "/path/1", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, @@ -8,7 +8,9 @@ {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} ], "KeyUsages": [ - {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"]}, - {"Endpoint": "/foo/bar", "Identifiers": ["key1", "key2"]} + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} ] } diff --git a/config/testdata/testconf-bad-non-x509-cert-for-x509-endpoint.json b/config/testdata/testconf-bad-non-x509-cert-for-x509-endpoint.json new file mode 100644 index 00000000..19b2ebf8 --- /dev/null +++ b/config/testdata/testconf-bad-non-x509-cert-for-x509-endpoint.json @@ -0,0 +1,16 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"Identifier": "key1", "KeyLabel": "foo", "SlotNumber": 1, "UserPinPath" : "/path/1", "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key2", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/config/testdata/testconf-bad-same-slot-different-pin-path.json b/config/testdata/testconf-bad-same-slot-different-pin-path.json new file mode 100644 index 00000000..31a0a989 --- /dev/null +++ b/config/testdata/testconf-bad-same-slot-different-pin-path.json @@ -0,0 +1,17 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"Identifier": "key1", "KeyLabel": "foo", "SlotNumber": 1, "UserPinPath" : "/path/1", "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key2", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key2-1", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2-1"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/config/testdata/testconf-bad-unsupported-key-type.json b/config/testdata/testconf-bad-unsupported-key-type.json new file mode 100644 index 00000000..6d61a8e6 --- /dev/null +++ b/config/testdata/testconf-bad-unsupported-key-type.json @@ -0,0 +1,16 @@ +{ + "TLSServerName": "foo.example.com", + "TLSClientAuthMode": 4, + "X509CACertLocation":"testdata/cacert.pem", + "Keys": [ + {"Identifier": "key1", "KeyLabel": "foo", "KeyType":3, "SlotNumber": 1, "UserPinPath" : "/path/1", "X509CACertLocation": "/path/foo", "CreateCACertIfNotExist": true, "CommonName": "My CA"}, + {"Identifier": "key2", "KeyLabel": "bar", "SlotNumber": 2, "UserPinPath" : "/path/2"}, + {"Identifier": "key3", "KeyLabel": "baz", "SlotNumber": 3, "UserPinPath" : "/path/3", "X509CACertLocation": "/path/baz"} + ], + "KeyUsages": [ + {"Endpoint": "/sig/x509-cert", "Identifiers": ["key1", "key3"], "MaxValidity": 3600}, + {"Endpoint": "/sig/ssh-host-cert", "Identifiers": ["key1", "key2"], "MaxValidity": 36000}, + {"Endpoint": "/sig/ssh-user-cert", "Identifiers": ["key3"], "MaxValidity": 36000}, + {"Endpoint": "/sig/blob", "Identifiers": ["key1"], "MaxValidity": 36000} + ] +} diff --git a/pkcs11/login.go b/pkcs11/login.go new file mode 100644 index 00000000..d093384f --- /dev/null +++ b/pkcs11/login.go @@ -0,0 +1,105 @@ +package pkcs11 + +import ( + "bytes" + "crypto/rand" + "errors" + "fmt" + "io/ioutil" + + p11 "github.com/miekg/pkcs11" + + "github.com/yahoo/crypki/config" +) + +// secureBuffer cached an array of bytes as secret. +// It has a `clear` method to overwrite secret with random data. +type secureBuffer struct { + secret []byte +} + +func (b *secureBuffer) get() string { + return string(b.secret) +} + +func (b *secureBuffer) clear() { + _, _ = rand.Read(b.secret) +} + +func newSecureBuffer(secret []byte) *secureBuffer { + return &secureBuffer{secret: bytes.TrimSpace(secret)} +} + +type loginHelper interface { + getUserPinCode(string) (*secureBuffer, error) +} + +type defaultHelper struct{} + +func (h *defaultHelper) getUserPinCode(path string) (*secureBuffer, error) { + return getUserPinCode(path) +} + +// openLoginSession opens a PKCS11 session and tries to log in. +func openLoginSession(context PKCS11Ctx, slot uint, userPin string) (p11.SessionHandle, error) { + session, err := context.OpenSession(slot, p11.CKF_SERIAL_SESSION) + if err != nil { + return 0, errors.New("makeLoginSession: error in OpenSession: " + err.Error()) + } + + if err = context.Login(session, p11.CKU_USER, userPin); err != nil { + context.CloseSession(session) + return 0, errors.New("makeSigner: error in Login: " + err.Error()) + } + return session, nil +} + +type loginOption interface { + config(map[uint]*secureBuffer) loginHelper +} + +// getLoginSessions opens the PKCS11 login session for all keys in the configuration. +// The pin codes used to login are stored in the secure buffer and are overwritten with random data just before function +// returns. +func getLoginSessions(p11ctx PKCS11Ctx, keys []config.KeyConfig, opts ...loginOption) (map[uint]p11.SessionHandle, error) { + login := make(map[uint]p11.SessionHandle) + keyMap := make(map[uint]*secureBuffer) + defer func() { + for _, buffer := range keyMap { + buffer.clear() + } + }() + + // `helper` provides the `getUserPinCode` method to get the pin code from input path. + // This helper is also used to help unit test to check the status of the function-scoped variable. + helper := loginHelper(&defaultHelper{}) + for _, opt := range opts { + helper = opt.config(keyMap) + } + + for _, key := range keys { + // config validation makes sure the pin code paths are equal for the same slot. + if _, exist := keyMap[key.SlotNumber]; !exist { + pin, err := helper.getUserPinCode(key.UserPinPath) + if err != nil { + return nil, fmt.Errorf("unable to read user pin for key with identifier %q, pin path: %v, err: %v", key.Identifier, key.UserPinPath, err) + } + keyMap[key.SlotNumber] = pin + session, err := openLoginSession(p11ctx, key.SlotNumber, pin.get()) + if err != nil { + return nil, fmt.Errorf("failed to create a login session for key with identifier %q, pin path: %v, err: %v", key.Identifier, key.UserPinPath, err) + } + login[key.SlotNumber] = session + } + } + return login, nil +} + +// getUserPinCode reads the pin code from the path and stores the pin code into the secure buffer. +func getUserPinCode(path string) (*secureBuffer, error) { + pin, err := ioutil.ReadFile(path) + if err != nil { + return nil, errors.New("Failed to open pin file: " + err.Error()) + } + return newSecureBuffer(pin), nil +} diff --git a/pkcs11/login_test.go b/pkcs11/login_test.go new file mode 100644 index 00000000..a92effe0 --- /dev/null +++ b/pkcs11/login_test.go @@ -0,0 +1,316 @@ +package pkcs11 + +import ( + "errors" + "reflect" + "testing" + + "github.com/golang/mock/gomock" + p11 "github.com/miekg/pkcs11" + + "github.com/yahoo/crypki/config" + mock "github.com/yahoo/crypki/pkcs11/mock_pkcs11" +) + +func Test_openLoginSession(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pin string + slot uint + session p11.SessionHandle + wantErr bool + errMsg map[string]error + }{ + { + name: "good", + wantErr: false, + }, + { + name: "bad_OpenSession", + wantErr: true, + errMsg: map[string]error{ + "OpenSession": errors.New("failed to open a new session"), + }, + }, + { + name: "bad_slot_pass", + wantErr: true, + errMsg: map[string]error{ + "Login": errors.New("bad pin"), + }, + }, + { + name: "bad_user_already_login", + wantErr: true, + errMsg: map[string]error{ + "Login": errors.New("user already login"), + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockCtx := mock.NewMockPKCS11Ctx(mockCtrl) + mockCtx.EXPECT(). + OpenSession(tt.slot, gomock.Any()). + Return(tt.session, tt.errMsg["OpenSession"]). + AnyTimes() + + mockCtx.EXPECT(). + Login(tt.session, p11.CKU_USER, gomock.Any()). + Return(tt.errMsg["Login"]). + AnyTimes() + + mockCtx.EXPECT(). + CloseSession(tt.session). + Return(tt.errMsg["CloseSession"]). + AnyTimes() + + _, err := openLoginSession(mockCtx, tt.slot, tt.pin) + if tt.wantErr { + if err == nil { + t.Error("expected error, but got nil") + } + } else if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +type mockHelper struct { + keyMap map[uint]*secureBuffer + mockGetUserPinCode func(string) (*secureBuffer, error) +} + +func (h *mockHelper) getUserPinCode(path string) (*secureBuffer, error) { + return h.mockGetUserPinCode(path) +} + +func (h *mockHelper) config(keyMap map[uint]*secureBuffer) loginHelper { + h.keyMap = keyMap + return h +} + +func Test_getLoginSessions(t *testing.T) { + type args struct { + p11ctx *mock.MockPKCS11Ctx + keys []config.KeyConfig + helper *mockHelper + } + + defaultPathAsPinCode := func(path string) (*secureBuffer, error) { + return newSecureBuffer([]byte(path)), nil + } + failedGetPinCode := func(_ string) (*secureBuffer, error) { + return nil, errors.New("some error") + } + + tests := []struct { + name string + setup func(t *testing.T) (*gomock.Controller, args) + want map[uint]p11.SessionHandle + wantErr bool + errMsg map[string]error + }{ + { + name: "nil key", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: nil, + helper: &mockHelper{ + mockGetUserPinCode: defaultPathAsPinCode, + }, + } + }, + want: make(map[uint]p11.SessionHandle), + wantErr: false, + }, + { + name: "one key", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: []config.KeyConfig{ + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + }, + helper: &mockHelper{ + mockGetUserPinCode: defaultPathAsPinCode, + }, + } + }, + want: map[uint]p11.SessionHandle{ + 0: p11.SessionHandle(0), + }, + wantErr: false, + }, + { + name: "two keys using same slot and same pin code", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: []config.KeyConfig{ + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + }, + helper: &mockHelper{ + mockGetUserPinCode: defaultPathAsPinCode, + }, + } + }, + want: map[uint]p11.SessionHandle{ + 0: p11.SessionHandle(0), + }, + wantErr: false, + }, + { + name: "multiple keys", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: []config.KeyConfig{ + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + { + SlotNumber: 1, + UserPinPath: "pin for slot 1", + }, + }, + helper: &mockHelper{ + mockGetUserPinCode: defaultPathAsPinCode, + }, + } + }, + want: map[uint]p11.SessionHandle{ + 0: p11.SessionHandle(0), + 1: p11.SessionHandle(1), + }, + wantErr: false, + }, + { + name: "fail to get pin code", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: []config.KeyConfig{ + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + }, + helper: &mockHelper{ + mockGetUserPinCode: failedGetPinCode, + }, + } + }, + want: nil, + wantErr: true, + }, + { + name: "fail to open login session", + setup: func(t *testing.T) (*gomock.Controller, args) { + mockCtrl := gomock.NewController(t) + return mockCtrl, args{ + p11ctx: mock.NewMockPKCS11Ctx(mockCtrl), + keys: []config.KeyConfig{ + { + SlotNumber: 0, + UserPinPath: "pin for slot 0", + }, + }, + helper: &mockHelper{ + mockGetUserPinCode: defaultPathAsPinCode, + }, + } + }, + want: nil, + wantErr: true, + errMsg: map[string]error{ + "OpenSession": errors.New("error to open session"), + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + mockCtrl, args := tt.setup(t) + defer mockCtrl.Finish() + + args.p11ctx.EXPECT(). + OpenSession(gomock.Any(), gomock.Any()). + DoAndReturn(func(slotID uint, _ uint) (p11.SessionHandle, error) { + return p11.SessionHandle(slotID), tt.errMsg["OpenSession"] + }). + AnyTimes() + + args.p11ctx.EXPECT(). + Login(gomock.Any(), p11.CKU_USER, gomock.Any()). + Return(tt.errMsg["Login"]). + AnyTimes() + + args.p11ctx.EXPECT(). + CloseSession(gomock.Any()). + Return(tt.errMsg["CloseSession"]). + AnyTimes() + + got, err := getLoginSessions(args.p11ctx, args.keys, args.helper) + if tt.wantErr { + if err == nil { + t.Error("expected error, but got nil") + } + } else if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("login session map is missmatched, got: %#v, want: %#v", got, tt.want) + } + + // We store the first pin of the slot as real key. + // If the first pin of the slot is not real, then the function will fail anyway and the + // expected pin code in the buffer is also the first pin. + keyMap := make(map[uint]string) + for _, key := range args.keys { + if _, exist := keyMap[key.SlotNumber]; !exist { + keyMap[key.SlotNumber] = key.UserPinPath + } + } + + // Check if the pin code stores in the secure buffer is cleaned. + for slot := range args.helper.keyMap { + if buffer, exist := args.helper.keyMap[slot]; !exist { + t.Errorf("the pin buffer for slot #%d is missing", slot) + } else if buffer.get() == keyMap[slot] { + t.Errorf("the pin buffer for slot #%d is not cleaned", slot) + } else { + t.Logf("%v\n%v\n", buffer, keyMap[slot]) + } + } + }) + } +} diff --git a/pkcs11/p11signer.go b/pkcs11/p11signer.go index 44fd8f95..9a56dc67 100644 --- a/pkcs11/p11signer.go +++ b/pkcs11/p11signer.go @@ -9,6 +9,7 @@ import ( "io" p11 "github.com/miekg/pkcs11" + "github.com/yahoo/crypki" ) @@ -20,19 +21,12 @@ type p11Signer struct { keyType crypki.PublicKeyAlgorithm } -func makeSigner(context PKCS11Ctx, login bool, slot uint, tokenLabel string, userPin string, keyType crypki.PublicKeyAlgorithm) (*p11Signer, error) { +func makeSigner(context PKCS11Ctx, slot uint, tokenLabel string, keyType crypki.PublicKeyAlgorithm) (*p11Signer, error) { session, err := context.OpenSession(slot, p11.CKF_SERIAL_SESSION) if err != nil { return nil, errors.New("makeSigner: error in OpenSession: " + err.Error()) } - if login { - if err = context.Login(session, p11.CKU_USER, userPin); err != nil { - context.CloseSession(session) - return nil, errors.New("makeSigner: error in Login: " + err.Error()) - } - } - privateKey, err := getKey(context, session, tokenLabel, p11.CKO_PRIVATE_KEY) if err != nil { context.CloseSession(session) diff --git a/pkcs11/p11signer_test.go b/pkcs11/p11signer_test.go index c999d01d..5abf5d37 100644 --- a/pkcs11/p11signer_test.go +++ b/pkcs11/p11signer_test.go @@ -13,6 +13,7 @@ import ( "github.com/golang/mock/gomock" p11 "github.com/miekg/pkcs11" + "github.com/yahoo/crypki/pkcs11/mock_pkcs11" ) @@ -281,5 +282,4 @@ func TestSignECDSA(t *testing.T) { } }) } - } diff --git a/pkcs11/pkcs11.go b/pkcs11/pkcs11.go index 4f07c37b..c43fc081 100644 --- a/pkcs11/pkcs11.go +++ b/pkcs11/pkcs11.go @@ -8,6 +8,7 @@ import ( "fmt" p11 "github.com/miekg/pkcs11" + "github.com/yahoo/crypki" ) diff --git a/pkcs11/signer.go b/pkcs11/signer.go index 8507a27f..3c0f7abb 100644 --- a/pkcs11/signer.go +++ b/pkcs11/signer.go @@ -11,19 +11,23 @@ import ( "io/ioutil" "log" "net" - "strings" "time" + p11 "github.com/miekg/pkcs11" + "golang.org/x/crypto/ssh" + "github.com/yahoo/crypki" "github.com/yahoo/crypki/config" "github.com/yahoo/crypki/x509cert" - "golang.org/x/crypto/ssh" ) // signer implements crypki.CertSign interface. type signer struct { x509CACerts map[string]*x509.Certificate sPool map[string]sPool + + // login keeps all login session using the slot number as key. + login map[uint]p11.SessionHandle } // NewCertSign initializes a CertSign object that interacts with PKCS11 compliant device. @@ -32,16 +36,19 @@ func NewCertSign(pkcs11ModulePath string, keys []config.KeyConfig, requireX509CA if err != nil { return nil, fmt.Errorf("unable to initialize PKCS11 context: %v", err) } + + login, err := getLoginSessions(p11ctx, keys) + if err != nil { + return nil, fmt.Errorf("failed to create login sessions, err: %v", err) + } + s := &signer{ x509CACerts: make(map[string]*x509.Certificate), sPool: make(map[string]sPool), + login: login, } for _, key := range keys { - pin, err := getUserPin(key.UserPinPath) - if err != nil { - return nil, fmt.Errorf("unable to read user pin for key with identifier %q, pin path: %v, err: %v", key.Identifier, key.UserPinPath, err) - } - pool, err := newSignerPool(p11ctx, key.SessionPoolSize, key.SlotNumber, key.KeyLabel, pin, key.KeyType) + pool, err := newSignerPool(p11ctx, key.SessionPoolSize, key.SlotNumber, key.KeyLabel, key.KeyType) if err != nil { return nil, fmt.Errorf("unable to initialize key with identifier %q: %v", key.Identifier, err) } @@ -50,7 +57,7 @@ func NewCertSign(pkcs11ModulePath string, keys []config.KeyConfig, requireX509CA if requireX509CACert[key.Identifier] { cert, err := getX509CACert(key, pool, hostname, ips) if err != nil { - log.Fatalf("failed to get x509 CA cert for key %q: %v", key.Identifier, err) + log.Fatalf("failed to get x509 CA cert for key with identifier %q, err: %v", key.Identifier, err) } s.x509CACerts[key.Identifier] = cert log.Printf("x509 CA cert loaded for key %q", key.Identifier) @@ -245,16 +252,6 @@ func getX509CACert(key config.KeyConfig, pool sPool, hostname string, ips []net. return cert, nil } -func getUserPin(pinFilePath string) (string, error) { - userPin, err := ioutil.ReadFile(pinFilePath) - if err != nil { - return "", errors.New("Failed to open pin file: " + err.Error()) - } - userPinStr := string(userPin) - userPinStr = strings.TrimSpace(userPinStr) // for removing trailing '/n' - return userPinStr, nil -} - func getSignatureAlgorithm(pka crypki.PublicKeyAlgorithm) x509.SignatureAlgorithm { switch pka { case crypki.RSA: diff --git a/pkcs11/signer_test.go b/pkcs11/signer_test.go index d788ceef..b0afa3a6 100644 --- a/pkcs11/signer_test.go +++ b/pkcs11/signer_test.go @@ -17,8 +17,9 @@ import ( "testing" "time" - "github.com/yahoo/crypki" "golang.org/x/crypto/ssh" + + "github.com/yahoo/crypki" ) const ( diff --git a/pkcs11/signerpool.go b/pkcs11/signerpool.go index 42cb94e3..da6deb48 100644 --- a/pkcs11/signerpool.go +++ b/pkcs11/signerpool.go @@ -25,28 +25,20 @@ type signerWithSignAlgorithm interface { // each key is corresponding with a SignerPool type SignerPool struct { signers chan signerWithSignAlgorithm - // per pkcs11, by default, Login() is required only once. - // we use dummySigner to login to the token, and for absolutely nothing else. - dummySigner *p11Signer } // newSignerPool initializes a signer pool based on the configuration parameters -func newSignerPool(context PKCS11Ctx, nSigners int, slot uint, tokenLabel string, pin string, keyType crypki.PublicKeyAlgorithm) (sPool, error) { - dummySigner, err := makeSigner(context, true, slot, tokenLabel, pin, keyType) - if err != nil { - return &SignerPool{nil, nil}, fmt.Errorf("error making dummy signer: %v", err) - } +func newSignerPool(context PKCS11Ctx, nSigners int, slot uint, tokenLabel string, keyType crypki.PublicKeyAlgorithm) (sPool, error) { signers := make(chan signerWithSignAlgorithm, nSigners) for i := 0; i < nSigners; i++ { - signerInstance, err := makeSigner(context, false, slot, tokenLabel, pin, keyType) + signerInstance, err := makeSigner(context, slot, tokenLabel, keyType) if err != nil { - return &SignerPool{nil, nil}, fmt.Errorf("error making signer: %v", err) + return &SignerPool{nil}, fmt.Errorf("error making signer: %v", err) } signers <- signerInstance } return &SignerPool{ - signers: signers, - dummySigner: dummySigner, + signers: signers, }, nil } diff --git a/pkcs11/signerpool_test.go b/pkcs11/signerpool_test.go index cbbc1f02..c7d086e9 100644 --- a/pkcs11/signerpool_test.go +++ b/pkcs11/signerpool_test.go @@ -12,6 +12,7 @@ import ( "github.com/golang/mock/gomock" p11 "github.com/miekg/pkcs11" + "github.com/yahoo/crypki" "github.com/yahoo/crypki/pkcs11/mock_pkcs11" ) @@ -82,72 +83,55 @@ func TestNewSignerPool(t *testing.T) { }{ "good": { nSigners: 10, - pin: "", keyType: crypki.UnknownPublicKeyAlgorithm, // should default to RSA objects: []p11.ObjectHandle{1, 2}, expectError: false, }, "good_zero_signers": { nSigners: 0, - pin: "", keyType: crypki.RSA, objects: []p11.ObjectHandle{1, 2}, expectError: false, }, "bad_OpenSession": { nSigners: 10, - pin: "", keyType: crypki.RSA, objects: []p11.ObjectHandle{1, 2}, expectError: true, errMsg: map[string]error{ - "OpenSession": errors.New("Failed to open a new Session"), - }, - }, - "bad_slot_pass": { - nSigners: 10, - pin: "", - keyType: crypki.RSA, - objects: []p11.ObjectHandle{1, 2}, - expectError: true, - errMsg: map[string]error{ - "Login": errors.New("bad pin"), + "OpenSession": errors.New("failed to open a new Session"), }, }, "bad_FindObjectsInit": { nSigners: 10, - pin: "", keyType: crypki.RSA, objects: []p11.ObjectHandle{1, 2}, expectError: true, errMsg: map[string]error{ - "OpenSession": errors.New("Failed to FindObjectsInit"), + "FindObjectsInit": errors.New("failed to FindObjectsInit"), }, }, "bad_FindObjects": { nSigners: 10, - pin: "", objects: []p11.ObjectHandle{1, 2}, expectError: true, errMsg: map[string]error{ - "OpenSession": errors.New("Failed to FindObjects"), + "FindObjects": errors.New("failed to FindObjects"), }, }, "bad_no_objects": { nSigners: 10, - pin: "", keyType: crypki.RSA, objects: []p11.ObjectHandle{}, expectError: true, }, "bad_FindObjectsFinal": { nSigners: 10, - pin: "", keyType: crypki.RSA, objects: []p11.ObjectHandle{1, 2}, expectError: true, errMsg: map[string]error{ - "OpenSession": errors.New("Failed to FindObjectsFinal"), + "FindObjectsFinal": errors.New("failed to FindObjectsFinal"), }, }, } @@ -190,7 +174,7 @@ func TestNewSignerPool(t *testing.T) { Return(tt.errMsg["FindObjectsFinal"]). AnyTimes() - ret, err := newSignerPool(mockCtx, tt.nSigners, tt.slot, tt.token, tt.pin, tt.keyType) + ret, err := newSignerPool(mockCtx, tt.nSigners, tt.slot, tt.token, tt.keyType) if tt.expectError { if err == nil { t.Error("expected error, but got nil") @@ -211,7 +195,6 @@ func TestNewSignerPool(t *testing.T) { t.Errorf("Expect %v Signers, but got %v", tt.nSigners, len(got.signers)) return } - }) } }