From f99df57840dade09f8e7020d2f680adbc58e9f35 Mon Sep 17 00:00:00 2001 From: Freddy Date: Fri, 22 Jul 2022 14:42:23 -0600 Subject: [PATCH] [OSS] Add new peering ACL rule (#13848) This commit adds a new ACL rule named "peering" to authorize actions taken against peering-related endpoints. The "peering" rule has several key properties: - It is scoped to a partition, and MUST be defined in the default namespace. - Its access level must be "read', "write", or "deny". - Granting an access level will apply to all peerings. This ACL rule cannot be used to selective grant access to some peerings but not others. - If the peering rule is not specified, we fall back to the "operator" rule and then the default ACL rule. --- acl/acl_test.go | 344 +++++++++++++++++++++++++++++++++ acl/authorizer.go | 14 +- acl/authorizer_test.go | 26 ++- acl/chained_authorizer.go | 16 ++ acl/chained_authorizer_test.go | 16 ++ acl/policy.go | 7 + acl/policy_authorizer.go | 31 +++ acl/policy_authorizer_test.go | 2 + acl/policy_merger.go | 15 +- acl/policy_test.go | 122 ++++++++---- acl/static_authorizer.go | 14 ++ agent/acl_endpoint_test.go | 18 ++ agent/structs/acl.go | 1 + 13 files changed, 574 insertions(+), 52 deletions(-) diff --git a/acl/acl_test.go b/acl/acl_test.go index 3ce0fa59b9..fae37e5a64 100644 --- a/acl/acl_test.go +++ b/acl/acl_test.go @@ -27,6 +27,7 @@ func legacyPolicy(policy *Policy) *Policy { Keyring: policy.Keyring, Operator: policy.Operator, Mesh: policy.Mesh, + Peering: policy.Peering, }, } } @@ -117,6 +118,14 @@ func checkAllowMeshWrite(t *testing.T, authz Authorizer, prefix string, entCtx * require.Equal(t, Allow, authz.MeshWrite(entCtx)) } +func checkAllowPeeringRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Allow, authz.PeeringRead(entCtx)) +} + +func checkAllowPeeringWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Allow, authz.PeeringWrite(entCtx)) +} + func checkAllowOperatorRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { require.Equal(t, Allow, authz.OperatorRead(entCtx)) } @@ -241,6 +250,14 @@ func checkDenyMeshWrite(t *testing.T, authz Authorizer, prefix string, entCtx *A require.Equal(t, Deny, authz.MeshWrite(entCtx)) } +func checkDenyPeeringRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Deny, authz.PeeringRead(entCtx)) +} + +func checkDenyPeeringWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Deny, authz.PeeringWrite(entCtx)) +} + func checkDenyOperatorRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { require.Equal(t, Deny, authz.OperatorRead(entCtx)) } @@ -365,6 +382,14 @@ func checkDefaultMeshWrite(t *testing.T, authz Authorizer, prefix string, entCtx require.Equal(t, Default, authz.MeshWrite(entCtx)) } +func checkDefaultPeeringRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Default, authz.PeeringRead(entCtx)) +} + +func checkDefaultPeeringWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { + require.Equal(t, Default, authz.PeeringWrite(entCtx)) +} + func checkDefaultOperatorRead(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) { require.Equal(t, Default, authz.OperatorRead(entCtx)) } @@ -446,6 +471,8 @@ func TestACL(t *testing.T) { {name: "DenyNodeWrite", check: checkDenyNodeWrite}, {name: "DenyMeshRead", check: checkDenyMeshRead}, {name: "DenyMeshWrite", check: checkDenyMeshWrite}, + {name: "DenyPeeringRead", check: checkDenyPeeringRead}, + {name: "DenyPeeringWrite", check: checkDenyPeeringWrite}, {name: "DenyOperatorRead", check: checkDenyOperatorRead}, {name: "DenyOperatorWrite", check: checkDenyOperatorWrite}, {name: "DenyPreparedQueryRead", check: checkDenyPreparedQueryRead}, @@ -480,6 +507,8 @@ func TestACL(t *testing.T) { {name: "AllowNodeWrite", check: checkAllowNodeWrite}, {name: "AllowMeshRead", check: checkAllowMeshRead}, {name: "AllowMeshWrite", check: checkAllowMeshWrite}, + {name: "AllowPeeringRead", check: checkAllowPeeringRead}, + {name: "AllowPeeringWrite", check: checkAllowPeeringWrite}, {name: "AllowOperatorRead", check: checkAllowOperatorRead}, {name: "AllowOperatorWrite", check: checkAllowOperatorWrite}, {name: "AllowPreparedQueryRead", check: checkAllowPreparedQueryRead}, @@ -514,6 +543,8 @@ func TestACL(t *testing.T) { {name: "AllowNodeWrite", check: checkAllowNodeWrite}, {name: "AllowMeshRead", check: checkAllowMeshRead}, {name: "AllowMeshWrite", check: checkAllowMeshWrite}, + {name: "AllowPeeringRead", check: checkAllowPeeringRead}, + {name: "AllowPeeringWrite", check: checkAllowPeeringWrite}, {name: "AllowOperatorRead", check: checkAllowOperatorRead}, {name: "AllowOperatorWrite", check: checkAllowOperatorWrite}, {name: "AllowPreparedQueryRead", check: checkAllowPreparedQueryRead}, @@ -1217,6 +1248,319 @@ func TestACL(t *testing.T) { {name: "WriteAllowed", check: checkAllowMeshWrite}, }, }, + { + name: "PeeringDefaultAllowPolicyDeny", + defaultPolicy: AllowAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + name: "PeeringDefaultAllowPolicyRead", + defaultPolicy: AllowAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + name: "PeeringDefaultAllowPolicyWrite", + defaultPolicy: AllowAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + name: "PeeringDefaultAllowPolicyNone", + defaultPolicy: AllowAll(), + policyStack: []*Policy{ + {}, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + name: "PeeringDefaultDenyPolicyDeny", + defaultPolicy: DenyAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + name: "PeeringDefaultDenyPolicyRead", + defaultPolicy: DenyAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + name: "PeeringDefaultDenyPolicyWrite", + defaultPolicy: DenyAll(), + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Peering: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + name: "PeeringDefaultDenyPolicyNone", + defaultPolicy: DenyAll(), + policyStack: []*Policy{ + {}, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:deny, p:deny = deny + name: "PeeringOperatorDenyPolicyDeny", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyDeny, + Peering: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:read, p:deny = deny + name: "PeeringOperatorReadPolicyDeny", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyRead, + Peering: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:write, p:deny = deny + name: "PeeringOperatorWritePolicyDeny", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyWrite, + Peering: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:deny, p:read = read + name: "PeeringOperatorDenyPolicyRead", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyDeny, + Peering: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:read, p:read = read + name: "PeeringOperatorReadPolicyRead", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyRead, + Peering: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:write, p:read = read + name: "PeeringOperatorWritePolicyRead", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyWrite, + Peering: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:deny, p:write = write + name: "PeeringOperatorDenyPolicyWrite", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyDeny, + Peering: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + // o:read, p:write = write + name: "PeeringOperatorReadPolicyWrite", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyRead, + Peering: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + // o:write, p:write = write + name: "PeeringOperatorWritePolicyWrite", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyWrite, + Peering: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, + { + // o:deny, p: = deny + name: "PeeringOperatorDenyPolicyNone", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyDeny, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadDenied", check: checkDenyPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:read, p: = read + name: "PeeringOperatorReadPolicyNone", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyRead, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteDenied", check: checkDenyPeeringWrite}, + }, + }, + { + // o:write, p: = write + name: "PeeringOperatorWritePolicyNone", + defaultPolicy: nil, // test both + policyStack: []*Policy{ + { + PolicyRules: PolicyRules{ + Operator: PolicyWrite, + }, + }, + }, + checks: []aclCheck{ + {name: "ReadAllowed", check: checkAllowPeeringRead}, + {name: "WriteAllowed", check: checkAllowPeeringWrite}, + }, + }, { name: "OperatorDefaultAllowPolicyDeny", defaultPolicy: AllowAll(), diff --git a/acl/authorizer.go b/acl/authorizer.go index fe28c05ed6..b0e5326bc8 100644 --- a/acl/authorizer.go +++ b/acl/authorizer.go @@ -114,6 +114,14 @@ type Authorizer interface { // functions can be used. MeshWrite(*AuthorizerContext) EnforcementDecision + // PeeringRead determines if the read-only Consul peering functions + // can be used. + PeeringRead(*AuthorizerContext) EnforcementDecision + + // PeeringWrite determines if the stage-changing Consul peering + // functions can be used. + PeeringWrite(*AuthorizerContext) EnforcementDecision + // NodeRead checks for permission to read (discover) a given node. NodeRead(string, *AuthorizerContext) EnforcementDecision @@ -542,12 +550,11 @@ func Enforce(authz Authorizer, rsc Resource, segment string, access string, ctx return authz.SessionWrite(segment, ctx), nil } case ResourcePeering: - // TODO (peering) switch this over to using PeeringRead & PeeringWrite methods once implemented switch lowerAccess { case "read": - return authz.OperatorRead(ctx), nil + return authz.PeeringRead(ctx), nil case "write": - return authz.OperatorWrite(ctx), nil + return authz.PeeringWrite(ctx), nil } default: if processed, decision, err := enforceEnterprise(authz, rsc, segment, lowerAccess, ctx); processed { @@ -561,6 +568,7 @@ func Enforce(authz Authorizer, rsc Resource, segment string, access string, ctx // NewAuthorizerFromRules is a convenience function to invoke NewPolicyFromSource followed by NewPolicyAuthorizer with // the parse policy. +// TODO(ACL-Legacy-Compat): remove syntax arg after removing SyntaxLegacy func NewAuthorizerFromRules(rules string, syntax SyntaxVersion, conf *Config, meta *EnterprisePolicyMeta) (Authorizer, error) { policy, err := NewPolicyFromSource(rules, syntax, conf, meta) if err != nil { diff --git a/acl/authorizer_test.go b/acl/authorizer_test.go index f8aeda3d42..03c0517a16 100644 --- a/acl/authorizer_test.go +++ b/acl/authorizer_test.go @@ -139,6 +139,20 @@ func (m *mockAuthorizer) MeshWrite(ctx *AuthorizerContext) EnforcementDecision { return ret.Get(0).(EnforcementDecision) } +// PeeringRead determines if the read-only Consul peering functions +// can be used. +func (m *mockAuthorizer) PeeringRead(ctx *AuthorizerContext) EnforcementDecision { + ret := m.Called(ctx) + return ret.Get(0).(EnforcementDecision) +} + +// PeeringWrite determines if the state-changing Consul peering +// functions can be used. +func (m *mockAuthorizer) PeeringWrite(ctx *AuthorizerContext) EnforcementDecision { + ret := m.Called(ctx) + return ret.Get(0).(EnforcementDecision) +} + // OperatorRead determines if the read-only Consul operator functions // can be used. ret := m.Called(segment, ctx) func (m *mockAuthorizer) OperatorRead(ctx *AuthorizerContext) EnforcementDecision { @@ -463,29 +477,25 @@ func TestACL_Enforce(t *testing.T) { err: "Invalid access level", }, { - // TODO (peering) Update to use PeeringRead - method: "OperatorRead", + method: "PeeringRead", resource: ResourcePeering, access: "read", ret: Allow, }, { - // TODO (peering) Update to use PeeringRead - method: "OperatorRead", + method: "PeeringRead", resource: ResourcePeering, access: "read", ret: Deny, }, { - // TODO (peering) Update to use PeeringWrite - method: "OperatorWrite", + method: "PeeringWrite", resource: ResourcePeering, access: "write", ret: Allow, }, { - // TODO (peering) Update to use PeeringWrite - method: "OperatorWrite", + method: "PeeringWrite", resource: ResourcePeering, access: "write", ret: Deny, diff --git a/acl/chained_authorizer.go b/acl/chained_authorizer.go index 77df69a3ea..cf81cc4b18 100644 --- a/acl/chained_authorizer.go +++ b/acl/chained_authorizer.go @@ -161,6 +161,22 @@ func (c *ChainedAuthorizer) MeshWrite(entCtx *AuthorizerContext) EnforcementDeci }) } +// PeeringRead determines if the read-only Consul peering functions +// can be used. +func (c *ChainedAuthorizer) PeeringRead(entCtx *AuthorizerContext) EnforcementDecision { + return c.executeChain(func(authz Authorizer) EnforcementDecision { + return authz.PeeringRead(entCtx) + }) +} + +// PeeringWrite determines if the state-changing Consul peering +// functions can be used. +func (c *ChainedAuthorizer) PeeringWrite(entCtx *AuthorizerContext) EnforcementDecision { + return c.executeChain(func(authz Authorizer) EnforcementDecision { + return authz.PeeringWrite(entCtx) + }) +} + // NodeRead checks for permission to read (discover) a given node. func (c *ChainedAuthorizer) NodeRead(node string, entCtx *AuthorizerContext) EnforcementDecision { return c.executeChain(func(authz Authorizer) EnforcementDecision { diff --git a/acl/chained_authorizer_test.go b/acl/chained_authorizer_test.go index 5f33d01667..284a1bd0e7 100644 --- a/acl/chained_authorizer_test.go +++ b/acl/chained_authorizer_test.go @@ -68,6 +68,12 @@ func (authz testAuthorizer) MeshRead(*AuthorizerContext) EnforcementDecision { func (authz testAuthorizer) MeshWrite(*AuthorizerContext) EnforcementDecision { return EnforcementDecision(authz) } +func (authz testAuthorizer) PeeringRead(*AuthorizerContext) EnforcementDecision { + return EnforcementDecision(authz) +} +func (authz testAuthorizer) PeeringWrite(*AuthorizerContext) EnforcementDecision { + return EnforcementDecision(authz) +} func (authz testAuthorizer) OperatorRead(*AuthorizerContext) EnforcementDecision { return EnforcementDecision(authz) } @@ -128,6 +134,8 @@ func TestChainedAuthorizer(t *testing.T) { checkDenyNodeWrite(t, authz, "foo", nil) checkDenyMeshRead(t, authz, "foo", nil) checkDenyMeshWrite(t, authz, "foo", nil) + checkDenyPeeringRead(t, authz, "foo", nil) + checkDenyPeeringWrite(t, authz, "foo", nil) checkDenyOperatorRead(t, authz, "foo", nil) checkDenyOperatorWrite(t, authz, "foo", nil) checkDenyPreparedQueryRead(t, authz, "foo", nil) @@ -160,6 +168,8 @@ func TestChainedAuthorizer(t *testing.T) { checkDenyNodeWrite(t, authz, "foo", nil) checkDenyMeshRead(t, authz, "foo", nil) checkDenyMeshWrite(t, authz, "foo", nil) + checkDenyPeeringRead(t, authz, "foo", nil) + checkDenyPeeringWrite(t, authz, "foo", nil) checkDenyOperatorRead(t, authz, "foo", nil) checkDenyOperatorWrite(t, authz, "foo", nil) checkDenyPreparedQueryRead(t, authz, "foo", nil) @@ -192,6 +202,8 @@ func TestChainedAuthorizer(t *testing.T) { checkAllowNodeWrite(t, authz, "foo", nil) checkAllowMeshRead(t, authz, "foo", nil) checkAllowMeshWrite(t, authz, "foo", nil) + checkAllowPeeringRead(t, authz, "foo", nil) + checkAllowPeeringWrite(t, authz, "foo", nil) checkAllowOperatorRead(t, authz, "foo", nil) checkAllowOperatorWrite(t, authz, "foo", nil) checkAllowPreparedQueryRead(t, authz, "foo", nil) @@ -224,6 +236,8 @@ func TestChainedAuthorizer(t *testing.T) { checkDenyNodeWrite(t, authz, "foo", nil) checkDenyMeshRead(t, authz, "foo", nil) checkDenyMeshWrite(t, authz, "foo", nil) + checkDenyPeeringRead(t, authz, "foo", nil) + checkDenyPeeringWrite(t, authz, "foo", nil) checkDenyOperatorRead(t, authz, "foo", nil) checkDenyOperatorWrite(t, authz, "foo", nil) checkDenyPreparedQueryRead(t, authz, "foo", nil) @@ -254,6 +268,8 @@ func TestChainedAuthorizer(t *testing.T) { checkAllowNodeWrite(t, authz, "foo", nil) checkAllowMeshRead(t, authz, "foo", nil) checkAllowMeshWrite(t, authz, "foo", nil) + checkAllowPeeringRead(t, authz, "foo", nil) + checkAllowPeeringWrite(t, authz, "foo", nil) checkAllowOperatorRead(t, authz, "foo", nil) checkAllowOperatorWrite(t, authz, "foo", nil) checkAllowPreparedQueryRead(t, authz, "foo", nil) diff --git a/acl/policy.go b/acl/policy.go index d4ebd5976a..59c3df8b3c 100644 --- a/acl/policy.go +++ b/acl/policy.go @@ -85,6 +85,7 @@ type PolicyRules struct { Keyring string `hcl:"keyring"` Operator string `hcl:"operator"` Mesh string `hcl:"mesh"` + Peering string `hcl:"peering"` } // Policy is used to represent the policy specified by an ACL configuration. @@ -289,6 +290,10 @@ func (pr *PolicyRules) Validate(conf *Config) error { return fmt.Errorf("Invalid mesh policy: %#v", pr.Mesh) } + // Validate the peering policy - this one is allowed to be empty + if pr.Peering != "" && !isPolicyValid(pr.Peering, false) { + return fmt.Errorf("Invalid peering policy: %#v", pr.Peering) + } return nil } @@ -309,6 +314,7 @@ func parseCurrent(rules string, conf *Config, meta *EnterprisePolicyMeta) (*Poli return p, nil } +// TODO(ACL-Legacy-Compat): remove in phase 2 func parseLegacy(rules string, conf *Config) (*Policy, error) { p := &Policy{} @@ -436,6 +442,7 @@ func NewPolicyFromSource(rules string, syntax SyntaxVersion, conf *Config, meta var policy *Policy var err error switch syntax { + // TODO(ACL-Legacy-Compat): remove and remove as argument from function case SyntaxLegacy: policy, err = parseLegacy(rules, conf) case SyntaxCurrent: diff --git a/acl/policy_authorizer.go b/acl/policy_authorizer.go index 3b79a63169..4a24b6bd03 100644 --- a/acl/policy_authorizer.go +++ b/acl/policy_authorizer.go @@ -43,6 +43,9 @@ type policyAuthorizer struct { // meshRule contains the mesh policies. meshRule *policyAuthorizerRule + // peeringRule contains the peering policies. + peeringRule *policyAuthorizerRule + // embedded enterprise policy authorizer enterprisePolicyAuthorizer } @@ -322,6 +325,15 @@ func (p *policyAuthorizer) loadRules(policy *PolicyRules) error { p.meshRule = &policyAuthorizerRule{access: access} } + // Load the peering policy + if policy.Peering != "" { + access, err := AccessLevelFromString(policy.Peering) + if err != nil { + return err + } + p.peeringRule = &policyAuthorizerRule{access: access} + } + return nil } @@ -692,6 +704,25 @@ func (p *policyAuthorizer) MeshWrite(ctx *AuthorizerContext) EnforcementDecision return p.OperatorWrite(ctx) } +// PeeringRead determines if the read-only peering functions are allowed. +func (p *policyAuthorizer) PeeringRead(ctx *AuthorizerContext) EnforcementDecision { + if p.peeringRule != nil { + return enforce(p.peeringRule.access, AccessRead) + } + // default to OperatorRead access + return p.OperatorRead(ctx) +} + +// PeeringWrite determines if the state-changing peering functions are +// allowed. +func (p *policyAuthorizer) PeeringWrite(ctx *AuthorizerContext) EnforcementDecision { + if p.peeringRule != nil { + return enforce(p.peeringRule.access, AccessWrite) + } + // default to OperatorWrite access + return p.OperatorWrite(ctx) +} + // OperatorRead determines if the read-only operator functions are allowed. func (p *policyAuthorizer) OperatorRead(*AuthorizerContext) EnforcementDecision { if p.operatorRule != nil { diff --git a/acl/policy_authorizer_test.go b/acl/policy_authorizer_test.go index d2f69a4ebc..57f41993ae 100644 --- a/acl/policy_authorizer_test.go +++ b/acl/policy_authorizer_test.go @@ -50,6 +50,8 @@ func TestPolicyAuthorizer(t *testing.T) { {name: "DefaultNodeWrite", prefix: "foo", check: checkDefaultNodeWrite}, {name: "DefaultMeshRead", prefix: "foo", check: checkDefaultMeshRead}, {name: "DefaultMeshWrite", prefix: "foo", check: checkDefaultMeshWrite}, + {name: "DefaultPeeringRead", prefix: "foo", check: checkDefaultPeeringRead}, + {name: "DefaultPeeringWrite", prefix: "foo", check: checkDefaultPeeringWrite}, {name: "DefaultOperatorRead", prefix: "foo", check: checkDefaultOperatorRead}, {name: "DefaultOperatorWrite", prefix: "foo", check: checkDefaultOperatorWrite}, {name: "DefaultPreparedQueryRead", prefix: "foo", check: checkDefaultPreparedQueryRead}, diff --git a/acl/policy_merger.go b/acl/policy_merger.go index d4a454bc16..3a617aa1e1 100644 --- a/acl/policy_merger.go +++ b/acl/policy_merger.go @@ -10,6 +10,7 @@ type policyRulesMergeContext struct { keyRules map[string]*KeyRule keyPrefixRules map[string]*KeyRule meshRule string + peeringRule string nodeRules map[string]*NodeRule nodePrefixRules map[string]*NodeRule operatorRule string @@ -33,6 +34,7 @@ func (p *policyRulesMergeContext) init() { p.keyRules = make(map[string]*KeyRule) p.keyPrefixRules = make(map[string]*KeyRule) p.meshRule = "" + p.peeringRule = "" p.nodeRules = make(map[string]*NodeRule) p.nodePrefixRules = make(map[string]*NodeRule) p.operatorRule = "" @@ -119,10 +121,6 @@ func (p *policyRulesMergeContext) merge(policy *PolicyRules) { } } - if takesPrecedenceOver(policy.Mesh, p.meshRule) { - p.meshRule = policy.Mesh - } - for _, np := range policy.Nodes { update := true if permission, found := p.nodeRules[np.Name]; found { @@ -145,6 +143,14 @@ func (p *policyRulesMergeContext) merge(policy *PolicyRules) { } } + if takesPrecedenceOver(policy.Mesh, p.meshRule) { + p.meshRule = policy.Mesh + } + + if takesPrecedenceOver(policy.Peering, p.peeringRule) { + p.peeringRule = policy.Peering + } + if takesPrecedenceOver(policy.Operator, p.operatorRule) { p.operatorRule = policy.Operator } @@ -235,6 +241,7 @@ func (p *policyRulesMergeContext) fill(merged *PolicyRules) { merged.Keyring = p.keyringRule merged.Operator = p.operatorRule merged.Mesh = p.meshRule + merged.Peering = p.peeringRule // All the for loop appends are ugly but Go doesn't have a way to get // a slice of all values within a map so this is necessary diff --git a/acl/policy_test.go b/acl/policy_test.go index 5416eb5572..362451e98c 100644 --- a/acl/policy_test.go +++ b/acl/policy_test.go @@ -65,6 +65,7 @@ func TestPolicySourceParse(t *testing.T) { } operator = "deny" mesh = "deny" + peering = "deny" service_prefix "" { policy = "write" } @@ -147,6 +148,7 @@ func TestPolicySourceParse(t *testing.T) { }, "operator": "deny", "mesh": "deny", + "peering": "deny", "service_prefix": { "": { "policy": "write" @@ -253,6 +255,7 @@ func TestPolicySourceParse(t *testing.T) { }, Operator: PolicyDeny, Mesh: PolicyDeny, + Peering: PolicyDeny, PreparedQueryPrefixes: []*PreparedQueryRule{ { Prefix: "", @@ -743,6 +746,13 @@ func TestPolicySourceParse(t *testing.T) { RulesJSON: `{ "mesh": "nope" }`, Err: "Invalid mesh policy", }, + { + Name: "Bad Policy - Peering", + Syntax: SyntaxCurrent, + Rules: `peering = "nope"`, + RulesJSON: `{ "peering": "nope" }`, + Err: "Invalid peering policy", + }, { Name: "Keyring Empty", Syntax: SyntaxCurrent, @@ -764,6 +774,13 @@ func TestPolicySourceParse(t *testing.T) { RulesJSON: `{ "mesh": "" }`, Expected: &Policy{PolicyRules: PolicyRules{Mesh: ""}}, }, + { + Name: "Peering Empty", + Syntax: SyntaxCurrent, + Rules: `peering = ""`, + RulesJSON: `{ "peering": "" }`, + Expected: &Policy{PolicyRules: PolicyRules{Peering: ""}}, + }, } for _, tc := range cases { @@ -1453,66 +1470,90 @@ func TestMergePolicies(t *testing.T) { { name: "Write Precedence", input: []*Policy{ - {PolicyRules: PolicyRules{ - ACL: PolicyRead, - Keyring: PolicyRead, - Operator: PolicyRead, - Mesh: PolicyRead, - }}, - {PolicyRules: PolicyRules{ + { + PolicyRules: PolicyRules{ + ACL: PolicyRead, + Keyring: PolicyRead, + Operator: PolicyRead, + Mesh: PolicyRead, + Peering: PolicyRead, + }, + }, + { + PolicyRules: PolicyRules{ + ACL: PolicyWrite, + Keyring: PolicyWrite, + Operator: PolicyWrite, + Mesh: PolicyWrite, + Peering: PolicyWrite, + }, + }, + }, + expected: &Policy{ + PolicyRules: PolicyRules{ ACL: PolicyWrite, Keyring: PolicyWrite, Operator: PolicyWrite, Mesh: PolicyWrite, - }}, + Peering: PolicyWrite, + }, }, - expected: &Policy{PolicyRules: PolicyRules{ - ACL: PolicyWrite, - Keyring: PolicyWrite, - Operator: PolicyWrite, - Mesh: PolicyWrite, - }}, }, { name: "Deny Precedence", input: []*Policy{ - {PolicyRules: PolicyRules{ - ACL: PolicyWrite, - Keyring: PolicyWrite, - Operator: PolicyWrite, - Mesh: PolicyWrite, - }}, - {PolicyRules: PolicyRules{ + { + PolicyRules: PolicyRules{ + ACL: PolicyWrite, + Keyring: PolicyWrite, + Operator: PolicyWrite, + Mesh: PolicyWrite, + Peering: PolicyWrite, + }, + }, + { + PolicyRules: PolicyRules{ + ACL: PolicyDeny, + Keyring: PolicyDeny, + Operator: PolicyDeny, + Mesh: PolicyDeny, + Peering: PolicyDeny, + }, + }, + }, + expected: &Policy{ + PolicyRules: PolicyRules{ ACL: PolicyDeny, Keyring: PolicyDeny, Operator: PolicyDeny, Mesh: PolicyDeny, - }}, + Peering: PolicyDeny, + }, }, - expected: &Policy{PolicyRules: PolicyRules{ - ACL: PolicyDeny, - Keyring: PolicyDeny, - Operator: PolicyDeny, - Mesh: PolicyDeny, - }}, }, { name: "Read Precedence", input: []*Policy{ - {PolicyRules: PolicyRules{ + { + PolicyRules: PolicyRules{ + ACL: PolicyRead, + Keyring: PolicyRead, + Operator: PolicyRead, + Mesh: PolicyRead, + Peering: PolicyRead, + }, + }, + {}, + }, + expected: &Policy{ + PolicyRules: PolicyRules{ ACL: PolicyRead, Keyring: PolicyRead, Operator: PolicyRead, Mesh: PolicyRead, - }}, - {}, + Peering: PolicyRead, + }, }, - expected: &Policy{PolicyRules: PolicyRules{ - ACL: PolicyRead, - Keyring: PolicyRead, - Operator: PolicyRead, - Mesh: PolicyRead, - }}, }, } @@ -1524,6 +1565,7 @@ func TestMergePolicies(t *testing.T) { require.Equal(t, exp.Keyring, act.Keyring) require.Equal(t, exp.Operator, act.Operator) require.Equal(t, exp.Mesh, act.Mesh) + require.Equal(t, exp.Peering, act.Peering) require.ElementsMatch(t, exp.Agents, act.Agents) require.ElementsMatch(t, exp.AgentPrefixes, act.AgentPrefixes) require.ElementsMatch(t, exp.Events, act.Events) @@ -1597,6 +1639,9 @@ operator = "write" # comment mesh = "write" + +# comment +peering = "write" ` expected := ` @@ -1652,6 +1697,9 @@ operator = "write" # comment mesh = "write" + +# comment +peering = "write" ` output, err := TranslateLegacyRules([]byte(input)) diff --git a/acl/static_authorizer.go b/acl/static_authorizer.go index 951b026f37..07cc845114 100644 --- a/acl/static_authorizer.go +++ b/acl/static_authorizer.go @@ -170,6 +170,20 @@ func (s *staticAuthorizer) MeshWrite(*AuthorizerContext) EnforcementDecision { return Deny } +func (s *staticAuthorizer) PeeringRead(*AuthorizerContext) EnforcementDecision { + if s.defaultAllow { + return Allow + } + return Deny +} + +func (s *staticAuthorizer) PeeringWrite(*AuthorizerContext) EnforcementDecision { + if s.defaultAllow { + return Allow + } + return Deny +} + func (s *staticAuthorizer) OperatorRead(*AuthorizerContext) EnforcementDecision { if s.defaultAllow { return Allow diff --git a/agent/acl_endpoint_test.go b/agent/acl_endpoint_test.go index 60a512ef41..5cffef6ee3 100644 --- a/agent/acl_endpoint_test.go +++ b/agent/acl_endpoint_test.go @@ -2044,6 +2044,14 @@ func TestACL_Authorize(t *testing.T) { Resource: "mesh", Access: "write", }, + { + Resource: "peering", + Access: "read", + }, + { + Resource: "peering", + Access: "write", + }, { Resource: "query", Segment: "foo", @@ -2186,6 +2194,14 @@ func TestACL_Authorize(t *testing.T) { Resource: "mesh", Access: "write", }, + { + Resource: "peering", + Access: "read", + }, + { + Resource: "peering", + Access: "write", + }, { Resource: "query", Segment: "foo", @@ -2238,6 +2254,8 @@ func TestACL_Authorize(t *testing.T) { true, // operator:write true, // mesh:read true, // mesh:write + true, // peering:read + true, // peering:write false, // query:read false, // query:write true, // service:read diff --git a/agent/structs/acl.go b/agent/structs/acl.go index 82d19b8aca..1fd3f1d935 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -60,6 +60,7 @@ node_prefix "" { } operator = "write" mesh = "write" +peering = "write" query_prefix "" { policy = "write" }