[FAB-3350] Increase test coverage for cryptogen

As we start to package this tool, need to make
sure the test coverage is adequate.  This CR
ups the current test coverage to >90%. It adds
some logic to existing tests and modifies some
functions to use nested if logic as independent
checks for errors were not needed / efficient.

Will need to add coverage for main.go in a follow-up
change.  Also addresses staticcheck errors from
[FAB-3177]

Change-Id: Ia7def11383e7c680ece59e5630cf9debaa6be2b0
Signed-off-by: Gari Singh <[email protected]>

Author: mastersingh24

common/tools/cryptogen/ca/ca_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package ca_test
 
 import (
+	"crypto/ecdsa"
 	"crypto/x509"
 	"os"
 	"path/filepath"
@@ -53,7 +54,7 @@ func TestNewCA(t *testing.T) {
 
 }
 
-func TestGenerateSignedCertificate(t *testing.T) {
+func TestGenerateSignCertificate(t *testing.T) {
 
 	caDir := filepath.Join(testDir, "ca")
 	certDir := filepath.Join(testDir, "certs")
@@ -77,6 +78,17 @@ func TestGenerateSignedCertificate(t *testing.T) {
 	pemFile := filepath.Join(certDir, testName+"-cert.pem")
 	assert.Equal(t, true, checkForFile(pemFile),
 		"Expected to find file "+pemFile)
+
+	err = rootCA.SignCertificate(certDir, "empty/CA", ecPubKey)
+	assert.Error(t, err, "Bad name should fail")
+
+	// use an empty CA to test error path
+	badCA := &ca.CA{
+		Name:     "badCA",
+		SignCert: &x509.Certificate{},
+	}
+	err = badCA.SignCertificate(certDir, testName, &ecdsa.PublicKey{})
+	assert.Error(t, err, "Empty CA should not be able to sign")
 	cleanup(testDir)
 
 }

common/tools/cryptogen/ca/generator.go

@@ -42,64 +42,53 @@ type CA struct {
 // baseDir/name
 func NewCA(baseDir, name string) (*CA, error) {
 
-	err := os.MkdirAll(baseDir, 0755)
-	if err != nil {
-		return nil, err
-	}
-
-	//key, err := genKeyECDSA(caDir, name)
-	priv, signer, err := csp.GeneratePrivateKey(baseDir)
-	// get public signing certificate
-	ecPubKey, err := csp.GetECPublicKey(priv)
-	if err != nil {
-		return nil, err
-	}
-
-	template, err := x509Template()
-
-	if err != nil {
-		return nil, err
-	}
-
-	//this is a CA
-	template.IsCA = true
-	template.KeyUsage |= x509.KeyUsageCertSign | x509.KeyUsageCRLSign
-	template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageAny, x509.ExtKeyUsageServerAuth}
-
-	//set the organization for the subject
-	subject := subjectTemplate()
-	subject.Organization = []string{name}
-	subject.CommonName = name
-
-	template.Subject = subject
-	template.SubjectKeyId = priv.SKI()
+	var response error
+	var ca *CA
 
-	x509Cert, err := genCertificateECDSA(baseDir, name, &template, &template,
-		ecPubKey, signer)
-
-	if err != nil {
-		return nil, err
-	}
-
-	ca := &CA{
-		Name:     name,
-		Signer:   signer,
-		SignCert: x509Cert,
+	err := os.MkdirAll(baseDir, 0755)
+	if err == nil {
+		priv, signer, err := csp.GeneratePrivateKey(baseDir)
+		response = err
+		if err == nil {
+			// get public signing certificate
+			ecPubKey, err := csp.GetECPublicKey(priv)
+			response = err
+			if err == nil {
+				template := x509Template()
+				//this is a CA
+				template.IsCA = true
+				template.KeyUsage |= x509.KeyUsageCertSign | x509.KeyUsageCRLSign
+				template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageAny, x509.ExtKeyUsageServerAuth}
+
+				//set the organization for the subject
+				subject := subjectTemplate()
+				subject.Organization = []string{name}
+				subject.CommonName = name
+
+				template.Subject = subject
+				template.SubjectKeyId = priv.SKI()
+
+				x509Cert, err := genCertificateECDSA(baseDir, name, &template, &template,
+					ecPubKey, signer)
+				response = err
+				if err == nil {
+					ca = &CA{
+						Name:     name,
+						Signer:   signer,
+						SignCert: x509Cert,
+					}
+				}
+			}
+		}
 	}
-
-	return ca, nil
+	return ca, response
 }
 
 // SignCertificate creates a signed certificate based on a built-in template
 // and saves it in baseDir/name
 func (ca *CA) SignCertificate(baseDir, name string, pub *ecdsa.PublicKey) error {
 
-	template, err := x509Template()
-
-	if err != nil {
-		return err
-	}
-
+	template := x509Template()
 	template.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
 
 	//set the organization for the subject
@@ -108,7 +97,7 @@ func (ca *CA) SignCertificate(baseDir, name string, pub *ecdsa.PublicKey) error
 
 	template.Subject = subject
 
-	_, err = genCertificateECDSA(baseDir, name, &template, ca.SignCert,
+	_, err := genCertificateECDSA(baseDir, name, &template, ca.SignCert,
 		pub, ca.Signer)
 
 	if err != nil {
@@ -128,14 +117,11 @@ func subjectTemplate() pkix.Name {
 }
 
 // default template for X509 certificates
-func x509Template() (x509.Certificate, error) {
+func x509Template() x509.Certificate {
 
 	//generate a serial number
 	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
-	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
-	if err != nil {
-		return x509.Certificate{}, err
-	}
+	serialNumber, _ := rand.Int(rand.Reader, serialNumberLimit)
 
 	now := time.Now()
 	//basic template to use
@@ -146,7 +132,7 @@ func x509Template() (x509.Certificate, error) {
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
 		BasicConstraintsValid: true,
 	}
-	return x509, nil
+	return x509
 
 }
 

common/tools/cryptogen/csp/csp.go

@@ -31,30 +31,27 @@ func GeneratePrivateKey(keystorePath string) (bccsp.Key,
 	crypto.Signer, error) {
 
 	csp := factory.GetDefault()
+	var response error
+	var priv bccsp.Key
+	signer := &signer.CryptoSigner{}
 
 	// generate a key
 	priv, err := csp.KeyGen(&bccsp.ECDSAP256KeyGenOpts{Temporary: true})
-	if err != nil {
-		return nil, nil, err
-	}
-	// write it to the keystore
-	ks, err := sw.NewFileBasedKeyStore(nil, keystorePath, false)
-	if err != nil {
-		return nil, nil, err
-	}
-	err = ks.StoreKey(priv)
-	if err != nil {
-		return nil, nil, err
+	response = err
+	if err == nil {
+		// write it to the keystore
+		ks, err := sw.NewFileBasedKeyStore(nil, keystorePath, false)
+		response = err
+		if err == nil {
+			err = ks.StoreKey(priv)
+			response = err
+			if err == nil {
+				// create a crypto.Signer
+				err = signer.Init(csp, priv)
+			}
+		}
 	}
-
-	// create a crypto.Signer
-	signer := &signer.CryptoSigner{}
-	err = signer.Init(csp, priv)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return priv, signer, nil
+	return priv, signer, response
 
 }
 

common/tools/cryptogen/csp/csp_test.go

@@ -18,14 +18,43 @@ package csp_test
 import (
 	"crypto/ecdsa"
 	"encoding/hex"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/hyperledger/fabric/bccsp"
 	"github.com/hyperledger/fabric/common/tools/cryptogen/csp"
 	"github.com/stretchr/testify/assert"
 )
 
+// mock implementation of bccsp.Key interface
+type mockKey struct {
+	pubKeyErr error
+	bytesErr  error
+	pubKey    bccsp.Key
+}
+
+func (mk *mockKey) Bytes() ([]byte, error) {
+	if mk.bytesErr != nil {
+		return nil, mk.bytesErr
+	}
+	return []byte{1, 2, 3, 4}, nil
+}
+
+func (mk *mockKey) PublicKey() (bccsp.Key, error) {
+	if mk.pubKeyErr != nil {
+		return nil, mk.pubKeyErr
+	}
+	return mk.pubKey, nil
+}
+
+func (mk *mockKey) SKI() []byte { return []byte{1, 2, 3, 4} }
+
+func (mk *mockKey) Symmetric() bool { return false }
+
+func (mk *mockKey) Private() bool { return false }
+
 var testDir = filepath.Join(os.TempDir(), "csp-test")
 
 func TestGeneratePrivateKey(t *testing.T) {
@@ -52,6 +81,32 @@ func TestGetECPublicKey(t *testing.T) {
 	assert.NoError(t, err, "Failed to get public key from private key")
 	assert.IsType(t, &ecdsa.PublicKey{}, ecPubKey,
 		"Failed to return an ecdsa.PublicKey")
+
+	// force errors using mockKey
+	priv = &mockKey{
+		pubKeyErr: nil,
+		bytesErr:  nil,
+		pubKey:    &mockKey{},
+	}
+	_, err = csp.GetECPublicKey(priv)
+	assert.Error(t, err, "Expected an error with a invalid pubKey bytes")
+	priv = &mockKey{
+		pubKeyErr: nil,
+		bytesErr:  nil,
+		pubKey: &mockKey{
+			bytesErr: errors.New("bytesErr"),
+		},
+	}
+	_, err = csp.GetECPublicKey(priv)
+	assert.EqualError(t, err, "bytesErr", "Expected bytesErr")
+	priv = &mockKey{
+		pubKeyErr: errors.New("pubKeyErr"),
+		bytesErr:  nil,
+		pubKey:    &mockKey{},
+	}
+	_, err = csp.GetECPublicKey(priv)
+	assert.EqualError(t, err, "pubKeyErr", "Expected pubKeyErr")
+
 	cleanup(testDir)
 }