smi-spec icon indicating copy to clipboard operation
smi-spec copied to clipboard

Additional comments/questions on Traffic Access Control

Open aanandr opened this issue 5 years ago • 8 comments

Had the following questions/comments

  1. There is a note that says authentication is handled by underlying implementation. But we need a way to explicitly say that mTLS is needed to communicate with certain services and also specify exceptions. Some of this was there in the first version of the spec.

  2. Specifying the destination a. Selection is happening based on service account. Multiple Pods across multiple services may have the same service account. It should be possible to also specify service names in the destination. It would be the name that customer uses to create the service.

    b. The spec currently says the following

Allowing destination traffic should only be possible with permission of the service owner. Therefore, RBAC rules should be configured to control the pods which are allowed to assign the ServiceAccount defined in the TrafficTarget destination.

This addresses the concern that we dont want spurious services illegally using a service-account but when we allow service name to be specified in the destination then client side enforcement will be required and so it will be good to also include service-account info in the destination.

  1. Specifying the source a. We need to be able to specify the service names here too (that are allowed) along with the service account info

  2. Out of scope section Egress Policy - I think we cant keep this out of scope for too long. Istio supports Pods accessing resources outside the cluster. We may need some policies to restrict what can be accessed. Do we use Network Policies for that? Ingress Policy - is this about accessing services from outside the cluster?

aanandr avatar May 15 '19 02:05 aanandr

Hey @aanandr, few answers for you

  1. I agree, we really need to start the part of the spec which defines the service itself including ports and association to route.
  2. At the moment service account is used for identification purposes, in all honesty, I think the missing piece of the puzzle is the definition of a service. Identity should be associated with Service and service referenced in the destination.
  3. I think this is pretty much related to 2, and I agree
  4. Egress is incredibly important and in my humble opinion one of the most underused mechanisms to secure applications. With controlled Egress, you can not create reverse SSH shells, install packages, download binaries, all useful mechanisms for lateral movement if a service is compromised. It feels like this could be an HTTPRouteGroup with a direction applied. I might try and draft something on that tomorrow. Ingress specifically relates to authenticated traffic from within the cluster, not related to K8s ingress resources.

nicholasjackson avatar May 15 '19 17:05 nicholasjackson

indeed, MTLS is missing from the spec, or specifying routes with HTTPS

abonas avatar May 22 '19 11:05 abonas

There is a note that says authentication is handled by underlying implementation. But we need a way to explicitly say that mTLS is needed to communicate with certain services and also specify exceptions. Some of this was there in the first version of the spec.

Any ideas here? By definition, if an identity is required, mTLS is required (currently at least). As the spec is defined, it is possible to required authenticated users but not specify the specific names. This functionally requires mTLS.

I'm all on board for explicitly saying mTLS is required, just feels like extra configuration at the moment at least. Mostly, I've not come up with a good idea myself yet.

a. Selection is happening based on service account. Multiple Pods across multiple services may have the same service account. It should be possible to also specify service names in the destination. It would be the name that customer uses to create the service.

Can you explain a little bit more where you're going here? I don't quite understand the use case.

a. We need to be able to specify the service names here too (that are allowed) along with the service account info

Not sure I understand here either, you can't send traffic from a service.

Egress Policy - I think we cant keep this out of scope for too long. Istio supports Pods accessing resources outside the cluster. We may need some policies to restrict what can be accessed. Do we use Network Policies for that?

Totally agreed! Network policies won't work, as they don't allow for DNS egress and an egress policy should really be separate from access control. That's why we changed the name from policy early on in the process =)

Ingress Policy - is this about accessing services from outside the cluster?

I don't understand this use case. Could you explain a little bit more?

grampelberg avatar Jun 13 '19 16:06 grampelberg

There is a note that says authentication is handled by underlying implementation. But we need a way to explicitly say that mTLS is needed to communicate with certain services and also specify exceptions. Some of this was there in the first version of the spec.

Any ideas here? By definition, if an identity is required, mTLS is required (currently at least). As the spec is defined, it is possible to required authenticated users but not specify the specific names. This functionally requires mTLS.

I'm all on board for explicitly saying mTLS is required, just feels like extra configuration at the moment at least. Mostly, I've not come up with a good idea myself yet.

We dont have to explicitly state that mTLS is required but we can provide a way to explicitly state that it is optional (equivalent of permissive mode in Istio). One idea is to add it to the spec portion under Destination in TrafficTarget

a. Selection is happening based on service account. Multiple Pods across multiple services may have the same service account. It should be possible to also specify service names in the destination. It would be the name that customer uses to create the service.

Can you explain a little bit more where you're going here? I don't quite understand the use case.

