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

securing a cluster: add recommendations about cloud metadata APIs #6659

Merged

Conversation

ericchiang
Copy link
Contributor

@ericchiang ericchiang commented Dec 12, 2017

Updates kubernetes/kubernetes#52184

Exposing the metadata API to pods is a very common install practice and can potentially lead to escalations beyond a cluster and into the cloud account.

cc @kubernetes/sig-auth-pr-reviews
cc @bgeesaman


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 12, 2017
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Dec 12, 2017

Deploy preview ready!

Built with commit 59e6d06

https://deploy-preview-6659--kubernetes-io-master-staging.netlify.com


When running Kubernetes on a cloud platform limit permissions given to instance credentials, use
[network policies](/docs/tasks/administer-cluster/declare-network-policy/) to restrict pod access
to the metadata API, and avoid using provising data to deliver secrets.
Copy link
Member

Choose a reason for hiding this comment

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

provisioning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙈

@ericchiang ericchiang force-pushed the securing-a-cluster/metadata-api branch from 7cd1d83 to 59e6d06 Compare December 12, 2017 07:29
@bgeesaman
Copy link

/lgtm Thanks!

@mikedanese
Copy link
Member

@ihmccreery

@mattmoyer
Copy link
Contributor

LGTM, thanks for getting this documented!

I'm not sure if we want to get into it in this doc, but if we want an example NetworkPolicy this is the one we're using in the Heptio AWS quickstart (thanks @bgeesaman!):

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
metadata:
  name: deny-metadata
  namespace: default
spec:
  podSelector: {}
  egress:
  - to:
    - ipBlock:
        cidr: 0.0.0.0/0
        except:
        - 169.254.169.254/32
  policyTypes:
  - Egress

@ericchiang
Copy link
Contributor Author

@mattmoyer default deny might be better to recommend? Particularly since GCE's metadata service isn't a fixed IP. This is one from @bgeesaman's slides that seems reasonable:

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
metadata:
  name: default-deny
  namespace: default
spec:
  podSelector: {}
  egress:
  - to:
    - podSelector:
        matchLabels:
          k8s-app: kube-dns
  - ports:
    - protocol: UDP
      port: 53
  policyTypes:
  - Ingress
  - Egress

NetworkPolicy docs could probably use some more "best practices" items and workflows. E.g. default deny + ship your network policies with your manifests. Will try to hack around a bit and take a look at that document.

@bgeesaman
Copy link

For the Heptio Quickstart on AWS, blocking just the metadata IP is/was an extremely safe assumption and made a lot of sense in that context for folks just getting a cluster going to learn on.

As a general recommendation, though, the advice I'd give is the latter policy as it forces you to explicitly declare what can talk where in subsequent "allow" policies. Using the default-deny policy has greater benefits for a namespace than just blocking access to the user-data/meta-data/IAM credentials assuming the cluster has a CNI plugin that supports egress policies. It blocks access to ports on the underlying nodes (cadvisor, kubelet, node-exporter, SSH, etcd, and more) as well as other subnets "behind the firewall" or inside the VPC. Finally, it serves to isolate workloads from each other.

@mattmoyer
Copy link
Contributor

+1 for describing a default deny policy (or more example policies in general).

Another challenge now as a distributor or cluster admin is you have to create that default deny policy with each new namespace or write some controller to do it for you.

@ericchiang
Copy link
Contributor Author

Any objections to getting this in then do the network policy improvements as a follow up?

@bgeesaman
Copy link

Good with me.

@liggitt
Copy link
Member

liggitt commented Dec 13, 2017

The posture of network policy is that it should be under the control of the namespaced app owner user. Per that, write permissions were added to the admin and edit roles. Is this expecting to be able to set up a deny all role that the namespaced user would be unable to remove?

@davidopp
Copy link
Member

BTW this doc is very relevant to this discussion, as there is large overlap between security and multitenancy.

(Join the kubernetes-dev mailing list to view the doc.)

@ericchiang
Copy link
Contributor Author

ericchiang commented Dec 13, 2017

Thanks @davidopp for the link. Yeah, I see a lot of overlap between these topics.

For those following, the PR to add NetworkPolicy to the default roles is here kubernetes/kubernetes#56650 (targeted for 1.10). Has some good discussion about who should be able to modify NetworkPolicy.

