add a static usermode helper by tych0 · Pull Request #2037 · linuxkit/linuxkit

tych0 · 2017-06-13T22:16:53Z

The binary is used in tandem with CONFIG_STATIC_USERMODEHELPER=y in 4.11+,
see the big comment in the binary for the current whitelist of binaries.

Should fix #1760.

rn

thanks for adding this. A few comments, some to help my understanding on how this works...

rn · 2017-06-13T22:25:41Z

pkg/init/Dockerfile

@@ -1,3 +1,9 @@
+FROM alpine:edge AS build


this should use linuxkit/alpine:<latest hash> just like the next FROM line. the alpine base image should have all the tools you need to compile it.

Right, I'll update thanks.

Hmm. It doesn't seem to have make or gcc?

~/packages/linuxkit/pkg/init usermode-helper make docker build --no-cache --network=none -t linuxkit/init:8ea972f0537944e1cf96d4f914d82bea0f9fe62c . Sending build context to Docker daemon 18.94kB Step 1/16 : FROM linuxkit/alpine:630ee558e4869672fae230c78364e367b8ea67a9 AS mirror ---> 72ff54f67634 Step 2/16 : ADD usermode-helper.c . ---> 3fba2922bf2b Removing intermediate container 50e00ff17303 Step 3/16 : RUN make usermode-helper ---> Running in 992e606cfe3f /bin/sh: make: not found The command '/bin/sh -c make usermode-helper' returned a non-zero code: 127 Makefile:11: recipe for target 'tag' failed make: *** [tag] Error 127 ~/packages/linuxkit/pkg/init usermode-helper 2 ls bin/ Dockerfile etc/ init* Makefile usermode-helper.c ~/packages/linuxkit/pkg/init usermode-helper vi Makefile ~/packages/linuxkit/pkg/init usermode-helper vi Dockerfile ~/packages/linuxkit/pkg/init usermode-helper make docker build --no-cache --network=none -t linuxkit/init:8ea972f0537944e1cf96d4f914d82bea0f9fe62c . Sending build context to Docker daemon 18.94kB Step 1/16 : FROM linuxkit/alpine:630ee558e4869672fae230c78364e367b8ea67a9 AS mirror ---> 72ff54f67634 Step 2/16 : ADD usermode-helper.c . ---> a25aa94cf323 Removing intermediate container 537002280a38 Step 3/16 : RUN gcc usermode-helper.c -o usermode-helper ---> Running in 69826310ac85 /bin/sh: gcc: not found The command '/bin/sh -c gcc usermode-helper.c -o usermode-helper' returned a non-zero code: 127 Makefile:11: recipe for target 'tag' failed make: *** [tag] Error 127

it does, but it's in a APK mirror. You still need to apk add them as in:

FROM linuxkit/alpine:630ee558e4869672fae230c78364e367b8ea67a9 AS build RUN mkdir -p /out/etc/apk && cp -r /etc/apk/* /out/etc/apk/ RUN apk add --no-cache --initdb -p /out alpine-baselayout make gcc ...

[note to self that I need to rewrite the packages document...]

when I use -p out, RUN still doesn't pick up the commands; I dropped that, which I think is fine since this is just a stage in a multi-stage build, so it won't bloat the final image.

rn · 2017-06-13T22:26:59Z

pkg/init/Makefile

 HASH?=$(shell git ls-tree HEAD -- ../$(notdir $(CURDIR)) | awk '{print $$3}')

 tag: $(DEPS)
-	docker build --no-cache --network=none -t $(ORG)/$(IMAGE):$(HASH) .


If you use the alpine base image you don't need network

rn · 2017-06-13T22:27:19Z

pkg/init/usermode-helper.c

+		 * features to linuxkit this whitelist may need changing (or a
+		 * policy, like always allow stuff in /sbin).
+		 */
+		exit(2);


should we log the binary name (or is this already logged?)

It's not logged by the kernel as far as I can tell. Where/how would you like to log it?

maybe fprintf(stderr, ) for now? I don't know where that will end up :)

