Update naming and definition for access control #20

grampelberg · 2019-05-02T23:04:15Z

Access Control

Change TrafficRole to TrafficTarget. This is to hopefully clear up that this is authorization and for the target of traffic, not the mutual connection between client and server.
Move aware from resources to pod selectors. This is to allow a little bit more flexible policy (fit into the canary use case) and make the target definition a little bit more clear. Pods being selected is now explicit instead of the implicit relationship through higher order resources such as deployments.
Change specs to matches. This feels like a slightly better word and is 1:1 with the new definition in HTTPRoutes.
Change TrafficRoleBinding to IdentityBinding. This makes it explicit that the binding is an identity and not something else.
Call out the use of groups a little bit more. This is the best way to do allow all policies and disable identity for some endpoints (via system:unauthenticated).
Show an example of using matchExpressions to show a TrafficTarget that works for everything (or as close as possible).

Traffic Spec

Change routes to matches so that the interface can be general across protocols.
Add filters to allow for matching things other than a URI. This is currently useful for filtering the Host header FQDN/port combination.

grampelberg · 2019-05-02T23:06:34Z

@slack do these new names and structures make the access control side of this a little more obvious?

@aanandr check out the IdentityBinding section, in particular system:unauthenticated. Does that work for your mesh external use case?

@nicholasjackson this all fits together a little bit better, there were some ugly edge cases that I tripped over earlier today. WDYT?

grampelberg · 2019-05-02T23:12:35Z

@stefanprodan check out how HTTPRoutes and TrafficTargets work together. The idea is to have a new object that works with HTTPRoutes to apply other types of policy across those routes (retries, timeouts and rate limits). Check out #19 to track the thinking there. Definitely needs some more thought first =)

traffic-access-control.md

slack · 2019-05-06T18:12:52Z

@grampelberg I think this is a lot better.

I was thinking through the typical "client/server" mental model over the weekend, I think that's the thing that makes this hilariously slippery!

The statement "clients have access to services" in SMI becomes "IdentityBindings have permission to TrafficTargets". Would changing IdentityBinding to TrafficSource or TrafficSourceBinding help the mental model?

Otherwise, this is a great improvement over Role-bits. LGTM.

grampelberg · 2019-05-06T22:36:54Z

I thought about TrafficSource and had a good reason for not going down that route. @olix0r you wouldn't happen to remember would you? I think it didn't work when thinking about other policies like rate limits and retries.

nicholasjackson · 2019-05-07T05:09:58Z

LGTM

aanandr · 2019-05-14T06:37:32Z

traffic-access-control.md

+### ClusterTrafficTarget
+
+A `ClusterTrafficTarget` allows policy to be applied to targets that live in
+multiple namespaces. This is primarily useful to system wide endpoints such as


What does this mean? The target is a service. What does it mean to say that target resides in multiple namespaces?

Think global allow policies, there would be multiple things serving traffic and they'd live in multiple namespaces.

Thanks, that makes sense

aanandr · 2019-05-14T06:41:34Z

traffic-access-control.md

-*server* side of a connection. It is up to implementations to decide whether
-they would also like to enforce access control on the *client* side
-of the connection as well.
+This example uses the `HTTPRoutes` definition from


A few questions here

How will we handle access definition to multiple such services? Would we create a separate ClusterTrafficTarget spec for each or is the matchexpression rich in terms of accepting lists/wildcards?

What is the namespace parameter under "rules" for? So far rules were generic and didnt have any namespace associated with them

aanandr · 2019-05-14T13:37:40Z

