Enable running as non root

[abjugard]

Addresses part of #104

Fixes #273

[francislavoie]

I don't understand what CI is doing :thinking: it's erroring out on the nsswitch.conf thing, but we removed that.

This will need @hairyhenderson's eyes on it I think.

I opened my own PR #275 and I guess this one precludes that one since it makes the same fix.

[abjugard]

I don't understand what CI is doing 🤔 it's erroring out on the nsswitch.conf thing, but we removed that.

Yeah, very strange, that stage seems to be building using the latest commit on master, with the test before it building and succeeding using my branch?

What's the purpose of this step that builds the old code?

[abjugard]

(No change, just added the #273 issue tag to the relevant commit)

[abjugard]

@francislavoie another thing to mention as well (didn't look like it was part of this repo otherwise I would have updated it myself), the documentation (https://hub.docker.com/_/caddy) for building your own caddy using the caddy-builder image, and copying the resulting binary onto this image, should also include the libcap stuff in my opinion.

I've pushed another commit adding libcap to the builder image so that the documentation can be updated to suggest the following Dockerfile:

FROM caddy:<version>-builder AS builder

RUN xcaddy build \
    --with github.com/caddyserver/nginx-adapter \
    --with github.com/hairyhenderson/caddy-teapot-module@v0.0.3-0

RUN setcap cap_net_bind_service=+eip /usr/bin/caddy

FROM caddy:<version>

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

[francislavoie]

Oh, really? It needs to be set on the binary before copying it over? Wow, that's really annoying. I thought setcap would write a rule to the existing container (i.e. the final stage) that would persist when the binary changes at the same path. I guess that's not how it works? I've never dug into how setcap really works.

I'd really hope not to have to add that to the recommended instructions. Is there anything we could do to make it smoother without needing to add that line?

An idea, we could add an option to xcaddy to try to run setcap on the binary after it's done building it. Would that work at all? Then we could use an env var to instruct xcaddy to do so, so the user doesn't see any difference in their own Dockerfile.

[abjugard]

Having xcaddy run setcap on the binary is enough yeah, and sounds like an elegant solution in my opinion! However... how do we instruct users that it's possible to do that, since that would still require changes to the Dockerfile?

And no, the capability is set on the binary itself so it doesn't survive if you copy another binary over it.

Another possible solution is to put a simple "loader" binary in the caddy image, then in the image we would RUN setcap cap_net_bind_service=+eip /usr/bin/caddy_loader, this loader would be set as the entrypoint, and it would load /usr/bin/caddy, transferring the capability to the loaded caddy binary.

It's less elegant but it requires zero changes to both the builder Dockerfile and xcaddy.

[francislavoie]

how do we instruct users that it's possible to do that, since that would still require changes to the Dockerfile?

We'd add it to here, maybe XCADDY_SETCAP_NET=1 or something:

https://github.com/caddyserver/caddy-docker/blob/aca93aecdbd9ba4b4229fba1e6c7ee7b87d5cf7f/Dockerfile.builder.tmpl#L11

Then users don't need to do anything, cause the builder image would just do it.

Edit: See https://github.com/caddyserver/xcaddy/issues/128

[jjlin]

I tried doing something similar for another project a couple of years ago, and there are some big caveats. I haven't checked whether they still apply, but I wouldn't be surprised at all if they do.

1. Capabilities are associated with an executable via extended file attributes. The COPY instruction doesn't copy extended attributes, so you can't do a multi-stage build where you set the capability in the build stage and then copy that out to a runtime stage.

2. If you set a capability via a separate RUN instruction, Docker seems to make another copy of the executable in a new layer (with an image where the bulk of the content is the executable, the image basically doubled in size). So it's probably best to build the executable and set the capability via a single RUN.

3. I've read that Docker support for extended attributes may not be the same across all Linux platforms, though I don't have personal experience beyond the common ones. Since the Caddy image is built for some of the less common platforms, that might be something to verify.

This blog post is a couple of years old, but seems to cover most of what I observed, and more.

[francislavoie]

Thanks @jjlin, that's a useful article.

1. Capabilities are associated with an executable via extended file attributes. The COPY instruction doesn't copy extended attributes, so you can't do a multi-stage build where you set the capability in the build stage and then copy that out to a runtime stage.

That seems to have been true for the original Docker builder, but with Buildx/BuildKit, it is preserved. The article you linked mentions that. BuildKit is on by default now in latest versions of Docker, so it should work fine if users are keeping Docker up to date.

2. If you set a capability via a separate RUN instruction, Docker seems to make another copy of the executable in a new layer (with an image where the bulk of the content is the executable, the image basically doubled in size). So it's probably best to build the executable and set the capability via a single RUN.

Yep, that's a good point, and that's why I'd like for xcaddy to do it for us.

This PR adds setcap to the main RUN already for the main image, so that's already good :+1:

3. I've read that Docker support for extended attributes may not be the same across all Linux platforms, though I don't have personal experience beyond the common ones. Since the Caddy image is built for some of the less common platforms, that might be something to verify.

:thinking: yeah, I don't have experience with this. We'll raise it with the Docker Library team when we push it there, they can tell us what's up.

[abjugard]

🤔 yeah, I don't have experience with this. We'll raise it with the Docker Library team when we push it there, they can tell us what's up.

For what it's worth it works as we expect it to on my machine (Intel Mac running colima in vz), documented my experience in https://github.com/caddyserver/xcaddy/issues/128

[jjlin]

I tried implementing this again in my other project, and with DOCKER_BUILDKIT=1 (BuildKit doesn't seem to be enabled by default), the COPY instruction does carry over the capabilities, so that's nice.

While testing this, I noticed that executables running as non-root can (unexpectedly) bind to "privileged" ports even without setting cap_net_bind_service. Apparently this behavior was introduced in Docker 20.10.0 via moby/moby#41030. To disable this feature for testing, you need to do something like docker run --sysctl net.ipv4.ip_unprivileged_port_start=1024 ....

Nevertheless, setting cap_net_bind_service should still be useful for people running earlier versions of Docker, or using another container runtime that doesn't have this feature.

[francislavoie]

Oooh that's awesome, thanks for trying that and finding that new feature in Moby!

I think buildkit might only be enabled by default in new installs, not in upgraded ones. That's what I read in the docs I think.

[hairyhenderson]

@abjugard can you revert the changes to library/caddy for now? I usually only update that separately after PRs have merged, to ensure the commits are correct for the main branch.

[abjugard]

@abjugard can you revert the changes to library/caddy for now? I usually only update that separately after PRs have merged, to ensure the commits are correct for the main branch.

@hairyhenderson Done!

[francislavoie]

Opened a PR in xcaddy: https://github.com/caddyserver/xcaddy/pull/129

[hairyhenderson]

Do we need caddyserver/xcaddy#129 to merge first? otherwise this LGTM (except that this needs to be rebased, and the Dockerfiles regenerated now that the errant .editorconfig is gone)

[francislavoie]

Yeah, I think we should wait for xcaddy. This is only a half solution until we can make the builder also work correctly. Shouldn't take long, Matt was on vacation but will get back into thing soon.

[francislavoie]

Alright @abjugard, https://github.com/caddyserver/xcaddy/releases/tag/v0.3.2 is released! Got time to finish this up? :tada:

Caddy v2.6.3 is gonna release sometime this week, would be great to get this in for that version.

[abjugard]

Absolutely @francislavoie. Want me to bump xcaddy in this PR then?

[abjugard]

How's that @francislavoie?

[abjugard]

Sorry, copy-pasting hashes is hard 😅. Now it's all correct!