It doesn't end up anywhere that I could figure out, when debugging this i just logged to a file in /tmp. I'll see if I can figure out where it goes.

Does syslog(3) end up somewhere useful?

Not as far as I can tell. Should it go somewhere besides dmesg in linuxkit?

Oh, there doesn't seem to be a syslog, so no perhaps not. Not sure if this is what projects/logging is looking to address or not.

cc @MagnusS ^

projects/logging contains an in-memory logging daemon that could capture the logs. I've not made it syslog compatible (yet) as it captures data from fd's passed to it (typically stderr/stdout from onboot/service/dockerd containers) so we don't have to bind mount /dev/log, but if we need it we could add it. It's a bit hard to enable at the moment unfortunately as it needs to run as a service before the onboot containers, which requires a modified init

rn · 2017-06-13T22:28:21Z

kernel/kernel_config-4.11.x

 # CONFIG_HARDENED_USERCOPY_PAGESPAN is not set
-# CONFIG_STATIC_USERMODEHELPER is not set
+CONFIG_STATIC_USERMODEHELPER=y
+CONFIG_STATIC_USERMODEHELPER_PATH="/sbin/usermode-helper"


does this need to be hard coded here or is there a sysctl/syfs entry we can use? Seems weird to have this in the kernel config...

In fact this hard coding is the bit that's designed to prevent exploits: people used to rewrite the sysctl for dynamic binaries like modprobe to something that they controlled. By hard coding it, they have to overwrite the binary itself instead.

see https://outflux.net/blog/archives/2017/05/02/security-things-in-linux-v4-11/

Cool, yes, that makes a lot of sense

rn · 2017-06-13T22:35:36Z

pkg/init/usermode-helper.c

+	} else if (!strcmp(argv[0], "/sbin/modprobe")) {
+		/* allow modprobe */
+		execv(argv[0], argv);
+	} else {


we probably want to add /sbin/reboot and /sbin/poweroffas this is used by orderly_reboot() and orderly_poweroff() in kernel/reboot.c and on Hyper-V this can be triggered from the host (see drivers/hv/hv_util.c

Good catch, thanks!

some residual memory from a day or two of debugging/cursing a year or so ago :)

riyazdf · 2017-06-13T23:37:59Z

pkg/init/Dockerfile

@@ -1,3 +1,10 @@
+FROM linuxkit/alpine:630ee558e4869672fae230c78364e367b8ea67a9 AS build
+# RUN mkdir -p /out/etc/apk && cp -r /etc/apk/* /out/etc/apk/


remove unused line? Though it seems like we use this in other images so I'm wondering if we need this here

Yep, sorry about that.

sorry, partly my fault. this line is not needed. It's only needed when you build a new root fs for a package. since we are not doing this in this stage it is not needed

rn · 2017-06-13T23:46:15Z

pkg/init/Dockerfile

 COPY init /
 COPY etc etc/
 COPY bin bin/
+COPY --from=build usermode-helper /sbin/


I would move that higher to have the COPY lines from other stages grouped together.

Fixed, thanks.

rn · 2017-06-13T23:47:10Z

could you squash the commits?

tych0 · 2017-06-14T02:55:52Z

Yep, just squashed them.

rn

you might also want to add undermode-helper.c the DEPS in the Makefile

rn · 2017-06-14T05:36:24Z

pkg/init/Dockerfile

+RUN apk add --no-cache --initdb alpine-baselayout make gcc musl-dev
+
+ADD usermode-helper.c .
+RUN make usermode-helper


how does this work without Makefile in the PR?

using make's "implicit targets" trick, https://www.gnu.org/software/make/manual/html_node/Implicit-Rules.html

You can do it with any .c file in any directory, assuming that file doesn't need any special compiler or linker args.

ah, I learned something today

ijc · 2017-06-14T09:52:55Z

pkg/init/usermode-helper.c

+{
+	if (!strcmp(argv[0], "/sbin/mdev")) {
+		/* busybox uses /sbin/mdev for early uevent bootstrapping */
+		execv(argv[0], argv);


For each of these it would be good for debugability if it would log the error if the execv returns.

Yes, good point. I'll add something like this when I figure out where to log stuff :)

Maybe we can add fprintf(stderr,...) for now and update to something else later.

tych0 · 2017-06-14T14:53:16Z

Yes, absolutely. I just added usermode-helper.c to the makefile deps

rn · 2017-06-14T17:05:46Z

pkg/init/usermode-helper.c

+
+	if (!strcmp(argv[0], "/sbin/mdev")) {
+		/* busybox uses /sbin/mdev for early uevent bootstrapping */
+		execv(argv[0], argv);


shouldn't we catch and log if execv errors? @ijc commented on this earlier...

just spotted the perror at the end and with the logging of the command line options that should provide enough info for debug

rn · 2017-06-14T17:06:16Z

pkg/init/usermode-helper.c

+		 * features to linuxkit this whitelist may need changing (or a
+		 * policy, like always allow stuff in /sbin).
+		 */
+		exit(2);


and log the command we denied

I think its ok to ignore logging for now, but maybe we can add a comment in saying that it may be useful to add later.

Or log to stdout like the perror below, and add a note saying stdout should be redirected to a log in future.

Yep, we are logging the command (and error, in case of exec failure). I just added a comment pointing out that the output doesn't go anywhere useful now, but once we have syslog, it should probably go there.

The binary is used in tandem with CONFIG_STATIC_USERMODEHELPER=y in 4.11+, see the big comment in the binary for the current whitelist of binaries. Signed-off-by: Tycho Andersen <tycho@docker.com>

rn · 2017-06-15T18:03:59Z

once this and #2047 is merged I'll kick off a kernel build/push

rn · 2017-06-15T18:05:37Z

well, and after I've updated to 4.11.5, 4.9.32, 4.4.72

tych0 · 2017-06-15T18:13:06Z

On Thu, Jun 15, 2017 at 11:04:05AM -0700, Rolf Neugebauer wrote: once this and #2047 is merged I'll kick off a kernel build/push

We'll need an init push too, otherwise 4.11 will probably fail to boot (or the devices will be all screwed up, at least).

GordonTheTurtle added the status/0-triage label Jun 13, 2017

rn reviewed Jun 13, 2017

View reviewed changes

riyazdf reviewed Jun 13, 2017

View reviewed changes

rn reviewed Jun 13, 2017

View reviewed changes

tych0 force-pushed the usermode-helper branch from b922a0f to 2417fa3 Compare June 14, 2017 02:55

rn reviewed Jun 14, 2017

View reviewed changes

ijc reviewed Jun 14, 2017

View reviewed changes

tych0 force-pushed the usermode-helper branch from 2417fa3 to 6d907ea Compare June 14, 2017 14:51

tych0 force-pushed the usermode-helper branch from 6d907ea to 483a7bd Compare June 14, 2017 16:41

rn reviewed Jun 14, 2017

View reviewed changes

add a static usermode helper

a4e0a59

The binary is used in tandem with CONFIG_STATIC_USERMODEHELPER=y in 4.11+, see the big comment in the binary for the current whitelist of binaries. Signed-off-by: Tycho Andersen <tycho@docker.com>

tych0 force-pushed the usermode-helper branch from 483a7bd to a4e0a59 Compare June 15, 2017 17:40

justincormack approved these changes Jun 15, 2017

View reviewed changes

rn merged commit d48715c into linuxkit:master Jun 15, 2017

		@@ -1,3 +1,10 @@
		FROM linuxkit/alpine:630ee558e4869672fae230c78364e367b8ea67a9 AS build
		# RUN mkdir -p /out/etc/apk && cp -r /etc/apk/* /out/etc/apk/

Conversation

tych0 commented Jun 13, 2017

Uh oh!

rn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rn commented Jun 13, 2017

Uh oh!

tych0 commented Jun 14, 2017

Uh oh!

rn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment