Fix broken pkcs11 tests

Running go test -tags pkcs11 was failing for bccsp. This was probably not caught in CI because CI runs with the short flag and the failing tests are omitted. Signed-off-by: Gari Singh <[email protected]>
hyperledger · Sep 1, 2020 · 80d3934 · 80d3934
1 parent c8d124f
commit 80d3934
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 40 deletions.
diff --git a/bccsp/pkcs11/impl_test.go b/bccsp/pkcs11/impl_test.go
@@ -47,7 +47,6 @@ type testConfig struct {
 	hashFamily    string
 	softVerify    bool
 	immutable     bool
-	altId         string
 }
 
 func TestMain(m *testing.M) {
@@ -71,18 +70,16 @@ func testMain(m *testing.M) int {
 
 	lib, pin, label := FindPKCS11Lib()
 	tests := []testConfig{
-		{256, "SHA2", true, false, ""},
-		{256, "SHA3", false, false, ""},
-		{384, "SHA2", false, false, ""},
-		{384, "SHA3", false, false, ""},
-		{384, "SHA3", true, false, ""},
+		{256, "SHA2", true, false},
+		{256, "SHA3", false, false},
+		{384, "SHA2", false, false},
+		{384, "SHA3", false, false},
+		{384, "SHA3", true, false},
 	}
 
 	if strings.Contains(lib, "softhsm") {
 		tests = append(tests, []testConfig{
-			{256, "SHA2", true, false, ""},
-			{256, "SHA2", true, true, ""},
-			{256, "SHA2", true, true, "AnAltId"},
+			{256, "SHA2", true, true},
 		}...)
 	}
 
@@ -99,8 +96,6 @@ func testMain(m *testing.M) int {
 		opts.SecLevel = config.securityLevel
 		opts.SoftVerify = config.softVerify
 		opts.Immutable = config.immutable
-		opts.AltId = config.altId
-		fmt.Printf("Immutable = [%v]\n", opts.Immutable)
 		currentBCCSP, err = New(opts, keyStore)
 		if err != nil {
 			fmt.Printf("Failed initiliazing BCCSP at [%+v] \n%s\n", opts, err)
@@ -221,10 +216,6 @@ func TestInvalidNewParameter(t *testing.T) {
 }
 
 func TestInvalidSKI(t *testing.T) {
-	if currentTestConfig.altId != "" {
-		t.Skip("Skipping TestInvalidSKI since AltId is set for this test run.")
-	}
-
 	k, err := currentBCCSP.GetKey(nil)
 	if err == nil {
 		t.Fatal("Error should be different from nil in this case")
@@ -243,10 +234,6 @@ func TestInvalidSKI(t *testing.T) {
 }
 
 func TestInvalidAltId(t *testing.T) {
-	if currentTestConfig.altId == "" {
-		t.Skip("Skipping TestInvalidAltId since AltId not set for this test run.")
-	}
-
 	opts := PKCS11Opts{
 		HashFamily: currentTestConfig.hashFamily,
 		SecLevel:   currentTestConfig.securityLevel,
@@ -262,23 +249,14 @@ func TestInvalidAltId(t *testing.T) {
 	lib, _, _ := FindPKCS11Lib()
 	opts.Library = lib
 
-	// Generate a KeyPair associated to the initial label
-	k, err := currentBCCSP.KeyGen(&bccsp.ECDSAP256KeyGenOpts{Temporary: false})
-	if err != nil {
-		t.Fatalf("Failed generating ECDSA P256 key [%s]", err)
-	}
-	if k == nil {
-		t.Fatal("Failed generating ECDSA P256 key. Key must be different from nil")
-	}
-
 	// Create temporary BCCSP set with an initial label
 	testBCCSP, err := New(opts, currentKS)
 	if err != nil {
 		t.Fatalf("Failed initiliazing Test BCCSP at [%+v] \n%s\n", opts, err)
 	}
 
 	// Now, try to retrieve the key using a different label
-	k, err = testBCCSP.GetKey([]byte{0, 1, 2, 3, 4, 5, 6})
+	k, err := testBCCSP.GetKey([]byte{0, 1, 2, 3, 4, 5, 6})
 	if err == nil {
 		t.Fatal("Error should be different from nil in this case")
 	}

diff --git a/bccsp/pkcs11/pkcs11.go b/bccsp/pkcs11/pkcs11.go
@@ -105,7 +105,7 @@ func createSession(ctx *pkcs11.Ctx, slot uint, pin string) pkcs11.SessionHandle
 		time.Sleep(100 * time.Millisecond)
 	}
 	if err != nil {
-		logger.Fatalf("OpenSession failed [%s]", err)
+		logger.Panicf("OpenSession failed [%s]", err)
 	}
 	logger.Debugf("Created new pkcs11 session %+v on slot %d\n", s, slot)
 	session := s
@@ -233,15 +233,11 @@ func (csp *impl) generateECKey(curve asn1.ObjectIdentifier, ephemeral bool) (ski
 		// Generate using the SKI process and then make keypair immutable according to csp.immutable
 		keylabel = fmt.Sprintf("BCP%s", nextIDCtr().Text(16))
 		updateSKI = true
-	} else if csp.altId != "" && csp.immutable {
+	} else if csp.altId != "" {
 		// Generate the key pair using AltID process.
 		// No need to worry about immutable since AltID is used with Write-Once HSMs
 		keylabel = csp.altId
 		updateSKI = false
-	} else if csp.altId != "" && !csp.immutable {
-		// Raise an error since AltID is used with Write-Once HSMs
-		// So cannot make support AltID with immutable = false.
-		return nil, nil, fmt.Errorf("Cannot generate a mutable key using AltID.")
 	}
 
 	marshaledOID, err := asn1.Marshal(curve)

diff --git a/bccsp/pkcs11/pkcs11_test.go b/bccsp/pkcs11/pkcs11_test.go
@@ -91,10 +91,6 @@ func TestOIDFromNamedCurve(t *testing.T) {
 }
 
 func TestAlternateLabelGeneration(t *testing.T) {
-	if currentTestConfig.altId == "" {
-		t.Skip("Skipping TestAlternateLabelGeneration since AltId is not set for this test run.")
-	}
-
 	// We generate a unique Alt ID for the test
 	uniqueAltId := strconv.FormatInt(time.Now().UnixNano()/1e6, 10)
 	fakeSKI := []byte("FakeSKI")
@@ -218,8 +214,8 @@ func TestPKCS11ECKeySignVerify(t *testing.T) {
 	}
 
 	msg1 := []byte("This is my very authentic message")
-	msg2 := []byte("This is my very unauthentic message")
 	hash1, _ := currentBCCSP.Hash(msg1, &bccsp.SHAOpts{})
+	msg2 := []byte("This is my very unauthentic message")
 	hash2, _ := currentBCCSP.Hash(msg2, &bccsp.SHAOpts{})
 
 	var oid asn1.ObjectIdentifier
@@ -270,4 +266,5 @@ func TestPKCS11ECKeySignVerify(t *testing.T) {
 	if pass != false {
 		t.Fatal("Signature should not match with software verification!")
 	}
+
 }