traffic-access-control.md

  name: path-specific
 ```

 This example allows any unauthenticated client access to the rules defined in
-`path-specific`.
+the `path-specific` TrafficTarget. Groups are more flexible than just
+unauthenticated users. Kubernetes defines many groups by default,


Does such a group (similar to the system one used in this example) have to be created upfront? Would like to understand this a bit more

aanandr · 2019-05-14T13:50:49Z

traffic-access-control.md

-## RBAC
+Continuing with the Prometheus example from above, it is possible to have a
+IdentityBinding that grants a specific system service access to an endpoint on
+every pod in the cluster.


Would this be done by playing with the "matches" param a bit and providing an "access all" kind of option in HttpRoutes?

aanandr · 2019-05-14T13:58:57Z

traffic-access-control.md


 ## Example Implementation

+TODO ...
+
 ## Tradeoffs



I would like to understand how all this will be used by customers to define access policies for their service mesh. From my understanding it looks like the moment a Policy is defined and applied all communication between all services (both originating from within the cluster and from outside) and destined to the selected service (e.g. "foo" used here) will be blocked except for the ones allowed per the policy. This is a good design but i am also wondering how customers will handle situations where there are many services wanting to talk to a single service. We have a mechanism for talking "TO" multiple services. Do we need one for requests originating "FROM" multiple services and going to a single service?

If i want to restrict traffic originating from a Pod/Service and going to an endpoint outside the cluster how do i do that? For e.g., i may want the Pod to be able to access only specific endpoints in my network and residing outside the cluster. I think this has been called out as out of scope below - maybe the solution is something else but bringing up the point here because we are discussing access policies. Would we use Network Policies?

aanandr · 2019-05-14T14:03:16Z

traffic-access-control.md

+  access control as it selects specific pods and not hostnames. Another object
+  will need to be created to manage this use case.
+
+* Ingress policy - assuming clients present the correct identity, this *should*


Ingress Policy kind of touches upon the point i raised earlier

aanandr · 2019-05-14T14:04:09Z

traffic-access-control.md

+  common use cases (filtering by hostname) and will need to be expanded to cover
+  this use case.
+
+* Other types of policy - having policy around retries, timeouts and rate limits


Wouldnt retries and rate limiting also fall under traffic mgmt/routing?

aanandr · 2019-05-14T14:05:36Z

traffic-access-control.md


-* Non-HTTP traffic - this specification will need to be expanded to support
-  traffic such as Kafka or MySQL.
+* Other types of identity - there is room to expand the `kind` accepted for


This will be very useful, for e.g. tying identity to cloud resident identity providers/services like AAD. But for now K8S service accounts is a good start.

aanandr · 2019-05-14T14:09:56Z

traffic-access-control.md

-This example grants the ability to access the routes in `path-specific` to any
-client providing the identity `bar` based on a ServiceAccount.
+This example grants the ability to access the `/metrics` route to any client
+providing the identity `bar`, based on a ServiceAccount.

 As access control is additive, it is important to provide definitions that allow
 non-authenticated traffic access. Imagine rolling a service mesh out


Cool. I think groups here are addressing the earlier comment i had for not enforcing mTLS for a service for all communication that it terminates

stefanprodan reviewed May 3, 2019

View reviewed changes

traffic-access-control.md Outdated Show resolved Hide resolved

traffic-access-control.md Outdated Show resolved Hide resolved

grampelberg added 2 commits May 6, 2019 15:39

Update naming and definition for access control

2e11030

Add out of scope for other types of policies

435f8b7

grampelberg force-pushed the grampelberg/access-updates branch from 3a3c579 to 435f8b7 Compare May 6, 2019 22:39

Fix typos

7cb1a8c

grampelberg merged commit 9d420fd into master May 7, 2019

grampelberg deleted the grampelberg/access-updates branch May 7, 2019 15:14

aanandr reviewed May 14, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update naming and definition for access control #20

Update naming and definition for access control #20

grampelberg commented May 2, 2019

grampelberg commented May 2, 2019

grampelberg commented May 2, 2019

slack commented May 6, 2019

grampelberg commented May 6, 2019

nicholasjackson commented May 7, 2019

aanandr May 14, 2019

grampelberg May 14, 2019

aanandr May 14, 2019

aanandr May 14, 2019 •

edited

Loading

aanandr May 14, 2019

aanandr May 14, 2019

aanandr May 14, 2019 •

edited

Loading

aanandr May 14, 2019

aanandr May 14, 2019

aanandr May 14, 2019

aanandr May 14, 2019

Update naming and definition for access control #20

Update naming and definition for access control #20

Conversation

grampelberg commented May 2, 2019

Access Control

Traffic Spec

grampelberg commented May 2, 2019

grampelberg commented May 2, 2019

slack commented May 6, 2019

grampelberg commented May 6, 2019

nicholasjackson commented May 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aanandr May 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aanandr May 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aanandr May 14, 2019 •

edited

Loading

aanandr May 14, 2019 •

edited

Loading