diff --git a/client/internal/pkcs11/driver_linux.go b/client/internal/pkcs11/driver_linux.go index 0d9eb8705..1ad8cd001 100644 --- a/client/internal/pkcs11/driver_linux.go +++ b/client/internal/pkcs11/driver_linux.go @@ -25,6 +25,12 @@ const ( userTypeUser = 0x1 findBatch = 32 + + // Upper bounds on sizes a module reports, so a buggy module cannot make the daemon + // allocate without limit. Certificates and keys are far smaller; an RSA-16384 + // signature is 2 KiB. + maxAttributeLen = 1 << 20 + maxSignatureLen = 1 << 16 ) type version struct { @@ -211,7 +217,7 @@ func (m *module) login(session uint, pin []byte) error { } rv := m.cLogin(ulong(session), userTypeUser, pinPtr, ulong(len(pin))) runtime.KeepAlive(pin) - if rv != rvOK && rv != rvUserAlreadyLoggedIn { + if rv != rvOK { return Error{Op: "C_Login", Code: uint(rv)} } return nil @@ -222,9 +228,10 @@ func (m *module) logout(session uint) { } func (m *module) findObjects(session uint, template []Attribute) ([]Object, error) { - attrs := toAttributes(template) + var pinner runtime.Pinner + attrs := toAttributes(template, &pinner) rv := m.cFindObjectsInit(ulong(session), first(attrs), ulong(len(attrs))) - runtime.KeepAlive(template) + pinner.Unpin() if rv != rvOK { return nil, Error{Op: "C_FindObjectsInit", Code: uint(rv)} } @@ -237,6 +244,9 @@ func (m *module) findObjects(session uint, template []Attribute) ([]Object, erro if rv := m.cFindObjects(ulong(session), &batch[0], findBatch, &count); rv != rvOK { return nil, Error{Op: "C_FindObjects", Code: uint(rv)} } + if count > findBatch { + return nil, fmt.Errorf("C_FindObjects reported %d handles for a batch of %d", count, findBatch) + } for _, handle := range batch[:count] { objects = append(objects, Object(handle)) } @@ -257,13 +267,21 @@ func (m *module) attribute(session uint, obj Object, typ uint) ([]byte, error) { if attr.len == 0 { return nil, nil } + if attr.len > maxAttributeLen { + return nil, fmt.Errorf("attribute 0x%x reports %d bytes, over the %d byte limit", typ, attr.len, maxAttributeLen) + } value := make([]byte, attr.len) + var pinner runtime.Pinner + pinner.Pin(&value[0]) attr.value = unsafe.Pointer(&value[0]) rv := m.cGetAttributeValue(ulong(session), ulong(obj), &attr, 1) - runtime.KeepAlive(value) + pinner.Unpin() if rv != rvOK { return nil, Error{Op: "C_GetAttributeValue", Code: uint(rv)} } + if attr.len > ulong(len(value)) { + return nil, fmt.Errorf("attribute 0x%x grew from %d to %d bytes between calls", typ, len(value), attr.len) + } return value[:attr.len], nil } @@ -272,14 +290,15 @@ func (m *module) sign(session uint, mech Mechanism, key Object, data []byte) ([] return nil, errors.New("nothing to sign") } native := mechanism{typ: ulong(mech.Type)} - var params *pssParams + var pinner runtime.Pinner if mech.PSS != nil { - params = &pssParams{hashAlg: ulong(mech.PSS.Hash), mgf: ulong(mech.PSS.MGF), saltLen: ulong(mech.PSS.SaltLen)} + params := &pssParams{hashAlg: ulong(mech.PSS.Hash), mgf: ulong(mech.PSS.MGF), saltLen: ulong(mech.PSS.SaltLen)} + pinner.Pin(params) native.parameter = unsafe.Pointer(params) native.len = ulong(unsafe.Sizeof(*params)) } rv := m.cSignInit(ulong(session), &native, ulong(key)) - runtime.KeepAlive(params) + pinner.Unpin() if rv != rvOK { return nil, Error{Op: "C_SignInit", Code: uint(rv)} } @@ -288,31 +307,41 @@ func (m *module) sign(session uint, mech Mechanism, key Object, data []byte) ([] if rv := m.cSign(ulong(session), &data[0], ulong(len(data)), nil, &size); rv != rvOK { return nil, Error{Op: "C_Sign", Code: uint(rv)} } + if size == 0 || size > maxSignatureLen { + return nil, fmt.Errorf("C_Sign reports a %d byte signature", size) + } signature := make([]byte, size) rv = m.cSign(ulong(session), &data[0], ulong(len(data)), &signature[0], &size) runtime.KeepAlive(data) if rv != rvOK { return nil, Error{Op: "C_Sign", Code: uint(rv)} } + if size > ulong(len(signature)) { + return nil, fmt.Errorf("C_Sign wrote %d bytes into a %d byte buffer", size, len(signature)) + } return signature[:size], nil } func (m *module) createObject(session uint, template []Attribute) (Object, error) { - attrs := toAttributes(template) + var pinner runtime.Pinner + attrs := toAttributes(template, &pinner) var object ulong rv := m.cCreateObject(ulong(session), first(attrs), ulong(len(attrs)), &object) - runtime.KeepAlive(template) + pinner.Unpin() if rv != rvOK { return 0, Error{Op: "C_CreateObject", Code: uint(rv)} } return Object(object), nil } -func toAttributes(template []Attribute) []attribute { +// toAttributes builds the C template for template. The values stay Go memory referenced +// from Go memory passed to C, so each is pinned; the caller unpins after the call. +func toAttributes(template []Attribute, pinner *runtime.Pinner) []attribute { attrs := make([]attribute, len(template)) for i, a := range template { attrs[i].typ = ulong(a.Type) if len(a.Value) > 0 { + pinner.Pin(&a.Value[0]) attrs[i].value = unsafe.Pointer(&a.Value[0]) attrs[i].len = ulong(len(a.Value)) } diff --git a/client/internal/pkcs11/pkcs11.go b/client/internal/pkcs11/pkcs11.go index 74903954e..4bae03369 100644 --- a/client/internal/pkcs11/pkcs11.go +++ b/client/internal/pkcs11/pkcs11.go @@ -82,6 +82,11 @@ func PINRejected(err error) bool { return false } +func isCode(err error, code uint) bool { + var e Error + return errors.As(err, &e) && e.Code == code +} + var returnValueNames = map[uint]string{ 0x2: "CKR_HOST_MEMORY", 0x3: "CKR_SLOT_ID_INVALID", @@ -195,11 +200,17 @@ func (m *Module) openSession(label string, pin []byte, readWrite bool) (*Session if pin == nil { return s, nil } - if err := m.d.login(handle, pin); err != nil { + err = m.d.login(handle, pin) + switch { + case err == nil: + s.loggedIn = true + case isCode(err, rvUserAlreadyLoggedIn): + // Login state belongs to the application, not the session, so another session + // holds it; logging out on Close would pull it from under that session. + default: s.Close() return nil, err } - s.loggedIn = true return s, nil } diff --git a/client/internal/pkcs11/session_test.go b/client/internal/pkcs11/session_test.go new file mode 100644 index 000000000..c64bba1ee --- /dev/null +++ b/client/internal/pkcs11/session_test.go @@ -0,0 +1,56 @@ +package pkcs11 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeDriver stands in for a loaded module and records the calls a session makes. +type fakeDriver struct { + loginErr error + logouts int + closes int +} + +func (f *fakeDriver) tokens() ([]Token, error) { return []Token{{Slot: 1, Label: "netbird"}}, nil } +func (f *fakeDriver) openSession(uint, bool) (uint, error) { return 7, nil } +func (f *fakeDriver) closeSession(uint) { f.closes++ } +func (f *fakeDriver) login(uint, []byte) error { return f.loginErr } +func (f *fakeDriver) logout(uint) { f.logouts++ } +func (f *fakeDriver) findObjects(uint, []Attribute) ([]Object, error) { + return nil, nil +} +func (f *fakeDriver) attribute(uint, Object, uint) ([]byte, error) { return nil, nil } +func (f *fakeDriver) sign(uint, Mechanism, Object, []byte) ([]byte, error) { + return nil, nil +} +func (f *fakeDriver) createObject(uint, []Attribute) (Object, error) { return 0, nil } + +func TestOpenSession_LogsOutOnlyALoginItOwns(t *testing.T) { + t.Run("own login is logged out on close", func(t *testing.T) { + d := &fakeDriver{} + s, err := (&Module{d: d}).OpenSession("netbird", []byte("1234")) + require.NoError(t, err) + s.Close() + assert.Equal(t, 1, d.logouts, "the session that logged in logs out again") + }) + + t.Run("login held by another session is left alone", func(t *testing.T) { + d := &fakeDriver{loginErr: Error{Op: "C_Login", Code: rvUserAlreadyLoggedIn}} + s, err := (&Module{d: d}).OpenSession("netbird", []byte("1234")) + require.NoError(t, err, "an existing login is good enough to use the token") + s.Close() + assert.Zero(t, d.logouts, "logging out would end the login of the session that owns it") + assert.Equal(t, 1, d.closes, "the session itself is still closed") + }) + + t.Run("rejected pin closes the session", func(t *testing.T) { + d := &fakeDriver{loginErr: Error{Op: "C_Login", Code: rvPINIncorrect}} + _, err := (&Module{d: d}).OpenSession("netbird", []byte("0000")) + assert.True(t, PINRejected(err), "the PIN error reaches the caller") + assert.Zero(t, d.logouts, "nothing to log out after a failed login") + assert.Equal(t, 1, d.closes, "the session opened for the login is closed") + }) +}