From beab0cd96289ff1274fa8914fd374015accbede3 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 2 Sep 2021 10:43:56 -0600 Subject: [PATCH 1/4] Account for partition when matching src intentions --- agent/xds/rbac.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/agent/xds/rbac.go b/agent/xds/rbac.go index 3a81784aae..1c7f6c1021 100644 --- a/agent/xds/rbac.go +++ b/agent/xds/rbac.go @@ -489,10 +489,11 @@ func removeSameSourceIntentions(intentions structs.Intentions) structs.Intention // 'against' service name via wildcard rules. // // For instance: -// - (web, api) => false, because these have no wildcards -// - (web, *) => true, because "all services" includes "web" -// - (default/web, default/*) => true, because "all services in the default NS" includes "default/web" -// - (default/*, */*) => true, "any service in any NS" includes "all services in the default NS" +// - (web, api) => false, because these have no wildcards +// - (web, *) => true, because "all services" includes "web" +// - (default/web, default/*) => true, because "all services in the default NS" includes "default/web" +// - (default/*, */*) => true, "any service in any NS" includes "all services in the default NS" +// - (default/default/*, other/*/*) => false, "any service in "other" partition" does NOT include services in the default partition" func ixnSourceMatches(tester, against structs.ServiceName) bool { // We assume that we can't have the same intention twice before arriving // here. @@ -505,13 +506,19 @@ func ixnSourceMatches(tester, against structs.ServiceName) bool { return false } + matchesAP := tester.PartitionOrDefault() == against.PartitionOrDefault() || against.PartitionOrDefault() == structs.WildcardSpecifier matchesNS := tester.NamespaceOrDefault() == against.NamespaceOrDefault() || against.NamespaceOrDefault() == structs.WildcardSpecifier matchesName := tester.Name == against.Name || against.Name == structs.WildcardSpecifier - return matchesNS && matchesName + return matchesAP && matchesNS && matchesName } // countWild counts the number of wildcard values in the given namespace and name. func countWild(src structs.ServiceName) int { + // If Partition is wildcard, panic because it's not supported + if src.PartitionOrDefault() == structs.WildcardSpecifier { + panic("invalid state: intention references wildcard partition") + } + // If NS is wildcard, it must be 2 since wildcards only follow exact if src.NamespaceOrDefault() == structs.WildcardSpecifier { return 2 From 19da23be2890521fe2c9593d5d31b33f2fe3a793 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 2 Sep 2021 12:12:51 -0600 Subject: [PATCH 2/4] Expand testing of removeSameSourceIntentions for partitions --- agent/xds/rbac.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/agent/xds/rbac.go b/agent/xds/rbac.go index 1c7f6c1021..12ce411131 100644 --- a/agent/xds/rbac.go +++ b/agent/xds/rbac.go @@ -457,6 +457,16 @@ func makeRBACRules(intentions structs.Intentions, intentionDefaultAllow bool, is return rbac, nil } +// removeSameSourceIntentions will iterate over intentions and remove any lower precedence +// intentions that share the same source. Intentions are sorted by descending precedence +// so once a source has been seen, additional intentions with the same source can be dropped. +// +// Example for the default/web service: +// input: [(backend/* -> default/web), (backend/* -> default/*)] +// output: [(backend/* -> default/web)] +// +// (backend/* -> default/*) was dropped because it is already known that any service +// in the backend namespace can target default/web. func removeSameSourceIntentions(intentions structs.Intentions) structs.Intentions { if len(intentions) < 2 { return intentions From 5e54f253d75124d243b58affb1fa64f2c2c734b0 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 2 Sep 2021 12:33:56 -0600 Subject: [PATCH 3/4] Expand testing of simplifyNotSourceSlice for partitions --- agent/xds/rbac.go | 7 ++++--- agent/xds/rbac_test.go | 11 ----------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/agent/xds/rbac.go b/agent/xds/rbac.go index 12ce411131..a9a2425fd1 100644 --- a/agent/xds/rbac.go +++ b/agent/xds/rbac.go @@ -296,15 +296,16 @@ func (p *rbacPermission) Flatten() *envoy_rbac_v3.Permission { return andPermissions(parts) } +// simplifyNotSourceSlice will collapse NotSources elements together if any element is +// a subset of another. +// For example "default/web" is a subset of "default/*" because it is covered by the wildcard. func simplifyNotSourceSlice(notSources []structs.ServiceName) []structs.ServiceName { if len(notSources) <= 1 { return notSources } - // Collapse NotSources elements together if any element is a subset of - // another. - // Sort, keeping the least wildcarded elements first. + // More specific elements have a higher precedence over more wildcarded elements. sort.SliceStable(notSources, func(i, j int) bool { return countWild(notSources[i]) < countWild(notSources[j]) }) diff --git a/agent/xds/rbac_test.go b/agent/xds/rbac_test.go index 44fac77334..9d182022e3 100644 --- a/agent/xds/rbac_test.go +++ b/agent/xds/rbac_test.go @@ -887,14 +887,3 @@ func makeServiceNameSlice(slice []string) []structs.ServiceName { } return out } - -func unmakeServiceNameSlice(slice []structs.ServiceName) []string { - if len(slice) == 0 { - return nil - } - var out []string - for _, src := range slice { - out = append(out, src.String()) - } - return out -} From d90e30f0091af45cc19b61bd3c17024012f351b7 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 9 Sep 2021 18:24:43 -0600 Subject: [PATCH 4/4] Update spiffe ID patterns used for RBAC --- agent/xds/rbac.go | 49 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/agent/xds/rbac.go b/agent/xds/rbac.go index a9a2425fd1..64cdcdd8d8 100644 --- a/agent/xds/rbac.go +++ b/agent/xds/rbac.go @@ -13,6 +13,7 @@ import ( envoy_network_rbac_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3" envoy_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" ) @@ -564,7 +565,7 @@ func notPrincipal(id *envoy_rbac_v3.Principal) *envoy_rbac_v3.Principal { } func idPrincipal(src structs.ServiceName) *envoy_rbac_v3.Principal { - pattern := makeSpiffePattern(src.NamespaceOrDefault(), src.Name) + pattern := makeSpiffePattern(src.PartitionOrDefault(), src.NamespaceOrDefault(), src.Name) return &envoy_rbac_v3.Principal{ Identifier: &envoy_rbac_v3.Principal_Authenticated_{ @@ -578,21 +579,41 @@ func idPrincipal(src structs.ServiceName) *envoy_rbac_v3.Principal { }, } } -func makeSpiffePattern(sourceNS, sourceName string) string { - const ( - anyPath = `[^/]+` - spiffeTemplate = `^spiffe://%s/ns/%s/dc/%s/svc/%s$` - ) - switch { - case sourceNS != structs.WildcardSpecifier && sourceName != structs.WildcardSpecifier: - return fmt.Sprintf(spiffeTemplate, anyPath, sourceNS, anyPath, sourceName) - case sourceNS != structs.WildcardSpecifier && sourceName == structs.WildcardSpecifier: - return fmt.Sprintf(spiffeTemplate, anyPath, sourceNS, anyPath, anyPath) - case sourceNS == structs.WildcardSpecifier && sourceName == structs.WildcardSpecifier: - return fmt.Sprintf(spiffeTemplate, anyPath, anyPath, anyPath, anyPath) - default: + +func makeSpiffePattern(sourceAP, sourceNS, sourceName string) string { + if sourceNS == structs.WildcardSpecifier && sourceName != structs.WildcardSpecifier { panic(fmt.Sprintf("not possible to have a wildcarded namespace %q but an exact service %q", sourceNS, sourceName)) } + if sourceAP == structs.WildcardSpecifier { + panic("not possible to have a wildcarded source partition") + } + + const anyPath = `[^/]+` + + // Match on any namespace or service if it is a wildcard, or on a specific value otherwise. + ns := sourceNS + if sourceNS == structs.WildcardSpecifier { + ns = anyPath + } + + svc := sourceName + if sourceName == structs.WildcardSpecifier { + svc = anyPath + } + + id := connect.SpiffeIDService{ + Namespace: ns, + Service: svc, + + // Trust domain and datacenter are not verified by RBAC, so we match on any value. + Host: anyPath, + Datacenter: anyPath, + + // Partition can only ever be an exact value. + Partition: sourceAP, + } + + return fmt.Sprintf(`^%s://%s%s$`, id.URI().Scheme, id.Host, id.URI().Path) } func anyPermission() *envoy_rbac_v3.Permission {