[FAB-1989] Fix leaking authority to delegates

The roles and delegates roles were not being checked correctly during
registration, thus allowing an identity to gain privileges they should
not have.  For example, a registrar's delegate roles were incorrectly
allowed to be a superset of the registrar's roles.  This allowed a
user1 to register user2 who could in turn create user3 with more privileges
than user1.  This change set fixes this problems and adds quite a few
test cases for various registration scenarios.

Change-Id: Ic2284d48a9efb2a0553bcacffe386d341cb2fa70
Signed-off-by: Keith Smith <bksmith@us.ibm.com>

lib/identity.go

@@ -53,6 +53,11 @@ func (i *Identity) GetName() string {
 	return i.name
 }
 
+// GetClient returns the client associated with this identity
+func (i *Identity) GetClient() *Client {
+	return i.client
+}
+
 // GetECert returns the enrollment certificate signer for this identity
 func (i *Identity) GetECert() *Signer {
 	return i.ecert
@@ -101,6 +106,25 @@ func (i *Identity) Register(req *api.RegistrationRequest) (rr *api.RegistrationR
 	return resp, nil
 }
 
+// RegisterAndEnroll registers and enrolls an identity and returns the identity
+func (i *Identity) RegisterAndEnroll(req *api.RegistrationRequest) (*Identity, error) {
+	if i.client == nil {
+		return nil, errors.New("No client is associated with this identity")
+	}
+	rresp, err := i.Register(req)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to register %s: %s", req.Name, err)
+	}
+	eresp, err := i.client.Enroll(&api.EnrollmentRequest{
+		Name:   req.Name,
+		Secret: rresp.Secret,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("Failed to enroll %s: %s", req.Name, err)
+	}
+	return eresp.Identity, nil
+}
+
 // Reenroll reenrolls an existing Identity and returns a new Identity
 // @param req The reenrollment request
 func (i *Identity) Reenroll(req *api.ReenrollmentRequest) (*EnrollmentResponse, error) {

lib/server.go

@@ -45,6 +45,15 @@ import (
 const (
 	defaultClientAuth         = "noclientcert"
 	fabricCAServerProfilePort = "FABRIC_CA_SERVER_PROFILE_PORT"
+	allRoles                  = "user,app,peer,orderer,client,validator,auditor"
+)
+
+// Attribute names
+const (
+	attrRoles          = "hf.Registrar.Roles"
+	attrDelegateRoles  = "hf.Registrar.DelegateRoles"
+	attrRevoker        = "hf.Revoker"
+	attrIntermediateCA = "hf.IntermediateCA"
 )
 
 // Server is the fabric-ca server
@@ -140,10 +149,10 @@ func (s *Server) RegisterBootstrapUser(user, pass, affiliation string) error {
 		Affiliation:    affiliation,
 		MaxEnrollments: s.CA.Config.Registry.MaxEnrollments,
 		Attrs: map[string]string{
-			"hf.Registrar.Roles":         "client,user,peer,validator,auditor",
-			"hf.Registrar.DelegateRoles": "client,user,validator,auditor",
-			"hf.Revoker":                 "true",
-			"hf.IntermediateCA":          "true",
+			attrRoles:          allRoles,
+			attrDelegateRoles:  allRoles,
+			attrRevoker:        "true",
+			attrIntermediateCA: "true",
 		},
 	}
 

lib/server_test.go

@@ -95,6 +95,8 @@ func TestRootServer(t *testing.T) {
 		t.Fatalf("Failed to enroll admin/adminpw: %s", err)
 	}
 	admin = eresp.Identity
+	// test registration permissions wrt roles and affiliation
+	testRegistration(admin, t)
 	// Register user1
 	rr, err = admin.Register(&api.RegistrationRequest{
 		Name:        "user1",
@@ -971,3 +973,76 @@ func getTLSConfig(srv *Server, clientAuthType string, clientRootCerts []string)
 
 	return srv
 }
+
+func testRegistration(admin *Identity, t *testing.T) {
+	name := "testRegistrationUser1"
+	topAffiliation := "hyperledger"
+	midAffiliation := "hyperledger.fabric"
+	botAffiliation := "hyperledger.fabric.security"
+	_, err := admin.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: midAffiliation,
+		Attributes:  makeAttrs(t, "hf.Registrar.Roles=user", "hf.Registrar.DelegateRoles=user,peer"),
+	})
+	if err == nil {
+		t.Error("Should have failed to register delegate roles which exceed roles")
+	}
+	id1, err := admin.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: midAffiliation,
+		Attributes:  makeAttrs(t, "hf.Registrar.Roles=user,peer", "hf.Registrar.DelegateRoles=user"),
+	})
+	if err != nil {
+		t.Fatalf("Failed to register %s: %s", name, err)
+	}
+	_, err = id1.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: botAffiliation,
+		Attributes:  makeAttrs(t, "hf.Registrar.Roles=peer"),
+	})
+	if err == nil {
+		t.Error("ID1 should not be allowed to delegate peer registration to another identity")
+	}
+	_, err = id1.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: topAffiliation,
+	})
+	if err == nil {
+		t.Error("ID1 should not be allowed to registrar outside of its affiliation hierarchy")
+	}
+	name = "testRegistrationUser2"
+	id2, err := id1.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: botAffiliation,
+	})
+	if err != nil {
+		t.Fatalf("ID1 failed to register %s: %s", name, err)
+	}
+	name = "testRegistrationUser3"
+	_, err = id2.RegisterAndEnroll(&api.RegistrationRequest{
+		Name:        name,
+		Type:        "user",
+		Affiliation: botAffiliation,
+	})
+	if err == nil {
+		t.Error("ID2 should not be allowed to register")
+	}
+}
+
+func makeAttrs(t *testing.T, args ...string) []api.Attribute {
+	attrs := make([]api.Attribute, len(args))
+	for idx, attr := range args {
+		eles := strings.Split(attr, "=")
+		if len(eles) != 2 {
+			t.Fatalf("Not two elements in %s", attr)
+		}
+		attrs[idx].Name = eles[0]
+		attrs[idx].Value = eles[1]
+	}
+	return attrs
+}

