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

Create non-root images #368

Merged
merged 1 commit into from
Jun 19, 2019
Merged

Conversation

dprotaso
Copy link
Contributor

Fixes: #235

@mattmoor @dlorenc @jonjohnsonjr I don't have opinions about how things are tagged. Let me know and I can wrap this up

@chanseokoh
Copy link
Member

My concern is that the home directory of nobody is pointing to /nonexistent. I think a non-root user (other than nobody) has to have a proper home. A lot of applications depend on the existence of a home directory.

@loosebazooka
Copy link
Member

Would it need to be an actual user instead of nobody? Perhaps nonroot with an actual home directory?

@mattmoor
Copy link
Contributor

cc @sharifelgamal This is what I was asking for at Kubecon! thanks @dprotaso

@mattmoor
Copy link
Contributor

cc @loosebazooka may want these for Jib as well.

@dprotaso
Copy link
Contributor Author

dprotaso commented May 29, 2019

I added a home directory for the 'nobody' user (/home/nobody). I changed the root user's home directory to /root

I noticed running tests don't work on Mac

+ base/certs_test.image --norun
Loaded image ID: sha256:101cd50a552664199d987a2506fd3bcfecc301019c462adc1a4c98eab217309f
Tagging 101cd50a552664199d987a2506fd3bcfecc301019c462adc1a4c98eab217309f as basecheck_certs_image:intermediate
+ ../structure_test_linux/file/downloaded version
/private/var/tmp/_bazel_dprotasowski/d4d2a8e3e6c0766b8b5af3d4f8a07ed0/sandbox/darwin-sandbox/84/execroot/distroless/bazel-out/k8-fastbuild/bin/base/certs_test.runfiles/distroless/base/certs_test: line 7: ../structure_test_linux/file/downloaded: cannot execute binary file

@briandealwis
Copy link
Member

@dprotaso run with --cpu=darwin

Provocative question: why don't we make all images be non-root? Is there any reason to have root-based images?

@loosebazooka
Copy link
Member

I'm still not sure if we should reuse nobody for this non-root user. I would expect nobody to have no file ownership. It seems odd to diverge from this convention.

@dprotaso
Copy link
Contributor Author

@loosebazooka what was the original purpose of the 'nobody' user?

@chanseokoh
Copy link
Member

AFAICT, it is sometimes advised to run some processes as nobody, but I do not think it is for running any general applications. Besides, I think there is no reason to be obsessed with nobody here. You can always create and use a proper user for non-root purposes.

@loosebazooka
Copy link
Member

@dprotaso mostly just historical context. People used it to isolate daemons, but then moved off that to other daemon-specific low-privilege users to isolate the daemons from eachother.

I can't really speak to how users are using it now though in distroless, as it doesn't seem like anyone has ever needed a home directory for nobody. I think the convention of nobody has been for it to be the lowest of low privileged users though, without the ability to write to anything (how did they write logs?).

Maybe I'm being too pedantic about the purpose of nobody and this implementation is fine?

@loosebazooka
Copy link
Member

Sorry, I think I may have misunderstood the question: other context: nobody was added in #335 because of #332, and I do not have enough context but @thockin might have something to add here.

@dprotaso
Copy link
Contributor Author

So outstanding questions for me:

Which user/group do folks want to be default in non-root images

  1. nobody user with a home directory
  2. nonroot with their own home directory (will use uid:gid 65533 - one lower than nobody)

Note the /root home directory changes with both - I'm unaware if this haves any effects downstream.

Image tags

How should we reference/upload non-root images to/from a registry. We can tweak tags or repo path. My preference is for change the repo path

ie. currently we have

gcr.io/distroless/static
gcr.io/distroless/base
gcr.io/distroless/base:debug

becomes (need to update the PR once consensus is made)

gcr.io/distroless/nonroot/static
gcr.io/distroless/nonroot/base
gcr.io/distroless/nonroot/base:debug

@patflynn
Copy link

It seems sad to make the less secure option the default. I imagine the concern here is breaking existing users so I'm wondering if we have other options.

@chanseokoh
Copy link
Member

