root:root owns docker.sock by default

[lsm5]

See: https://docs.docker.com/installation/binaries/#giving-non-root-access

Signed-off-by: Lokesh Mandvekar lsm5@fedoraproject.org

[tianon]

Except that Docker can't change the user/group on the socket if systemd was the one that created it, since Docker doesn't even know where that socket file is (it only has the file descriptor systemd passed in).

[lsm5]

sorry, not sure if i'm reading ^ right, would you prefer systemd enforce root:root via Socket{User,Group} ?

[tianon]

No. I mean that the documentation you've linked specifically only applies when Docker itself is responsible for creating docker.sock, which isn't the case for systemd.

[lsm5]

aah sorry about that, perhaps this projectatomic thread explains better

[jessfraz]

I do not agree with this change. I used to install docker from the get.docker.com script which installs this systemd service on debian jessie. Those users are going to update to docker (say this is merged) and find they have to sudo everything all of a sudden NOT LGTM

[tianon]

Indeed, I'd actually be a whole lot more +1 on removing socket activation altogether and letting the Docker daemon handle this intelligently than I am on this particular change.

[rhatdan]

I agree, I actually think we should do away with socket activation, and let docker handle this. With socket activation, docker autostarting of containers does not work either, I believe.

[cgwalters]

Systemd could have SocketGroupIfExists= ...a little ugly. But probably easy to implement on that side. (Then of course we get into the versioning discussion)

[lsm5]

@philips ping, wdyt?

[phemmer]

I'm really confused on this PR. The documentation that is linked to says "ownership of the Unix socket read/writable by the docker group", and the PR is to change this behavior such that the documentation will be wrong/inconsistent (nevermind the fact that this only changes it for systemd, and not if docker creates the socket).

I highly disagree with making the socket group root. If the concern is that adding users to the 'docker' group is dangerous, then don't add them to the docker group.
My hard drives are read/write by group 'disk'. /dev/kmem is readable by group 'kmem'. Both of these are extremely dangerous if an untrusted user is added to the groups, yet they exist and it's not an issue.

This has also come up numerous times in the past: #5656 #7306 #9976

[cgwalters]

Yes, but there's no sane reason to add normal users to the disk group, so no one clamors for it. However, people are tempted to not have to type sudo in front of docker, but the fact that Docker allows this out of the box means that many people are confused by the fact that it is root equivalent, as you showed with those previous issues.

I guess at this point it may be too hard to change this by default in upstream Docker, but I also think this is something that downstream distributions that want to provide more secure defaults should be able to easily turn off by default.

Turning it back on is not too hard - you groupadd docker and create an /etc/systemd/system/docker.socket.d/dockergroup.conf that overrides. Or just copy the unit file into /etc and change there.

[SvenDowideit]

If you are going to merge something like this, then it must be accompanied with some documenation changes that explain why that distro is annoying its users (even if the change is sensible, users don't like it), and how they fix it. (as getting rid of sudo is an extremely regularly requested thing)

[jessfraz]

ya thats why I NOT LGTM'd it

On Mon, Jan 19, 2015 at 4:16 PM, Sven Dowideit notifications@github.com
wrote:

If you are going to merge something like this, then it must be
accompanied with some documenation changes that explain why that distro is
annoying its users (even if the change is sensible, users don't like it),
and how they fix it. (as getting rid of sudo is an extremely regularly
requested thing)

—
Reply to this email directly or view it on GitHub
#10144 (comment).

[lsm5]

thanks all for comments, I guess it's best for now to set this in the rpms instead.

[lsm5]

ugh, earlier comment was made before the 2 comments got loaded on this page.

@SvenDowideit I'll send another PR updating the fedora/centos/rhel docs about the PITAs we cause.

[rhatdan]

The long term solution to this is to add proper auditing/logging to docker to record who is doing what on the machine.
Similarly we need to add RBAC (Role Based Access Control) to docker.

Can dwalsh start/stop/exec containers A,B,C?
Can dwalsh run a container with privilege? Privilege means --privileged as well as --cap_add/cap_remove, --security-opt ...
Can @jfrazelle list the images, containers?

Can I prove it is jfrazelle and dwalsh by asking them to provide a password within a specified period of time like sudo does.

First step to do this is to get proper authentication into docker. We are looking into adding gssproxy/saslauthd support to allow authentication on remote connections.

Next step is to create a potential authorization database, could be a simple json db, which can be modified to add users, permissions and lists of images/containers.

Right now adding dwalsh to the docker group allows a user to become root by executing

docker run -ti --rm --permissive -v /:/host fedora chroot /host

There is no information in the /var/log/secure or journal that the user did this. This is worse then adding NOPASSWD to a sudoers file, where at least the command you execute as root gets logged.

By having this on by default, we are encouraging users to do something that is potentially dangerous on their system.