lib/serverregister.go

@@ -144,6 +144,14 @@ func (h *registerHandler) registerUserID(req *api.RegistrationRequestNet, caname
 		return "", fmt.Errorf("Unlimited enrollments not allowed, value must be equal to or less than %d", maxEnrollments)
 	}
 
+	// Make sure delegateRoles is not larger than roles
+	roles := GetAttrValue(req.Attributes, attrRoles)
+	delegateRoles := GetAttrValue(req.Attributes, attrDelegateRoles)
+	err := util.IsSubsetOf(delegateRoles, roles)
+	if err != nil {
+		return "", fmt.Errorf("delegateRoles is superset of roles: %s", err)
+	}
+
 	insert := spi.UserInfo{
 		Name:           req.Name,
 		Pass:           req.Secret,
@@ -155,7 +163,7 @@ func (h *registerHandler) registerUserID(req *api.RegistrationRequestNet, caname
 
 	registry := h.server.caMap[caname].registry
 
-	_, err := registry.GetUser(req.Name, nil)
+	_, err = registry.GetUser(req.Name, nil)
 	if err == nil {
 		return "", fmt.Errorf("Identity '%s' is already registered", req.Name)
 	}

lib/util.go

@@ -26,6 +26,7 @@ import (
 	"io/ioutil"
 
 	"github.com/cloudflare/cfssl/log"
+	"github.com/hyperledger/fabric-ca/api"
 	"github.com/hyperledger/fabric-ca/util"
 	"github.com/spf13/viper"
 )
@@ -130,3 +131,14 @@ func UnmarshalConfig(config interface{}, vp *viper.Viper, caFile string, server,
 	}
 	return nil
 }
+
+// GetAttrValue searches 'attrs' for the attribute with name 'name' and returns
+// its value, or "" if not found.
+func GetAttrValue(attrs []api.Attribute, name string) string {
+	for _, attr := range attrs {
+		if attr.Name == name {
+			return attr.Value
+		}
+	}
+	return ""
+}

util/util.go

@@ -377,6 +377,23 @@ func StrContained(str string, strs []string) bool {
 	return false
 }
 
+// IsSubsetOf returns an error if there is something in 'small' that
+// is not in 'big'.  Both small and big are assumed to be comma-separated
+// strings.  All string comparisons are case-insensitive.
+// Examples:
+// 1) IsSubsetOf('a,B', 'A,B,C') returns nil
+// 2) IsSubsetOf('A,B,C', 'B,C') returns an error because A is not in the 2nd set.
+func IsSubsetOf(small, big string) error {
+	bigSet := strings.Split(big, ",")
+	smallSet := strings.Split(small, ",")
+	for _, s := range smallSet {
+		if s != "" && !StrContained(s, bigSet) {
+			return fmt.Errorf("'%s' is not a member of '%s'", s, big)
+		}
+	}
+	return nil
+}
+
 // HTTPRequestToString returns a string for an HTTP request for debuggging
 func HTTPRequestToString(req *http.Request) string {
 	body, _ := ioutil.ReadAll(req.Body)

util/util_test.go

@@ -572,3 +572,23 @@ func getPEM(file string, t *testing.T) []byte {
 	assert.NoError(t, err)
 	return buf
 }
+
+func TestIsSubsetOf(t *testing.T) {
+	testIsSubsetOf(t, "a,b", "b,a,c", true)
+	testIsSubsetOf(t, "a,b", "b,a", true)
+	testIsSubsetOf(t, "a,b,c", "a,b", false)
+	testIsSubsetOf(t, "a,b,c", "", false)
+}
+
+func testIsSubsetOf(t *testing.T, small, large string, expectToPass bool) {
+	err := IsSubsetOf(small, large)
+	if expectToPass {
+		if err != nil {
+			t.Errorf("IsSubsetOf('%s','%s') failed: %s", small, large, err)
+		}
+	} else {
+		if err == nil {
+			t.Errorf("IsSubsetOf('%s','%s') expected error but passed", small, large)
+		}
+	}
+}