From 80d3934494bdac9a4be8aec456c2b153161314ab Mon Sep 17 00:00:00 2001 From: Gari Singh Date: Thu, 27 Aug 2020 11:47:55 -0400 Subject: [PATCH] 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 --- bccsp/pkcs11/impl_test.go | 36 +++++++----------------------------- bccsp/pkcs11/pkcs11.go | 8 ++------ bccsp/pkcs11/pkcs11_test.go | 7 ++----- 3 files changed, 11 insertions(+), 40 deletions(-) diff --git a/bccsp/pkcs11/impl_test.go b/bccsp/pkcs11/impl_test.go index da0358e074d..0e17873ebdc 100644 --- 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,15 +249,6 @@ 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 { @@ -278,7 +256,7 @@ func TestInvalidAltId(t *testing.T) { } // 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 index 159b26da263..f30e03c008f 100644 --- 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 index 7aabcf2daf8..25970ca84ce 100644 --- 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!") } + }