Skip to content

Commit

Permalink
RBAC: add the support of subject.group (istio#8048)
Browse files Browse the repository at this point in the history 
* RBAC: add the support of subject.group

In the RBAC policy, add the support of subject.group to the subjects in a ServiceRoleBinding.
A user can use subject.group or "request.auth.claims[groups]" to specify the
groups RBAC policy.

* Change based on the review comments

* subject.group overrides request.auth.claims[groups]

* Add a utility function to create test policy

* Add a log when group is defined twice
  • Loading branch information
@lei-tang @istio-testing
lei-tang authored and istio-testing committed Aug 24, 2018
1 parent cf98675 commit 1f3328d
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 11 deletions.
29 changes: 20 additions & 9 deletions pilot/pkg/networking/plugin/authz/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,15 @@ const (
attrRequestHeader = "request.headers" // header name is surrounded by brackets, e.g. "request.headers[User-Agent]".

// attributes that could be used in a ServiceRoleBinding property.
attrSrcIP = "source.ip" // supports both single ip and cidr, e.g. "10.1.2.3" or "10.1.0.0/16".
attrSrcNamespace = "source.namespace" // e.g. "default".
attrSrcUser = "source.user" // source identity, e.g. "cluster.local/ns/default/sa/productpage".
attrSrcPrincipal = "source.principal" // source identity, e,g, "cluster.local/ns/default/sa/productpage".
attrRequestPrincipal = "request.auth.principal" // authenticated principal of the request.
attrRequestAudiences = "request.auth.audiences" // intended audience(s) for this authentication information.
attrRequestPresenter = "request.auth.presenter" // authorized presenter of the credential.
attrRequestClaims = "request.auth.claims" // claim name is surrounded by brackets, e.g. "request.auth.claims[iss]".
attrSrcIP = "source.ip" // supports both single ip and cidr, e.g. "10.1.2.3" or "10.1.0.0/16".
attrSrcNamespace = "source.namespace" // e.g. "default".
attrSrcUser = "source.user" // source identity, e.g. "cluster.local/ns/default/sa/productpage".
attrSrcPrincipal = "source.principal" // source identity, e,g, "cluster.local/ns/default/sa/productpage".
attrRequestPrincipal = "request.auth.principal" // authenticated principal of the request.
attrRequestAudiences = "request.auth.audiences" // intended audience(s) for this authentication information.
attrRequestPresenter = "request.auth.presenter" // authorized presenter of the credential.
attrRequestClaims = "request.auth.claims" // claim name is surrounded by brackets, e.g. "request.auth.claims[iss]".
attrRequestClaimGroups = "request.auth.claims[groups]" // groups claim".

// attributes that could be used in a ServiceRole constraint.
attrDestIP = "destination.ip" // supports both single ip and cidr, e.g. "10.1.2.3" or "10.1.0.0/16".
Expand Down Expand Up @@ -577,7 +578,14 @@ func convertToPrincipal(subject *rbacproto.Subject) *policyproto.Principal {
}

if subject.Group != "" {
rbacLog.Errorf("ignored Subject.group %s, not implemented", subject.Group)
// Treat subject.Group as the request.auth.claims[groups] property. If
// request.auth.claims[groups] has been defined for the subject, subject.Group
// overrides request.auth.claims[groups].
if subject.Properties[attrRequestClaimGroups] != "" {
rbacLog.Errorf("Both subject.group and request.auth.claims[groups] are defined.\n")
}
rbacLog.Debugf("Treat subject.Group (%s) as the request.auth.claims[groups]\n", subject.Group)
subject.Properties[attrRequestClaimGroups] = subject.Group
}

if len(subject.Properties) != 0 {
Expand Down Expand Up @@ -675,6 +683,9 @@ func permissionForKeyValues(key string, values []string) *policyproto.Permission
return &policyproto.Permission{Rule: orRules}
}

// Create a Principal based on the key and the value.
// key: the key of a subject property.
// value: the value of a subject property.
func principalForKeyValue(key, value string) *policyproto.Principal {
switch {
case key == attrSrcIP:
Expand Down
92 changes: 90 additions & 2 deletions pilot/pkg/networking/plugin/authz/rbac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ import (
"reflect"
"testing"

"github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
"github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
rbacconfig "github.com/envoyproxy/go-control-plane/envoy/config/filter/http/rbac/v2"
policy "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v2alpha"
metadata "github.com/envoyproxy/go-control-plane/envoy/type/matcher"
"github.com/envoyproxy/go-control-plane/pkg/util"
"github.com/gogo/protobuf/types"

rbacproto "istio.io/api/rbac/v1alpha1"
"istio.io/istio/pilot/pkg/config/memory"
"istio.io/istio/pilot/pkg/model"

"github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
"github.com/gogo/protobuf/types"
)

func newIstioStoreWithConfigs(configs []model.Config, t *testing.T) model.IstioConfigStore {
Expand Down Expand Up @@ -232,6 +233,17 @@ func TestConvertRbacRulesToFilterConfig(t *testing.T) {
},
},
},
{
ConfigMeta: model.ConfigMeta{Name: "service-role-3"},
Spec: &rbacproto.ServiceRole{
Rules: []*rbacproto.AccessRule{
{
Services: []string{"allow-group"},
Methods: []string{"GET"},
},
},
},
},
}
bindings := []model.Config{
{
Expand Down Expand Up @@ -273,6 +285,21 @@ func TestConvertRbacRulesToFilterConfig(t *testing.T) {
},
},
},
{
ConfigMeta: model.ConfigMeta{Name: "service-role-binding-3"},
Spec: &rbacproto.ServiceRoleBinding{
Subjects: []*rbacproto.Subject{
{
Group: "group*",
Properties: map[string]string{},
},
},
RoleRef: &rbacproto.RoleRef{
Kind: "ServiceRole",
Name: "service-role-3",
},
},
},
}

policy1 := &policy.Policy{
Expand Down Expand Up @@ -545,6 +572,8 @@ func TestConvertRbacRulesToFilterConfig(t *testing.T) {
}},
}

