Test-infra: Update planter.sh to allow mounting volumes and relabel for SElinux

[leifmadsen]

Currently, if you run planter.sh on a system with SElinux enabled, things will fail due to volume mounts not being relabeled for SElinux purposes.

Error when running with SElinux enabled:

[centos@k8sdev test-infra]$ ./planter/planter.sh bazel test //...
Trying to pull repository gcr.io/k8s-testimages/planter ... 
0.7.0: Pulling from gcr.io/k8s-testimages/planter
Digest: sha256:f8ad2e2aeb5ab310e36025d95df8004da2aec6d2658cd5b1b309291f718b97db
Error: mkdir('/home/centos/.cache/bazel/_bazel_centos'): (error: 13): Permission denied

The fix is to add :z to the end of the -v volume flag in planter.sh. I'm not entirely sure how best to handle this, other than to add some flag or option to the bash script to add :z. Alternatively, just set it and do a basic check like running getenforce ?

Happy to provide a patch, just wanted to figure out what might make the most sense before tackling it.

[leifmadsen]

CC: @BenTheElder

[BenTheElder]

@ixdy this is how it grows :joy: https://github.com/kubernetes/test-infra/pull/5260#issuecomment-340872360

I want planter to continue to work most importantly on macOS since our builds are typically only functional in a linux environment, so blindly calling getenforce sounds problematic. However I'm also not particularly familiar with SELinux so I'm not sure what the best option is here. Perhaps we can detect if getenforce is present?

if command -v getenforce > /dev/null 2>&1; then
   # make sure :z is set here
fi

/area planter

[leifmadsen]

FYI I'm working on a very basic Ansible playbook that will bootstrap an environment so that I can run planter against Kubernetes right now. I'm hoping that what I think planter does, is what it actually does :)

(I'm interested in using planter to build me kubernetes binaries that I can then consume in a test enviroment.)

[leifmadsen]

Also I'm totally fine with that approach. We can wrap it in a if command and then do the needful checks to see if selinux is enabled via getenforce and then append the :z to the -v flag.

[ixdy]

Is there any harm in adding :z always? We've had changes in k/k like https://github.com/kubernetes/kubernetes/pull/16159 which have done this for other scripts.

[leifmadsen]

I'm not sure how Docker reacts when SElinux isn't enabled, or doesn't exist at all on the system. I suppose the easiest test would be to try running it with :z on an Ubuntu or MacOS box?

[leifmadsen]

Also, if we're not doing multiple containers, it looks like :Z (capitalized) is probably the preferred (doesn't allow sharing of data between containers).

[leifmadsen]

I just realized we can't just add :Z right now, because the default $HOME volume is.... your home directory, and we shouldn't probably just relabel that all willy-nilly ;) I wonder if the default $HOME should really be something like $HOME/.planter ?

[BenTheElder]

adding :Z seems to be fine on macOS tentatively, but https://github.com/moby/moby/issues/30934 seems a bit concerning

[leifmadsen]

Heh, I found the same bug report when searching around :)

[leifmadsen]

Guess my question would be, why is $HOME mounted into the container at all? Does it pull certain things out of the default path?

[BenTheElder]

@leifmadsen $HOME/.planter:$HOME/.planter might work, we want the paths in bazel's output to exist outside the container though at the very least. Ideally though it should just be the same as using bazel outside of planter.
Is there a another way to allow access to these paths?

[BenTheElder]

Guess my question would be, why is $HOME mounted into the container at all? Does it pull certain things out of the default path?

Bazel's cache is in $HOME/.cache/... and the log outputs are in there, bazel on a typical run will put verbose logging into these files and print the paths of logs for failed tests to stdout, so we want these log paths to exist outside the container. This also means if you eventually switch to native bazel you have nothing to clean up. We could probably ensure the existence of and mount $HOME/.cache but we already trusted the container.

I really don't actually want to change the ownership of these things on the host, the user should be able to read/write/delete the bazel output, can we escalate planter's permission to these paths in some other way?

[leifmadsen]

Not that I'm aware of. The escalation is basically being done with the :Z flag to mark the directories properly for SElinux. The alternative is to say, "we don't support SElinux" and force the user to just turn it off (which kind of sucks).

I guess my question around $HOME was... does the build really need to run directly in $HOME or can it run under a subdirectory, or something else? The ideal would be to just create a working directory that we can apply :Z to, and then move on with our lives :)

The question around $HOME was because I wasn't sure if it was pulling paths from that location, or reading other files, or what was going on from there, rather than just operating in a working directory (if the point is to build a .cache or whatever else is necessary).

[BenTheElder]

It's possible to override the bazel cache location though the TEST_TMPDIR
flag which we (ab)use in CI, but this is meant to be a magic variable for
bazel developers to test bazel with and isn't a fantastic option.

I agree that we want to support SElinux users, but it looks to me like :Z
changes bits on the host side which is probably never going to not be
dangerous.

