Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Security Model docs #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update Security Model docs #106

wants to merge 1 commit into from

Conversation

yannh
Copy link
Collaborator

@yannh yannh commented Dec 28, 2020

This should address #39
You should be able to configure kube-secret-syncer with environment variables/config files/... instead of using the IAM Role of the instance, and benefit from IAM restrictions if the IAM roles can be assumed by the "master" role kube-secret-syncer uses.

@yannh yannh requested review from argvk and elblivion December 28, 2020 17:12
@codecov
Copy link

codecov bot commented Dec 28, 2020

Codecov Report

Merging #106 (e884381) into master (e22412e) will decrease coverage by 1.7%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #106     +/-   ##
========================================
- Coverage    61.1%   59.5%   -1.6%     
========================================
  Files           8       8             
  Lines         370     370             
========================================
- Hits          226     220      -6     
- Misses        115     119      +4     
- Partials       29      31      +2     
Impacted Files Coverage Δ
controllers/syncedsecret_controller.go 59.5% <0.0%> (-5.9%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e22412e...e884381. Read the comment docs.

synced secrets have an IAMRole field defined, kube-secret-syncer will assume that role before retrieving the secret. This
implies that the role specified by IAMRole can be assumed by the role of the Kubernetes node kube-secret-syncer runs on.
Kube-secret-syncer relies on the AWS Go SDK to communicate with AWS - and supports the different ways of
authenticating to AWS described in the [AWS Go SDK documentation](https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html#specifying-credentials).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should explicitly point out that additional steps are required to prevent that the iam roles used kube-secret-syncer aren't assumable by other pods in the k8s cluster as this would otherwise break the security model.

E.g. You shouldn't use ec2 instance profile based authentication and you need to enable namespace restrictions if you use kube2iam https://github.com/jtblin/kube2iam#namespace-restrictions, which isn't enabled by default.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd second this -- at a minimum it's probably worth calling out which auth methods would break the security model, and the necessary requirements to maintain the security model, like:

prevent that the iam roles used kube-secret-syncer aren't assumable by other pods in the k8s cluster

I can understand not giving explicit setup instructions (for kube2iam, kiam, IRSA, etc.) as the projects themselves will always have the most complete/up-to-date documentation. But might also be worth a link or two to some of those projects that can be configured to meet the requirements?

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

Successfully merging this pull request may close these issues.

3 participants