Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NET-6640] Adds "Policy" BindType to BindingRule #19499

Merged
merged 2 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/19499.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
acl: add policy bindtype to binding rules.
```
4 changes: 4 additions & 0 deletions acl/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
47 changes: 46 additions & 1 deletion agent/consul/acl_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand Down
32 changes: 29 additions & 3 deletions agent/consul/auth/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
143 changes: 124 additions & 19 deletions agent/consul/auth/binder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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},
Expand All @@ -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}
}
Expand Down
1 change: 1 addition & 0 deletions agent/consul/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading