docs(aws): scoping the IAM policy to explicitely defined Route53 zones

[crtr109]

What does it do ?

It makes end users to pass a zone/zones instead of allowing the policy to edit all zones hosted in an AWS account.

Motivation

Applying the principle of least privilege is one of the fundamental security best practices. It reduces blast radius and makes permissions more clear and auditable. It's also recommended by AWS https://docs.aws.amazon.com/IAM/latest/UserGuide/best-practices.html#grant-least-privilege

More

• [x ] Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: crtr109 (9ad952f, fb3c87e, 62b5a52, 4d6310d, 681f1f7)

[k8s-ci-robot]

Welcome @crtr109!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @crtr109. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ivankatliarchuk]

There is a scoped role with ABAC as well https://github.com/kubernetes-sigs/external-dns/blob/62b5a52340dbce1716ad6735bb603628f34aaf5b/docs/tutorials/aws.md#iam-permissions-with-abac

[crtr109]

hey @ivankatliarchuk

example from the link you provided implements filtering based on hostnames, the same way you can do from the external-dns side using the --domain-filter option

however the value for aws route53 resource remains *, meaning all zones that are hosted in that account
example.com can be hosted in one account as both private and public zones and access matrices to the resources can differ

my suggestion is a simple implementation of defense in depth approach, i.e having as much protective layers as possible

[mloiseleur]

@crtr109 thanks for your improvement on "route53:ListResourceRecordSets" and "route53:ListTagsForResources". It provides better security and makes sense.

For the default, I prefer to keep it with the *. It's better for userXP to have ready-to-go YAML. There is a clear invitation, with the note just above, to follow least privilege and set the expected zone id.

=> Would you please rework this PR accordingly ?

[crtr109]

hey @mloiseleur

are you suggesting to keep the default policy as it is to make it more beginner friendly but tighten down the second example which implements ABAC?

if i understood you correctly yes i can update the PR

otherwise please elaborate on how it should be reworked

[mloiseleur]

@crtr109 I've done some suggestion on the PR. I suggest also to add on L78 something like this:

On production, it's recommended to replace wildcard (*) with only explicit Hosted Zone IDs. It reduces blast radius and makes permissions more clear and auditable.

[crtr109]

hey @mloiseleur

thank you for clarification

i've applied the suggested changes and added a section called Further improvements , please take a look

[ivankatliarchuk]

This will not work with service specific conditions.

[ivankatliarchuk]

ListTagsForResources will not work as it will require healthcheck resource id as well.

[crtr109]

should then ListTagsForResources be placed under a separate allow statement?
this way it will possible to fine tune it later passing either zone(s) or health check(s) to it

[ivankatliarchuk]

Hard to say. IAM is tricky. I just pointed that is not going to work, probably if you have access to AWS, worth to validate a policy.

[crtr109]

i do have a live cluster but unfortunately without health checks configured

i'm also not really sure how to test it since i've never come across that functionality to be honest
is that the relevant part in the docs? https://github.com/kubernetes-sigs/external-dns/blob/master/docs/tutorials/aws.md#associating-dns-records-with-healthchecks
if you can give me more hints on how to properly test i will do it

so for ListTagsForResources my suggestion will be the following:

      "Effect": "Allow",
      "Action": [
        "route53:ListTagsForResources"
      ],
      "Resource": [
        "arn:aws:route53:::hostedzone/*",
         "arn:aws:route53:::healthcheck/*",
      ]

this way if a user wants to make the policy tighter he has all the needed info to do it

please let me know how to proceed

[ivankatliarchuk]

yeah arn:aws:route53:::healthcheck/* is nice to have, but I'm not using it as well, so not sure what are the exact requirement are, and how to lock them down. Here is some helper https://www.awsiamactions.io/?o=ListTagsForResources

[ivankatliarchuk]

/ok-to-test

[ivankatliarchuk]

I think we could further improve the AWS IAM docs in follow-ups

Currently arn:aws:route53:::healthcheck/* and similar resource conditions are missing. As well there is an issue #5773

/lgtm

[mloiseleur]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mloiseleur]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment