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

[WIP] Adding support for encrypted container images #78975

Closed
wants to merge 2 commits into from

Conversation

harche
Copy link
Contributor

@harche harche commented Jun 13, 2019

What type of PR is this?

/kind feature

What this PR does / why we need it:
This PR adds support for upcoming encryption support for container images

Which issue(s) this PR fixes:

Fixes kubernetes/enhancements#1067

Special notes for your reviewer:
This is still a work-in-progress. We are on the verge of getting OCI spec followed by the support in containerd for encrypted images merged.

Does this PR introduce a user-facing change?: yes

Add support to encrypted images

@k8s-ci-robot
Copy link
Contributor

Welcome @harche!

It looks like this is your first PR to kubernetes/kubernetes 🎉. 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/kubernetes 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 k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 13, 2019
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 13, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @harche. Thanks for your PR.

I'm waiting for a kubernetes 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.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 13, 2019
@k8s-ci-robot k8s-ci-robot added area/kubectl area/kubelet area/test kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 13, 2019
@harche
Copy link
Contributor Author

harche commented Jun 13, 2019

"I signed it"

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 13, 2019
@harche
Copy link
Contributor Author

harche commented Jun 14, 2019

/assign @derekwaynecarr @liggitt

@harche
Copy link
Contributor Author

harche commented Jun 14, 2019

/joke

@dims
Copy link
Member

dims commented Aug 13, 2019

@harche Which version of containerd is likely to have support for this feature? Is there a plan to help other runtimes get there? (especially support via docker API)

@harche
Copy link
Contributor Author

harche commented Aug 16, 2019

@harche Which version of containerd is likely to have support for this feature? Is there a plan to help other runtimes get there? (especially support via docker API)

CRI->containerd is yet to support the decryption in their upstream. Although I have a working POC of CRI-Containerd as well as CRI-O, they both won't be able to merge my changes because they vendor k8s APIs which need modification for this to work. So as soon as we have the code in k8s merged, I can submit the PRs for CRI-Containerd as well CRI-O to support this feature by updating their respective vendor directories from k8s that handle CRI interface.

Once the CRI interface in k8s gets updated to pass decryption keys to runtimes, adding support in the runtimes should fairly simple, including docker.

For your reference this is how it's supposed to work, I gave the example of CRI-O in that diagram but you can very well replace with any runtime that implements CRI interface.

@dims
Copy link
Member

dims commented Aug 16, 2019

Gotcha. Thanks @harche

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 14, 2019
@harche
Copy link
Contributor Author

harche commented Nov 14, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 14, 2019
@liggitt liggitt assigned smarterclayton and unassigned liggitt Feb 7, 2020
@duyanghao
Copy link

@harche Great Work, but how things progress? Does kubernetes support image encryption now?
There is an article about how to use kubernetes image encryption based on cri-o, which seems not to be consistent with your KEP status right now.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 21, 2020
@k8s-ci-robot
Copy link
Contributor

@harche: PR needs rebase.

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/test-infra repository.

@harche
Copy link
Contributor Author

harche commented Feb 21, 2020

@harche Great Work, but how things progress? Does kubernetes support image encryption now?
There is an article about how to use kubernetes image encryption based on cri-o, which seems not to be consistent with your KEP status right now.

Thanks @duyanghao. The article talks about the work I did with @lumjjb on adding decryption capabilities in CRIO using the keys available on the worker node.

This code change is about adding the decryption capabilities in kubernetes by adding the new secret type to hold the required private keys. However, based on this comment, we are trying add the image encryption/decryption support in other container related projects to increase the adoption. Such as,

containers/skopeo#732
containers/skopeo#814
cri-o/cri-o#2813
containerd/cri#1340

Apart from these, we are in talks with Quay registry folks to add the support for encrypted images. Hoping to get that through soon.

@duyanghao
Copy link

duyanghao commented Feb 21, 2020

@harche Great Work, but how things progress? Does kubernetes support image encryption now?
There is an article about how to use kubernetes image encryption based on cri-o, which seems not to be consistent with your KEP status right now.

Thanks @duyanghao. The article talks about the work I did with @lumjjb on adding decryption capabilities in CRIO using the keys available on the worker node.

This code change is about adding the decryption capabilities in kubernetes by adding the new secret type to hold the required private keys. However, based on this comment, we are trying add the image encryption/decryption support in other container related projects to increase the adoption. Such as,

containers/skopeo#732
containers/skopeo#814
cri-o/cri-o#2813
containerd/cri#1340

Apart from these, we are in talks with Quay registry folks to add the support for encrypted images. Hoping to get that through soon.

@harche Thanks for your reply, I still have some questions about your work:
1. Is there a practical version of your work in production environments?
As I understand it, both containerd/cri and cri-o has supported basic image encryption and decryption right now(maybe not fully as you are still developing). So I am curious about the practical version of the kubernetes, cri-o or containerd in production environments.
2. What's the current stage of your work? Alpha, Beta or Stable?
I am trying to put your work onto our production environments(kubernetes + docker), so I am worry about the security of image encryption and decryption itself. After all, this new feature seem to have not used by others yet.

@lumjjb
Copy link

lumjjb commented Feb 21, 2020

Hi @duyanghao, thanks for your interest in our work! Here is some more information, but would be glad to chat offline to hear some of the usecases and plans.

  1. Is there a practical version of your work in production environments?

We have an ongoing offering that will be using this technology that is going to be available in the later half of this year. We have plans to work with the IBM IKS team to get this integrated this year as well. The feature should also be available in OpenShift 4.4 when they start using k8s >=1.17.

  1. What's the current stage of your work? Alpha, Beta or Stable?

This would be made first available with kubernetes 1.17 (through cri-o 1.17 and the respective containerd cri version), without any changes in kubernetes, so if I were to put a label on it, it would be alpha. But containerd and cri-o doesn't officially have any special labeling on the feature.

Also, just to clarify, there are two models of using encrypted images with kubernetes, what is do-able today with containerd/cri and cri-o is independent of this KEP. This KEP is an effort to provide the ability to have stronger multitenancy using encrypted images.

@duyanghao
Copy link

duyanghao commented Feb 21, 2020

@lumjjb Thanks for your reply

Also, just to clarify, there are two models of using encrypted images with kubernetes, what is do-able today with containerd/cri and cri-o is independent of this KEP. This KEP is an effort to provide the ability to have stronger multitenancy using encrypted images.

It seems that I can now use the kubernetes(>=1.17) and docker(containerd) to achieve image encryption&decryption with the model independent of this KEP, but is there any official kubernetes documents about how to use it besides this article?

@lumjjb
Copy link

lumjjb commented Feb 21, 2020

It seems that I can now use the kubernetes(>=1.17) and docker(containerd) to achieve image encryption&decryption with the model independent of this KEP, but is there any official kubernetes documents about how to use it besides this article?

For containerd, it is currently pending on this PR, which is waiting on 1 more approval.

containerd/cri#1340

The actual doc that will merge with this is: https://github.com/containerd/cri/blob/de21864c7684ddbb9c72c22769d704ed0d42cbbf/docs/encryption.md

I think @Random-Liu can probably shed a bit more light on timeline and containerd release cycles.

@harche
Copy link
Contributor Author

harche commented Feb 21, 2020

@lumjjb Thanks for your reply

Also, just to clarify, there are two models of using encrypted images with kubernetes, what is do-able today with containerd/cri and cri-o is independent of this KEP. This KEP is an effort to provide the ability to have stronger multitenancy using encrypted images.

It seems that I can now use the kubernetes(>=1.17) and docker(containerd) to achieve image encryption&decryption with the model independent of this KEP, but is there any official kubernetes documents about how to use it besides this article?

That article talks about the threat model where you only trust the worker nodes of your k8s cluster (but not other components, like api server, docker registry etc). It requires that the decryption keys are kept on the target worker node. The support for this is already present in upstream in CRIO. So you might have to use CRIO instead of Containerd with your k8s setup. You don't need to change anything in k8s for this to work.

We are also hoping to get our changes merged soon in CRI-Containerd too.

As Brandon mentioned, the code in this KEP is part of the effort to provide stronger multi-tenancy using encrypted images. In this the kubernetes api server and worker nodes are both trusted entities (but docker registry is not). But as I mentioned earlier, this code is not upstreamed yet.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 21, 2020
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 21, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 20, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubectl area/kubelet area/test cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Encrypted Images in Kubernetes
9 participants