Skip to content

Commit

Permalink
Isolate softVerify to impacted test paths
Browse files Browse the repository at this point in the history
The SoftwareVerify option only get used when validating ECDSA
signatures. By isolating the configuration to the test that requires it,
we can reduce the number of times all tests are executed.

Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>
  • Loading branch information
sykesm committed Aug 21, 2020
1 parent 82b867f commit f55ef45
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 20 deletions.
3 changes: 1 addition & 2 deletions bccsp/pkcs11/pkcs11.go
Expand Up @@ -181,7 +181,6 @@ func (csp *impl) KeyImport(raw interface{}, opts bccsp.KeyImportOpts) (k bccsp.K
}

switch opts.(type) {

case *bccsp.X509PublicKeyImportOpts:
x509Cert, ok := raw.(*x509.Certificate)
if !ok {
Expand Down Expand Up @@ -648,7 +647,7 @@ func (csp *impl) verifyP11ECDSA(ski []byte, msg []byte, R, S *big.Int, byteSize
}
defer csp.returnSession(session)

logger.Debugf("Verify ECDSA\n")
logger.Debugf("Verify ECDSA")

publicKey, err := csp.findKeyPairFromSKI(session, ski, publicKeyType)
if err != nil {
Expand Down
44 changes: 26 additions & 18 deletions bccsp/pkcs11/pkcs11_test.go
Expand Up @@ -45,7 +45,6 @@ var (
type testConfig struct {
securityLevel int
hashFamily string
softVerify bool
immutable bool
}

Expand All @@ -69,16 +68,16 @@ func testMain(m *testing.M) int {
currentKS = keyStore

lib, pin, label := FindPKCS11Lib()
tests := []testConfig{
{securityLevel: 256, hashFamily: "SHA2", softVerify: true, immutable: false},
{securityLevel: 256, hashFamily: "SHA3", softVerify: false, immutable: false},
{securityLevel: 384, hashFamily: "SHA2", softVerify: false, immutable: false},
{securityLevel: 384, hashFamily: "SHA3", softVerify: false, immutable: false},
{securityLevel: 384, hashFamily: "SHA3", softVerify: true, immutable: false},
var tests []testConfig
tests = []testConfig{
{securityLevel: 256, hashFamily: "SHA2", immutable: false},
{securityLevel: 256, hashFamily: "SHA3", immutable: false},
{securityLevel: 384, hashFamily: "SHA2", immutable: false},
{securityLevel: 384, hashFamily: "SHA3", immutable: false},
}

if strings.Contains(lib, "softhsm") {
tests = append(tests, testConfig{securityLevel: 256, hashFamily: "SHA2", softVerify: true, immutable: true})
tests = append(tests, testConfig{securityLevel: 256, hashFamily: "SHA2", immutable: true})
}

opts := PKCS11Opts{
Expand All @@ -92,7 +91,6 @@ func testMain(m *testing.M) int {

opts.Hash = config.hashFamily
opts.Security = config.securityLevel
opts.SoftwareVerify = config.softVerify
opts.Immutable = config.immutable

provider, err := New(opts, keyStore)
Expand Down Expand Up @@ -370,23 +368,33 @@ func TestECDSASign(t *testing.T) {
func TestECDSAVerify(t *testing.T) {
k, err := currentBCCSP.KeyGen(&bccsp.ECDSAKeyGenOpts{Temporary: false})
require.NoError(t, err)
pk, err := k.PublicKey()
require.NoError(t, err)

digest, err := currentBCCSP.Hash([]byte("Hello World"), &bccsp.SHAOpts{})
require.NoError(t, err)

signature, err := currentBCCSP.Sign(k, digest, nil)
require.NoError(t, err)

valid, err := currentBCCSP.Verify(k, signature, digest, nil)
require.NoError(t, err)
require.True(t, valid, "signature should be valid from private key")
tests := map[string]bool{
"WithSoftVerify": true,
"WithoutSoftVerify": false,
}
for name, softVerify := range tests {
t.Run(name, func(t *testing.T) {
defer func(s bool) { currentBCCSP.softVerify = s }(currentBCCSP.softVerify)
currentBCCSP.softVerify = softVerify

pk, err := k.PublicKey()
require.NoError(t, err)
valid, err := currentBCCSP.Verify(k, signature, digest, nil)
require.NoError(t, err)
require.True(t, valid, "signature should be valid from private key")

valid, err = currentBCCSP.Verify(pk, signature, digest, nil)
require.NoError(t, err)
require.True(t, valid, "signature should be valid from public key")
valid, err = currentBCCSP.Verify(pk, signature, digest, nil)
require.NoError(t, err)
require.True(t, valid, "signature should be valid from public key")
})
}

t.Run("MissingKey", func(t *testing.T) {
_, err := currentBCCSP.Verify(nil, signature, digest, nil)
Expand Down Expand Up @@ -416,7 +424,7 @@ func TestECDSAVerify(t *testing.T) {
pk2, err := currentBCCSP.GetKey(pk.SKI())
require.NoError(t, err)

valid, err = currentBCCSP.Verify(pk2, signature, digest, nil)
valid, err := currentBCCSP.Verify(pk2, signature, digest, nil)
require.NoError(t, err)
require.True(t, valid, "signature should be valid from public key")
})
Expand Down

0 comments on commit f55ef45

Please sign in to comment.