policy3 := generatePolicyWithHTTPMethodAndGroupClaim("GET", "group*")

expectRbac1 := &policy.RBAC{
Action: policy.RBAC_ALLOW,
Policies: map[string]*policy.Policy{
Expand All @@ -558,6 +587,12 @@ func TestConvertRbacRulesToFilterConfig(t *testing.T) {
"service-role-2": policy2,
},
}
expectRbac3 := &policy.RBAC{
Action: policy.RBAC_ALLOW,
Policies: map[string]*policy.Policy{
"service-role-3": policy3,
},
}
testCases := []struct {
name string
service *serviceMetadata
Expand Down Expand Up @@ -599,6 +634,13 @@ func TestConvertRbacRulesToFilterConfig(t *testing.T) {
},
rbac: expectRbac2,
},
{
name: "test group",
service: &serviceMetadata{
name: "allow-group",
},
rbac: expectRbac3,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -979,3 +1021,49 @@ func generatePrincipal(principalName string) *policy.Principal {
},
}
}

func generatePolicyWithHTTPMethodAndGroupClaim(methodName, claimName string) *policy.Policy {
policy := &policy.Policy{
Permissions: []*policy.Permission{{
Rule: &policy.Permission_AndRules{
AndRules: &policy.Permission_Set{
Rules: []*policy.Permission{
{
Rule: &policy.Permission_OrRules{
OrRules: &policy.Permission_Set{
Rules: []*policy.Permission{
{
Rule: &policy.Permission_Header{
Header: &route.HeaderMatcher{
Name: ":method",
HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
ExactMatch: methodName,
},
},
},
},
},
},
},
},
},
},
},
}},
Principals: []*policy.Principal{{
Identifier: &policy.Principal_AndIds{
AndIds: &policy.Principal_Set{
Ids: []*policy.Principal{
{
Identifier: &policy.Principal_Metadata{
Metadata: generateMetadataListMatcher(
[]string{attrRequestClaims, "groups"}, claimName),
},
},
},
},
},
}},
}
return policy
}

0 comments on commit 1f3328d

Please sign in to comment.