Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

envoy/rbac: add support for server side RBAC fitler #2054

Merged
merged 2 commits into from
Nov 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions pkg/envoy/lds/rbac.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package lds

import (
"fmt"

xds_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
xds_rbac "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
xds_network_rbac "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/rbac/v3"
xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
"github.com/envoyproxy/go-control-plane/pkg/wellknown"

"github.com/openservicemesh/osm/pkg/envoy"
"github.com/openservicemesh/osm/pkg/identity"
"github.com/openservicemesh/osm/pkg/service"
)

// buildRBACFilter builds an RBAC filter based on SMI TrafficTarget policies.
// The returned RBAC filter has policies that gives downstream principals full access to the local service.
func (lb *listenerBuilder) buildRBACFilter() (*xds_listener.Filter, error) {
networkRBACPolicy, err := lb.buildInboundRBACPolicies()
shashankram marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
log.Error().Err(err).Msgf("Error building inbound RBAC policies for principal %q", lb.svcAccount)
return nil, err
}

marshalledNetworkRBACPolicy, err := envoy.MessageToAny(networkRBACPolicy)
if err != nil {
log.Error().Err(err).Msgf("Error marshalling RBAC policy: %v", networkRBACPolicy)
return nil, err
}

rbacFilter := &xds_listener.Filter{
Name: wellknown.RoleBasedAccessControl,
ConfigType: &xds_listener.Filter_TypedConfig{TypedConfig: marshalledNetworkRBACPolicy},
}

return rbacFilter, nil
}

// buildInboundRBACPolicies builds the RBAC policies based on allowed principals
func (lb *listenerBuilder) buildInboundRBACPolicies() (*xds_network_rbac.RBAC, error) {
allowsInboundSvcAccounts, err := lb.meshCatalog.ListAllowedInboundServiceAccounts(lb.svcAccount)
if err != nil {
log.Error().Err(err).Msgf("Error listing allowed inbound ServiceAccounts for ServiceAccount %q", lb.svcAccount)
return nil, err
}

log.Trace().Msgf("Building RBAC policies for ServiceAccount %q with allowed inbound %v", lb.svcAccount, allowsInboundSvcAccounts)

// Each downstream is a principal in the RBAC policy, which will have its own permissions
// based on SMI TrafficTarget policies.
rbacPolicies := make(map[string]*xds_rbac.Policy)
shashankram marked this conversation as resolved.
Show resolved Hide resolved
for _, downstreamSvcAccount := range allowsInboundSvcAccounts {
policyName := getPolicyName(downstreamSvcAccount, lb.svcAccount)
principal := identity.GetKubernetesServiceIdentity(downstreamSvcAccount, identity.ClusterLocalTrustDomain)
rbacPolicies[policyName] = buildAllowAllPermissionsPolicy(principal)
}

// Create an inbound RBAC policy that denies a request by default, unless a policy explicitly allows it
networkRBACPolicy := &xds_network_rbac.RBAC{
StatPrefix: "RBAC",
Rules: &xds_rbac.RBAC{
Action: xds_rbac.RBAC_ALLOW, // Allows the request if and only if there is a policy that matches the request
Policies: rbacPolicies,
},
}

return networkRBACPolicy, nil
}

// buildAllowAllPermissionsPolicy creates an XDS RBAC policy for the given client principal to be granted all access
func buildAllowAllPermissionsPolicy(clientPrincipal identity.ServiceIdentity) *xds_rbac.Policy {
return &xds_rbac.Policy{
Permissions: []*xds_rbac.Permission{
{
// Grant the given principal all access
Rule: &xds_rbac.Permission_Any{Any: true},
},
},
Principals: []*xds_rbac.Principal{
{
Identifier: &xds_rbac.Principal_OrIds{
OrIds: &xds_rbac.Principal_Set{
Ids: []*xds_rbac.Principal{
getPrincipalAuthenticated(clientPrincipal.String()),
},
},
},
},
},
}
}

// getPolicyName returns a policy name for the policy used to authorize a downstream service account by the upstream
func getPolicyName(downstream, upstream service.K8sServiceAccount) string {
return fmt.Sprintf("%s to %s", downstream, upstream)
}

func getPrincipalAuthenticated(principalName string) *xds_rbac.Principal {
return &xds_rbac.Principal{
Identifier: &xds_rbac.Principal_Authenticated_{
Authenticated: &xds_rbac.Principal_Authenticated{
PrincipalName: &xds_matcher.StringMatcher{
MatchPattern: &xds_matcher.StringMatcher_Exact{
Exact: principalName,
},
},
},
},
}
}
154 changes: 154 additions & 0 deletions pkg/envoy/lds/rbac_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package lds

import (
"fmt"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

xds_rbac "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
"github.com/envoyproxy/go-control-plane/pkg/wellknown"

"github.com/openservicemesh/osm/pkg/catalog"
"github.com/openservicemesh/osm/pkg/service"
)

func TestBuildInboundRBACPolicies(t *testing.T) {
assert := assert.New(t)
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

mockCatalog := catalog.NewMockMeshCataloger(mockCtrl)
proxySvcAccount := service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"}

lb := &listenerBuilder{
meshCatalog: mockCatalog,
svcAccount: proxySvcAccount,
}

testCases := []struct {
draychev marked this conversation as resolved.
Show resolved Hide resolved
name string
allowedInboundSvcAccounts []service.K8sServiceAccount
expectedPrincipals []string
expectErr bool
}{
{
name: "multiple client allowed",
allowedInboundSvcAccounts: []service.K8sServiceAccount{
{Name: "sa-2", Namespace: "ns-2"},
{Name: "sa-3", Namespace: "ns-3"},
},
expectedPrincipals: []string{
"sa-2.ns-2.cluster.local",
"sa-3.ns-3.cluster.local",
},
expectErr: false, // no error
},
{
name: "no clients allowed",
allowedInboundSvcAccounts: []service.K8sServiceAccount{},
expectedPrincipals: []string{},
expectErr: false, // no error
},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("Testing test case %d: %s", i, tc.name), func(t *testing.T) {
// Mock the calls to catalog
mockCatalog.EXPECT().ListAllowedInboundServiceAccounts(lb.svcAccount).Return(tc.allowedInboundSvcAccounts, nil).Times(1)

// Test the RBAC policies
networkRBAC, err := lb.buildInboundRBACPolicies()
assert.Equal(err != nil, tc.expectErr)

assert.Equal(networkRBAC.Rules.GetAction(), xds_rbac.RBAC_ALLOW)

rbacPolicies := networkRBAC.Rules.Policies

// Expect 1 policy per client principal
assert.Len(rbacPolicies, len(tc.expectedPrincipals))

// Loop through the policies and ensure there is a policy corresponding to each principal
var actualPrincipals []string
for _, policy := range rbacPolicies {
principalName := policy.Principals[0].GetOrIds().Ids[0].GetAuthenticated().PrincipalName.GetExact()
actualPrincipals = append(actualPrincipals, principalName)

assert.Len(policy.Permissions, 1) // Any permission
assert.True(policy.Permissions[0].GetAny())
}
assert.ElementsMatch(tc.expectedPrincipals, actualPrincipals)
})
}
}

func TestBuildRBACFilter(t *testing.T) {
assert := assert.New(t)
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

mockCatalog := catalog.NewMockMeshCataloger(mockCtrl)
proxySvcAccount := service.K8sServiceAccount{Name: "sa-1", Namespace: "ns-1"}

lb := &listenerBuilder{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use newListenerBuilder()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this allows to fill in minimal stuff into the object needed to build the filter. When a new field is added to the struct, if the test fails it helps find dependency issues faster.

meshCatalog: mockCatalog,
svcAccount: proxySvcAccount,
}

testCases := []struct {
name string
allowedInboundSvcAccounts []service.K8sServiceAccount
expectErr bool
}{
{
name: "multiple clients allowed",
allowedInboundSvcAccounts: []service.K8sServiceAccount{
{Name: "sa-2", Namespace: "ns-2"},
{Name: "sa-3", Namespace: "ns-3"},
},
expectErr: false, // no error
},
{
name: "no clients allowed",
allowedInboundSvcAccounts: []service.K8sServiceAccount{},
expectErr: false, // no error
},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("Testing test case %d", i), func(t *testing.T) {
// Mock the calls to catalog
mockCatalog.EXPECT().ListAllowedInboundServiceAccounts(lb.svcAccount).Return(tc.allowedInboundSvcAccounts, nil).Times(1)

// Test the RBAC filter
rbacFilter, err := lb.buildRBACFilter()
assert.Equal(err != nil, tc.expectErr)

assert.Equal(rbacFilter.Name, wellknown.RoleBasedAccessControl)
})
}
}

func TestGetPolicyName(t *testing.T) {
assert := assert.New(t)

testCases := []struct {
downstream service.K8sServiceAccount
upstream service.K8sServiceAccount
expectedName string
}{
{
downstream: service.K8sServiceAccount{Name: "foo", Namespace: "ns-1"},
upstream: service.K8sServiceAccount{Name: "bar", Namespace: "ns-2"},
expectedName: "ns-1/foo to ns-2/bar",
},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("Testing test case %d", i), func(t *testing.T) {
actual := getPolicyName(tc.downstream, tc.upstream)
assert.Equal(actual, tc.expectedName)
})
}
}
37 changes: 32 additions & 5 deletions pkg/envoy/lds/response.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package lds

import (
xds_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
xds_discovery "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3"
"github.com/golang/protobuf/ptypes"

Expand All @@ -9,6 +10,7 @@ import (
"github.com/openservicemesh/osm/pkg/configurator"
"github.com/openservicemesh/osm/pkg/constants"
"github.com/openservicemesh/osm/pkg/envoy"
"github.com/openservicemesh/osm/pkg/service"
)

const (
Expand All @@ -22,21 +24,29 @@ const (
// 1. Inbound listener to handle incoming traffic
// 2. Outbound listener to handle outgoing traffic
// 3. Prometheus listener for metrics
func NewResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_discovery.DiscoveryRequest, cfg configurator.Configurator, _ certificate.Manager) (*xds_discovery.DiscoveryResponse, error) {
svcList, err := catalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName())
func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_discovery.DiscoveryRequest, cfg configurator.Configurator, _ certificate.Manager) (*xds_discovery.DiscoveryResponse, error) {
svcList, err := meshCatalog.GetServicesFromEnvoyCertificate(proxy.GetCommonName())
if err != nil {
log.Error().Err(err).Msgf("Error looking up MeshService for Envoy with CN=%q", proxy.GetCommonName())
return nil, err
}
// Github Issue #1575
proxyServiceName := svcList[0]

svcAccount, err := catalog.GetServiceAccountFromProxyCertificate(proxy.GetCommonName())
if err != nil {
log.Error().Err(err).Msgf("Error retrieving SerivceAccount for proxy %s", proxy.GetCommonName())
return nil, err
}

resp := &xds_discovery.DiscoveryResponse{
TypeUrl: string(envoy.TypeLDS),
}

lb := newListenerBuilder(meshCatalog, svcAccount)
draychev marked this conversation as resolved.
Show resolved Hide resolved

// --- OUTBOUND -------------------
outboundListener, err := newOutboundListener(catalog, cfg, svcList)
outboundListener, err := newOutboundListener(meshCatalog, cfg, svcList)
if err != nil {
log.Error().Err(err).Msgf("Error making outbound listener config for proxy %s", proxyServiceName)
} else {
Expand All @@ -55,13 +65,23 @@ func NewResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_disco
inboundListener := newInboundListener()
if meshFilterChain, err := getInboundInMeshFilterChain(proxyServiceName, cfg); err != nil {
log.Error().Err(err).Msgf("Error making in-mesh filter chain for proxy %s", proxy.GetCommonName())
} else if meshFilterChain != nil {
} else {
if !cfg.IsPermissiveTrafficPolicyMode() {
draychev marked this conversation as resolved.
Show resolved Hide resolved
// Apply RBAC policies on the inbound filters based on configured policies
rbacFilter, err := lb.buildRBACFilter()
draychev marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
log.Error().Err(err).Msgf("Error applying RBAC filter for service %s", proxyServiceName)
return nil, err
}
// RBAC filter should be the very first filter in the filter chain
draychev marked this conversation as resolved.
Show resolved Hide resolved
meshFilterChain.Filters = append([]*xds_listener.Filter{rbacFilter}, meshFilterChain.Filters...)
}
inboundListener.FilterChains = append(inboundListener.FilterChains, meshFilterChain)
}

// --- INGRESS -------------------
// Apply an ingress filter chain if there are any ingress routes
if ingressRoutesPerHost, err := catalog.GetIngressRoutesPerHost(proxyServiceName); err != nil {
if ingressRoutesPerHost, err := meshCatalog.GetIngressRoutesPerHost(proxyServiceName); err != nil {
log.Error().Err(err).Msgf("Error getting ingress routes per host for service %s", proxyServiceName)
} else {
thereAreIngressRoutes := len(ingressRoutesPerHost) > 0
Expand Down Expand Up @@ -102,3 +122,10 @@ func NewResponse(catalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_disco

return resp, nil
}

func newListenerBuilder(meshCatalog catalog.MeshCataloger, svcAccount service.K8sServiceAccount) *listenerBuilder {
return &listenerBuilder{
meshCatalog: meshCatalog,
svcAccount: svcAccount,
}
}
8 changes: 8 additions & 0 deletions pkg/envoy/lds/types.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
package lds

import (
"github.com/openservicemesh/osm/pkg/catalog"
"github.com/openservicemesh/osm/pkg/logger"
"github.com/openservicemesh/osm/pkg/service"
)

var (
log = logger.New("envoy/lds")
)

// listenerBuilder is a type containing data to build the listener configurations
type listenerBuilder struct {
svcAccount service.K8sServiceAccount
meshCatalog catalog.MeshCataloger
draychev marked this conversation as resolved.
Show resolved Hide resolved
}