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

resolving destination service accounts to workload

Open rickducott opened this issue 5 years ago • 9 comments

Currently the traffic targets define access control rules along a set of routes between a source and destination (https://github.com/deislabs/smi-spec/blob/master/traffic-access-control.md). Since we are trying to define rules such as "A can talk to B", I understand why A translates to an identity. However, I'm curious why we're using service accounts as destinations for B, instead of the service or workload itself?

I think this becomes problematic when trying to write an adapter for istio 1.4+, which has now deprecated the destination.user constraint on ServiceRole and the guidance is to migrate to workload selectors. These are simply pod selectors that may only conventionally encode service account info. In figuring out how to implement the controller for the SMI istio adapter on istio 1.4+, we'll need to either translate the service account in the target destination to a service, or translate it to a label selector -- I think this may be challenging in general. Here's how I discovered the issue, and the corresponding istio issue:

SMI adapter issue: https://github.com/deislabs/smi-adapter-istio/issues/70

Istio bug fix + discussion about path forward: https://github.com/istio/istio/pull/17430

Istio workload selector: https://github.com/istio/api/blob/1187adbd148251b20e7cd8e91f73ebcc09ac7ef1/type/v1beta1/selector.proto

rickducott avatar Oct 31 '19 22:10 rickducott

How does something like this look?

kind: TrafficTarget
apiVersion: access.smi-spec.io/v1alpha1
metadata:
  name: path-specific
  namespace: default
destination:
  kind: Service
  name: service-a
  namespace: default
  port: 8080
specs:
- kind: HTTPRouteGroup
  name: the-routes
  matches:
  - metrics
sources:
- kind: ServiceAccount
  name: prometheus
  namespace: default

If I remember correctly, there were implementation details at the last moment with Consul that made implementing something like this more difficult.

I dislike the destination requiring a service account personally and would love to revision the spec. This has the potential for being a little verbose when creating policies that span multiple services, but it probably isn't any more verbose than roles and rolebindings.

grampelberg avatar Oct 31 '19 22:10 grampelberg

I like this. I think the service ref is much more intuitive here, curious what made it tricky for Consul.

We could consider a service selector or a list of destinations reduce verbosity.

rickducott avatar Oct 31 '19 22:10 rickducott

Going through the git history, it was originally something more like:

selector:
  matchLabels:
    app: foo

Which would reduce verbosity, but I think it makes it harder to reason about what's going on. As service discovery happens on the Service level in k8s and TrafficSplit uses services natively, it feels like the right level of abstraction.

grampelberg avatar Oct 31 '19 22:10 grampelberg

Makes sense and agreed - much prefer things to be explicit, particularly on things affecting authorization.

rickducott avatar Oct 31 '19 22:10 rickducott

Didn't we not originally bounce the proposal of using labels as it was inherently insecure. Service accounts can be controlled using RBAC however I am not sure adding metadata can?

nicholasjackson avatar Nov 04 '19 16:11 nicholasjackson

@nicholasjackson can't you add arbitrary service accounts to pod specs? It feels like this might be a job for an admission controller like gatekeeper again...

grampelberg avatar Nov 04 '19 18:11 grampelberg

If you added an arbitrary service account to your pod, you may circumvent RBAC restrictions on incoming traffic, but potentially be sacrificing other needed permissions on the workload itself. I can see how this is less of a security concern than using services / selectors.

That said, the original problem stands: the loose correlation between services/service accounts in Kubernetes makes it unclear how to reason about or translate SMI traffic targets. Changing the destination to a service seems to be a security posture others (i.e. Istio) are comfortable with, and would resolve this. I'm open to alternatives as well, I just don't think the current spec is viable.

When time permits, I'll move this discussion to a PR.

rickducott avatar Nov 06 '19 03:11 rickducott

Discussed in the community meeting 1/8/20 - no real update yet. Related to discussion of how to extend the mesh to non-kubernetes VMs, and using service accounts is a very kube-specific abstraction. Related - Avinash also setting up a proposal for non-service account based representation of identity in the spec. Extending the mesh with istio is a good place to start looking at this.

rickducott avatar Jan 08 '20 18:01 rickducott

Related proposal in #105

lachie83 avatar Feb 19 '20 17:02 lachie83