uswitch / kiam

Integrate AWS IAM with Kubernetes
Apache License 2.0
1.15k stars 238 forks source link

Customizing suffix of the assumed role ARN for auditing purpose? #38

Open mumoshu opened 6 years ago

mumoshu commented 6 years ago

Hi! Thanks for starting & maintaining this great project 👍

I recently realized that running get-caller-identity with the kiam-provided AWS credentials produce:

$ aws sts get-caller-identity --region ap-northeast-1
{
    "Account": "$myaccountid",
    "UserId": "AROAICVHQ4GZUSQIQRRHY:kiam-kiam",
    "Arn": "arn:aws:sts::$myaccountid:assumed-role/my-k8s-service-role/kiam-kiam"
}

my-k8s-service-role is from the pod annotation and kiam-kiam seems to be coming from kiam.

I couldn't find whether it is hard-coded into kiam or not. Anyway, it would be great if I could configure the part to e.g. kiam-$mycluster-$ns-$pod_name so that it looks kiam-mycluster-kube-system-mytestpod which would add more traceability via CloudTrail logs. More concretely, it would be nice if I could trace from CloudTrail logs which pod in which cluster/namespace called which AWS API.

mumoshu commented 6 years ago

Seems like these are where it is set: https://github.com/uswitch/kiam/blob/e809a1aeeda7b5066329e269606ea43fcf6a575b/pkg/aws/sts/cache.go#L53 https://github.com/uswitch/kiam/blob/bbbc49733609414f333aa06864727c7b38fb160f/pkg/server/server.go#L159 https://github.com/uswitch/kiam/blob/bbbc49733609414f333aa06864727c7b38fb160f/cmd/server/main.go#L56

So, ideally a template to the session flag like --session ${cluster_name}-{{.PodNamespace}}-{{.PodName}} executed with the metadata from the credentials-requestor pod would be great.

Note that ${cluster_name} is not needed to be templated. A cluster provisioning tool knows the cluster name at kiam installation time.

pingles commented 6 years ago

As you say- it's configurable with --session but currently kiam will request credentials (and cache) per-role, so the session would be tied to the originally requesting Pod (if you have the same role used by multiple pods it might be misleading) so given it's current implementation it'd mostly be useful for cluster name.

We re-use credentials as we often have many replicas of deployments/jobs etc. that use the same credentials and this helps reduce the number of AWS calls we make.

I guess another option would be to add an additional annotation that would name the session- then have credentials cached using the role name and session name (so pods in the same deployment/job etc. would share the same?).

pingles commented 6 years ago

Also @mumoshu sorry for only just noticing this issue now :)

pingles commented 6 years ago

I had a think about doing this today and I don't think it's feasible without quite a large refactoring/change that would also unlock #14. I'm going to make some sketches and talk about it with people. Hopefully we get a little time to do this soon as it'd improve quite a lot of the code!

pingles commented 6 years ago

I'm going to move this back to 3.1- I'd rather we release with all the stuff that's been done so far and then add this later.

It's a feature which shouldn't rely on changing the behaviour of any existing flags so should be possible to add without breaking the existing behaviour.

aidansteele commented 5 years ago

@pingles What's the status of this effort? I see 3.0-3.2 have been released since your last message. Is this something that others can help with?

pingles commented 5 years ago

It’s something we’d love to have contributed but i don’t think we have the capacity to do ourselves.

If someone wants to pick to I’d be happy to help answer questions here or on slack to help guide if they need it.

On Wed, 26 Jun 2019 at 12:09, Aidan Steele notifications@github.com wrote:

@pingles https://github.com/pingles What's the status of this effort? I see 3.0-3.2 have been released since your last message. Is this something that others can help with?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/uswitch/kiam/issues/38?email_source=notifications&email_token=AAAAI7QG6GJRIZHE7G7TCV3P4NE63A5CNFSM4ERPIXOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTFTHY#issuecomment-505829791, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAAI7VP5HIBPSGPKDWY6A3P4NE63ANCNFSM4ERPIXOA .

stefansedich commented 4 years ago

Was there every any movement on this feature? we really need something like this so that we can have better auditiing of our S3 resources from JupyterHub, if I could apply the username as part of the prefix it could solve this in a simple way.

@pingles I am happy to pick this up if you want to have a chat with any ideas you might have had at the time? my initial thoughts in another pod annotation defining the suffix which can then be applied.

stefansedich commented 4 years ago

As you say- it's configurable with --session but currently kiam will request credentials (and cache) per-role, so the session would be tied to the originally requesting Pod (if you have the same role used by multiple pods it might be misleading) so given it's current implementation it'd mostly be useful for cluster name.

We re-use credentials as we often have many replicas of deployments/jobs etc. that use the same credentials and this helps reduce the number of AWS calls we make.

I guess another option would be to add an additional annotation that would name the session- then have credentials cached using the role name and session name (so pods in the same deployment/job etc. would share the same?).

Have been digging into this @pingles and have come to the same conclusion, supporting this is going to require a new annotation but also change the cache key to have to include the role and session name.

Now before I go down this long dirty road just want to check in to see that this feature would be welcomed or if there is other work going on that could make the caching changes simpler.

My current thought is that the cache key would become some concatenation of <role-name>|<role-session-name> which would allow us to have as many role/role-session-name combos as we want.

Would be interested to hear your thoughts?

stefansedich commented 4 years ago

@pingles as talked about in #399 adding my 2c here.

I would like to see it work using a pod template like we do today for the role and soon the extrernal ID, however the idea you mentioned to support templates could also be a viable alternative option if we could support both however I imagine from an implementation perspective and since we are already adding extrernal-id support might be easiest to just go with the annotation?

bburky commented 3 years ago

I see #430 references this and was merged, what is the status of this now?

Another nice feature would be to allow specifying the tags used during AssumeRole. AWS allows parameterizing IAM policies based on the role's tags. If it were possible to provide the namespace as a tag, it would allow writing some extremely generic IAM policies parameterized based on namespace.

"Condition": {
    "StringEquals": {"aws:ResourceTag/namespace": "${aws:PrincipalTag/namespace}"}
}

Maybe this is a separate feature request, let me know if this needs a different ticket.

stefansedich commented 3 years ago

I see #430 references this and was merged, what is the status of this now?

Another nice feature would be to allow specifying the tags used during AssumeRole. AWS allows parameterizing IAM policies based on the role's tags. If it were possible to provide the namespace as a tag, it would allow writing some extremely generic IAM policies parameterized based on namespace.

"Condition": {
    "StringEquals": {"aws:ResourceTag/namespace": "${aws:PrincipalTag/namespace}"}
}

Maybe this is a separate feature request, let me know if this needs a different ticket.

@bburky this sounds interesting and would not be overly difficult to add with the refactoring that was done to support session-name and external-id, I imagine we could just add support for session-tags as another pod annotation which are used when assuming the role.

I think another feature request is a good route here.