I am saying that the destination selection shouldnt be done based on just ServiceAccount. We should be able to do the selection based on a service name. We could add a new kind under destination for service.

a. We need to be able to specify the service names here too (that are allowed) along with the service account info

Not sure I understand here either, you can't send traffic from a service.

True. But we could still specify a service here which could translate and apply to Pods that make up the service.

Egress Policy - I think we cant keep this out of scope for too long. Istio supports Pods accessing resources outside the cluster. We may need some policies to restrict what can be accessed. Do we use Network Policies for that?

Totally agreed! Network policies won't work, as they don't allow for DNS egress and an egress policy should really be separate from access control. That's why we changed the name from policy early on in the process =)

A new HttpRouteGroup as suggested by @nicholasjackson is a good start and it can be used to cover FQDNs and maybe IP ranges too (?)

Ingress Policy - is this about accessing services from outside the cluster?

I don't understand this use case. Could you explain a little bit more?

Please ignore this

aanandr avatar Jun 22 '19 02:06 aanandr

@aanandr any reasons you closed the issue? Feels like a great discussion!

We dont have to explicitly state that mTLS is required but we can provide a way to explicitly state that it is optional (equivalent of permissive mode in Istio). One idea is to add it to the spec portion under Destination in TrafficTarget

That was called out in a previous version of the spec as well, you can just use system:unauthenticated and it is permissive.

I am saying that the destination selection shouldnt be done based on just ServiceAccount. We should be able to do the selection based on a service name. We could add a new kind under destination for service.

I’m still not understanding, you can’t be authenticated as a service name and just like ServiceAccounts, multiple pods can be in multiple services. In fact, it is worse because with ServiceAccounts the identity is locked to the pod’s lifecycle. With services, it is dynamic and pretty much non-discoverable.

True. But we could still specify a service here which could translate and apply to Pods that make up the service.

We tried this a couple times and it was extremely confusing for users. I’d like to have everything be based off selectors that are explicit. That matches the k8s API and other upcoming specs closely and gives you the ability to do binding. This ended up getting moved out of the spec because of time constraints and it is something I’d love to add back in.

A new HttpRouteGroup as suggested by @nicholasjackson is a good start and it can be used to cover FQDNs and maybe IP ranges too (?)

I’d be a separate resource as HttpRouteGroup is really, specifically, for just defining routes. Something like TrafficEgress would match the current naming scheme pretty well. I’d leave IP ranges 100% to NetworkPolicy. That’s the right layer for IP level filtering.

grampelberg avatar Jun 26 '19 15:06 grampelberg

I think i accidentally closed this issue. Reopened now. Thanks for your comments @grampelberg . My main point is shouldnt we have policies that allow customers to specify which services are allowed to talk to each other. In a microservices world where there are apps mapping to 100s of services, doesnt this become an important policy capability? I agree that service account also plays an important role and we should include it. But how do we bring in the service angle? Or, is there mostly a one to one mapping between service account and service (this is what i observed in the examples used for Istio)?

The other point is that we should have the ability to make authorization optional and do only mTLS for service-service encryption. Many customers want only encryption for security purposes to begin with.

aanandr avatar Jun 27 '19 02:06 aanandr

My main point is shouldnt we have policies that allow customers to specify which services are allowed to talk to each other. In a microservices world where there are apps mapping to 100s of services, doesnt this become an important policy capability? I agree that service account also plays an important role and we should include it. But how do we bring in the service angle? Or, is there mostly a one to one mapping between service account and service (this is what i observed in the examples used for Istio)?

  • I don’t think we should stop at using service accounts for identity, there are other forms of identity that are super important (like users with JWTs).
  • If we’re talking about “services” in a general sense, I agree 100%. We’ll need to come up with a solid source of identity and map it to pods for that to work. Unfortunately, k8s Service isn’t particularly usable for this use case and we might want to build more on top of that to solve the use case.

The other point is that we should have the ability to make authorization optional and do only mTLS for service-service encryption. Many customers want only encryption for security purposes to begin with.

Yup, that’s my point around the identity being system:authenticated instead of a specific service account. Technically the spec already supports that use case, I suspect there will be some bumps along the road of implementation though.

grampelberg avatar Jun 27 '19 17:06 grampelberg

One thing to add here and I am not sure we really got this nailed in the initial spec but...

With services we are covering a specific implementation based on Istio's configuration, if we introduce service as target type we end up with a non-portable implementation. The goal should be to create SMI specs which when related to an application could run on any mesh without change.

Kinda echoing @grampelberg thoughts here, but maybe we need to better define the abstraction for identity. The underlying controller should be responsible for translating this into an implementation. In this world ServiceAccount is a low level implementation not high level abstraction.

nicholasjackson avatar Jul 02 '19 04:07 nicholasjackson