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

resolving destination service accounts to workload #82

Closed
rickducott opened this issue Oct 31, 2019 · 9 comments
Closed

resolving destination service accounts to workload #82

rickducott opened this issue Oct 31, 2019 · 9 comments

Comments

@rickducott
Copy link

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:
servicemeshinterface/smi-adapter-istio#70

Istio bug fix + discussion about path forward:
istio/istio#17430

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

@grampelberg
Copy link
Collaborator

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.

@rickducott
Copy link
Author

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.

@grampelberg
Copy link
Collaborator

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.

@rickducott
Copy link
Author

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

@nicholasjackson
Copy link
Collaborator

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?

@grampelberg
Copy link
Collaborator

grampelberg commented Nov 4, 2019

@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...

@rickducott
Copy link
Author

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
Copy link
Author

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.

@lachie83
Copy link
Contributor

Related proposal in #105

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants