Skip to content

Commit

Permalink
ACL Node Identities (#7970)
Browse files Browse the repository at this point in the history 
A Node Identity is very similar to a service identity. Its main targeted use is to allow creating tokens for use by Consul agents that will grant the necessary permissions for all the typical agent operations (node registration, coordinate updates, anti-entropy).

Half of this commit is for golden file based tests of the acl token and role cli output. Another big updates was to refactor many of the tests in agent/consul/acl_endpoint_test.go to use the same style of tests and the same helpers. Besides being less boiler plate in the tests it also uses a common way of starting a test server with ACLs that should operate without any warnings regarding deprecated non-uuid master tokens etc.
  • Loading branch information
@mkeeler
mkeeler authored Jun 16, 2020
1 parent ef37628 commit d3881dd
Show file tree
Hide file tree
Showing 62 changed files with 2,595 additions and 1,423 deletions.
28 changes: 28 additions & 0 deletions agent/acl_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,12 @@ func TestACL_HTTP(t *testing.T) {
Name: policyMap[idMap["policy-read-all-nodes"]].Name,
},
},
NodeIdentities: []*structs.ACLNodeIdentity{
&structs.ACLNodeIdentity{
NodeName: "web-node",
Datacenter: "foo",
},
},
}

req, _ := http.NewRequest("PUT", "/v1/acl/role?token=root", jsonBody(roleInput))
Expand All @@ -423,6 +429,7 @@ func TestACL_HTTP(t *testing.T) {
require.Equal(t, roleInput.Name, role.Name)
require.Equal(t, roleInput.Description, role.Description)
require.Equal(t, roleInput.Policies, role.Policies)
require.Equal(t, roleInput.NodeIdentities, role.NodeIdentities)
require.True(t, role.CreateIndex > 0)
require.Equal(t, role.CreateIndex, role.ModifyIndex)
require.NotNil(t, role.Hash)
Expand Down Expand Up @@ -502,6 +509,12 @@ func TestACL_HTTP(t *testing.T) {
ServiceName: "web-indexer",
},
},
NodeIdentities: []*structs.ACLNodeIdentity{
&structs.ACLNodeIdentity{
NodeName: "web-node",
Datacenter: "foo",
},
},
}

req, _ := http.NewRequest("PUT", "/v1/acl/role/"+idMap["role-test"]+"?token=root", jsonBody(roleInput))
Expand All @@ -518,6 +531,7 @@ func TestACL_HTTP(t *testing.T) {
require.Equal(t, roleInput.Description, role.Description)
require.Equal(t, roleInput.Policies, role.Policies)
require.Equal(t, roleInput.ServiceIdentities, role.ServiceIdentities)
require.Equal(t, roleInput.NodeIdentities, role.NodeIdentities)
require.True(t, role.CreateIndex > 0)
require.True(t, role.CreateIndex < role.ModifyIndex)
require.NotNil(t, role.Hash)
Expand Down Expand Up @@ -623,6 +637,12 @@ func TestACL_HTTP(t *testing.T) {
Name: policyMap[idMap["policy-read-all-nodes"]].Name,
},
},
NodeIdentities: []*structs.ACLNodeIdentity{
&structs.ACLNodeIdentity{
NodeName: "foo",
Datacenter: "bar",
},
},
}

req, _ := http.NewRequest("PUT", "/v1/acl/token?token=root", jsonBody(tokenInput))
Expand All @@ -638,6 +658,7 @@ func TestACL_HTTP(t *testing.T) {
require.Len(t, token.SecretID, 36)
require.Equal(t, tokenInput.Description, token.Description)
require.Equal(t, tokenInput.Policies, token.Policies)
require.Equal(t, tokenInput.NodeIdentities, token.NodeIdentities)
require.True(t, token.CreateIndex > 0)
require.Equal(t, token.CreateIndex, token.ModifyIndex)
require.NotNil(t, token.Hash)
Expand Down Expand Up @@ -741,6 +762,12 @@ func TestACL_HTTP(t *testing.T) {
Name: policyMap[idMap["policy-read-all-nodes"]].Name,
},
},
NodeIdentities: []*structs.ACLNodeIdentity{
&structs.ACLNodeIdentity{
NodeName: "foo",
Datacenter: "bar",
},
},
}

