From e86ea5bd698b1573c87ef87718cf2d56d2776911 Mon Sep 17 00:00:00 2001 From: Gari Singh Date: Sun, 23 Apr 2017 08:21:53 -0400 Subject: [PATCH] [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 --- common/tools/cryptogen/ca/ca_test.go | 14 +++- common/tools/cryptogen/ca/generator.go | 100 ++++++++++-------------- common/tools/cryptogen/csp/csp.go | 37 ++++----- common/tools/cryptogen/csp/csp_test.go | 55 +++++++++++++ common/tools/cryptogen/msp/generator.go | 91 +++++++++------------ common/tools/cryptogen/msp/msp_test.go | 15 ++++ 6 files changed, 181 insertions(+), 131 deletions(-) diff --git a/common/tools/cryptogen/ca/ca_test.go b/common/tools/cryptogen/ca/ca_test.go index 1f3f437cb92..3cc0529aa07 100644 --- a/common/tools/cryptogen/ca/ca_test.go +++ b/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) } diff --git a/common/tools/cryptogen/ca/generator.go b/common/tools/cryptogen/ca/generator.go index 9dae526e949..5bdfcb96e48 100644 --- a/common/tools/cryptogen/ca/generator.go +++ b/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 } diff --git a/common/tools/cryptogen/csp/csp.go b/common/tools/cryptogen/csp/csp.go index 44e5a2c875f..658ce9ad9d7 100644 --- a/common/tools/cryptogen/csp/csp.go +++ b/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 } diff --git a/common/tools/cryptogen/csp/csp_test.go b/common/tools/cryptogen/csp/csp_test.go index 7f6cad6ca7b..53f0b0a83ef 100644 --- a/common/tools/cryptogen/csp/csp_test.go +++ b/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) } diff --git a/common/tools/cryptogen/msp/generator.go b/common/tools/cryptogen/msp/generator.go index e13e54af123..27d54bd63de 100644 --- a/common/tools/cryptogen/msp/generator.go +++ b/common/tools/cryptogen/msp/generator.go @@ -27,65 +27,53 @@ import ( func GenerateLocalMSP(baseDir, name string, rootCA *ca.CA) error { + var response error // create folder structure err := createFolderStructure(baseDir) - if err != nil { - return err - } - - // generate private key - priv, _, err := csp.GeneratePrivateKey(filepath.Join(baseDir, "keystore")) - if err != nil { - return err - } - - // get public signing certificate - ecPubKey, err := csp.GetECPublicKey(priv) - if err != nil { - return err - } - err = rootCA.SignCertificate(filepath.Join(baseDir, "signcerts"), - name, ecPubKey) - if err != nil { - return err - } - - // write root cert to folders - err = x509ToFile(filepath.Join(baseDir, "admincerts"), rootCA.Name, rootCA.SignCert) - if err != nil { - return err - } - err = x509ToFile(filepath.Join(baseDir, "cacerts"), rootCA.Name, rootCA.SignCert) - if err != nil { - return err + response = err + if err == nil { + // generate private key + priv, _, err := csp.GeneratePrivateKey(filepath.Join(baseDir, "keystore")) + response = err + if err == nil { + // get public signing certificate + ecPubKey, err := csp.GetECPublicKey(priv) + response = err + if err == nil { + err = rootCA.SignCertificate(filepath.Join(baseDir, "signcerts"), + name, ecPubKey) + response = err + if err == nil { + // write root cert to folders + folders := []string{"admincerts", "cacerts"} + for _, folder := range folders { + err = x509ToFile(filepath.Join(baseDir, folder), rootCA.Name, rootCA.SignCert) + if err != nil { + return err + } + } + } + } + } } - return nil - + return response } func GenerateVerifyingMSP(baseDir string, rootCA *ca.CA) error { // create folder structure err := createFolderStructure(baseDir) - if err != nil { - return err - } - - // write public cert to appropriate folders - err = x509ToFile(filepath.Join(baseDir, "admincerts"), rootCA.Name, rootCA.SignCert) - if err != nil { - return err - } - err = x509ToFile(filepath.Join(baseDir, "cacerts"), rootCA.Name, rootCA.SignCert) - if err != nil { - return err - } - err = x509ToFile(filepath.Join(baseDir, "signcerts"), rootCA.Name, rootCA.SignCert) - if err != nil { - return err + if err == nil { + // write public cert to appropriate folders + folders := []string{"admincerts", "cacerts", "signcerts"} + for _, folder := range folders { + err = x509ToFile(filepath.Join(baseDir, folder), rootCA.Name, rootCA.SignCert) + if err != nil { + return err + } + } } - - return nil + return err } func createFolderStructure(rootDir string) error { @@ -118,10 +106,7 @@ func x509ToFile(baseDir, name string, cert *x509.Certificate) error { //pem encode the cert err = pem.Encode(certFile, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) certFile.Close() - if err != nil { - return err - } - return nil + return err } diff --git a/common/tools/cryptogen/msp/msp_test.go b/common/tools/cryptogen/msp/msp_test.go index f41f646679d..594600eda3c 100644 --- a/common/tools/cryptogen/msp/msp_test.go +++ b/common/tools/cryptogen/msp/msp_test.go @@ -36,6 +36,10 @@ var testDir = filepath.Join(os.TempDir(), "msp-test") func TestGenerateLocalMSP(t *testing.T) { cleanup(testDir) + + err := msp.GenerateLocalMSP(testDir, testName, &ca.CA{}) + assert.Error(t, err, "Empty CA should have failed") + caDir := filepath.Join(testDir, "ca") mspDir := filepath.Join(testDir, "msp") rootCA, err := ca.NewCA(caDir, testCAName) @@ -63,6 +67,11 @@ func TestGenerateLocalMSP(t *testing.T) { assert.NoError(t, err, "Error creating new BCCSP MSP") err = testMSP.Setup(testMSPConfig) assert.NoError(t, err, "Error setting up local MSP") + + rootCA.Name = "test/fail" + err = msp.GenerateLocalMSP(testDir, testName, rootCA) + assert.Error(t, err, "Should have failed with CA name 'test/fail'") + t.Log(err) cleanup(testDir) } @@ -72,6 +81,7 @@ func TestGenerateVerifyingMSP(t *testing.T) { caDir := filepath.Join(testDir, "ca") mspDir := filepath.Join(testDir, "msp") rootCA, err := ca.NewCA(caDir, testCAName) + assert.NoError(t, err, "Failed to create new CA") err = msp.GenerateVerifyingMSP(mspDir, rootCA) assert.NoError(t, err, "Failed to generate verifying MSP") @@ -94,6 +104,11 @@ func TestGenerateVerifyingMSP(t *testing.T) { assert.NoError(t, err, "Error creating new BCCSP MSP") err = testMSP.Setup(testMSPConfig) assert.NoError(t, err, "Error setting up verifying MSP") + + rootCA.Name = "test/fail" + err = msp.GenerateVerifyingMSP(mspDir, rootCA) + assert.Error(t, err, "Should have failed with CA name 'test/fail'") + t.Log(err) cleanup(testDir) }