[FAB-4567] Fix for id attributes security issue

This change set makes a server-side check to require that the
CN (Common Name) in the CSR (Certificate Signing Request) on
an enroll request be the same as the enrollment ID.
The check was previously only made on the client-side, leaving
a security hole which could be exploited by a roque client.

The testMasqueradeEnroll test fails prior to this change and
passes with this change.

Change-Id: Id7c87e6958f5df19ca6308dd9ef8009a5f4a1d74
Signed-off-by: Keith Smith <[email protected]>

lib/client_whitebox_test.go

@@ -24,6 +24,7 @@ import (
 	"path"
 	"testing"
 
+	"github.com/cloudflare/cfssl/log"
 	"github.com/cloudflare/cfssl/signer"
 	"github.com/hyperledger/fabric-ca/api"
 	"github.com/hyperledger/fabric-ca/util"
@@ -91,6 +92,7 @@ func TestTLSClientAuth(t *testing.T) {
 	}
 	id := eresp.Identity
 	testImpersonation(id, t)
+	testMasqueradeEnroll(t, client, id)
 	// Stop server
 	err = server.Stop()
 	if err != nil {
@@ -245,6 +247,37 @@ func testImpersonation(id *Identity, t *testing.T) {
 	}
 }
 
+func testMasqueradeEnroll(t *testing.T, c *Client, id *Identity) {
+	// Register masqueradeUser
+	log.Debug("Entering testMasqueradeEnroll")
+	name := "masqueradeUser"
+	rr, err := id.Register(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: "hyperledger.fabric.security",
+	})
+	if err != nil {
+		t.Fatalf("Failed to register maqueradeUser: %s", err)
+	}
+	// Try to enroll user1 but masquerading as 'admin'
+	_, err = masqueradeEnroll(c, "admin", false, &api.EnrollmentRequest{
+		Name:   name,
+		Secret: rr.Secret,
+	})
+	if err == nil {
+		t.Fatalf("%s masquerading as admin (false) should have failed", name)
+	}
+	log.Debugf("testMasqueradeEnroll (false) error: %s", err)
+	_, err = masqueradeEnroll(c, "admin", true, &api.EnrollmentRequest{
+		Name:   name,
+		Secret: rr.Secret,
+	})
+	if err == nil {
+		t.Fatalf("%s masquerading as admin (true) should have failed", name)
+	}
+	log.Debugf("testMasqueradeEnroll (true) error: %s", err)
+}
+
 func getEnrollmentPayload(t *testing.T, c *Client) ([]byte, error) {
 	req := &api.EnrollmentRequest{
 		Name:   user,
@@ -320,3 +353,46 @@ func getTestClient(port int) *Client {
 		HomeDir: testdataDir,
 	}
 }
+
+// Enroll enrolls a new identity
+// @param req The enrollment request
+func masqueradeEnroll(c *Client, id string, passInSubject bool, req *api.EnrollmentRequest) (*EnrollmentResponse, error) {
+	err := c.Init()
+	if err != nil {
+		return nil, err
+	}
+	csrPEM, key, err := c.GenCSR(req.CSR, id)
+	if err != nil {
+		log.Debugf("Enroll failure generating CSR: %s", err)
+		return nil, err
+	}
+	reqNet := &api.EnrollmentRequestNet{
+		CAName: req.CAName,
+	}
+	if req.CSR != nil {
+		reqNet.SignRequest.Hosts = req.CSR.Hosts
+	}
+	reqNet.SignRequest.Request = string(csrPEM)
+	reqNet.SignRequest.Profile = req.Profile
+	reqNet.SignRequest.Label = req.Label
+	if passInSubject {
+		reqNet.SignRequest.Subject = &signer.Subject{CN: id}
+	}
+	body, err := util.Marshal(reqNet, "SignRequest")
+	if err != nil {
+		return nil, err
+	}
+	// Send the CSR to the fabric-ca server with basic auth header
+	post, err := c.newPost("enroll", body)
+	if err != nil {
+		return nil, err
+	}
+	post.SetBasicAuth(req.Name, req.Secret)
+	var result enrollmentResponseNet
+	err = c.SendReq(post, &result)
+	if err != nil {
+		return nil, err
+	}
+	// Create the enrollment response
+	return c.newEnrollmentResponse(&result, req.Name, key)
+}

lib/serverenroll.go

@@ -110,7 +110,8 @@ func (sh *signHandler) handle(w http.ResponseWriter, r *http.Request) error {
 
 	// Make any authorization checks needed, depending on the contents
 	// of the CSR (Certificate Signing Request)
-	err = sh.csrAuthCheck(&req.SignRequest, r)
+	enrollmentID := r.Header.Get(enrollmentIDHdrName)
+	err = sh.csrAuthCheck(&req.SignRequest, enrollmentID, r)
 	if err != nil {
 		return err
 	}
@@ -135,7 +136,7 @@ func (sh *signHandler) handle(w http.ResponseWriter, r *http.Request) error {
 // of the CSR (Certificate Signing Request).
 // In particular, if the request is for an intermediate CA certificate,
 // the caller must have the "hf.IntermediateCA" attribute.
-func (sh *signHandler) csrAuthCheck(req *signer.SignRequest, r *http.Request) error {
+func (sh *signHandler) csrAuthCheck(req *signer.SignRequest, enrollmentID string, r *http.Request) error {
 	// Decode and parse the request into a CSR so we can make checks
 	caname := r.Header.Get(caHdrName)
 	block, _ := pem.Decode([]byte(req.Request))
@@ -150,6 +151,10 @@ func (sh *signHandler) csrAuthCheck(req *signer.SignRequest, r *http.Request) er
 	if err != nil {
 		return err
 	}
+	log.Debugf("csrAuthCheck: enrollment ID=%s, CommonName=%s, Subject=%+v", enrollmentID, csrReq.Subject.CommonName, req.Subject)
+	if (req.Subject != nil && req.Subject.CN != enrollmentID) || csrReq.Subject.CommonName != enrollmentID {
+		return fmt.Errorf("The CSR subject common name must equal the enrollment ID")
+	}
 	// Check the CSR for the X.509 BasicConstraints extension (RFC 5280, 4.2.1.9)
 	for _, val := range csrReq.Extensions {
 		if val.Id.Equal(basicConstraintsOID) {