acl: fix intention:*:write checks (#11061)

This is a partial revert of #10793
This commit is contained in:
R.B. Boyer 2021-09-16 11:08:45 -05:00 committed by GitHub
parent cd08a36ce0
commit ca73abdea1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 47 additions and 5 deletions

View File

@ -536,9 +536,6 @@ func (p *policyAuthorizer) IntentionRead(prefix string, _ *AuthorizerContext) En
// IntentionWrite checks if writing (creating, updating, or deleting) of an // IntentionWrite checks if writing (creating, updating, or deleting) of an
// intention is allowed. // intention is allowed.
func (p *policyAuthorizer) IntentionWrite(prefix string, _ *AuthorizerContext) EnforcementDecision { func (p *policyAuthorizer) IntentionWrite(prefix string, _ *AuthorizerContext) EnforcementDecision {
if prefix == "" {
return Deny
}
if prefix == "*" { if prefix == "*" {
return p.allAllowed(p.intentionRules, AccessWrite) return p.allAllowed(p.intentionRules, AccessWrite)
} }

View File

@ -2188,6 +2188,42 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega
require.Equal(t, acl.Deny, authz.OperatorRead(nil)) require.Equal(t, acl.Deny, authz.OperatorRead(nil))
require.Equal(t, acl.Allow, authz.ServiceRead("foo", 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) { func TestACLResolver_Legacy(t *testing.T) {

View File

@ -175,8 +175,9 @@ func TestIntentionApply_createWithID(t *testing.T) {
Datacenter: "dc1", Datacenter: "dc1",
Op: structs.IntentionOpCreate, Op: structs.IntentionOpCreate,
Intention: &structs.Intention{ Intention: &structs.Intention{
ID: generateUUID(), ID: generateUUID(),
SourceName: "test", SourceName: "test",
DestinationName: "test2",
}, },
} }
var reply string var reply string

View File

@ -329,6 +329,14 @@ func (ixn *Intention) CanRead(authz acl.Authorizer) bool {
} }
func (ixn *Intention) CanWrite(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 var authzContext acl.AuthorizerContext
ixn.FillAuthzContext(&authzContext, true) ixn.FillAuthzContext(&authzContext, true)
return authz.IntentionWrite(ixn.DestinationName, &authzContext) == acl.Allow return authz.IntentionWrite(ixn.DestinationName, &authzContext) == acl.Allow