req, _ := http.NewRequest("PUT", "/v1/acl/token/"+originalToken.AccessorID+"?token=root", jsonBody(tokenInput))
Expand All @@ -754,6 +781,7 @@ func TestACL_HTTP(t *testing.T) {
require.Equal(t, originalToken.SecretID, token.SecretID)
require.Equal(t, tokenInput.Description, token.Description)
require.Equal(t, tokenInput.Policies, token.Policies)
require.Equal(t, tokenInput.NodeIdentities, token.NodeIdentities)
require.True(t, token.CreateIndex > 0)
require.True(t, token.CreateIndex < token.ModifyIndex)
require.NotNil(t, token.Hash)
Expand Down
55 changes: 54 additions & 1 deletion agent/consul/acl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ 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
}
Expand Down Expand Up @@ -648,8 +652,9 @@ 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 {
if len(policyIDs) == 0 && len(serviceIdentities) == 0 && len(roleIDs) == 0 && len(nodeIdentities) == 0 {
policy := identity.EmbeddedPolicy()
if policy != nil {
return []*structs.ACLPolicy{policy}, nil
Expand All @@ -671,14 +676,17 @@ 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
Expand All @@ -705,6 +713,19 @@ 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

Expand Down Expand Up @@ -739,6 +760,38 @@ 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...)
Expand Down
37 changes: 23 additions & 14 deletions agent/consul/acl_authmethod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ 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.
Expand All @@ -46,13 +52,13 @@ func (s *Server) evaluateRoleBindings(
verifiedIdentity *authmethod.Identity,
methodMeta *structs.EnterpriseMeta,
targetMeta *structs.EnterpriseMeta,
) ([]*structs.ACLServiceIdentity, []structs.ACLTokenRoleLink, error) {
) (*aclBindings, 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, nil, err
return nil, err
} else if len(rules) == 0 {
return nil, nil, nil
return nil, nil
}

// Find all binding rules that match the provided fields.
Expand All @@ -63,36 +69,39 @@ func (s *Server) evaluateRoleBindings(
}
}
if len(matchingRules) == 0 {
return nil, nil, nil
return nil, nil
}

// For all matching rules compute the attributes of a token.
var (
roleLinks []structs.ACLTokenRoleLink
serviceIdentities []*structs.ACLServiceIdentity
)
var bindings aclBindings
for _, rule := range matchingRules {
bindName, valid, err := computeBindingRuleBindName(rule.BindType, rule.BindName, verifiedIdentity.ProjectedVars)
if err != nil {
return nil, nil, fmt.Errorf("cannot compute %q bind name for bind target: %v", rule.BindType, err)
return nil, fmt.Errorf("cannot compute %q bind name for bind target: %v", rule.BindType, err)
} else if !valid {
return nil, nil, fmt.Errorf("computed %q bind name for bind target is invalid: %q", rule.BindType, bindName)
return nil, fmt.Errorf("computed %q bind name for bind target is invalid: %q", rule.BindType, bindName)
}

switch rule.BindType {
case structs.BindingRuleBindTypeService:
serviceIdentities = append(serviceIdentities, &structs.ACLServiceIdentity{
bindings.serviceIdentities = append(bindings.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, nil, err
return nil, err
}

if role != nil {
roleLinks = append(roleLinks, structs.ACLTokenRoleLink{
bindings.roles = append(bindings.roles, structs.ACLTokenRoleLink{
ID: role.ID,
})
}
Expand All @@ -102,7 +111,7 @@ func (s *Server) evaluateRoleBindings(
}
}

return serviceIdentities, roleLinks, nil
return &bindings, nil
}

// doesSelectorMatch checks that a single selector matches the provided vars.
Expand Down
53 changes: 48 additions & 5 deletions agent/consul/acl_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ 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}$`)
)
Expand Down Expand Up @@ -319,6 +321,7 @@ 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,
Expand Down Expand Up @@ -615,6 +618,19 @@ 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")
}
Expand Down Expand Up @@ -700,7 +716,8 @@ 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)

Expand All @@ -722,6 +739,17 @@ 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
Expand Down Expand Up @@ -1572,6 +1600,19 @@ 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)

Expand Down Expand Up @@ -1892,6 +1933,7 @@ 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)
Expand Down Expand Up @@ -2365,14 +2407,14 @@ func (a *ACL) tokenSetFromAuthMethod(
}

// 3. send map through role bindings
serviceIdentities, roleLinks, err := a.srv.evaluateRoleBindings(validator, verifiedIdentity, entMeta, targetMeta)
bindings, 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 len(serviceIdentities) == 0 && len(roleLinks) == 0 {
if bindings == nil || (len(bindings.serviceIdentities) == 0 && len(bindings.nodeIdentities) == 0 && len(bindings.roles) == 0) {
return acl.ErrPermissionDenied
}

Expand All @@ -2392,8 +2434,9 @@ func (a *ACL) tokenSetFromAuthMethod(
Description: description,
Local: true,
AuthMethod: method.Name,
ServiceIdentities: serviceIdentities,
Roles: roleLinks,
ServiceIdentities: bindings.serviceIdentities,
NodeIdentities: bindings.nodeIdentities,
Roles: bindings.roles,
ExpirationTTL: method.MaxTokenTTL,
EnterpriseMeta: *targetMeta,
}
Expand Down
Loading

0 comments on commit d3881dd

Please sign in to comment.