ingress/rbac: allow any downstream principal to access the service

Since ingress policies are not based on service accounts, traffic
policy routes derived from ingress rules should not enforce RBAC
policies based on the downstream's service account.
This change adds support to wildcard the allowed service accounts
for an ingress route so that the corresponding RBAC policy
can create a rule that allows any downstream principal.

This is required for ingress to work with routesV2 and RBAC
per route policies.

Signed-off-by: Shashank Ram <shashr2204@gmail.com>

DESIGN.md

@@ -349,7 +349,7 @@ type MeshCataloger interface {
   GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.HTTPRouteMatch, error)
 
   // GetIngressPoliciesForService returns the inbound traffic policies associated with an ingress service
-  GetIngressPoliciesForService(service.MeshService, service.K8sServiceAccount) ([]*trafficpolicy.InboundTrafficPolicy, error)
+  GetIngressPoliciesForService(service.MeshService) ([]*trafficpolicy.InboundTrafficPolicy, error)
 }
 ```
 

pkg/catalog/ingress.go

@@ -55,8 +55,12 @@ func (mc *MeshCatalog) GetIngressRoutesPerHost(service service.MeshService) (map
 }
 
 // GetIngressPoliciesForService returns a list of inbound traffic policies for a service as defined in observed ingress k8s resources.
-func (mc *MeshCatalog) GetIngressPoliciesForService(svc service.MeshService, sa service.K8sServiceAccount) ([]*trafficpolicy.InboundTrafficPolicy, error) {
+func (mc *MeshCatalog) GetIngressPoliciesForService(svc service.MeshService) ([]*trafficpolicy.InboundTrafficPolicy, error) {
 	inboundIngressPolicies := []*trafficpolicy.InboundTrafficPolicy{}
+	// Ingress does not depend on k8s service accounts, program a wildcard (empty struct) to indicate
+	// to RDS that an inbound traffic policy for ingress should not enforce service account based RBAC policies.
+	wildcardServiceAccount := service.K8sServiceAccount{}
+
 	ingresses, err := mc.ingressMonitor.GetIngressResources(svc)
 	if err != nil {
 		log.Error().Err(err).Msgf("Failed to get ingress resources for service %s", svc)
@@ -72,7 +76,7 @@ func (mc *MeshCatalog) GetIngressPoliciesForService(svc service.MeshService, sa
 	for _, ingress := range ingresses {
 		if ingress.Spec.Backend != nil && ingress.Spec.Backend.ServiceName == svc.Name {
 			wildcardIngressPolicy := trafficpolicy.NewInboundTrafficPolicy(buildIngressPolicyName(ingress.ObjectMeta.Name, ingress.ObjectMeta.Namespace, constants.WildcardHTTPMethod), []string{constants.WildcardHTTPMethod})
-			wildcardIngressPolicy.AddRule(*trafficpolicy.NewRouteWeightedCluster(wildCardRouteMatch, ingressWeightedCluster), sa)
+			wildcardIngressPolicy.AddRule(*trafficpolicy.NewRouteWeightedCluster(wildCardRouteMatch, ingressWeightedCluster), wildcardServiceAccount)
 			inboundIngressPolicies = trafficpolicy.MergeInboundPolicies(false, inboundIngressPolicies, wildcardIngressPolicy)
 		}
 
@@ -91,7 +95,7 @@ func (mc *MeshCatalog) GetIngressPoliciesForService(svc service.MeshService, sa
 				if routePolicy.PathRegex != "" {
 					routePolicy.PathRegex = ingressPath.Path
 				}
-				ingressPolicy.AddRule(*trafficpolicy.NewRouteWeightedCluster(routePolicy, ingressWeightedCluster), sa)
+				ingressPolicy.AddRule(*trafficpolicy.NewRouteWeightedCluster(routePolicy, ingressWeightedCluster), wildcardServiceAccount)
 			}
 
 			inboundIngressPolicies = trafficpolicy.MergeInboundPolicies(false, inboundIngressPolicies, ingressPolicy)

pkg/catalog/ingress_test.go

@@ -314,12 +314,8 @@ func TestGetIngressPoliciesForService(t *testing.T) {
 		Namespace: fakeIngressNamespace,
 		Name:      fakeIngressService,
 	}
-	fakeServiceAccount := service.K8sServiceAccount{
-		Namespace: fakeIngressNamespace,
-		Name:      fakeIngressService,
-	}
 
-	inboundIngressPolicies, err := mc.GetIngressPoliciesForService(fakeService, fakeServiceAccount)
+	inboundIngressPolicies, err := mc.GetIngressPoliciesForService(fakeService)
 	assert.Empty(err)
 
 	// The number of ingress inbound policies is the number of unique hosts across the ingress resources :
@@ -340,6 +336,12 @@ func TestGetIngressPoliciesForService(t *testing.T) {
 			// on the implementation of 'GetIngressPoliciesForServices()', we relax the check here
 			// to match on any of the ingress paths corresponding to the host.
 			assert.True(pathContains(fakeIngressPaths[inboundIngressPolicies[i].Hostnames[0]], rule.Route.HTTPRouteMatch.PathRegex))
+
+			// Allowed service accounts should be wildcarded with an empty service account for ingress rules
+			assert.NotNil(rule.AllowedServiceAccounts)
+			assert.Equal(1, rule.AllowedServiceAccounts.Cardinality()) // single wildcard service account
+			allowedSvcAccounts := rule.AllowedServiceAccounts.Pop().(service.K8sServiceAccount)
+			assert.True((allowedSvcAccounts.IsEmpty()))
 		}
 	}
 }

pkg/catalog/types.go

@@ -132,7 +132,7 @@ type MeshCataloger interface {
 	GetIngressRoutesPerHost(service.MeshService) (map[string][]trafficpolicy.HTTPRouteMatch, error)
 
 	// GetIngressPoliciesForService returns the inbound traffic policies associated with an ingress service
-	GetIngressPoliciesForService(service.MeshService, service.K8sServiceAccount) ([]*trafficpolicy.InboundTrafficPolicy, error)
+	GetIngressPoliciesForService(service.MeshService) ([]*trafficpolicy.InboundTrafficPolicy, error)
 
 	// ListMonitoredNamespaces lists namespaces monitored by the control plane
 	ListMonitoredNamespaces() []string

pkg/envoy/rds/new_response.go

@@ -40,7 +40,7 @@ func newResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_disco
 
 	// Get Ingress inbound policies for the proxy
 	for _, svc := range services {
-		ingressInboundPolicies, err := catalog.GetIngressPoliciesForService(svc, proxyIdentity)
+		ingressInboundPolicies, err := catalog.GetIngressPoliciesForService(svc)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error looking up ingress policies for service=%s", svc.String())
 			return nil, err

pkg/envoy/rds/new_response_test.go

@@ -96,7 +96,7 @@ func TestNewResponse(t *testing.T) {
 
 	mockCatalog.EXPECT().ListInboundTrafficPolicies(gomock.Any(), gomock.Any()).Return(testInbound).AnyTimes()
 	mockCatalog.EXPECT().ListOutboundTrafficPolicies(gomock.Any()).Return(testOutbound).AnyTimes()
-	mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any(), gomock.Any()).Return(testIngressInbound, nil).AnyTimes()
+	mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any()).Return(testIngressInbound, nil).AnyTimes()
 	mockCatalog.EXPECT().GetServicesFromEnvoyCertificate(gomock.Any()).Return([]service.MeshService{tests.BookstoreV1Service}, nil).AnyTimes()
 
 	mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes()
@@ -219,7 +219,7 @@ func TestNewResponseWithPermissiveMode(t *testing.T) {
 
 	mockCatalog.EXPECT().GetServicesFromEnvoyCertificate(gomock.Any()).Return([]service.MeshService{tests.BookstoreV1Service}, nil).AnyTimes()
 	mockCatalog.EXPECT().ListPoliciesForPermissiveMode(gomock.Any()).Return(testPermissiveInbound, testOutbound).AnyTimes()
-	mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any(), gomock.Any()).Return(testIngressInbound, nil).AnyTimes()
+	mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any()).Return(testIngressInbound, nil).AnyTimes()
 
 	mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(true).AnyTimes()
 

pkg/envoy/route/rbac.go

@@ -31,16 +31,26 @@ func buildInboundRBACFilterForRule(rule *trafficpolicy.Rule) (map[string]*any.An
 	// Create the list of principals for this policy
 	var principalRuleList []rbac.RulesList
 	for downstream := range rule.AllowedServiceAccounts.Iter() {
+		var principalRule rbac.RulesList
 		downstreamIdentity := downstream.(service.K8sServiceAccount)
-		// The downstream principal in an RBAC policy is an authenticated principal type, which
-		// means the principal must correspond to the fully qualified SAN in the certificate presented
-		// by the downstream.
-		downstreamPrincipal := identity.GetKubernetesServiceIdentity(downstreamIdentity, identity.ClusterLocalTrustDomain)
-		principalRule := rbac.RulesList{
-			OrRules: []rbac.Rule{
-				{Attribute: rbac.DownstreamAuthPrincipal, Value: downstreamPrincipal.String()},
-			},
+
+		if downstreamIdentity.IsEmpty() {
+			// When the downstream identity in a traffic policy rule is set to be empty, it implies
+			// we must allow all downstream principals. This can be accomplished by setting an empty
+			// principal rules list to generate an RBAC policy with principals set to ANY (all downstreams).
+			principalRule = rbac.RulesList{}
+		} else {
+			// The downstream principal in an RBAC policy is an authenticated principal type, which
+			// means the principal must correspond to the fully qualified SAN in the certificate presented
+			// by the downstream.
+			downstreamPrincipal := identity.GetKubernetesServiceIdentity(downstreamIdentity, identity.ClusterLocalTrustDomain)
+			principalRule = rbac.RulesList{
+				OrRules: []rbac.Rule{
+					{Attribute: rbac.DownstreamAuthPrincipal, Value: downstreamPrincipal.String()},
+				},
+			}
 		}
+
 		principalRuleList = append(principalRuleList, principalRule)
 	}
 

pkg/envoy/route/rbac_test.go

@@ -27,7 +27,7 @@ func TestBuildInboundRBACFilterForRule(t *testing.T) {
 		expectError        bool
 	}{
 		{
-			name: "valid trafficpolicy rule",
+			name: "valid trafficpolicy rule with restricted downstream identities",
 			rule: &trafficpolicy.Rule{
 				Route: trafficpolicy.RouteWeightedClusters{
 					HTTPRouteMatch:   tests.BookstoreBuyHTTPRoute,
@@ -67,6 +67,31 @@ func TestBuildInboundRBACFilterForRule(t *testing.T) {
 			},
 			expectError: false,
 		},
+		{
+			name: "valid trafficpolicy rule which allows all downstream identities",
+			rule: &trafficpolicy.Rule{
+				Route: trafficpolicy.RouteWeightedClusters{
+					HTTPRouteMatch:   tests.BookstoreBuyHTTPRoute,
+					WeightedClusters: set.NewSet(tests.BookstoreV1DefaultWeightedCluster),
+				},
+				AllowedServiceAccounts: set.NewSetFromSlice([]interface{}{
+					service.K8sServiceAccount{}, // setting an empty service account will result in all downstreams being allowed
+				}),
+			},
+			expectedRBACPolicy: &xds_rbac.Policy{
+				Principals: []*xds_rbac.Principal{
+					{
+						Identifier: &xds_rbac.Principal_Any{Any: true},
+					},
+				},
+				Permissions: []*xds_rbac.Permission{
+					{
+						Rule: &xds_rbac.Permission_Any{Any: true},
+					},
+				},
+			},
+			expectError: false,
+		},
 		{
 			name: "invalid trafficpolicy rule with Rule.AllowedServiceAccounts not specified",
 			rule: &trafficpolicy.Rule{

pkg/service/types.go

@@ -32,10 +32,16 @@ type K8sServiceAccount struct {
 	Name      string
 }
 
+// String returns the string representation of the service account object
 func (sa K8sServiceAccount) String() string {
 	return strings.Join([]string{sa.Namespace, namespaceNameSeparator, sa.Name}, "")
 }
 
+// IsEmpty returns true if the given service account object is empty
+func (sa K8sServiceAccount) IsEmpty() bool {
+	return (K8sServiceAccount{}) == sa
+}
+
 // GetSyntheticService creates a MeshService for the given K8s Service Account,
 // which has a unique name and is in lieu of an existing Kubernetes service.
 func (sa K8sServiceAccount) GetSyntheticService() MeshService {