Skip to content

Commit

Permalink
[FAB-18250] Check Error Before Returning Session to Pool
Browse files Browse the repository at this point in the history
FAB-17722 introduced logic for evicting invalid sessions from
the pkcs11 session pool. The GetSessionInfo call was expensive,
taking nearly a full second per call. The change was reverted
in FAB-18242.

This change introduces a new method for evicting sessions by
checking the error message returned when pkcs11 call fail
and closing sessions that resulted in SESSION errors.

Signed-off-by: Brett Logan <brett.t.logan@ibm.com>
  • Loading branch information
Brett Logan committed Sep 28, 2020
1 parent fc0c3e2 commit a288fe4
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 4 deletions.
21 changes: 17 additions & 4 deletions bccsp/pkcs11/pkcs11.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"fmt"
"math/big"
"os"
"regexp"
"sync"
"time"

Expand All @@ -28,6 +29,7 @@ import (
)

var logger = flogging.MustGetLogger("bccsp_p11")
var regex = regexp.MustCompile(".*0xB.:\\sCKR.+")

type Provider struct {
bccsp.BCCSP
Expand Down Expand Up @@ -374,7 +376,7 @@ func (csp *Provider) getECKey(ski []byte) (pubKey *ecdsa.PublicKey, isPriv bool,
if err != nil {
return nil, false, err
}
defer csp.returnSession(session)
defer func() { csp.handleSessionReturn(err, session) }()

isPriv = true
_, err = csp.findKeyPairFromSKI(session, ski, privateKeyType)
Expand Down Expand Up @@ -464,7 +466,7 @@ func (csp *Provider) generateECKey(curve asn1.ObjectIdentifier, ephemeral bool)
if err != nil {
return nil, nil, err
}
defer csp.returnSession(session)
defer func() { csp.handleSessionReturn(err, session) }()

id := nextIDCtr()
publabel := fmt.Sprintf("BCPUB%s", id.Text(16))
Expand Down Expand Up @@ -583,7 +585,7 @@ func (csp *Provider) signP11ECDSA(ski []byte, msg []byte) (R, S *big.Int, err er
if err != nil {
return nil, nil, err
}
defer csp.returnSession(session)
defer func() { csp.handleSessionReturn(err, session) }()

privateKey, err := csp.findKeyPairFromSKI(session, ski, privateKeyType)
if err != nil {
Expand Down Expand Up @@ -615,7 +617,7 @@ func (csp *Provider) verifyP11ECDSA(ski []byte, msg []byte, R, S *big.Int, byteS
if err != nil {
return false, err
}
defer csp.returnSession(session)
defer func() { csp.handleSessionReturn(err, session) }()

logger.Debugf("Verify ECDSA")

Expand Down Expand Up @@ -800,6 +802,17 @@ func (csp *Provider) ecPoint(session pkcs11.SessionHandle, key pkcs11.ObjectHand
return ecpt, oid, nil
}

func (csp *Provider) handleSessionReturn(err error, session pkcs11.SessionHandle) {
if err != nil {
if regex.MatchString(err.Error()) {
logger.Infof("PKCS11 session invalidated, closing session: %v", err)
csp.closeSession(session)
return
}
}
csp.returnSession(session)
}

func listAttrs(p11lib *pkcs11.Ctx, session pkcs11.SessionHandle, obj pkcs11.ObjectHandle) {
var cktype, ckclass uint
var ckaid, cklabel []byte
Expand Down
21 changes: 21 additions & 0 deletions bccsp/pkcs11/pkcs11_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -763,3 +763,24 @@ func TestDelegation(t *testing.T) {
require.Equal(t, msg, pt)
})
}

func TestHandleSessionReturn(t *testing.T) {
opts := defaultOptions()
opts.sessionCacheSize = 5
csp, cleanup := newProvider(t, opts)
defer cleanup()

// Retrieve and destroy default session created during initialization
session, err := csp.getSession()
require.NoError(t, err)
csp.closeSession(session)

// Verify session pool is empty and place invalid session in pool
require.Empty(t, csp.sessPool, "sessionPool should be empty")
csp.returnSession(pkcs11.SessionHandle(^uint(0)))

// Attempt to generate key with invalid session
_, err = csp.KeyGen(&bccsp.ECDSAP256KeyGenOpts{Temporary: false})
require.EqualError(t, err, "Failed generating ECDSA P256 key: P11: keypair generate failed [pkcs11: 0xB3: CKR_SESSION_HANDLE_INVALID]")
require.Empty(t, csp.sessPool, "sessionPool should be empty")
}

0 comments on commit a288fe4

Please sign in to comment.