diff --git a/agent/connect/authz.go b/agent/connect/authz.go new file mode 100644 index 0000000000..605a447374 --- /dev/null +++ b/agent/connect/authz.go @@ -0,0 +1,49 @@ +package connect + +import ( + "github.com/hashicorp/consul/agent/structs" +) + +// AuthorizeIntentionTarget determines whether the destination is covered by the given intention +// and whether the intention action allows a connection. +// This is a generalized version of the old CertURI.Authorize(), and can be evaluated against sources or destinations. +// +// The return value of `auth` is only valid if the second value `match` is true. +// If `match` is false, then the intention doesn't match this target and any result should be ignored. +func AuthorizeIntentionTarget( + target, targetNS string, + ixn *structs.Intention, + matchType structs.IntentionMatchType, +) (auth bool, match bool) { + + switch matchType { + case structs.IntentionMatchDestination: + if ixn.DestinationNS != structs.WildcardSpecifier && ixn.DestinationNS != targetNS { + // Non-matching namespace + return false, false + } + + if ixn.DestinationName != structs.WildcardSpecifier && ixn.DestinationName != target { + // Non-matching name + return false, false + } + + case structs.IntentionMatchSource: + if ixn.SourceNS != structs.WildcardSpecifier && ixn.SourceNS != targetNS { + // Non-matching namespace + return false, false + } + + if ixn.SourceName != structs.WildcardSpecifier && ixn.SourceName != target { + // Non-matching name + return false, false + } + + default: + // Reject on any un-recognized match type + return false, false + } + + // The name and namespace match, so the destination is covered + return ixn.Action == structs.IntentionActionAllow, true +} diff --git a/agent/connect/authz_test.go b/agent/connect/authz_test.go new file mode 100644 index 0000000000..4033abb86f --- /dev/null +++ b/agent/connect/authz_test.go @@ -0,0 +1,196 @@ +package connect + +import ( + "github.com/hashicorp/consul/agent/structs" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestAuthorizeIntentionTarget(t *testing.T) { + cases := []struct { + name string + target string + targetNS string + ixn *structs.Intention + matchType structs.IntentionMatchType + auth bool + match bool + }{ + // Source match type + { + name: "match exact source, not matching namespace", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: "db", + SourceNS: "different", + }, + matchType: structs.IntentionMatchSource, + auth: false, + match: false, + }, + { + name: "match exact source, not matching name", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: "db", + SourceNS: structs.IntentionDefaultNamespace, + }, + matchType: structs.IntentionMatchSource, + auth: false, + match: false, + }, + { + name: "match exact source, allow", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: "web", + SourceNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionAllow, + }, + matchType: structs.IntentionMatchSource, + auth: true, + match: true, + }, + { + name: "match exact source, deny", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: "web", + SourceNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionDeny, + }, + matchType: structs.IntentionMatchSource, + auth: false, + match: true, + }, + { + name: "match exact sourceNS for wildcard service, deny", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: structs.WildcardSpecifier, + SourceNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionDeny, + }, + matchType: structs.IntentionMatchSource, + auth: false, + match: true, + }, + { + name: "match exact sourceNS for wildcard service, allow", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + SourceName: structs.WildcardSpecifier, + SourceNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionAllow, + }, + matchType: structs.IntentionMatchSource, + auth: true, + match: true, + }, + + // Destination match type + { + name: "match exact destination, not matching namespace", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: "db", + DestinationNS: "different", + }, + matchType: structs.IntentionMatchDestination, + auth: false, + match: false, + }, + { + name: "match exact destination, not matching name", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: "db", + DestinationNS: structs.IntentionDefaultNamespace, + }, + matchType: structs.IntentionMatchDestination, + auth: false, + match: false, + }, + { + name: "match exact destination, allow", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: "web", + DestinationNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionAllow, + }, + matchType: structs.IntentionMatchDestination, + auth: true, + match: true, + }, + { + name: "match exact destination, deny", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: "web", + DestinationNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionDeny, + }, + matchType: structs.IntentionMatchDestination, + auth: false, + match: true, + }, + { + name: "match exact destinationNS for wildcard service, deny", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: structs.WildcardSpecifier, + DestinationNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionDeny, + }, + matchType: structs.IntentionMatchDestination, + auth: false, + match: true, + }, + { + name: "match exact destinationNS for wildcard service, allow", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: structs.WildcardSpecifier, + DestinationNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionAllow, + }, + matchType: structs.IntentionMatchDestination, + auth: true, + match: true, + }, + { + name: "unknown match type", + target: "web", + targetNS: structs.IntentionDefaultNamespace, + ixn: &structs.Intention{ + DestinationName: structs.WildcardSpecifier, + DestinationNS: structs.IntentionDefaultNamespace, + Action: structs.IntentionActionAllow, + }, + matchType: structs.IntentionMatchType("unknown"), + auth: false, + match: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + auth, match := AuthorizeIntentionTarget(tc.target, tc.targetNS, tc.ixn, tc.matchType) + assert.Equal(t, tc.auth, auth) + assert.Equal(t, tc.match, match) + }) + } +} diff --git a/agent/connect/uri.go b/agent/connect/uri.go index 5d155d4742..2f44546320 100644 --- a/agent/connect/uri.go +++ b/agent/connect/uri.go @@ -5,8 +5,6 @@ import ( "net/url" "regexp" "strings" - - "github.com/hashicorp/consul/agent/structs" ) // CertURI represents a Connect-valid URI value for a TLS certificate. @@ -17,13 +15,6 @@ import ( // However, we anticipate that we may accept URIs that are also not SPIFFE // compliant and therefore the interface is named as such. type CertURI interface { - // Authorize tests the authorization for this URI as a client - // for the given intention. The return value `auth` is only valid if - // the second value `match` is true. If the second value `match` is - // false, then the intention doesn't match this client and any - // result should be ignored. - Authorize(*structs.Intention) (auth bool, match bool) - // URI is the valid URI value used in the cert. URI() *url.URL } diff --git a/agent/connect/uri_agent.go b/agent/connect/uri_agent.go index 86205dbdcd..981dee61fb 100644 --- a/agent/connect/uri_agent.go +++ b/agent/connect/uri_agent.go @@ -3,8 +3,6 @@ package connect import ( "fmt" "net/url" - - "github.com/hashicorp/consul/agent/structs" ) // SpiffeIDService is the structure to represent the SPIFFE ID for an agent. @@ -23,11 +21,6 @@ func (id *SpiffeIDAgent) URI() *url.URL { return &result } -// CertURI impl. -func (id *SpiffeIDAgent) Authorize(_ *structs.Intention) (bool, bool) { - return false, false -} - func (id *SpiffeIDAgent) CommonName() string { return AgentCN(id.Agent, id.Host) } diff --git a/agent/connect/uri_agent_test.go b/agent/connect/uri_agent_test.go index 8990f552c5..7e0cb81845 100644 --- a/agent/connect/uri_agent_test.go +++ b/agent/connect/uri_agent_test.go @@ -15,14 +15,3 @@ func TestSpiffeIDAgentURI(t *testing.T) { require.Equal(t, "spiffe://1234.consul/agent/client/dc/dc1/id/123", agent.URI().String()) } - -func TestSpiffeIDAgentAuthorize(t *testing.T) { - agent := &SpiffeIDAgent{ - Host: "1234.consul", - Agent: "uuid", - } - - auth, match := agent.Authorize(nil) - require.False(t, auth) - require.False(t, match) -} diff --git a/agent/connect/uri_service.go b/agent/connect/uri_service.go index 405bdcbd96..7136df8ab3 100644 --- a/agent/connect/uri_service.go +++ b/agent/connect/uri_service.go @@ -3,8 +3,6 @@ package connect import ( "fmt" "net/url" - - "github.com/hashicorp/consul/agent/structs" ) // SpiffeIDService is the structure to represent the SPIFFE ID for a service. @@ -25,22 +23,6 @@ func (id *SpiffeIDService) URI() *url.URL { return &result } -// CertURI impl. -func (id *SpiffeIDService) Authorize(ixn *structs.Intention) (bool, bool) { - if ixn.SourceNS != structs.WildcardSpecifier && ixn.SourceNS != id.Namespace { - // Non-matching namespace - return false, false - } - - if ixn.SourceName != structs.WildcardSpecifier && ixn.SourceName != id.Service { - // Non-matching name - return false, false - } - - // Match, return allow value - return ixn.Action == structs.IntentionActionAllow, true -} - func (id *SpiffeIDService) CommonName() string { return ServiceCN(id.Service, id.Namespace, id.Host) } diff --git a/agent/connect/uri_service_test.go b/agent/connect/uri_service_test.go deleted file mode 100644 index af917540f8..0000000000 --- a/agent/connect/uri_service_test.go +++ /dev/null @@ -1,104 +0,0 @@ -package connect - -import ( - "testing" - - "github.com/hashicorp/consul/agent/structs" - "github.com/stretchr/testify/assert" -) - -func TestSpiffeIDServiceAuthorize(t *testing.T) { - ns := structs.IntentionDefaultNamespace - serviceWeb := &SpiffeIDService{ - Host: "1234.consul", - Namespace: structs.IntentionDefaultNamespace, - Datacenter: "dc01", - Service: "web", - } - - cases := []struct { - Name string - URI *SpiffeIDService - Ixn *structs.Intention - Auth bool - Match bool - }{ - { - "exact source, not matching namespace", - serviceWeb, - &structs.Intention{ - SourceNS: "different", - SourceName: "db", - }, - false, - false, - }, - - { - "exact source, not matching name", - serviceWeb, - &structs.Intention{ - SourceNS: ns, - SourceName: "db", - }, - false, - false, - }, - - { - "exact source, allow", - serviceWeb, - &structs.Intention{ - SourceNS: serviceWeb.Namespace, - SourceName: serviceWeb.Service, - Action: structs.IntentionActionAllow, - }, - true, - true, - }, - - { - "exact source, deny", - serviceWeb, - &structs.Intention{ - SourceNS: serviceWeb.Namespace, - SourceName: serviceWeb.Service, - Action: structs.IntentionActionDeny, - }, - false, - true, - }, - - { - "exact namespace, wildcard service, deny", - serviceWeb, - &structs.Intention{ - SourceNS: serviceWeb.Namespace, - SourceName: structs.WildcardSpecifier, - Action: structs.IntentionActionDeny, - }, - false, - true, - }, - - { - "exact namespace, wildcard service, allow", - serviceWeb, - &structs.Intention{ - SourceNS: serviceWeb.Namespace, - SourceName: structs.WildcardSpecifier, - Action: structs.IntentionActionAllow, - }, - true, - true, - }, - } - - for _, tc := range cases { - t.Run(tc.Name, func(t *testing.T) { - auth, match := tc.URI.Authorize(tc.Ixn) - assert.Equal(t, tc.Auth, auth) - assert.Equal(t, tc.Match, match) - }) - } -} diff --git a/agent/connect/uri_signing.go b/agent/connect/uri_signing.go index 652f26422d..ad8de307e1 100644 --- a/agent/connect/uri_signing.go +++ b/agent/connect/uri_signing.go @@ -28,12 +28,6 @@ func (id *SpiffeIDSigning) Host() string { return strings.ToLower(fmt.Sprintf("%s.%s", id.ClusterID, id.Domain)) } -// CertURI impl. -func (id *SpiffeIDSigning) Authorize(ixn *structs.Intention) (bool, bool) { - // Never authorize as a client. - return false, true -} - // CanSign takes any CertURI and returns whether or not this signing entity is // allowed to sign CSRs for that entity (i.e. represents the trust domain for // that entity). diff --git a/agent/connect/uri_signing_test.go b/agent/connect/uri_signing_test.go index 6d04a5fab8..9a62ea2448 100644 --- a/agent/connect/uri_signing_test.go +++ b/agent/connect/uri_signing_test.go @@ -10,14 +10,6 @@ import ( "github.com/stretchr/testify/assert" ) -// Signing ID should never authorize -func TestSpiffeIDSigningAuthorize(t *testing.T) { - var id SpiffeIDSigning - auth, ok := id.Authorize(nil) - assert.False(t, auth) - assert.True(t, ok) -} - func TestSpiffeIDSigningForCluster(t *testing.T) { // For now it should just append .consul to the ID. config := &structs.CAConfiguration{ @@ -31,10 +23,6 @@ func TestSpiffeIDSigningForCluster(t *testing.T) { // about type fakeCertURI string -func (f fakeCertURI) Authorize(*structs.Intention) (auth bool, match bool) { - return false, false -} - func (f fakeCertURI) URI() *url.URL { u, _ := url.Parse(string(f)) return u diff --git a/agent/connect_auth.go b/agent/connect_auth.go index c498cb2e23..9b50ef183e 100644 --- a/agent/connect_auth.go +++ b/agent/connect_auth.go @@ -3,7 +3,6 @@ package agent import ( "context" "fmt" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" cachetype "github.com/hashicorp/consul/agent/cache-types" @@ -105,7 +104,8 @@ func (a *Agent) ConnectAuthorize(token string, // Figure out which source matches this request. var ixnMatch *structs.Intention for _, ixn := range reply.Matches[0] { - if _, ok := uriService.Authorize(ixn); ok { + // We match on the intention source because the uriService is the source of the connection to authorize. + if _, ok := connect.AuthorizeIntentionTarget(uriService.Service, uriService.Namespace, ixn, structs.IntentionMatchSource); ok { ixnMatch = ixn break } diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index fb19d9c11c..bedf8be3cd 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -3,6 +3,7 @@ package state import ( "errors" "fmt" + "github.com/hashicorp/consul/agent/connect" "sort" "github.com/hashicorp/go-memdb" @@ -743,7 +744,7 @@ func (s *Store) IntentionDecision( // Figure out which source matches this request. var ixnMatch *structs.Intention for _, ixn := range intentions { - if _, ok := AuthorizeIntentionTarget(target, targetNS, ixn, matchType); ok { + if _, ok := connect.AuthorizeIntentionTarget(target, targetNS, ixn, matchType); ok { ixnMatch = ixn break } @@ -775,37 +776,6 @@ func (s *Store) IntentionDecision( return resp, nil } -// AuthorizeIntentionTarget determines whether the destination is covered by the given intention -// and whether the intention action allows a connection. -func AuthorizeIntentionTarget(target, targetNS string, ixn *structs.Intention, matchType structs.IntentionMatchType) (bool, bool) { - switch matchType { - case structs.IntentionMatchDestination: - if ixn.DestinationNS != structs.WildcardSpecifier && ixn.DestinationNS != targetNS { - // Non-matching namespace - return false, false - } - - if ixn.DestinationName != structs.WildcardSpecifier && ixn.DestinationName != target { - // Non-matching name - return false, false - } - - case structs.IntentionMatchSource: - if ixn.SourceNS != structs.WildcardSpecifier && ixn.SourceNS != targetNS { - // Non-matching namespace - return false, false - } - - if ixn.SourceName != structs.WildcardSpecifier && ixn.SourceName != target { - // Non-matching name - return false, false - } - } - - // The name and namespace match, so the destination is covered - return ixn.Action == structs.IntentionActionAllow, true -} - // IntentionMatch returns the list of intentions that match the namespace and // name for either a source or destination. This applies the resolution rules // so wildcards will match any value. diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index cc4fc1a7bd..5989dff35c 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -1870,182 +1870,6 @@ func TestStore_IntentionDecision(t *testing.T) { } } -func TestAuthorizeIntentionTarget(t *testing.T) { - cases := []struct { - name string - target string - targetNS string - ixn *structs.Intention - matchType structs.IntentionMatchType - auth bool - match bool - }{ - // Source match type - { - name: "match exact source, not matching namespace", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: "db", - SourceNS: "different", - }, - matchType: structs.IntentionMatchSource, - auth: false, - match: false, - }, - { - name: "match exact source, not matching name", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: "db", - SourceNS: structs.IntentionDefaultNamespace, - }, - matchType: structs.IntentionMatchSource, - auth: false, - match: false, - }, - { - name: "match exact source, allow", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: "web", - SourceNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionAllow, - }, - matchType: structs.IntentionMatchSource, - auth: true, - match: true, - }, - { - name: "match exact source, deny", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: "web", - SourceNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionDeny, - }, - matchType: structs.IntentionMatchSource, - auth: false, - match: true, - }, - { - name: "match exact sourceNS for wildcard service, deny", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: structs.WildcardSpecifier, - SourceNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionDeny, - }, - matchType: structs.IntentionMatchSource, - auth: false, - match: true, - }, - { - name: "match exact sourceNS for wildcard service, allow", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - SourceName: structs.WildcardSpecifier, - SourceNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionAllow, - }, - matchType: structs.IntentionMatchSource, - auth: true, - match: true, - }, - - // Destination match type - { - name: "match exact destination, not matching namespace", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: "db", - DestinationNS: "different", - }, - matchType: structs.IntentionMatchDestination, - auth: false, - match: false, - }, - { - name: "match exact destination, not matching name", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: "db", - DestinationNS: structs.IntentionDefaultNamespace, - }, - matchType: structs.IntentionMatchDestination, - auth: false, - match: false, - }, - { - name: "match exact destination, allow", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: "web", - DestinationNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionAllow, - }, - matchType: structs.IntentionMatchDestination, - auth: true, - match: true, - }, - { - name: "match exact destination, deny", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: "web", - DestinationNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionDeny, - }, - matchType: structs.IntentionMatchDestination, - auth: false, - match: true, - }, - { - name: "match exact destinationNS for wildcard service, deny", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: structs.WildcardSpecifier, - DestinationNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionDeny, - }, - matchType: structs.IntentionMatchDestination, - auth: false, - match: true, - }, - { - name: "match exact destinationNS for wildcard service, allow", - target: "web", - targetNS: structs.IntentionDefaultNamespace, - ixn: &structs.Intention{ - DestinationName: structs.WildcardSpecifier, - DestinationNS: structs.IntentionDefaultNamespace, - Action: structs.IntentionActionAllow, - }, - matchType: structs.IntentionMatchDestination, - auth: true, - match: true, - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - auth, match := AuthorizeIntentionTarget(tc.target, tc.targetNS, tc.ixn, tc.matchType) - assert.Equal(t, tc.auth, auth) - assert.Equal(t, tc.match, match) - }) - } -} - func disableLegacyIntentions(s *Store) error { return s.SystemMetadataSet(1, &structs.SystemMetadataEntry{ Key: structs.SystemMetadataIntentionFormatKey,