Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAB-18250] Cherry-Pick -- Check Error Before Returning Session to Pool #1937

Merged
merged 1 commit into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions bccsp/pkcs11/pkcs11.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"fmt"
"math/big"
"os"
"regexp"
"sync"
"time"

Expand All @@ -31,6 +32,7 @@ const createSessionRetries = 10

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

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions bccsp/pkcs11/pkcs11_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}