Revert "ACL Node Identities (#7970)"

This reverts commit d3881dd.

agent/consul/acl.go

@@ -99,10 +99,6 @@ func (id *missingIdentity) ServiceIdentityList() []*structs.ACLServiceIdentity {
 	return nil
 }
 
-func (id *missingIdentity) NodeIdentityList() []*structs.ACLNodeIdentity {
-	return nil
-}
-
 func (id *missingIdentity) IsExpired(asOf time.Time) bool {
 	return false
 }
@@ -652,9 +648,8 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
 	policyIDs := identity.PolicyIDs()
 	roleIDs := identity.RoleIDs()
 	serviceIdentities := identity.ServiceIdentityList()
-	nodeIdentities := identity.NodeIdentityList()
 
-	if len(policyIDs) == 0 && len(serviceIdentities) == 0 && len(roleIDs) == 0 && len(nodeIdentities) == 0 {
+	if len(policyIDs) == 0 && len(serviceIdentities) == 0 && len(roleIDs) == 0 {
 		policy := identity.EmbeddedPolicy()
 		if policy != nil {
 			return []*structs.ACLPolicy{policy}, nil
@@ -676,17 +671,14 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
 			policyIDs = append(policyIDs, link.ID)
 		}
 		serviceIdentities = append(serviceIdentities, role.ServiceIdentities...)
-		nodeIdentities = append(nodeIdentities, role.NodeIdentityList()...)
 	}
 
 	// Now deduplicate any policies or service identities that occur more than once.
 	policyIDs = dedupeStringSlice(policyIDs)
 	serviceIdentities = dedupeServiceIdentities(serviceIdentities)
-	nodeIdentities = dedupeNodeIdentities(nodeIdentities)
 
 	// Generate synthetic policies for all service identities in effect.
 	syntheticPolicies := r.synthesizePoliciesForServiceIdentities(serviceIdentities, identity.EnterpriseMetadata())
-	syntheticPolicies = append(syntheticPolicies, r.synthesizePoliciesForNodeIdentities(nodeIdentities)...)
 
 	// For the new ACLs policy replication is mandatory for correct operation on servers. Therefore
 	// we only attempt to resolve policies locally
@@ -713,19 +705,6 @@ func (r *ACLResolver) synthesizePoliciesForServiceIdentities(serviceIdentities [
 	return syntheticPolicies
 }
 
-func (r *ACLResolver) synthesizePoliciesForNodeIdentities(nodeIdentities []*structs.ACLNodeIdentity) []*structs.ACLPolicy {
-	if len(nodeIdentities) == 0 {
-		return nil
-	}
-
-	syntheticPolicies := make([]*structs.ACLPolicy, 0, len(nodeIdentities))
-	for _, n := range nodeIdentities {
-		syntheticPolicies = append(syntheticPolicies, n.SyntheticPolicy())
-	}
-
-	return syntheticPolicies
-}
-
 func dedupeServiceIdentities(in []*structs.ACLServiceIdentity) []*structs.ACLServiceIdentity {
 	// From: https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable
 
@@ -760,38 +739,6 @@ func dedupeServiceIdentities(in []*structs.ACLServiceIdentity) []*structs.ACLSer
 	return in[:j+1]
 }
 
-func dedupeNodeIdentities(in []*structs.ACLNodeIdentity) []*structs.ACLNodeIdentity {
-	// From: https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable
-
-	if len(in) <= 1 {
-		return in
-	}
-
-	sort.Slice(in, func(i, j int) bool {
-		if in[i].NodeName < in[j].NodeName {
-			return true
-		}
-
-		return in[i].Datacenter < in[j].Datacenter
-	})
-
-	j := 0
-	for i := 1; i < len(in); i++ {
-		if in[j].NodeName == in[i].NodeName && in[j].Datacenter == in[i].Datacenter {
-			continue
-		}
-		j++
-		in[j] = in[i]
-	}
-
-	// Discard the skipped items.
-	for i := j + 1; i < len(in); i++ {
-		in[i] = nil
-	}
-
-	return in[:j+1]
-}
-
 func mergeStringSlice(a, b []string) []string {
 	out := make([]string, 0, len(a)+len(b))
 	out = append(out, a...)

agent/consul/acl_authmethod.go

@@ -36,12 +36,6 @@ func (s *Server) loadAuthMethodValidator(idx uint64, method *structs.ACLAuthMeth
 	return v, nil
 }
 
-type aclBindings struct {
-	roles             []structs.ACLTokenRoleLink
-	serviceIdentities []*structs.ACLServiceIdentity
-	nodeIdentities    []*structs.ACLNodeIdentity
-}
-
 // evaluateRoleBindings evaluates all current binding rules associated with the
 // given auth method against the verified data returned from the authentication
 // process.
@@ -52,13 +46,13 @@ func (s *Server) evaluateRoleBindings(
 	verifiedIdentity *authmethod.Identity,
 	methodMeta *structs.EnterpriseMeta,
 	targetMeta *structs.EnterpriseMeta,
-) (*aclBindings, error) {
+) ([]*structs.ACLServiceIdentity, []structs.ACLTokenRoleLink, error) {
 	// Only fetch rules that are relevant for this method.
 	_, rules, err := s.fsm.State().ACLBindingRuleList(nil, validator.Name(), methodMeta)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	} else if len(rules) == 0 {
-		return nil, nil
+		return nil, nil, nil
 	}
 
 	// Find all binding rules that match the provided fields.
@@ -69,39 +63,36 @@ func (s *Server) evaluateRoleBindings(
 		}
 	}
 	if len(matchingRules) == 0 {
-		return nil, nil
+		return nil, nil, nil
 	}
 
 	// For all matching rules compute the attributes of a token.
-	var bindings aclBindings
+	var (
+		roleLinks         []structs.ACLTokenRoleLink
+		serviceIdentities []*structs.ACLServiceIdentity
+	)
 	for _, rule := range matchingRules {
 		bindName, valid, err := computeBindingRuleBindName(rule.BindType, rule.BindName, verifiedIdentity.ProjectedVars)
 		if err != nil {
-			return nil, fmt.Errorf("cannot compute %q bind name for bind target: %v", rule.BindType, err)
+			return nil, nil, fmt.Errorf("cannot compute %q bind name for bind target: %v", rule.BindType, err)
 		} else if !valid {
-			return nil, fmt.Errorf("computed %q bind name for bind target is invalid: %q", rule.BindType, bindName)
+			return nil, nil, fmt.Errorf("computed %q bind name for bind target is invalid: %q", rule.BindType, bindName)
 		}
 
 		switch rule.BindType {
 		case structs.BindingRuleBindTypeService:
-			bindings.serviceIdentities = append(bindings.serviceIdentities, &structs.ACLServiceIdentity{
+			serviceIdentities = append(serviceIdentities, &structs.ACLServiceIdentity{
 				ServiceName: bindName,
 			})
 
-		case structs.BindingRuleBindTypeNode:
-			bindings.nodeIdentities = append(bindings.nodeIdentities, &structs.ACLNodeIdentity{
-				NodeName:   bindName,
-				Datacenter: s.config.Datacenter,
-			})
-
 		case structs.BindingRuleBindTypeRole:
 			_, role, err := s.fsm.State().ACLRoleGetByName(nil, bindName, targetMeta)
 			if err != nil {
-				return nil, err
+				return nil, nil, err
 			}
 
 			if role != nil {
-				bindings.roles = append(bindings.roles, structs.ACLTokenRoleLink{
+				roleLinks = append(roleLinks, structs.ACLTokenRoleLink{
 					ID: role.ID,
 				})
 			}
@@ -111,7 +102,7 @@ func (s *Server) evaluateRoleBindings(
 		}
 	}
 
-	return &bindings, nil
+	return serviceIdentities, roleLinks, nil
 }
 
 // doesSelectorMatch checks that a single selector matches the provided vars.

agent/consul/acl_endpoint.go

@@ -35,8 +35,6 @@ var (
 	validPolicyName              = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,128}$`)
 	validServiceIdentityName     = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-_]*[a-z0-9])?$`)
 	serviceIdentityNameMaxLength = 256
-	validNodeIdentityName        = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-_]*[a-z0-9])?$`)
-	nodeIdentityNameMaxLength    = 256
 	validRoleName                = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,256}$`)
 	validAuthMethod              = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,128}$`)
 )
@@ -322,7 +320,6 @@ func (a *ACL) TokenClone(args *structs.ACLTokenSetRequest, reply *structs.ACLTok
 			Policies:          token.Policies,
 			Roles:             token.Roles,
 			ServiceIdentities: token.ServiceIdentities,
-			NodeIdentities:    token.NodeIdentities,
 			Local:             token.Local,
 			Description:       token.Description,
 			ExpirationTime:    token.ExpirationTime,
@@ -619,19 +616,6 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
 	}
 	token.ServiceIdentities = dedupeServiceIdentities(token.ServiceIdentities)
 
-	for _, nodeid := range token.NodeIdentities {
-		if nodeid.NodeName == "" {
-			return fmt.Errorf("Node identity is missing the node name field on this token")
-		}
-		if nodeid.Datacenter == "" {
-			return fmt.Errorf("Node identity is missing the datacenter field on this token")
-		}
-		if !isValidNodeIdentityName(nodeid.NodeName) {
-			return fmt.Errorf("Node identity has an invalid name. Only alphanumeric characters, '-' and '_' are allowed")
-		}
-	}
-	token.NodeIdentities = dedupeNodeIdentities(token.NodeIdentities)
-
 	if token.Rules != "" {
 		return fmt.Errorf("Rules cannot be specified for this token")
 	}
@@ -717,8 +701,7 @@ func computeBindingRuleBindName(bindType, bindName string, projectedVars map[str
 	switch bindType {
 	case structs.BindingRuleBindTypeService:
 		valid = isValidServiceIdentityName(bindName)
-	case structs.BindingRuleBindTypeNode:
-		valid = isValidNodeIdentityName(bindName)
+
 	case structs.BindingRuleBindTypeRole:
 		valid = validRoleName.MatchString(bindName)
 
@@ -740,17 +723,6 @@ func isValidServiceIdentityName(name string) bool {
 	return validServiceIdentityName.MatchString(name)
 }
 
-// isValidNodeIdentityName returns true if the provided name can be used as
-// an ACLNodeIdentity NodeName. This is more restrictive than standard
-// catalog registration, which basically takes the view that "everything is
-// valid".
-func isValidNodeIdentityName(name string) bool {
-	if len(name) < 1 || len(name) > nodeIdentityNameMaxLength {
-		return false
-	}
-	return validNodeIdentityName.MatchString(name)
-}
-
 func (a *ACL) TokenDelete(args *structs.ACLTokenDeleteRequest, reply *string) error {
 	if err := a.aclPreCheck(); err != nil {
 		return err
@@ -1601,19 +1573,6 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e
 	}
 	role.ServiceIdentities = dedupeServiceIdentities(role.ServiceIdentities)
 
-	for _, nodeid := range role.NodeIdentities {
-		if nodeid.NodeName == "" {
-			return fmt.Errorf("Node identity is missing the node name field on this role")
-		}
-		if nodeid.Datacenter == "" {
-			return fmt.Errorf("Node identity is missing the datacenter field on this role")
-		}
-		if !isValidNodeIdentityName(nodeid.NodeName) {
-			return fmt.Errorf("Node identity has an invalid name. Only alphanumeric characters, '-' and '_' are allowed")
-		}
-	}
-	role.NodeIdentities = dedupeNodeIdentities(role.NodeIdentities)
-
 	// calculate the hash for this role
 	role.SetHash(true)
 
@@ -1934,7 +1893,6 @@ func (a *ACL) BindingRuleSet(args *structs.ACLBindingRuleSetRequest, reply *stru
 
 	switch rule.BindType {
 	case structs.BindingRuleBindTypeService:
-	case structs.BindingRuleBindTypeNode:
 	case structs.BindingRuleBindTypeRole:
 	default:
 		return fmt.Errorf("Invalid Binding Rule: unknown BindType %q", rule.BindType)
@@ -2408,14 +2366,14 @@ func (a *ACL) tokenSetFromAuthMethod(
 	}
 
 	// 3. send map through role bindings
-	bindings, err := a.srv.evaluateRoleBindings(validator, verifiedIdentity, entMeta, targetMeta)
+	serviceIdentities, roleLinks, err := a.srv.evaluateRoleBindings(validator, verifiedIdentity, entMeta, targetMeta)
 	if err != nil {
 		return err
 	}
 
 	// We try to prevent the creation of a useless token without taking a trip
 	// through the state store if we can.
-	if bindings == nil || (len(bindings.serviceIdentities) == 0 && len(bindings.nodeIdentities) == 0 && len(bindings.roles) == 0) {
+	if len(serviceIdentities) == 0 && len(roleLinks) == 0 {
 		return acl.ErrPermissionDenied
 	}
 
@@ -2435,9 +2393,8 @@ func (a *ACL) tokenSetFromAuthMethod(
 		Description:       description,
 		Local:             true,
 		AuthMethod:        method.Name,
-		ServiceIdentities: bindings.serviceIdentities,
-		NodeIdentities:    bindings.nodeIdentities,
-		Roles:             bindings.roles,
+		ServiceIdentities: serviceIdentities,
+		Roles:             roleLinks,
 		ExpirationTTL:     method.MaxTokenTTL,
 		EnterpriseMeta:    *targetMeta,
 	}

agent/consul/intention_endpoint_test.go

@@ -392,8 +392,13 @@ service "foo" {
 func TestIntention_WildcardACLEnforcement(t *testing.T) {
 	t.Parallel()
 
-	_, srv, codec := testACLServerWithConfig(t, nil, false)
-	waitForLeaderEstablishment(t, srv)
+	dir, srv := testACLServerWithConfig(t, nil, false)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	codec := rpcClient(t, srv)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, srv.RPC, "dc1")
 
 	// create some test policies.
 
@@ -1217,8 +1222,13 @@ func TestIntentionMatch_good(t *testing.T) {
 func TestIntentionMatch_acl(t *testing.T) {
 	t.Parallel()
 
-	_, srv, codec := testACLServerWithConfig(t, nil, false)
-	waitForLeaderEstablishment(t, srv)
+	dir1, s1 := testACLServerWithConfig(t, nil, false)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 
 	token, err := upsertTestTokenWithPolicyRules(codec, TestDefaultMasterToken, "dc1", `service "bar" { policy = "write" }`)
 	require.NoError(t, err)
@@ -1454,8 +1464,13 @@ service "bar" {
 func TestIntentionCheck_match(t *testing.T) {
 	t.Parallel()
 
-	_, srv, codec := testACLServerWithConfig(t, nil, false)
-	waitForLeaderEstablishment(t, srv)
+	dir1, s1 := testACLServerWithConfig(t, nil, false)
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 
 	token, err := upsertTestTokenWithPolicyRules(codec, TestDefaultMasterToken, "dc1", `service "api" { policy = "read" }`)
 	require.NoError(t, err)

agent/consul/leader.go

@@ -731,7 +731,6 @@ func (s *Server) legacyACLTokenUpgrade(ctx context.Context) error {
 			// Assign the global-management policy to legacy management tokens
 			if len(newToken.Policies) == 0 &&
 				len(newToken.ServiceIdentities) == 0 &&
-				len(newToken.NodeIdentities) == 0 &&
 				len(newToken.Roles) == 0 &&
 				newToken.Type == structs.ACLTokenTypeManagement {
 				newToken.Policies = append(newToken.Policies, structs.ACLTokenPolicyLink{ID: structs.ACLPolicyGlobalManagementID})

agent/consul/leader_test.go

@@ -1314,7 +1314,9 @@ func TestLeader_ACLLegacyReplication(t *testing.T) {
 		c.Datacenter = "dc2"
 		c.ACLTokenReplication = true
 	}
-	_, srv, _ := testACLServerWithConfig(t, cb, true)
+	dir, srv := testACLServerWithConfig(t, cb, true)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
 	waitForLeaderEstablishment(t, srv)
 
 	require.True(t, srv.leaderRoutineManager.IsRunning(legacyACLReplicationRoutineName))