From 302f994410a2df627d1fdf5ba3eb923d0df8853b Mon Sep 17 00:00:00 2001 From: Mike Nomitch Date: Mon, 20 Nov 2023 05:11:08 -0800 Subject: [PATCH] [NET-6640] Adds "Policy" BindType to BindingRule (#19499) feat: add bind type of policy Co-authored-by: Ronald Ekambi --- .changelog/19499.txt | 3 + acl/validation.go | 4 + agent/consul/acl_endpoint_test.go | 47 +++++- agent/consul/auth/binder.go | 32 +++- agent/consul/auth/binder_test.go | 143 +++++++++++++++--- agent/consul/auth/login.go | 1 + agent/structs/acl.go | 16 ++ api/acl.go | 3 + .../bindingrule/create/bindingrule_create.go | 2 +- .../bindingrule/update/bindingrule_update.go | 2 +- .../update/bindingrule_update_test.go | 33 ++++ .../commands/acl/binding-rule/create.mdx | 2 +- 12 files changed, 262 insertions(+), 26 deletions(-) create mode 100644 .changelog/19499.txt diff --git a/.changelog/19499.txt b/.changelog/19499.txt new file mode 100644 index 0000000000..83849637d4 --- /dev/null +++ b/.changelog/19499.txt @@ -0,0 +1,3 @@ +```release-note:feature +acl: add policy bindtype to binding rules. +``` \ No newline at end of file diff --git a/acl/validation.go b/acl/validation.go index c0017effa1..652a76e879 100644 --- a/acl/validation.go +++ b/acl/validation.go @@ -68,6 +68,10 @@ func IsValidRoleName(name string) bool { return validRoleName.MatchString(name) } +func IsValidPolicyName(name string) bool { + return ValidatePolicyName(name) == nil +} + // IsValidRoleName returns true if the provided name can be used as an // ACLAuthMethod Name. func IsValidAuthMethodName(name string) bool { diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 39840942d1..7033e90881 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -3663,6 +3663,37 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { require.Equal(t, "test-node", rule.BindName) }) + t.Run("Bind Policy", func(t *testing.T) { + req := structs.ACLBindingRuleSetRequest{ + Datacenter: "dc1", + BindingRule: structs.ACLBindingRule{ + Description: "foobar policy", + AuthMethod: testAuthMethod.Name, + Selector: "serviceaccount.name==abc", + BindType: structs.BindingRuleBindTypePolicy, + BindName: "test-policy", + }, + WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken}, + } + var resp structs.ACLBindingRule + + err := aclEp.BindingRuleSet(&req, &resp) + require.NoError(t, err) + require.NotNil(t, resp.ID) + + // Get the rule directly to validate that it exists + ruleResp, err := retrieveTestBindingRule(codec, TestDefaultInitialManagementToken, "dc1", resp.ID) + require.NoError(t, err) + rule := ruleResp.BindingRule + + require.NotEmpty(t, rule.ID) + require.Equal(t, rule.Description, "foobar policy") + require.Equal(t, rule.AuthMethod, testAuthMethod.Name) + require.Equal(t, "serviceaccount.name==abc", rule.Selector) + require.Equal(t, structs.BindingRuleBindTypePolicy, rule.BindType) + require.Equal(t, "test-policy", rule.BindName) + }) + t.Run("templated policy", func(t *testing.T) { req := structs.ACLBindingRuleSetRequest{ Datacenter: "dc1", @@ -3841,7 +3872,7 @@ func TestACLEndpoint_BindingRuleSet(t *testing.T) { t.Run("Create fails; invalid bind type", func(t *testing.T) { reqRule := newRule() reqRule.BindType = "invalid" - requireSetErrors(t, reqRule, "Invalid Binding Rule: unknown BindType") + requireSetErrors(t, reqRule, "invalid Binding Rule: unknown BindType") }) t.Run("Create fails; bind name with unknown vars", func(t *testing.T) { @@ -4540,6 +4571,11 @@ func TestACLEndpoint_Login(t *testing.T) { "fake-node", "default", "mynode", "jkl101", ) + testauth.InstallSessionToken( + testSessionID, + "fake-policy", // 1 rule (policy) + "default", "mypolicy", "jkl012", + ) method, err := upsertTestAuthMethod(codec, TestDefaultInitialManagementToken, "dc1", testSessionID) require.NoError(t, err) @@ -4587,6 +4623,15 @@ func TestACLEndpoint_Login(t *testing.T) { ) require.NoError(t, err) + // policy rule + _, err = upsertTestBindingRule( + codec, TestDefaultInitialManagementToken, "dc1", method.Name, + "serviceaccount.namespace==default and serviceaccount.name==mypolicy", + structs.BindingRuleBindTypePolicy, + "method-${serviceaccount.name}", + ) + require.NoError(t, err) + t.Run("do not provide a token", func(t *testing.T) { req := structs.ACLLoginRequest{ Auth: &structs.ACLLoginParams{ diff --git a/agent/consul/auth/binder.go b/agent/consul/auth/binder.go index 1964cc5a87..ba66bc916b 100644 --- a/agent/consul/auth/binder.go +++ b/agent/consul/auth/binder.go @@ -36,14 +36,16 @@ func NewBinder(store BinderStateStore, datacenter string) *Binder { type BinderStateStore interface { ACLBindingRuleList(ws memdb.WatchSet, methodName string, entMeta *acl.EnterpriseMeta) (uint64, structs.ACLBindingRules, error) ACLRoleGetByName(ws memdb.WatchSet, roleName string, entMeta *acl.EnterpriseMeta) (uint64, *structs.ACLRole, error) + ACLPolicyGetByName(ws memdb.WatchSet, policyName string, entMeta *acl.EnterpriseMeta) (uint64, *structs.ACLPolicy, error) } -// Bindings contains the ACL roles, service identities, node identities, +// Bindings contains the ACL roles, service identities, node identities, policies, // templated policies, and enterprise meta to be assigned to the created token. type Bindings struct { Roles []structs.ACLTokenRoleLink ServiceIdentities []*structs.ACLServiceIdentity NodeIdentities []*structs.ACLNodeIdentity + Policies []structs.ACLTokenPolicyLink TemplatedPolicies structs.ACLTemplatedPolicies EnterpriseMeta acl.EnterpriseMeta } @@ -58,7 +60,8 @@ func (b *Bindings) None() bool { return len(b.ServiceIdentities) == 0 && len(b.NodeIdentities) == 0 && len(b.TemplatedPolicies) == 0 && - len(b.Roles) == 0 + len(b.Roles) == 0 && + len(b.Policies) == 0 } // Bind collects the ACL roles, service identities, etc. to be assigned to the @@ -119,6 +122,24 @@ func (b *Binder) Bind(authMethod *structs.ACLAuthMethod, verifiedIdentity *authm } bindings.TemplatedPolicies = append(bindings.TemplatedPolicies, templatedPolicy) + case structs.BindingRuleBindTypePolicy: + bindName, err := computeBindName(rule.BindName, verifiedIdentity.ProjectedVars, acl.IsValidRoleName) + if err != nil { + return nil, err + } + + _, policy, err := b.store.ACLPolicyGetByName(nil, bindName, &bindings.EnterpriseMeta) + if err != nil { + return nil, err + } + + if policy != nil { + bindings.Policies = append(bindings.Policies, structs.ACLTokenPolicyLink{ + ID: policy.ID, + Name: policy.Name, + }) + } + case structs.BindingRuleBindTypeRole: bindName, err := computeBindName(rule.BindName, verifiedIdentity.ProjectedVars, acl.IsValidRoleName) if err != nil { @@ -177,8 +198,13 @@ func IsValidBindingRule(bindType, bindName string, bindVars *structs.ACLTemplate if _, err := computeBindName(bindName, fakeVarMap, acl.IsValidRoleName); err != nil { return fmt.Errorf("failed to validate bindType %q: %w", bindType, err) } + + case structs.BindingRuleBindTypePolicy: + if _, err := computeBindName(bindName, fakeVarMap, acl.IsValidPolicyName); err != nil { + return fmt.Errorf("failed to validate bindType %q: %w", bindType, err) + } default: - return fmt.Errorf("Invalid Binding Rule: unknown BindType %q", bindType) + return fmt.Errorf("invalid Binding Rule: unknown BindType %q", bindType) } return nil diff --git a/agent/consul/auth/binder_test.go b/agent/consul/auth/binder_test.go index 41e0d14ddc..3220ff2b30 100644 --- a/agent/consul/auth/binder_test.go +++ b/agent/consul/auth/binder_test.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/consul/agent/consul/authmethod" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/api" ) func TestBindings_None(t *testing.T) { @@ -27,11 +28,79 @@ func TestBindings_None(t *testing.T) { b = &Bindings{Roles: []structs.ACLTokenRoleLink{{ID: generateID(t)}}} require.False(t, b.None()) + b = &Bindings{Policies: []structs.ACLTokenPolicyLink{{ID: generateID(t)}}} + require.False(t, b.None()) + b = &Bindings{ServiceIdentities: []*structs.ACLServiceIdentity{{ServiceName: "web"}}} require.False(t, b.None()) b = &Bindings{NodeIdentities: []*structs.ACLNodeIdentity{{NodeName: "node-123"}}} require.False(t, b.None()) + + b = &Bindings{TemplatedPolicies: []*structs.ACLTemplatedPolicy{{TemplateName: api.ACLTemplatedPolicyDNSName}}} + require.False(t, b.None()) +} + +func TestBinder_Policy_Success(t *testing.T) { + store := testStateStore(t) + binder := &Binder{store: store} + + authMethod := &structs.ACLAuthMethod{ + Name: "test-auth-method", + Type: "testing", + } + require.NoError(t, store.ACLAuthMethodSet(0, authMethod)) + + targetPolicy := &structs.ACLPolicy{ + ID: generateID(t), + Name: "foo-policy", + } + require.NoError(t, store.ACLPolicySet(0, targetPolicy)) + + otherPolicy := &structs.ACLPolicy{ + ID: generateID(t), + Name: "not-my-policy", + } + require.NoError(t, store.ACLPolicySet(0, otherPolicy)) + + bindingRules := structs.ACLBindingRules{ + { + ID: generateID(t), + Selector: "role==engineer", + BindType: structs.BindingRuleBindTypePolicy, + BindName: "${editor}-policy", + AuthMethod: authMethod.Name, + }, + { + ID: generateID(t), + Selector: "role==engineer", + BindType: structs.BindingRuleBindTypePolicy, + BindName: "this-policy-does-not-exist", + AuthMethod: authMethod.Name, + }, + { + ID: generateID(t), + Selector: "language==js", + BindType: structs.BindingRuleBindTypePolicy, + BindName: otherPolicy.Name, + AuthMethod: authMethod.Name, + }, + } + require.NoError(t, store.ACLBindingRuleBatchSet(0, bindingRules)) + + result, err := binder.Bind(&structs.ACLAuthMethod{}, &authmethod.Identity{ + SelectableFields: map[string]string{ + "role": "engineer", + "language": "go", + }, + ProjectedVars: map[string]string{ + "editor": "foo", + }, + }) + require.NoError(t, err) + require.Equal(t, []structs.ACLTokenPolicyLink{ + {ID: targetPolicy.ID, Name: targetPolicy.Name}, + }, result.Policies) } func TestBinder_Roles_Success(t *testing.T) { @@ -122,6 +191,32 @@ func TestBinder_Roles_NameValidation(t *testing.T) { require.Contains(t, err.Error(), "invalid bind name") } +func TestBinder_Policy_NameValidation(t *testing.T) { + store := testStateStore(t) + binder := &Binder{store: store} + + authMethod := &structs.ACLAuthMethod{ + Name: "test-auth-method", + Type: "testing", + } + require.NoError(t, store.ACLAuthMethodSet(0, authMethod)) + + bindingRules := structs.ACLBindingRules{ + { + ID: generateID(t), + Selector: "", + BindType: structs.BindingRuleBindTypePolicy, + BindName: "INVALID!", + AuthMethod: authMethod.Name, + }, + } + require.NoError(t, store.ACLBindingRuleBatchSet(0, bindingRules)) + + _, err := binder.Bind(&structs.ACLAuthMethod{}, &authmethod.Identity{}) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid bind name") +} + func TestBinder_ServiceIdentities_Success(t *testing.T) { store := testStateStore(t) binder := &Binder{store: store} @@ -275,54 +370,60 @@ func Test_IsValidBindingRule(t *testing.T) { "invalid", "blah", nil, "", true}, // valid HIL, invalid name {"empty", - "both", "", nil, "", true}, + "all", "", nil, "", true}, {"just end", - "both", "}", nil, "", true}, + "all", "}", nil, "", true}, {"var without start", - "both", " item }", nil, "item", true}, + "all", " item }", nil, "item", true}, {"two vars missing second start", - "both", "before-${ item }after--more }", nil, "item,more", true}, + "all", "before-${ item }after--more }", nil, "item,more", true}, // names for the two types are validated differently {"@ is disallowed", - "both", "bad@name", nil, "", true}, + "all", "bad@name", nil, "", true}, {"leading dash", "role", "-name", nil, "", false}, + {"leading dash", + "policy", "-name", nil, "", false}, {"leading dash", "service", "-name", nil, "", true}, {"trailing dash", "role", "name-", nil, "", false}, + {"trailing dash", + "policy", "name-", nil, "", false}, {"trailing dash", "service", "name-", nil, "", true}, {"inner dash", - "both", "name-end", nil, "", false}, + "all", "name-end", nil, "", false}, {"upper case", "role", "NAME", nil, "", false}, + {"upper case", + "policy", "NAME", nil, "", false}, {"upper case", "service", "NAME", nil, "", true}, // valid HIL, valid name {"no vars", - "both", "nothing", nil, "", false}, + "all", "nothing", nil, "", false}, {"just var", - "both", "${item}", nil, "item", false}, + "all", "${item}", nil, "item", false}, {"var in middle", - "both", "before-${item}after", nil, "item", false}, + "all", "before-${item}after", nil, "item", false}, {"two vars", - "both", "before-${item}after-${more}", nil, "item,more", false}, + "all", "before-${item}after-${more}", nil, "item,more", false}, // bad {"no bind name", - "both", "", nil, "", true}, + "all", "", nil, "", true}, {"just start", - "both", "${", nil, "", true}, + "all", "${", nil, "", true}, {"backwards", - "both", "}${", nil, "", true}, + "all", "}${", nil, "", true}, {"no varname", - "both", "${}", nil, "", true}, + "all", "${}", nil, "", true}, {"missing map key", - "both", "${item}", nil, "", true}, + "all", "${item}", nil, "", true}, {"var without end", - "both", "${ item ", nil, "item", true}, + "all", "${ item ", nil, "item", true}, {"two vars missing first end", - "both", "before-${ item after-${ more }", nil, "item,more", true}, + "all", "before-${ item after-${ more }", nil, "item,more", true}, // bind type: templated policy - bad input {"templated-policy missing bindvars", "templated-policy", "builtin/service", nil, "", true}, @@ -338,12 +439,16 @@ func Test_IsValidBindingRule(t *testing.T) { "templated-policy", "builtin/service", &structs.ACLTemplatedPolicyVariables{Name: "before-${item}after-${more}"}, "item,more", false}, } { var cases []testcase - if test.bindType == "both" { + if test.bindType == "all" { test1 := test test1.bindType = "role" test2 := test test2.bindType = "service" - cases = []testcase{test1, test2} + test3 := test + test3.bindType = "policy" + test4 := test + test4.bindType = "node" + cases = []testcase{test1, test2, test3, test4} } else { cases = []testcase{test} } diff --git a/agent/consul/auth/login.go b/agent/consul/auth/login.go index a084f33bf1..0b9b5eeac7 100644 --- a/agent/consul/auth/login.go +++ b/agent/consul/auth/login.go @@ -48,6 +48,7 @@ func (l *Login) TokenForVerifiedIdentity(identity *authmethod.Identity, authMeth NodeIdentities: bindings.NodeIdentities, TemplatedPolicies: bindings.TemplatedPolicies, Roles: bindings.Roles, + Policies: bindings.Policies, EnterpriseMeta: bindings.EnterpriseMeta, } token.ACLAuthMethodEnterpriseMeta.FillWithEnterpriseMeta(&authMethod.EnterpriseMeta) diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 4d76dffa4e..d856ce0af2 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -1084,6 +1084,21 @@ const ( // }, // } BindingRuleBindTypeTemplatedPolicy = "templated-policy" + + // BindingRuleBindTypePolicy is the binding rule bind type that only allows + // the binding rule to function if a policy with the given name (BindName) + // exists at login-time. If it does the token that is created is directly + // linked to that policy like: + // + // &ACLToken{ + // ...other fields... + // Policies: *ACLTokenPolicyLink{ + // { Name: "" }, + // } + // } + // + // If it does not exist at login-time the rule is ignored. + BindingRuleBindTypePolicy = "policy" ) type ACLBindingRule struct { @@ -1106,6 +1121,7 @@ type ACLBindingRule struct { // - BindingRuleBindTypeService = "service" // - BindingRuleBindTypeNode = "node" // - BindingRuleBindTypeRole = "role" + // - BindingRuleBindTypePolicy = "policy" // - BindingRuleBindTypeTemplatedPolicy = "templated-policy" BindType string diff --git a/api/acl.go b/api/acl.go index 47b38eb6ca..e40ae1cd49 100644 --- a/api/acl.go +++ b/api/acl.go @@ -253,6 +253,9 @@ const ( // BindingRuleBindTypeNode binds to a node identity with given name. BindingRuleBindTypeNode BindingRuleBindType = "node" + // BindingRuleBindTypePolicy binds to a specific policy with given name. + BindingRuleBindTypePolicy BindingRuleBindType = "policy" + // BindingRuleBindTypeTemplatedPolicy binds to a templated policy with given template name and variables. BindingRuleBindTypeTemplatedPolicy BindingRuleBindType = "templated-policy" ) diff --git a/command/acl/bindingrule/create/bindingrule_create.go b/command/acl/bindingrule/create/bindingrule_create.go index ab6a16f3ed..0b09a5eb2f 100644 --- a/command/acl/bindingrule/create/bindingrule_create.go +++ b/command/acl/bindingrule/create/bindingrule_create.go @@ -73,7 +73,7 @@ func (c *cmd) init() { &c.bindType, "bind-type", string(api.BindingRuleBindTypeService), - "Type of binding to perform (\"service\", \"role\", \"node\" or \"templated-policy\").", + "Type of binding to perform (\"service\", \"role\", \"node\", \"policy\", or \"templated-policy\").", ) c.flags.Var( (*flags.FlagMapValue)(&c.bindVars), diff --git a/command/acl/bindingrule/update/bindingrule_update.go b/command/acl/bindingrule/update/bindingrule_update.go index ce70067487..40c03cab54 100644 --- a/command/acl/bindingrule/update/bindingrule_update.go +++ b/command/acl/bindingrule/update/bindingrule_update.go @@ -77,7 +77,7 @@ func (c *cmd) init() { &c.bindType, "bind-type", string(api.BindingRuleBindTypeService), - "Type of binding to perform (\"service\" or \"role\").", + "Type of binding to perform (\"service\", \"policy\", or \"role\").", ) c.flags.StringVar( &c.bindName, diff --git a/command/acl/bindingrule/update/bindingrule_update_test.go b/command/acl/bindingrule/update/bindingrule_update_test.go index 1bbcd35cc2..c873aadcb4 100644 --- a/command/acl/bindingrule/update/bindingrule_update_test.go +++ b/command/acl/bindingrule/update/bindingrule_update_test.go @@ -242,6 +242,39 @@ func TestBindingRuleUpdateCommand(t *testing.T) { require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) }) + t.Run("update all fields with policy", func(t *testing.T) { + id := createRule(t, false) + + ui := cli.NewMockUi() + cmd := New(ui) + + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-token=root", + "-id", id, + "-description=test rule edited", + "-bind-type", "policy", + "-bind-name=policy-updated", + "-selector=serviceaccount.namespace==alt and serviceaccount.name==demo", + } + + code := cmd.Run(args) + require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) + require.Empty(t, ui.ErrorWriter.String()) + + rule, _, err := client.ACL().BindingRuleRead( + id, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + require.Equal(t, "test rule edited", rule.Description) + require.Equal(t, "policy-updated", rule.BindName) + require.Equal(t, api.BindingRuleBindTypePolicy, rule.BindType) + require.Equal(t, "serviceaccount.namespace==alt and serviceaccount.name==demo", rule.Selector) + }) + t.Run("update all fields with templated policy", func(t *testing.T) { id := createRule(t, false) diff --git a/website/content/commands/acl/binding-rule/create.mdx b/website/content/commands/acl/binding-rule/create.mdx index fd925e8d93..bcca8b3db0 100644 --- a/website/content/commands/acl/binding-rule/create.mdx +++ b/website/content/commands/acl/binding-rule/create.mdx @@ -89,4 +89,4 @@ Description: just vault role BindType: role BindName: vault Selector: serviceaccount.namespace==default and serviceaccount.name==vault -``` +``` \ No newline at end of file