[FAB-18250] Check Error Before Returning Session to Pool (#1937)

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 <[email protected]>

bccsp/pkcs11/pkcs11.go

@@ -16,6 +16,7 @@ import (
 	"fmt"
 	"math/big"
 	"os"
+	"regexp"
 	"sync"
 	"time"
 
@@ -31,6 +32,7 @@ const createSessionRetries = 10
 
 var (
 	logger           = flogging.MustGetLogger("bccsp_p11")
+	regex            = regexp.MustCompile(".*0xB.:\\sCKR.+")
 	sessionCacheSize = 10
 )
 
@@ -373,7 +375,8 @@ func (csp *impl) getECKey(ski []byte) (pubKey *ecdsa.PublicKey, isPriv bool, err
 	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)
 	if err != nil {
@@ -451,7 +454,7 @@ func (csp *impl) generateECKey(curve asn1.ObjectIdentifier, ephemeral bool) (ski
 	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))
@@ -570,7 +573,7 @@ func (csp *impl) signP11ECDSA(ski []byte, msg []byte) (R, S *big.Int, err error)
 	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 {
@@ -602,7 +605,7 @@ func (csp *impl) verifyP11ECDSA(ski []byte, msg []byte, R, S *big.Int, byteSize
 	if err != nil {
 		return false, err
 	}
-	defer csp.returnSession(session)
+	defer func() { csp.handleSessionReturn(err, session) }()
 
 	logger.Debugf("Verify ECDSA\n")
 
@@ -787,6 +790,17 @@ func (csp *impl) ecPoint(session pkcs11.SessionHandle, key pkcs11.ObjectHandle)
 	return ecpt, oid, nil
 }
 
+func (csp *impl) 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

bccsp/pkcs11/pkcs11_test.go

@@ -1545,3 +1545,31 @@ func TestPKCS11ECKeySignVerify(t *testing.T) {
 		t.Fatal("Signature should not match with software verification!")
 	}
 }
+
+func TestHandleSessionReturn(t *testing.T) {
+	opts := PKCS11Opts{
+		HashFamily: "SHA2",
+		SecLevel:   256,
+		SoftVerify: false,
+	}
+	opts.Library, opts.Pin, opts.Label = FindPKCS11Lib()
+
+	provider, err := New(opts, currentKS)
+	require.NoError(t, err)
+	pi := provider.(*impl)
+	defer pi.ctx.Destroy()
+
+	// Retrieve and destroy default session created during initialization
+	session, err := pi.getSession()
+	require.NoError(t, err)
+	pi.closeSession(session)
+
+	// Verify session pool is empty and place invalid session in pool
+	require.Empty(t, pi.sessPool, "sessionPool should be empty")
+	pi.returnSession(pkcs11.SessionHandle(^uint(0)))
+
+	// Attempt to generate key with invalid session
+	_, err = pi.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, pi.sessPool, "sessionPool should be empty")
+}