The only thing it should be using home for is the cache so we could
probably mount just .cache though. All binaries, intermediate output, and
build / test logs live here and are symlinked to from your workspace and
are reused when possible between runs.

On Mon, Nov 20, 2017, 13:14 Leif Madsen [e-mail] wrote:

Not that I'm aware of. The escalation is basically being done with the :Z
flag to mark the directories properly for SElinux. The alternative is to
say, "we don't support SElinux" and force the user to just turn it off
(which kind of sucks).

I guess my question around $HOME was... does the build really need to run
directly in $HOME or can it run under a subdirectory, or something else?
The ideal would be to just create a working directory that we can apply :Z
to, and then move on with our lives :)

The question around $HOME was because I wasn't sure if it was pulling
paths from that location, or reading other files, or what was going on from
there, rather than just operating in a working directory (if the point is
to build a .cache or whatever else is necessary).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/issues/5607#issuecomment-345833943,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4Bq3DXLGyGqos2ShlUtaeMIfve3ZcYks5s4eutgaJpZM4QkzEG
.

[leifmadsen]

Well the main issue, is you can't just add :Z or :z to the -v ${HOME}:${HOME} because that'll relabel your entire home directory, which basically locks you out of it as soon as you logout :). Without the relabeling though, then Docker can't write to that volume, which doesn't allow you to create that .cache directory (or anything else).

Not really sure what the fix is here, but basically, you can't just add :Z to that ${HOME} volume mount, or you basically screw up the system.

[BenTheElder]

We can move to $HOME/.cache and relabel, but this still seems mildly risky
(bad $HOME ?) and it still won't be WAI unless the permissions for .cache
are still correct for the user outside of planter. It also appears that
relabeling will be quite slow, but it seems we will just have to live with
that.

On Mon, Nov 20, 2017, 13:28 Leif Madsen [e-mail] wrote:

Well the main issue, is you can't just add :Z or :z to the -v
${HOME}:${HOME} because that'll relabel your entire home directory, which
basically locks you out of it as soon as you logout :). Without the
relabeling though, then Docker can't write to that volume, which doesn't
allow you to create that .cache directory (or anything else).

Not really sure what the fix is here, but basically, you can't just add :Z
to that ${HOME} volume mount, or you basically screw up the system.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/issues/5607#issuecomment-345837590,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4Bq7fHcPHFhDyRVv6jhkOBolOLapHPks5s4e70gaJpZM4QkzEG
.

[BenTheElder]

I can experiment with this more soonish, I need to get some more infrastructure changes in ideally before code freeze though, will come back to this next.
/assign
/kind bug

[leifmadsen]

All good. I have a work around for now. Disable SELinux :)

[leifmadsen]

Having gone through all the documentation and such, I think until(?) Planter moves away from needing access to the entire $HOME directory, instead of relabeling, we just disable SELinux for that particular container instantiation (which at least keeps SELinux enabled on the system instead of just disabling it).

I just ran a test, and it seems to work if you do this (per documentation on Dan Walsh's blog post @ http://www.projectatomic.io/blog/2016/03/dwalsh_selinux_containers/)

diff --git a/planter/planter.sh b/planter/planter.sh
index 95a7127..c697fa1 100755
--- a/planter/planter.sh
+++ b/planter/planter.sh
@@ -28,7 +28,7 @@ VOLUMES="-v ${REPO}:${REPO} -v ${HOME}:${HOME} --tmpfs /tmp:exec,mode=777"
 GID="$(id -g ${USER})"
 ENV="-e USER=${USER} -e GID=${GID} -e UID=${UID} -e HOME=${HOME}"
 # the final command to run
-CMD="docker pull ${IMAGE} && docker run --rm ${VOLUMES} --user ${UID} -w ${PWD} ${ENV} ${DOCKER_EXTRA:-} ${IMAGE} ${@}"
+CMD="docker pull ${IMAGE} && docker run --security-opt label:disable --rm ${VOLUMES} --user ${UID} -w ${PWD} ${ENV} ${DOCKER_EXTRA:-} ${IMAGE} ${@}"
 if [ -n "${DRY_RUN+set}" ]; then
     echo "${CMD}"
 else

/cc @BenTheElder @soltysh

[BenTheElder]

This sounds good to me, we definitely want to move away from mounting the entire $HOME, but I'd be semi concerned that eg $REPO could accidentally be home anyhow (maybe you have a dotfiles setup with a .git in home?) so relabeling is always going to be somewhat risky imo.

[leifmadsen]

Agreed. I think my change to just disable SELinux for this container, and assume it is a "super container" or whatever, is a good start. In my case, it's fine, because I'm creating an destroying a dedicated VM for building.

[ixdy]

Planter no longer mounts $HOME (#6006).

[leifmadsen]

I'd suggest closing this out then! :)