Controller is vulnerable to being a confused deputy (without an external ID)

[seh]

In the current design, the controller creates a Kubernetes Secret object when it notices one or more pods that wish to mount a Secret with the right name. Any pods within that namespace can mount that same Secret object, regardless of governing RBAC permissions, and any pods in any namespace can assume whichever IAM roles trust the controller's initial role. That's a broad entitlement for role delegation.

In kiam, there's a way to limit which IAM roles pod authors can assume within a namespace, but even that doesn't go far enough: It allows the Kubernetes administrator some control over IAM role assumption, but it does not help the IAM roles' owners limit their trust scope. The IAM roles' owners must trust kiam or kube-aws-aim-controller for all the pods in an entire Kubernetes cluster; essentially, the role owners must trust an entire cluster. That's too broad of a scope with a multi-tenant cluster, where an IAM role owner might trust only a namespace, or only particular pods within the namespace.

By forcing the IAM role owners to trust the entire cluster, kube-aws-iam-controller becomes a confused deputy. Pod authors in one namespace may be able to learn the names of Secret objects in another namespace—as some multi-tenant clusters allow inter-namespace observation, if not mutation—and once they have that role name, they too can use it, even if the given IAM role owner only intended pods for one tenant to be able to assume it.

One way around this problem as prescribed by AWS is to use an External ID. When assuming the IAM role nominated by the Secret name suffix, the assuming STS client will include an external ID specific to the requesting party—in this case, a Kubernetes pod—that the IAM role owner can assess in his trust policy for the role being assumed.

Here, kube-aws-iam-controller could supply three pieces of information to the IAM role owner:

• pod name
• containing namespace
• cluster ID (optional)

With these three pieces, an IAM role owner could limit the scope of his trust, down from many Kubernetes clusters—in the case where kube-aws-iam-controller's initial assumed role is shared among clusters—to a single cluster, further down to a single namespace within a cluster, and even further down to a prefix name of the pods within a namespace.

The external ID syntax would need to preclude false aliasing. We know that neither namespace names nor pod names can contain a forward slash, but we have no idea what constitutes a cluster ID, so we can construct the external ID as

namespace name / pod name [ / cluster ID ]

treating the suffix as optional, only included if kube-aws-iam-controller is configured with a nonempty cluster ID. It may be useful to forbid a cluster ID to include a forward slash character ('/').

With this external ID include in its calls to sts:AssumeRole, kube-aws-iam-controller would allow the IAM role owner to include conditions in its trust policy, using the string condition operators to match expected values:

Constraint	Condition
Within namespace	{"StringLike": {"sts:ExternalId": "trusted-ns/*"}}
Pods with generated name	{"StringLike": {"sts:ExternalId": "trusted-ns/server-*"}}
Within cluster	{"StringLike": {"sts:ExternalId": "*/*/cluster-1"}}
Within specific pod	{"StringLike": {"sts:ExternalId": "trusted-ns/server-0/cluster-1"}}

Note that due to the limited capability of the StringLike operator, the "Within cluster" example above is vulnerable to false aliasing if a cluster ID can contain forward slashes, assuming that the StringLike operator uses greedy matching for wildcards. Likewise, it is difficult to match a pod name prefix of, say, "server-" when another set of pods could use the prefix "server-impostor-."

Some refinement is possible for the external ID format, but I hope this makes the idea clear enough: Without an external ID, kube-aws-iam-controller's trust scope is too broad for some environments, and AWS already has a way of narrowing such a scope.

The main problem I see here is that Kubernetes Secret names are scoped to the containing namespace, so kube-aws-iam-controller can't easily include a pod name in an external ID; any number of pods in the same namespace could mount the Secret, so long as the first pod's projected external ID satisfies the assumed IAM role's trust criteria. Trying to create a Secret that only some pods within a namespace could make use of is complicated, if not impossible. (Perhaps a validating admission control plugin could handle that, though an IAM role's trust criteria could change after the pod is created.) Given that, omitting the pod name and using just the namespace name and optional cluster ID (with the format reduced to namespace name / [ cluster ID ]) would still be an improvement.

I'd like to hear your thoughts on the proposal. In the meantime, thank you for your work on the project!

[mikkeloscar]

Thanks a lot for the very detailed feature request!

As you have noticed, until now, this project has solely focused on making a robust alternative to something like kube2iam, but has not touched on topics like multi-tenant clusters.

While I don't have a use case right now, for what you propose, I would be happy to review PRs making this an opt-in feature. :)

I like the idea of using externalID with the namespace and optional clusterID as you propose, however, the pod name part I would leave out because it has the problems you mention and this is more of a general "problem" of how Kubernetes secrets work. I would not try to solve the problem in this project, but rather on a higher level e.g. through an admission controller like you also mention.
Pods are also often dynamic and managed by a controller of some sort, so what you really would want to express is that: "only the pods of this 'application' (e.g. deployment) can read this and that secret", rather than naming the pods explicitly in the access granting feature.

[seh]

Are you amenable to reviewing a patch that introduces use of an external ID, as described above? I can see a path to introducing it that doesn’t look too invasive to my eye.

Still in need of investigation is how IAM role trust relationship constraint wildcards work. I want to understand that fully before committing to the format for these external IDs, as being able to match their components with wildcards—without falling prey to false aliases—is useful, if not essential.

[mikkeloscar]

Yes, I would be happy to review PRs for this. I think it's a good feature, just didn't have any time myself to look at it :)

[seh]

I have this implemented. It needs further testing with real STS calls to exercise the IAM policy condition operators, and then documentation. Once I'm sure that it works as intended, I'll post the PR, with documentation coming along once we agree on the detailed design choices.

[mikkeloscar]

Cool! Looking forward to that!