From 4e7b6888e3bac18d5cf4b0b8058b8221d3afe34a Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" <4903+rboyer@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:08:45 -0500 Subject: [PATCH] acl: fix intention:*:write checks (#11061) This is a partial revert of #10793 --- acl/policy_authorizer.go | 3 --- agent/consul/acl_test.go | 36 +++++++++++++++++++++++++ agent/consul/intention_endpoint_test.go | 5 ++-- agent/structs/intention.go | 8 ++++++ 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/acl/policy_authorizer.go b/acl/policy_authorizer.go index 5d06513ac..8cce120eb 100644 --- a/acl/policy_authorizer.go +++ b/acl/policy_authorizer.go @@ -536,9 +536,6 @@ func (p *policyAuthorizer) IntentionRead(prefix string, _ *AuthorizerContext) En // IntentionWrite checks if writing (creating, updating, or deleting) of an // intention is allowed. func (p *policyAuthorizer) IntentionWrite(prefix string, _ *AuthorizerContext) EnforcementDecision { - if prefix == "" { - return Deny - } if prefix == "*" { return p.allAllowed(p.intentionRules, AccessWrite) } diff --git a/agent/consul/acl_test.go b/agent/consul/acl_test.go index bae52aa1f..f68c3c096 100644 --- a/agent/consul/acl_test.go +++ b/agent/consul/acl_test.go @@ -2188,6 +2188,42 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega require.Equal(t, acl.Deny, authz.OperatorRead(nil)) require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil)) }) + + runTwiceAndReset("service and intention wildcard write", func(t *testing.T) { + delegate.UseTestLocalData([]interface{}{ + &structs.ACLToken{ + AccessorID: "5f57c1f6-6a89-4186-9445-531b316e01df", + SecretID: "with-intentions", + Policies: []structs.ACLTokenPolicyLink{ + {ID: "ixn-write"}, + }, + }, + &structs.ACLPolicy{ + ID: "ixn-write", + Name: "ixn-write", + Description: "ixn-write", + Rules: `service_prefix "" { policy = "write" intentions = "write" }`, + Syntax: acl.SyntaxCurrent, + RaftIndex: structs.RaftIndex{CreateIndex: 1, ModifyIndex: 2}, + }, + }) + authz, err := r.ResolveToken("with-intentions") + require.NoError(t, err) + require.NotNil(t, authz) + require.Equal(t, acl.Allow, authz.ServiceRead("", nil)) + require.Equal(t, acl.Allow, authz.ServiceRead("foo", nil)) + require.Equal(t, acl.Allow, authz.ServiceRead("bar", nil)) + require.Equal(t, acl.Allow, authz.ServiceWrite("", nil)) + require.Equal(t, acl.Allow, authz.ServiceWrite("foo", nil)) + require.Equal(t, acl.Allow, authz.ServiceWrite("bar", nil)) + require.Equal(t, acl.Allow, authz.IntentionRead("", nil)) + require.Equal(t, acl.Allow, authz.IntentionRead("foo", nil)) + require.Equal(t, acl.Allow, authz.IntentionRead("bar", nil)) + require.Equal(t, acl.Allow, authz.IntentionWrite("", nil)) + require.Equal(t, acl.Allow, authz.IntentionWrite("foo", nil)) + require.Equal(t, acl.Allow, authz.IntentionWrite("bar", nil)) + require.Equal(t, acl.Deny, authz.NodeRead("server", nil)) + }) } func TestACLResolver_Legacy(t *testing.T) { diff --git a/agent/consul/intention_endpoint_test.go b/agent/consul/intention_endpoint_test.go index bec1d6c4a..7ddf7d9d6 100644 --- a/agent/consul/intention_endpoint_test.go +++ b/agent/consul/intention_endpoint_test.go @@ -175,8 +175,9 @@ func TestIntentionApply_createWithID(t *testing.T) { Datacenter: "dc1", Op: structs.IntentionOpCreate, Intention: &structs.Intention{ - ID: generateUUID(), - SourceName: "test", + ID: generateUUID(), + SourceName: "test", + DestinationName: "test2", }, } var reply string diff --git a/agent/structs/intention.go b/agent/structs/intention.go index d30a4c0fc..49217b130 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -329,6 +329,14 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool { } func (ixn *Intention) CanWrite(authz acl.Authorizer) bool { + if ixn.DestinationName == "" { + // This is likely a strange form of legacy intention data validation + // that happened within the authorization check, since intentions without + // a destination cannot be written. + // This may be able to be removed later. + return false + } + var authzContext acl.AuthorizerContext ixn.FillAuthzContext(&authzContext, true) return authz.IntentionWrite(ixn.DestinationName, &authzContext) == acl.Allow