Merge pull request #24 from ThalesIgnite/improve-error-checking

Improve error checking

.travis.yml

@@ -9,9 +9,7 @@ go:
 
 # trusty only has softhsmv1
 before_script:
- - sudo add-apt-repository -y ppa:pkg-opendnssec/ppa
- - sudo apt-get update
- - sudo apt-get install softhsm2
+ - sudo add-apt-repository -y ppa:pkg-opendnssec/ppa && sudo apt-get update && sudo apt-get install softhsm2
  - curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh
 
 script:

Gopkg.toml

@@ -24,3 +24,7 @@
 [[constraint]]
   branch = "master"
   name = "github.com/miekg/pkcs11"
+
+[[constraint]]
+  name = "github.com/stretchr/testify"
+  version = "1.3.0"

common.go

@@ -72,7 +72,7 @@ func (sig *dsaSignature) marshalDER() ([]byte, error) {
 	return asn1.Marshal(*sig)
 }
 
-// Compute *DSA signature and marshal the result in DER fform
+// Compute *DSA signature and marshal the result in DER form
 func dsaGeneric(slot uint, key pkcs11.ObjectHandle, mechanism uint, digest []byte) ([]byte, error) {
 	var err error
 	var sigBytes []byte
@@ -88,7 +88,11 @@ func dsaGeneric(slot uint, key pkcs11.ObjectHandle, mechanism uint, digest []byt
 	if err != nil {
 		return nil, err
 	}
-	sig.unmarshalBytes(sigBytes)
+	err = sig.unmarshalBytes(sigBytes)
+	if err != nil {
+		return nil, err
+	}
+
 	return sig.marshalDER()
 }
 

crypto11.go

@@ -285,13 +285,16 @@ func Configure(config *PKCS11Config) (*pkcs11.Ctx, error) {
 // Note that if CRYPTO11_CONFIG_PATH is set in the environment,
 // configuration will be read from that file, overriding any later
 // runtime configuration.
-func ConfigureFromFile(configLocation string) (*pkcs11.Ctx, error) {
+func ConfigureFromFile(configLocation string) (ctx *pkcs11.Ctx, err error) {
 	file, err := os.Open(configLocation)
 	if err != nil {
 		log.Printf("Could not open config file: %s", configLocation)
 		return nil, err
 	}
-	defer file.Close()
+	defer func() {
+		err = file.Close()
+	}()
+
 	configDecoder := json.NewDecoder(file)
 	config := &PKCS11Config{}
 	err = configDecoder.Decode(config)

crypto11_test.go

@@ -27,6 +27,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"github.com/miekg/pkcs11"
+	"github.com/stretchr/testify/require"
 	"log"
 	"os"
 	"testing"
@@ -40,17 +41,22 @@ func TestInitializeFromConfig(t *testing.T) {
 	config.TokenSerial = "NoSuchToken"
 	config.TokenLabel = "NoSuchToken"
 	//assert.Panics(Configure(config), "Invalid config should panic")
-	ConfigureFromFile("config")
-	Close()
+	_, err := ConfigureFromFile("config")
+	require.NoError(t, err)
+	require.NoError(t, Close())
 }
 
 func TestLoginContext(t *testing.T) {
 	t.Run("key identity with login", func(t *testing.T) {
-		configureWithPin(t)
-		defer Close()
+		_, err := configureWithPin(t)
+		require.NoError(t, err)
+
+		defer func() {
+			err = Close()
+			require.NoError(t, err)
+		}()
 
 		// Generate a key and and close a session
-		var err error
 		var key *PKCS11PrivateKeyDSA
 		psize := dsa.L1024N160
 		if key, err = GenerateDSAKeyPair(dsaSizes[psize]); err != nil {
@@ -74,7 +80,8 @@ func TestLoginContext(t *testing.T) {
 		// Reopen a session and try to find a key.
 		// Valid session must enlist a key.
 		// If login is not performed than it will fail.
-		configureWithPin(t)
+		_, err = configureWithPin(t)
+		require.NoError(t, err)
 
 		var key2 crypto.PrivateKey
 		if key2, err = FindKeyPair(id, nil); err != nil {
@@ -89,11 +96,15 @@ func TestLoginContext(t *testing.T) {
 		defer func() {instance.cfg.IdleTimeout = prevIdleTimeout}()
 		instance.cfg.IdleTimeout = time.Second
 
-		configureWithPin(t)
-		defer Close()
+		_, err := configureWithPin(t)
+		require.NoError(t, err)
+
+		defer func() {
+			err = Close()
+			require.NoError(t, err)
+		}()
 
 		// Generate a key and and close a session
-		var err error
 		var key *PKCS11PrivateKeyDSA
 		psize := dsa.L1024N160
 		if key, err = GenerateDSAKeyPair(dsaSizes[psize]); err != nil {
@@ -123,11 +134,15 @@ func TestLoginContext(t *testing.T) {
 	})
 
 	t.Run("login context shared between sessions", func(t *testing.T) {
-		configureWithPin(t)
-		defer Close()
+		_, err := configureWithPin(t)
+		require.NoError(t, err)
+
+		defer func() {
+			err = Close()
+			require.NoError(t, err)
+		}()
 
 		// Generate a key and and close a session
-		var err error
 		var key *PKCS11PrivateKeyDSA
 		psize := dsa.L1024N160
 		if key, err = GenerateDSAKeyPair(dsaSizes[psize]); err != nil {
@@ -167,11 +182,15 @@ func TestIdentityExpiration(t *testing.T) {
 	defer func() {instance.cfg.IdleTimeout = prevIdleTimeout}()
 	instance.cfg.IdleTimeout = time.Second
 
-	configureWithPin(t)
-	defer Close()
+	_, err := configureWithPin(t)
+	require.NoError(t, err)
+
+	defer func() {
+		err = Close()
+		require.NoError(t, err)
+	}()
 
 	// Generate a key and and close a session
-	var err error
 	var key *PKCS11PrivateKeyDSA
 	psize := dsa.L1024N160
 	if key, err = GenerateDSAKeyPair(dsaSizes[psize]); err != nil {
@@ -211,13 +230,16 @@ func configureWithPin(t *testing.T) (*pkcs11.Ctx, error) {
 	return ctx, nil
 }
 
-func getConfig(configLocation string) (*PKCS11Config, error) {
+func getConfig(configLocation string) (ctx *PKCS11Config, err error) {
 	file, err := os.Open(configLocation)
 	if err != nil {
 		log.Printf("Could not open config file: %s", configLocation)
 		return nil, err
 	}
-	defer file.Close()
+	defer func() {
+		err = file.Close()
+	}()
+
 	configDecoder := json.NewDecoder(file)
 	config := &PKCS11Config{}
 	err = configDecoder.Decode(config)

dsa_test.go

@@ -28,6 +28,7 @@ import (
 	_ "crypto/sha1"
 	_ "crypto/sha256"
 	_ "crypto/sha512"
+	"github.com/stretchr/testify/require"
 	"io"
 	"math/big"
 	"testing"
@@ -99,11 +100,12 @@ func TestNativeDSA(t *testing.T) {
 }
 
 func TestHardDSA(t *testing.T) {
-	var err error
 	var key *PKCS11PrivateKeyDSA
 	var key2, key3 crypto.PrivateKey
 	var id, label []byte
-	ConfigureFromFile("config")
+	_, err := ConfigureFromFile("config")
+	require.NoError(t, err)
+
 	for psize, params := range dsaSizes {
 		if key, err = GenerateDSAKeyPair(params); err != nil {
 			t.Errorf("crypto11.GenerateDSAKeyPair: %v", err)
@@ -130,7 +132,7 @@ func TestHardDSA(t *testing.T) {
 		}
 		testDsaSigning(t, key3.(crypto.Signer), psize, "hard3")
 	}
-	Close()
+	require.NoError(t, Close())
 }
 
 func testDsaSigning(t *testing.T, key crypto.Signer, psize dsa.ParameterSizes, what string) {

ecdsa_test.go

@@ -29,6 +29,7 @@ import (
 	_ "crypto/sha1"
 	_ "crypto/sha256"
 	_ "crypto/sha512"
+	"github.com/stretchr/testify/require"
 	"testing"
 )
 
@@ -57,11 +58,12 @@ func TestNativeECDSA(t *testing.T) {
 }
 
 func TestHardECDSA(t *testing.T) {
-	var err error
 	var key *PKCS11PrivateKeyECDSA
 	var key2, key3 crypto.PrivateKey
 	var id, label []byte
-	ConfigureFromFile("config")
+	_, err := ConfigureFromFile("config")
+	require.NoError(t, err)
+
 	for _, curve := range curves {
 		if key, err = GenerateECDSAKeyPair(curve); err != nil {
 			t.Errorf("GenerateECDSAKeyPair: %v", err)
@@ -92,7 +94,7 @@ func TestHardECDSA(t *testing.T) {
 		}
 		testEcdsaSigning(t, key3.(crypto.Signer), crypto.SHA384)
 	}
-	Close()
+	require.NoError(t, Close())
 }
 
 func testEcdsaSigning(t *testing.T, key crypto.Signer, hashFunction crypto.Hash) {

hmac.go

@@ -214,7 +214,11 @@ func (hi *hmacImplementation) Sum(b []byte) []byte {
 
 func (hi *hmacImplementation) Reset() {
 	hi.Sum(nil) // Clean up
-	hi.initialize()
+
+	// Assign the error to "_" to indicate we are knowingly ignoring this. It may have been
+	// sensible to panic at this stage, but we cannot add a panic without breaking backwards
+	// compatibility.
+	_ = hi.initialize()
 }
 
 func (hi *hmacImplementation) Size() int {

hmac_test.go

@@ -24,14 +24,16 @@ package crypto11
 import (
 	"bytes"
 	"github.com/miekg/pkcs11"
+	"github.com/stretchr/testify/require"
 	"hash"
 	"testing"
 )
 
 func TestHmac(t *testing.T) {
-	ConfigureFromFile("config")
+	_, err := ConfigureFromFile("config")
+	require.NoError(t, err)
+
 	var info pkcs11.Info
-	var err error
 	if info, err = instance.ctx.GetInfo(); err != nil {
 		t.Errorf("GetInfo: %v", err)
 		return
@@ -48,7 +50,7 @@ func TestHmac(t *testing.T) {
 	t.Run("HMACSHA256", func(t *testing.T) {
 		testHmac(t, pkcs11.CKK_SHA256_HMAC, pkcs11.CKM_SHA256_HMAC, 0, 32, false)
 	})
-	Close()
+	require.NoError(t, Close())
 }
 
 func testHmac(t *testing.T, keytype int, mech int, length int, xlength int, full bool) {

keys.go

@@ -46,8 +46,7 @@ func (object *PKCS11Object) Identify() (id []byte, label []byte, err error) {
 
 // Find a key object.  For asymmetric keys this only finds one half so
 // callers will call it twice.
-func findKey(session *PKCS11Session, id []byte, label []byte, keyclass uint, keytype uint) (pkcs11.ObjectHandle, error) {
-	var err error
+func findKey(session *PKCS11Session, id []byte, label []byte, keyclass uint, keytype uint) (obj pkcs11.ObjectHandle, err error) {
 	var handles []pkcs11.ObjectHandle
 	var template []*pkcs11.Attribute
 	if keyclass != ^uint(0) {
@@ -65,7 +64,9 @@ func findKey(session *PKCS11Session, id []byte, label []byte, keyclass uint, key
 	if err = session.Ctx.FindObjectsInit(session.Handle, template); err != nil {
 		return 0, err
 	}
-	defer session.Ctx.FindObjectsFinal(session.Handle)
+	defer func() {
+		err = session.Ctx.FindObjectsFinal(session.Handle)
+	}()
 	if handles, _, err = session.Ctx.FindObjects(session.Handle, 1); err != nil {
 		return 0, err
 	}