-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat: chroot isolation #2169
feat: chroot isolation #2169
Conversation
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks promising! Thanks for working on this. 👍
pkg/chroot/root.go
Outdated
"os" | ||
) | ||
|
||
func TmpDirInHome() (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want os.UserHomeDir()
instead, which uses $HOME
and other heuristics.
pkg/executor/build.go
Outdated
func (s *stageBuilder) isolate() (exitFunc func() error, err error) { | ||
switch s.opts.Isolation { | ||
case "chroot": | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These {}
s are unnecessary:
switch s.opts.Isolation {
case "chroot":
newRoot, err := ...
...
case "none":
...
}
Signed-off-by: Höhl, Lukas <[email protected]>
…ation Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
mount /etc/hosts /etc/hostname and /etc/resolv.conf for networking Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
This reverts commit 5d7efa7.
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
During my development on this PR, I ran more and more into duplications with established isolation tools like runc and crun. Is it maybe worth considering investing into using those tools that are conform with the OCI spec standard? Reimplementing those isolation techniques is imo not easy for the long run. I'll look into the possibilities with runc's libcontainer and how it plays with kaniko. |
First of all, thanks for digging into this! I'm excited to learn what our options are if we want to retain the invariant that Kaniko doesn't require the kind of privilege that |
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Höhl, Lukas <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Hoehl <[email protected]>
I took a look around for other isolation tools (libcontainer, bubblewrap, crun) but all of those tools need privileged containers, because they create cgroups and those need writable /sys dirs, thus --privileged. I'll try to implement a sort of style that buildah did with their chroot isolation. Integration tests for isolation will be created aswell (breaking out of chroot, accessing "host" files). |
Signed-off-by: Lukas Hoehl <[email protected]>
Signed-off-by: Lukas Hoehl <[email protected]>
Signed-off-by: Lukas Hoehl <[email protected]>
Signed-off-by: Lukas Hoehl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
support for new{uid,gid}map in user namespace. Useful for future rootless implementation Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
Signed-off-by: Lukas Höhl <[email protected]>
I apologize for my ignorance here, but would it be possible to explain at a high level how does this PR circumvent the limitations noted here: #107 (comment)? Would a platform running containers without |
Glad you asked, since this is a draft PR I'm not 100% settled on the final implementation, but the current idea is the following:
Currently, this implementation has to set the seccomp and apparmor profiles to "unconfined" because those profiles are very strict in docker (CRI-O, Podman etc. allow mount syscalls), but it's being discussed in the moby repo: moby/moby#42455 The final goal is to be able to run --isolation chroot without SYS_ADMIN privilege and with a slightly different seccomp/apparmor profile than default (when running with docker). Also I think it's best to leave the default to --isolation none at first, since changing the default could break builds. |
Cool stuff: Some noobie question: Why is chroot isolation needed, what does it achieve? I know it solves a security issue... |
Hello guys, any news about this PR? |
hi guys, is there any progress about this PR? @hown3d |
Hey everyone,
Kaniko is just not designed to provide more isolation inside the container environment.
I'm hereby closing this PR, since I won't continue working on that feature. |
This PR implements isolation between build commands with chroot.
Should fix #2165 and #2153.
Description
Chroot isolation is implemented by creating a temporary directory inside the HOME directory which is later used as the base for chroot. Before chroot, the context directory, /proc and /dev are bind mounted into the new root directory.
Isolation is wrapped between each build command execution.
Isolation can be toggled with a flag --isolation which resolves by default to chroot. Providing anything else will disable isolation.
TODOs before removing Draft:
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Reviewer Notes
Release Notes
User facing changes:
--isolation
to specify the isolation method. The default will be chroot. Removing isolation can be done by setting the flag to none