[FAB-6932] Unmarshal key request object correctly

Currently, the key size and alogarithm specified in the
csr section of the client config file is not being used
in the creation of the CSR. This change set fixes that
problem.

Also, currently server key size and algorithm are always
set to 256 and ecdsa, respectively. With this change, 
csr.keyrequest config property will be used to set the 
key algorithm and size. If csr.keyrequest is not 
specified, key size and algo are set to bccsp.sw.security 
config property and 'ecdsa', respectively.

Change-Id: I12fe6c02cbeeeb03c8bedf0b2664ed5f3a5f922c
Signed-off-by: Anil Ambati <aambati@us.ibm.com>

api/client.go

@@ -252,12 +252,18 @@ type IdentityInfo struct {
 
 // CSRInfo is Certificate Signing Request (CSR) Information
 type CSRInfo struct {
-	CN           string               `json:"CN"`
-	Names        []csr.Name           `json:"names,omitempty"`
-	Hosts        []string             `json:"hosts,omitempty"`
-	KeyRequest   *csr.BasicKeyRequest `json:"key,omitempty"`
-	CA           *csr.CAConfig        `json:"ca,omitempty"`
-	SerialNumber string               `json:"serial_number,omitempty"`
+	CN           string           `json:"CN"`
+	Names        []csr.Name       `json:"names,omitempty"`
+	Hosts        []string         `json:"hosts,omitempty"`
+	KeyRequest   *BasicKeyRequest `json:"key,omitempty"`
+	CA           *csr.CAConfig    `json:"ca,omitempty"`
+	SerialNumber string           `json:"serial_number,omitempty"`
+}
+
+// BasicKeyRequest encapsulates size and algorithm for the key to be generated
+type BasicKeyRequest struct {
+	Algo string `json:"algo" yaml:"algo"`
+	Size int    `json:"size" yaml:"size"`
 }
 
 // Attribute is a name and value pair
@@ -293,3 +299,10 @@ func (ar *AttributeRequest) GetName() string {
 func (ar *AttributeRequest) IsRequired() bool {
 	return !ar.Optional
 }
+
+// NewBasicKeyRequest returns the BasicKeyRequest object that is constructed
+// from the object returned by the csr.NewBasicKeyRequest() function
+func NewBasicKeyRequest() *BasicKeyRequest {
+	bkr := csr.NewBasicKeyRequest()
+	return &BasicKeyRequest{Algo: bkr.A, Size: bkr.S}
+}

cmd/fabric-ca-client/main_test.go

@@ -241,21 +241,21 @@ func TestGenCRL(t *testing.T) {
 	t.Log("Testing GenCRL")
 	adminHome := filepath.Join(tdDir, "gencrladminhome")
 
-	// Remove server home directory if it exists
+	// Remove admin home directory if it exists
 	err := os.RemoveAll(adminHome)
 	if err != nil {
 		t.Fatalf("Failed to remove directory %s: %s", adminHome, err)
 	}
 
-	// Remove server home directory that this test is going to create before
+	// Remove admin home directory that this test is going to create before
 	// exiting the test case
 	defer os.RemoveAll(adminHome)
 
 	// Set up for the test case
 	srv := setupGenCRLTest(t, adminHome)
 
 	// Cleanup before exiting the test case
-	defer cleanupGenCRLTest(t, srv)
+	defer stopAndCleanupServer(t, srv)
 
 	// Error case 1: gencrl command should fail when called without enrollment info
 	tmpHome := filepath.Join(os.TempDir(), "gencrlhome")
@@ -754,6 +754,109 @@ func TestGencsr(t *testing.T) {
 	}
 }
 
+func TestDifferentKeySizeAlgos(t *testing.T) {
+	config := `csr:
+  cn: <<CN>>
+  names:
+    - C: US
+      ST: "North Carolina"
+      L:
+      O: Hyperledger
+      OU: Fabric
+  keyrequest:
+    algo: <<ALGO>>
+    size: <<SIZE>>
+  hosts:
+   - hostname
+`
+	writeConfig := func(cn, algo string, size int, dir string) error {
+		cfg := strings.Replace(config, "<<CN>>", cn, 1)
+		cfg = strings.Replace(cfg, "<<ALGO>>", algo, 1)
+		cfg = strings.Replace(cfg, "<<SIZE>>", strconv.Itoa(size), 1)
+		err := os.MkdirAll(dir, 0755)
+		if err != nil {
+			return err
+		}
+		fileName := filepath.Join(dir, "fabric-ca-client-config.yaml")
+		err = ioutil.WriteFile(fileName, []byte(cfg), os.ModePerm)
+		return err
+	}
+
+	testdata := []struct {
+		algo                  string
+		size                  int
+		errorExpected         bool
+		expectedSignatureAlgo x509.SignatureAlgorithm
+	}{
+		{"ecdsa", 256, false, x509.ECDSAWithSHA256},
+		{"ecdsa", 384, false, x509.ECDSAWithSHA384},
+		{"ecdsa", 521, true, x509.ECDSAWithSHA512},
+		{"rsa", 2048, false, x509.SHA256WithRSA},
+		{"rsa", 3072, false, x509.SHA384WithRSA},
+		{"rsa", 4096, false, x509.SHA512WithRSA},
+	}
+
+	homeDir := filepath.Join(tdDir, "genCSRDiffKeyReqs")
+	err := os.RemoveAll(homeDir)
+	if err != nil {
+		t.Fatalf("Failed to remove directory %s: %s", homeDir, err)
+	}
+
+	// Remove home directory that this test is going to create before
+	// exiting the test case
+	defer os.RemoveAll(homeDir)
+
+	for _, data := range testdata {
+		cn := "TestGenCSRWithDifferentKeyRequests" + data.algo + strconv.Itoa(data.size)
+		err := writeConfig(cn, data.algo, data.size, homeDir)
+		if err != nil {
+			t.Fatalf("Failed to write client config file in the %s directory: %s", homeDir, err)
+		}
+
+		err = RunMain([]string{cmdName, "gencsr", "-H", homeDir})
+		if !data.errorExpected {
+			assert.NoError(t, err, "GenCSR called with %s algorithm and %d key size should not have failed", data.algo, data.size)
+			csrFileName := cn + ".csr"
+			csrBytes, rerr := ioutil.ReadFile(filepath.Join(homeDir, "msp/signcerts", csrFileName))
+			assert.NoError(t, rerr, "Failed to read the generated CSR from the file %s:", csrFileName)
+
+			block, _ := pem.Decode(csrBytes)
+			if block == nil || block.Type != "CERTIFICATE REQUEST" {
+				t.Errorf("Block type read from the CSR file %s is not of type certificate request", csrFileName)
+			}
+			certReq, perr := x509.ParseCertificateRequest(block.Bytes)
+			assert.NoError(t, perr, "Failed to parse generated CSR")
+			assert.Equal(t, data.expectedSignatureAlgo, certReq.SignatureAlgorithm, "Not expected signature algorithm in the CSR")
+		} else {
+			if assert.Errorf(t, err, "GenCSR called with %s algorithm and %d key size should have failed", data.algo, data.size) {
+				assert.Contains(t, err.Error(), "Unsupported", "Not expected error message")
+			}
+		}
+	}
+
+	// Test enroll with ecdsa algorithm and 384 key size
+	srv := setupGenCSRTest(t, homeDir)
+	defer stopAndCleanupServer(t, srv)
+
+	// Enroll admin
+	err = RunMain([]string{cmdName, "enroll", "-H", homeDir, "-u", "http://admin:adminpw@localhost:7090"})
+	if err != nil {
+		t.Fatalf("Failed to enroll admin: %s", err)
+	}
+	certBytes, rerr1 := ioutil.ReadFile(filepath.Join(homeDir, "msp/signcerts/cert.pem"))
+	if rerr1 != nil {
+		t.Fatalf("Failed to read the enrollment certificate: %s", err)
+	}
+
+	block, _ := pem.Decode(certBytes)
+	if block == nil || block.Type != "CERTIFICATE" {
+		t.Errorf("Block type read from the cert file is not of type certificate")
+	}
+	cert, perr1 := x509.ParseCertificate(block.Bytes)
+	assert.NoError(t, perr1, "Failed to parse enrollment certificate")
+	assert.Equal(t, x509.ECDSAWithSHA384, cert.SignatureAlgorithm, "Not expected signature algorithm in the ecert")
+}
+
 // TestMOption tests to make sure that the key is stored in the correct
 // directory when the "-M" option is used.
 // This also ensures the intermediatecerts directory structure is populated
@@ -1790,9 +1893,9 @@ func setupGenCRLTest(t *testing.T, adminHome string) *lib.Server {
 	return srv
 }
 
-func cleanupGenCRLTest(t *testing.T, srv *lib.Server) {
-	defer os.RemoveAll(srv.HomeDir)
+func stopAndCleanupServer(t *testing.T, srv *lib.Server) {
 	if srv != nil {
+		defer os.RemoveAll(srv.HomeDir)
 		err := srv.Stop()
 		if err != nil {
 			t.Errorf("Server stop failed: %s", err)
@@ -1887,6 +1990,36 @@ func registerAndRevokeUsers(t *testing.T, admin *lib.Identity, num int) []*big.I
 	return serials
 }
 
+func setupGenCSRTest(t *testing.T, adminHome string) *lib.Server {
+	srvHome := filepath.Join(tdDir, "gencsrsrvhome")
+	err := os.RemoveAll(srvHome)
+	if err != nil {
+		t.Fatalf("Failed to remove home directory %s: %s", srvHome, err)
+	}
+
+	srv := lib.TestGetServer(serverPort, srvHome, "", -1, t)
+	srv.Config.Debug = true
+	srv.CA.Config.CSR.KeyRequest = &api.BasicKeyRequest{Algo: "ecdsa", Size: 384}
+
+	adminName := "admin"
+	adminPass := "adminpw"
+	err = srv.RegisterBootstrapUser(adminName, adminPass, "")
+	if err != nil {
+		t.Fatalf("Failed to register bootstrap user: %s", err)
+	}
+
+	err = srv.Start()
+	if err != nil {
+		t.Fatalf("Server start failed: %s", err)
+	}
+
+	err = RunMain([]string{cmdName, "enroll", "-u", enrollURL, "-H", adminHome})
+	if err != nil {
+		t.Fatalf("Failed to enroll admin: %s", err)
+	}
+	return srv
+}
+
 func extraArgErrorTest(in *TestData, t *testing.T) {
 	err := RunMain(in.input)
 	if err == nil {

lib/ca.go

@@ -341,12 +341,20 @@ func (ca *CA) getCACert() (cert []byte, err error) {
 		if csr.CA.Expiry == "" {
 			csr.CA.Expiry = defaultRootCACertificateExpiration
 		}
+		if csr.KeyRequest == nil {
+			if ca.Config.CSP.SwOpts != nil {
+				csr.KeyRequest = &api.BasicKeyRequest{Algo: "ecdsa", Size: ca.Config.CSP.SwOpts.SecLevel}
+			} else if ca.Config.CSP.Pkcs11Opts != nil {
+				csr.KeyRequest = &api.BasicKeyRequest{Algo: "ecdsa", Size: ca.Config.CSP.Pkcs11Opts.SecLevel}
+			} else {
+				csr.KeyRequest = api.NewBasicKeyRequest()
+			}
+		}
 		req := cfcsr.CertificateRequest{
-			CN:    csr.CN,
-			Names: csr.Names,
-			Hosts: csr.Hosts,
-			// FIXME: NewBasicKeyRequest only does ecdsa 256; use config
-			KeyRequest:   cfcsr.NewBasicKeyRequest(),
+			CN:           csr.CN,
+			Names:        csr.Names,
+			Hosts:        csr.Hosts,
+			KeyRequest:   &cfcsr.BasicKeyRequest{A: csr.KeyRequest.Algo, S: csr.KeyRequest.Size},
 			CA:           csr.CA,
 			SerialNumber: csr.SerialNumber,
 		}

lib/client.go

@@ -262,7 +262,7 @@ func (c *Client) GenCSR(req *api.CSRInfo, id string) ([]byte, bccsp.Key, error)
 	cr.CN = id
 
 	if cr.KeyRequest == nil {
-		cr.KeyRequest = csr.NewBasicKeyRequest()
+		cr.KeyRequest = newCfsslBasicKeyRequest(api.NewBasicKeyRequest())
 	}
 
 	key, cspSigner, err := util.BCCSPKeyRequestGenerate(cr, c.csp)
@@ -298,7 +298,7 @@ func (c *Client) newCertificateRequest(req *api.CSRInfo) *csr.CertificateRequest
 		}
 	}
 	if req != nil && req.KeyRequest != nil {
-		cr.KeyRequest = req.KeyRequest
+		cr.KeyRequest = newCfsslBasicKeyRequest(req.KeyRequest)
 	}
 	if req != nil {
 		cr.CA = req.CA
@@ -533,6 +533,10 @@ func (c *Client) CheckEnrollment() error {
 	return errors.New("Enrollment information does not exist. Please execute enroll command first. Example: fabric-ca-client enroll -u http://user:userpw@serverAddr:serverPort")
 }
 
+func newCfsslBasicKeyRequest(bkr *api.BasicKeyRequest) *csr.BasicKeyRequest {
+	return &csr.BasicKeyRequest{A: bkr.Algo, S: bkr.Size}
+}
+
 // NormalizeURL normalizes a URL (from cfssl)
 func NormalizeURL(addr string) (*url.URL, error) {
 	addr = strings.TrimSpace(addr)

lib/client_test.go

@@ -442,10 +442,10 @@ func testEnrollMiscFailures(c *Client, t *testing.T) {
 
 	c.Config.URL = ""
 	var r api.CSRInfo
-	var k csr.BasicKeyRequest
+	var k api.BasicKeyRequest
 	var n csr.Name
-	k.A = "dsa"
-	k.S = 256
+	k.Algo = "dsa"
+	k.Size = 256
 	n.C = "US"
 
 	r.KeyRequest = &k
@@ -1120,9 +1120,9 @@ func TestCLIGenCSR(t *testing.T) {
 	// Fail to gen key
 	config.CSR = api.CSRInfo{
 		CN: "TestGenCSR",
-		KeyRequest: &csr.BasicKeyRequest{
-			A: "dsa",
-			S: 256,
+		KeyRequest: &api.BasicKeyRequest{
+			Algo: "dsa",
+			Size: 256,
 		},
 	}
 	err = config.GenCSR(homeDir)

lib/client_whitebox_test.go

@@ -539,7 +539,7 @@ func TestCWBNewCertificateRequest(t *testing.T) {
 	req := &api.CSRInfo{
 		Names:      []csr.Name{},
 		Hosts:      []string{},
-		KeyRequest: csr.NewBasicKeyRequest(),
+		KeyRequest: api.NewBasicKeyRequest(),
 	}
 	if c.newCertificateRequest(req) == nil {
 		t.Error("newCertificateRequest failed")