Use of "iam.amazonaws.com/permitted" annotation causes conflicts with existing KIAM setups

[mmcaya]

I'm submitting a...

• bug report

• related to roleArn functionality described in issue Support to have IAM user Assume a Role when retrieving secret data #143
  and implemented in PR feat: add option to assume role #144

• Use of KIAM iam.amazonaws.com/permitted annotation for roleArn validation causes conflicts with existing KIAM setups.

• iam.amazonaws.com/permitted is a KIAM created and managed annotation, and not part of any known standard.
  (e.g. kube2iam uses a different annotation: iam.amazonaws.com/allowed-roles )

Issue 1:

What is the current behavior?

Kubernetes external secrets errors out when iam.amazonaws.com/permitted is present, has valid values intended for kiam, but is not provided a roleArn in the ExternalSecret descriptor.

This is a common use case when kiam is handling the IAM for the kubernetes-external-secret operator, and no additional role assumption is required by the individual ExternalSecret resources

To reproduce:

kind: ExternalSecret
metadata:
  annotations:
  name: test-service
secretDescriptor:
 #No Role ARN Provided
  backendType: secretsManager
  data:
  - key: test-service/password
    name: password

Namespace annotated for kiam support only:

kind: Namespace
metadata:
  annotations:
    iam.amazonaws.com/permitted: my-kiam-roles.* 

{"level":50,"time":XXX,"pid":16,"hostname":"XXX","type":"Error","stack":"Error: not allowed to fetch secret: test-service: namspace does not allow to assume role undefined
    at Poller._upsertKubernetesSecret (/app/lib/poller.js:110:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)","msg":"failure while polling the secret test-service","v":1}

What's the expected behavior?

Secret sync should be allowed when namespace annotation is present, and no roleArn is provided.

{
     // test undefined
     ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': 'my-kiam-role.*' } } },
     descriptor: { roleArn: undefined },
     permitted: true
}, 

Issue 2

What is the current behavior?

kiam or kubernetes-external-secrets will now incorrectly permit (i.e. pass the regex and proceed to attempt an sts call) assumption of roles in a namespace not intended to be used by kiam or kubernetes-external-secrets. The sharing of an annotation allows for nondeterministic behavior from each operator when used at the same time.

Scenario (AWS ARNs truncated for brevity):

• kubernetes external secret controller uses kiam to assume roles for processing, with that role (and all other allowed kiam roles) covered by the iam.amazonaws.com/permitted: kiam-roles.* annotation on the namespace

• kubernetes external secrets ExternalSecret has a desired roleArn value my-team-external-secret-role.*

• kiam policies are attached to kiam-server pods, and have no knowledge of ExternalSecret level kubernetes-external-secret role / polices

• kubernetes-external-secret policies are assumed at runtime from the kubernetes-external-secret controller AFTER it assumes a role via kiam, and have no knowledge of all the other kiam level role / polices

In order to support this, one of the following must now happen:

• regex updated to support both patterns: (kiam|my-team-external-secret-role).*
  • now kiam OR kubernetes-external-secrets is "permitting" roles they can not actually assume
• naming convention updated to support a single shared pattern: namespace-iam-roles.*
  • same result, kiam OR kubernetes-external-secrets is still "permitting" roles they can not actually assume

What's the expected behavior?

One of the following should be done to kubernetes-external-secrets prevent conflicts

• Backwards Compatible Break - change the support annotation for kubernetes-external-secrets to be something under the existing CRD naming of externalsecrets.kubernetes-client.io,

• Allow the annotation to be configurable so that both kiam and kubernetes-external-secrets can run without conflict

• Explicitly document this issue in the README.md

[moolen]

Thanks @mmcaya for taking the time to do such a good write-up <3

In regard to issue 2:

As far as i understand (correct me if i'm wrong), Kiam is designed to "just" intercept the metadata API. If a pod has already assumed a role, Kiam can do nothing if the pod tries to "escalate" privileges (i.e. assume other roles).
Since there is a 2-way trust relationship between roles needed i don't see a big security issue here. The issue lies in the design/implementation of Kiam.

I personally 100% support documenting this issue and making the externalsecrets.kubernetes-client.io/permitted (or however one will call it) configurable.

I need to take some more time to dig into issue 1 you mentioned.

Just for clarity so no one wastes effort: is someone already working on these issues?

[mmcaya]

👍 Sorry for the delayed response, thanks for the fixes!