nonroot with their own home directory (will use uid:gid 65533 - one lower than nobody

Others may have different opinions, but I prefer this. The home of nobody is currently /nonexistent, and nobody should mean "nobody", not a real user. I don't really like breaking the convention by having a usable home directory for it. And as I said, I think there is no reason to be obsessed with nobody.

For the uid:gid, I vote for 65432 for safety. I think 65533 has an increased chance of collision than 65432 (just like you picked it) and is too close to 65535.

Note the /root home directory changes with both - I'm unaware if this haves any effects downstream.

We could have /nonroot as a home directory for the new user instead of /home/nonroot to keep backward compatibility, but I'm not sure if that's a good idea either.

@mattmoor
Copy link
Contributor

@patflynn making nonroot the default would immediately break anyone depending on it (and there are some niche cases that need it today). If we were starting from scratch (no pun intended), armed with the knowledge we have now, then this would be the default.

IMO we should use tags to distinguish these, not repos. :root (same as :latest), and :nonroot (this PR). For :debug and :nonroot, I'd just pick a blending: :nonrootdebug.

If we get these published, and folks (ko, bazel, jib, kubernetes, ...) adopt the explicit tagging by default, then we can see if :latest is still being pulled in a few months and flip the default.

cc @tallclair @lizrice will be interested in this thread as well

@dprotaso dprotaso force-pushed the master branch 3 times, most recently from 9917033 to b9c0ce9 Compare May 29, 2019 15:50
@dprotaso dprotaso marked this pull request as ready for review May 29, 2019 15:51
@lizrice
Copy link

lizrice commented May 29, 2019

I love that you’re doing this!

+1 to @chanseokoh’s comment preferring a real user rather than nobody. It feels wrong to have a home directory for the nobody account; and in the limited occasions where people deliberately use the nobody account they might even be relying on there not being a home for it.

I think for back-compatibility you're right to keep the :latest tag, but you could document it prominently as deprecated to encourage people to use the :root and :nonroot tags

@loosebazooka
Copy link
Member

I think we might still need to decide if we want to move user:root's home dir to /root ?

@briandealwis
Copy link
Member

@mattmoor: If we get these published, and folks (ko, bazel, jib, kubernetes, ...) adopt the explicit tagging by default, then we can see if :latest is still being pulled in a few months and flip the default.

Would it be worth setting up an distroless-announce@ mailing list? Whether we change the default now or in several months, it's still going to break anybody depending on root who isn't aware of the change.

@mattmoor
Copy link
Contributor

@briandealwis I wasn't suggesting that we shouldn't socialize the transition, we should, but it probably makes sense to get any known things onto explicit tags first.

I'd love to get this in. @dprotaso ping me if you need anything.

@loosebazooka
Copy link
Member

@mattmoor, any idea what to do with the user-home dir? do we expect users to explicitly be referencing user home instead of through the env?

@mattmoor
Copy link
Contributor

mattmoor commented Jun 13, 2019

@loosebazooka There's no place like $HOME

@mattmoor
Copy link
Contributor

Unless you are one of those %USERPROFILE% types 🙄

@jeremyje
Copy link

jeremyje commented Jun 13, 2019

Once this PR goes in how long will it take for the images to be available? I'm monitoring this change because we'd like to use distroless with a PodSecurityPolicy with

runAsUser:
    # Require the container to run without root privileges.
    rule: 'MustRunAsNonRoot'

@mattmoor
Copy link
Contributor

@sharifelgamal ^^

@dprotaso
Copy link
Contributor Author

@donmccasland maybe?

@googlebot googlebot added the cla: yes CLAs look good label Jun 19, 2019
@sharifelgamal
Copy link
Contributor

Sorry for the delay, no idea how this fell off my radar.

@sharifelgamal sharifelgamal merged commit de7f130 into GoogleContainerTools:master Jun 19, 2019
@sharifelgamal
Copy link
Contributor

These should be live in 15 minutes or so.

@dprotaso
Copy link
Contributor Author

@sharifelgamal thanks!

cloud-robotics-github-robot pushed a commit to googlecloudrobotics/core that referenced this pull request Dec 20, 2019
This is required to use newer Bazel versions in CI.

Also grpc-gateway needs an update to cope with the use of virtual proto
imports in the newer protobuf release, and rules_docker needs to be
updated to cope with newer Bazel versions.

Updating rules_docker pulled in
GoogleContainerTools/distroless#368, which
changes the default home directory in the containers to /root, which
requires a corresponding change in robot-master.yaml.

Change-Id: I958daa2356af795120e6b00a5f37b0c37d49d21d
GitOrigin-RevId: 61eedfd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLAs look good
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RunAs unprivileged user
10 participants