Is this expecting to be able to set up a deny all role that the namespaced user would be unable to remove?

I think it'd be nice to have some sort of "cluster network policy" but I realize that's not possible today. It does seems odd that a resource created in a namespace could govern egress rules outside the cluster (like this example shows).

I still think this recommendation is appropriate. It's just stating "cloud metadata APIs are dangerous, and admins should be aware of that." One of the hardening strategies happens to be NetworkPolicy.

@bgeesaman
Copy link

I still think this recommendation is appropriate. It's just stating "cloud metadata APIs are dangerous, and admins should be aware of that." One of the hardening strategies happens to be NetworkPolicy.

I would agree that this guidance is meant to raise awareness of a very common security issue and provide a sensible/workable solution with what is available to be easily expressed today. The ongoing discussion of a cluster-admin vs namespace admin controlled NetworkPolicy object is a fantastic one, and I look forward to the added granularity and enforcement that will provide.

@ikehz
Copy link

ikehz commented Dec 13, 2017

This has the potential to cause problems with addons (at least on GCE), since many of them obtain credentials in precisely this fashion. We should include a recommended default for addons. Does this addons issue also apply to other cloud providers?

@ericchiang
Copy link
Contributor Author

We should include a recommended default for addons.

Yes. I think as we see a rollout of NetworkPolicy. like RBAC and PSP, more components will ship NetworkPolicy as part of their manifest bundle.

Does this addons issue also apply to other cloud providers?

I had trouble finding an AWS specific example in cluster/addons, but I imagine there are third party integrations that do use the EC2 metadata API this way. Azure's metadata API isn't as fully functional and I don't think you can request credentials from it.

@bgeesaman
Copy link

This has the potential to cause problems with addons (at least on GCE), since many of them obtain credentials in precisely this fashion. We should include a recommended default for addons. Does this addons issue also apply to other cloud providers?

I see where you are coming from. Would it make sense to add additional language in this recommendation that cluster-admins and namespace admins be aware of the valid uses (certain add-ons) for pod access to the cloud provider metadata API and to take that into account when applying networkpolicy as a control so as not to silently break necessary functionality? And if those add-ons are in use, to recommend placing them in a separate namespace to be able to more cleanly maintain separation from user-facing workloads?

@steveperry-53
Copy link
Contributor

@ericchiang @tengqm Are you ready for me to merge this? Or it the discussion still going on? Thanks.

@tengqm
Copy link
Contributor

tengqm commented Dec 29, 2017

I have voted for this 10 days ago. It looks good to me, @steveperry-53

@ericchiang
Copy link
Contributor Author

I don't have plans to modify this PR. Anyone object to making NetworkPolicy doc changes separately?

@steveperry-53 steveperry-53 merged commit 77d0855 into kubernetes:master Jan 8, 2018
@ericchiang ericchiang deleted the securing-a-cluster/metadata-api branch January 8, 2018 23:00
chenopis added a commit that referenced this pull request Jan 9, 2018
…elease-1.9

* 'master' of https://github.com/kubernetes/website: (140 commits)
  Update configure-multiple-schedulers.md (#6877)
  Explicitly specify the components that need a change (#6873)
  Updates to partner page (#6797)
  Add steps for extension api-server setup (#6667)
  securing a cluster: add recommendations about cloud metadata APIs (#6659)
  Namespace glossary term (#6607)
  Update device plugin doc with upgrade suggestions and known limitations. (#6501)
  Add Kubermatic to pick-right-solution.md (#6459)
  extra word
  typo
  Fix concepts/configuration/secret
  Link to extended resources instead of deprecated opaque integer resources
  Fix CI issue
  Corrected the group in runtime-config of ValidatingAdmissionWebhook and MutatingAdmissionWebhook. It should be admissionregistration.k8s.io (was admissionregistration).
  Correct references in Jekyll data files
  Update scheduling-hugepages.md
  Drop reference to outdated FAQ
  Follow sig list to kubernetes/community
  Update scheduling-hugepages.md
  Fix references to ConfigMap for the CN website
  ...

# Conflicts:
#	docs/concepts/policy/pod-security-policy.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/auth Categorizes an issue or PR as relevant to SIG Auth. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.