This PR has 2 main changes:

1. Change the policy builder's AddPrincipal, to AddIdentity so
that the builder accepts OSM constructs as inputs, and returns
envoy constructs.
2. Set the trust domain (currently an empty string) on the
listener builder so that it will be used in the future.

Part of #4754

Signed-off-by: Sean Teeling <[email protected]>

Author: steeling

pkg/certificate/manager.go

@@ -56,6 +56,13 @@ func (m *Manager) Start(checkInterval time.Duration, stop <-chan struct{}) {
 	}()
 }
 
+// GetTrustDomain returns the trust domain from the configured signingkey issuer.
+// Note that the CRD uses a default, so this value will always be set.
+func (m *Manager) GetTrustDomain() string {
+	// TODO(4754): implement
+	return ""
+}
+
 func (m *Manager) checkAndRotate() {
 	// NOTE: checkAndRotate can reintroduce a certificate that has been released, thereby creating an unbounded cache.
 	// A certificate can also have been rotated already, leaving the list of issued certs stale, and we re-rotate.

pkg/certificate/types.go

@@ -72,7 +72,8 @@ type Issuer interface {
 
 type issuer struct {
 	Issuer
-	ID string
+	ID          string
+	TrustDomain string
 	// memoized once the first certificate is issued
 	CertificateAuthority pem.RootCertificate
 }

pkg/envoy/lds/inmesh_test.go

@@ -406,7 +406,7 @@ func TestGetOutboundFilterChainMatchForService(t *testing.T) {
 	mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
 	mockCatalog := catalog.NewMockMeshCataloger(mockCtrl)
 
-	lb := newListenerBuilder(mockCatalog, tests.BookbuyerServiceIdentity, mockConfigurator, nil)
+	lb := newListenerBuilder(mockCatalog, tests.BookbuyerServiceIdentity, mockConfigurator, nil, "cluster.local")
 
 	testCases := []struct {
 		name                     string
@@ -581,7 +581,7 @@ func TestGetOutboundTCPFilter(t *testing.T) {
 			mockCatalog := catalog.NewMockMeshCataloger(mockCtrl)
 			mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
 
-			lb := newListenerBuilder(mockCatalog, tests.BookbuyerServiceIdentity, mockConfigurator, nil)
+			lb := newListenerBuilder(mockCatalog, tests.BookbuyerServiceIdentity, mockConfigurator, nil, "cluster.local")
 			filter, err := lb.getOutboundTCPFilter(tc.trafficMatch)
 
 			assert := tassert.New(t)

pkg/envoy/lds/listener_test.go

@@ -196,7 +196,7 @@ func TestNewOutboundListener(t *testing.T) {
 		EnableEgressPolicy: true,
 	}).Times(1)
 
-	lb := newListenerBuilder(meshCatalog, identity, cfg, nil)
+	lb := newListenerBuilder(meshCatalog, identity, cfg, nil, "cluster.local")
 
 	assert := tassert.New(t)
 	listener, err := lb.newOutboundListener()

pkg/envoy/lds/rbac.go

@@ -50,7 +50,7 @@ func (lb *listenerBuilder) buildInboundRBACPolicies() (*xds_network_rbac.RBAC, e
 	rbacPolicies := make(map[string]*xds_rbac.Policy)
 	// Build an RBAC policies based on SMI TrafficTarget policies
 	for _, targetPolicy := range trafficTargets {
-		rbacPolicies[targetPolicy.Name] = buildRBACPolicyFromTrafficTarget(targetPolicy)
+		rbacPolicies[targetPolicy.Name] = buildRBACPolicyFromTrafficTarget(targetPolicy, lb.trustDomain)
 	}
 
 	log.Debug().Msgf("RBAC policy for proxy with identity %s: %+v", proxyIdentity, rbacPolicies)
@@ -68,12 +68,12 @@ func (lb *listenerBuilder) buildInboundRBACPolicies() (*xds_network_rbac.RBAC, e
 }
 
 // buildRBACPolicyFromTrafficTarget creates an XDS RBAC policy from the given traffic target policy
-func buildRBACPolicyFromTrafficTarget(trafficTarget trafficpolicy.TrafficTargetWithRoutes) *xds_rbac.Policy {
+func buildRBACPolicyFromTrafficTarget(trafficTarget trafficpolicy.TrafficTargetWithRoutes, trustDomain string) *xds_rbac.Policy {
 	pb := &rbac.PolicyBuilder{}
 
-	// Create the list of principals for this policy
-	for _, downstreamPrincipal := range trafficTarget.Sources {
-		pb.AddPrincipal(downstreamPrincipal.String())
+	// Create the list of identities for this policy
+	for _, downstreamIdentity := range trafficTarget.Sources {
+		pb.AddIdentity(downstreamIdentity)
 	}
 	// Create the list of permissions for this policy
 	for _, tcpRouteMatch := range trafficTarget.TCPRouteMatches {
@@ -83,5 +83,7 @@ func buildRBACPolicyFromTrafficTarget(trafficTarget trafficpolicy.TrafficTargetW
 		}
 	}
 
+	pb.SetTrustDomain(trustDomain)
+
 	return pb.Build()
 }

pkg/envoy/lds/rbac_test.go

@@ -89,7 +89,7 @@ func TestBuildRBACPolicyFromTrafficTarget(t *testing.T) {
 			assert := tassert.New(t)
 
 			// Test the RBAC policies
-			policy := buildRBACPolicyFromTrafficTarget(tc.trafficTarget)
+			policy := buildRBACPolicyFromTrafficTarget(tc.trafficTarget, "cluster.local")
 
 			assert.Equal(tc.expectedPolicy, policy)
 		})

pkg/envoy/lds/response.go

@@ -19,15 +19,15 @@ import (
 // 1. Inbound listener to handle incoming traffic
 // 2. Outbound listener to handle outgoing traffic
 // 3. Prometheus listener for metrics
-func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_discovery.DiscoveryRequest, cfg configurator.Configurator, _ *certificate.Manager, proxyRegistry *registry.ProxyRegistry) ([]types.Resource, error) {
+func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_discovery.DiscoveryRequest, cfg configurator.Configurator, cm *certificate.Manager, proxyRegistry *registry.ProxyRegistry) ([]types.Resource, error) {
 	var ldsResources []types.Resource
 
 	var statsHeaders map[string]string
 	if featureflags := cfg.GetFeatureFlags(); featureflags.EnableWASMStats {
 		statsHeaders = proxy.StatsHeaders()
 	}
 
-	lb := newListenerBuilder(meshCatalog, proxy.Identity, cfg, statsHeaders)
+	lb := newListenerBuilder(meshCatalog, proxy.Identity, cfg, statsHeaders, cm.GetTrustDomain())
 
 	if proxy.Kind() == envoy.KindGateway && cfg.GetFeatureFlags().EnableMulticlusterMode {
 		gatewayListener, err := lb.buildMulticlusterGatewayListener()
@@ -96,11 +96,12 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d
 }
 
 // Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
-func newListenerBuilder(meshCatalog catalog.MeshCataloger, svcIdentity identity.ServiceIdentity, cfg configurator.Configurator, statsHeaders map[string]string) *listenerBuilder {
+func newListenerBuilder(meshCatalog catalog.MeshCataloger, svcIdentity identity.ServiceIdentity, cfg configurator.Configurator, statsHeaders map[string]string, trustDomain string) *listenerBuilder {
 	return &listenerBuilder{
 		meshCatalog:     meshCatalog,
 		serviceIdentity: svcIdentity,
 		cfg:             cfg,
 		statsHeaders:    statsHeaders,
+		trustDomain:     trustDomain,
 	}
 }

pkg/envoy/lds/types.go

@@ -19,4 +19,5 @@ type listenerBuilder struct {
 	meshCatalog     catalog.MeshCataloger
 	cfg             configurator.Configurator
 	statsHeaders    map[string]string
+	trustDomain     string
 }

pkg/envoy/rbac/policy.go

@@ -3,27 +3,30 @@ package rbac
 import (
 	xds_rbac "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
 	xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
+
+	"github.com/openservicemesh/osm/pkg/identity"
 )
 
 // PolicyBuilder is a utility for constructing *xds_rbac.Policy's
 type PolicyBuilder struct {
 	allowedPorts       []uint32
-	allowedPrincipals  []string
+	allowedIdentities  []identity.ServiceIdentity
 	allowAllPrincipals bool
 
 	// All permissions are applied using OR semantics by default. If applyPermissionsAsAnd is set to true, then
 	// permissions are applied using AND semantics.
 	applyPermissionsAsAnd bool
+	trustDomain           string
 }
 
 // Build constructs an RBAC policy for the policy object on which this method is called
 func (p *PolicyBuilder) Build() *xds_rbac.Policy {
 	policy := &xds_rbac.Policy{}
 
 	// Each RuleList follows OR semantics with other RuleList in the list of RuleList
-	prinicipals := make([]*xds_rbac.Principal, 0, len(p.allowedPrincipals))
-	for _, principal := range p.allowedPrincipals {
-		prinicipals = append(prinicipals, GetAuthenticatedPrincipal(principal))
+	prinicipals := make([]*xds_rbac.Principal, 0, len(p.allowedIdentities))
+	for _, svcIdentity := range p.allowedIdentities {
+		prinicipals = append(prinicipals, GetAuthenticatedPrincipal(svcIdentity.AsPrincipal(p.trustDomain)))
 	}
 	if len(prinicipals) == 0 {
 		// No principals specified for this policy, allow ANY
@@ -62,21 +65,21 @@ func (p *PolicyBuilder) UseANDForPermissions(val bool) {
 	p.applyPermissionsAsAnd = val
 }
 
-// AddPrincipal adds a principal to the list of allowed principals
-func (p *PolicyBuilder) AddPrincipal(principal string) {
+// AddIdentity adds an identity, later to be converted to a principal, to the list of allowed identities.
+func (p *PolicyBuilder) AddIdentity(svcIdentity identity.ServiceIdentity) {
 	// We need this extra defense in depth because it is currently possible to configure a wildcard principal
 	// in addition to specific principals. Future changes may look to avoid this.
-	if principal == "*" {
-		p.AllowAnyPrincipal()
+	if svcIdentity.IsWildcard() {
+		p.AllowAnyIdentity()
 	}
 	if !p.allowAllPrincipals {
-		p.allowedPrincipals = append(p.allowedPrincipals, principal)
+		p.allowedIdentities = append(p.allowedIdentities, svcIdentity)
 	}
 }
 
-// AllowAnyPrincipal allows any principal to access the permissions.
-func (p *PolicyBuilder) AllowAnyPrincipal() {
-	p.allowedPrincipals = nil
+// AllowAnyIdentity allows any principal to access the permissions.
+func (p *PolicyBuilder) AllowAnyIdentity() {
+	p.allowedIdentities = nil
 	p.allowAllPrincipals = true
 }
 
@@ -86,6 +89,11 @@ func (p *PolicyBuilder) AddAllowedDestinationPort(port uint16) {
 	p.allowedPorts = append(p.allowedPorts, uint32(port))
 }
 
+// SetTrustDomain sets the trust domain for the policy, which is used when converting a ServiceIdentity to a Principal.
+func (p *PolicyBuilder) SetTrustDomain(td string) {
+	p.trustDomain = td
+}
+
 // GetAuthenticatedPrincipal returns an authenticated RBAC principal object for the given principal
 func GetAuthenticatedPrincipal(principalName string) *xds_rbac.Principal {
 	return &xds_rbac.Principal{

pkg/envoy/rbac/policy_test.go

@@ -6,21 +6,24 @@ import (
 
 	tassert "github.com/stretchr/testify/assert"
 
+	"github.com/openservicemesh/osm/pkg/identity"
+
 	xds_rbac "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
 	xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
 )
 
 func TestBuild(t *testing.T) {
 	testCases := []struct {
 		name                  string
-		principals            []string
+		identities            []identity.ServiceIdentity
 		ports                 []uint16
 		applyPermissionsAsAND bool
+		trustDomain           string
 		expectedPolicy        *xds_rbac.Policy
 	}{
 		{
 			name:       "testing rules for single principal",
-			principals: []string{"foo.domain", "bar.domain"},
+			identities: []identity.ServiceIdentity{identity.New("foo", "domain"), identity.New("bar", "domain")},
 			ports:      []uint16{80},
 			expectedPolicy: &xds_rbac.Policy{
 				Principals: []*xds_rbac.Principal{
@@ -29,7 +32,7 @@ func TestBuild(t *testing.T) {
 							Authenticated: &xds_rbac.Principal_Authenticated{
 								PrincipalName: &xds_matcher.StringMatcher{
 									MatchPattern: &xds_matcher.StringMatcher_Exact{
-										Exact: "foo.domain",
+										Exact: "foo.domain.cluster.local",
 									},
 								},
 							},
@@ -40,7 +43,7 @@ func TestBuild(t *testing.T) {
 							Authenticated: &xds_rbac.Principal_Authenticated{
 								PrincipalName: &xds_matcher.StringMatcher{
 									MatchPattern: &xds_matcher.StringMatcher_Exact{
-										Exact: "bar.domain",
+										Exact: "bar.domain.cluster.local",
 									},
 								},
 							},
@@ -57,17 +60,18 @@ func TestBuild(t *testing.T) {
 			},
 		},
 		{
-			name:       "testing rules for single principal",
-			principals: []string{"foo.domain"},
-			ports:      []uint16{80, 443},
+			name:        "testing rules for single principal",
+			identities:  []identity.ServiceIdentity{identity.New("foo", "domain")},
+			trustDomain: "cluster.local",
+			ports:       []uint16{80, 443},
 			expectedPolicy: &xds_rbac.Policy{
 				Principals: []*xds_rbac.Principal{
 					{
 						Identifier: &xds_rbac.Principal_Authenticated_{
 							Authenticated: &xds_rbac.Principal_Authenticated{
 								PrincipalName: &xds_matcher.StringMatcher{
 									MatchPattern: &xds_matcher.StringMatcher_Exact{
-										Exact: "foo.domain",
+										Exact: "foo.domain.cluster.local",
 									},
 								},
 							},
@@ -92,17 +96,18 @@ func TestBuild(t *testing.T) {
 			// Note that AND ports wouldn't make sense, since you can't have 2 ports at once, but we use it to test
 			// the logic.
 			name:                  "testing rules for AND ports",
-			principals:            []string{"foo.domain"},
+			identities:            []identity.ServiceIdentity{identity.New("foo", "domain")},
 			ports:                 []uint16{80, 443},
 			applyPermissionsAsAND: true,
+			trustDomain:           "cluster.local",
 			expectedPolicy: &xds_rbac.Policy{
 				Principals: []*xds_rbac.Principal{
 					{
 						Identifier: &xds_rbac.Principal_Authenticated_{
 							Authenticated: &xds_rbac.Principal_Authenticated{
 								PrincipalName: &xds_matcher.StringMatcher{
 									MatchPattern: &xds_matcher.StringMatcher_Exact{
-										Exact: "foo.domain",
+										Exact: "foo.domain.cluster.local",
 									},
 								},
 							},
@@ -133,7 +138,7 @@ func TestBuild(t *testing.T) {
 		},
 		{
 			name:       "testing rule for ANY principal when no ports specified",
-			principals: []string{"foo.domain", "*"},
+			identities: []identity.ServiceIdentity{identity.New("foo", "domain"), identity.WildcardServiceIdentity},
 			expectedPolicy: &xds_rbac.Policy{
 				Principals: []*xds_rbac.Principal{
 					{
@@ -169,16 +174,16 @@ func TestBuild(t *testing.T) {
 			assert := tassert.New(t)
 
 			pb := &PolicyBuilder{}
-			for _, principal := range tc.principals {
-				pb.AddPrincipal(principal)
+			for _, svcIdentity := range tc.identities {
+				pb.AddIdentity(svcIdentity)
 			}
 			for _, port := range tc.ports {
 				pb.AddAllowedDestinationPort(port)
 			}
 
 			pb.UseANDForPermissions(tc.applyPermissionsAsAND)
 			policy := pb.Build()
-			assert.Equal(policy, tc.expectedPolicy)
+			assert.Equal(tc.expectedPolicy, policy)
 		})
 	}
 }

pkg/envoy/rds/route/rbac.go

@@ -30,7 +30,7 @@ func buildInboundRBACFilterForRule(rule *trafficpolicy.Rule) (map[string]*any.An
 
 	// Create the list of principals for this policy
 	for downstream := range rule.AllowedServiceIdentities.Iter() {
-		pb.AddPrincipal(downstream.(identity.ServiceIdentity).String())
+		pb.AddIdentity(downstream.(identity.ServiceIdentity))
 	}
 
 	// A single RBAC policy per route