linux: honor mount-label for the notify socket #419

giuseppe · 2020-06-25T13:26:44Z

Signed-off-by: Giuseppe Scrivano [email protected]

rhatdan · 2020-06-25T13:47:47Z

LGTM

edsantiago · 2020-06-25T14:26:44Z

Better - I can now see the directory and socket file - but systemd-notify still doesn't work:

$ NOTIFY_SOCKET=/tmp/podman-sdnotify/notify ./bin/podman run --rm --sdnotify=container fedora sh -c 'systemd-notify --ready'
Failed to notify init system: Permission denied

AVC is now:

type=AVC msg=audit(1593095163.591:144897): avc:  denied  { sendto } for  pid=3151035 comm="systemd-notify" path="/run/user/14904/crun/fd69a7326c6e96c6c099709fa7ed6613d2c1dc62751869078673f39bd11e987c/notify/notify" scontext=system_u:system_r:container_t:s0:c320,c811 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

giuseppe · 2020-06-25T15:08:18Z

@rhatdan how can we address this issue? Should we allow writing to the socket?

goochjj · 2020-06-25T17:33:45Z

@giuseppe It doesn't look like JUST a label problem... You're actually clobbering the /run tmpfs mount.

+  libcrun_warning ("NOTIFY MOUNT  %s to %s", host_notify_socket_path, container_notify_socket_path_dir);
   ret = do_mount (container, host_notify_socket_path, -1, container_notify_socket_path_dir, NULL,
                   MS_BIND | MS_REC | MS_PRIVATE, NULL, 0, err);

Gives me

NOTIFY MOUNT  /run/crun/38f89151b835932ded6db71c6d65ffb5e6ec115fc3480ee3ea0c33c91748e27a/notify to /var/lib/containers/storage/overlay/4497d77a73f723d03d7359d984b20b0ad2efe9d00c8694558a37a3556b1c94e2/merged/run

So yes, the label ends up being wrong... but it's wrong on /run which breaks everything

goochjj · 2020-06-25T18:04:13Z

@giuseppe It doesn't look like JUST a label problem... You're actually clobbering the /run tmpfs mount.

+  libcrun_warning ("NOTIFY MOUNT  %s to %s", host_notify_socket_path, container_notify_socket_path_dir);
   ret = do_mount (container, host_notify_socket_path, -1, container_notify_socket_path_dir, NULL,
                   MS_BIND | MS_REC | MS_PRIVATE, NULL, 0, err);

Gives me

NOTIFY MOUNT  /run/crun/38f89151b835932ded6db71c6d65ffb5e6ec115fc3480ee3ea0c33c91748e27a/notify to /var/lib/containers/storage/overlay/4497d77a73f723d03d7359d984b20b0ad2efe9d00c8694558a37a3556b1c94e2/merged/run

So yes, the label ends up being wrong... but it's wrong on /run which breaks everything

Context - my NOTIFY_SOCKET is /run/notifytest

goochjj · 2020-06-25T18:15:32Z

@edsantiago @giuseppe
NOTIFY_SOCKET=/run/notify/notify

AVC avc:  denied  { sendto } for  pid=1695219 comm="systemd-notify" path="/run/crun/1d2edf748b1bbe744f4a86ccf8ab7e77b0ed84bb8fe32a9a1c83ecbe12cf5466/notify/notify" scontext=system_u:system_r:container_init_t:s0:c340,c575 tcontext=unconfined_u:system_r:container_runtime_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

NOTIFY_SOCKET=/tmp/podman-sdnotify/notify

audit: type=1400 audit(1593108777.091:6377): avc:  denied  { sendto } for  pid=1696055 comm="systemd-notify" path="/run/crun/e93c79341dbf8c1cf1597acfad4c1e05e5d8448aaeb960c6ab58993fd2021723/notify/notify" scontext=system_u:system_r:container_init_t:s0:c241,c773 tcontext=unconfined_u:system_r:container_runtime_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

Also note my scontext is coming in as container_init_t instead of container_t... I'm on Fedora CoreOS.

@edsantiago does it work for you if you use /run instead of tmp?

goochjj · 2020-06-25T18:35:51Z

I've been banging my head against this, with and without crun, I'm just unsure why

I'm getting container_init_t context instead of container_t
If that matters
What permissions are allowed in the container-selinux policy that allows for notify sockets to work.. Is it label specific? Path specific?

edsantiago · 2020-06-25T19:22:30Z

Nope:

$ sudo env NOTIFY_SOCKET=/run/aaa/sock ./bin/podman run --rm --sdnotify=container fedora sh -c 'ls -lZ /run/aaa'
ls: cannot open directory '/run/aaa': Permission denied

Without enforcing:

srwxrwxrwx. 1 root root unconfined_u:object_r:container_var_run_t:s0 0 Jun 25 19:18 notify

goochjj · 2020-06-25T19:26:02Z

@edsantiago can you do 'ps -Z 1; ls -lZ /; ls -lZ /run; ls -lZ /run/aaa'

edsantiago · 2020-06-25T19:32:49Z

TIL there's no ps in the fedora image. Relevant ls in container:

drwxr-xr-x. 1 root root system_u:object_r:container_file_t:s0:c103,c681 4096 Jun 25 19:30 /
drwxr-xr-x. 1 root root system_u:object_r:container_file_t:s0:c103,c681 4096 Jun 25 19:30 /run
drwx------. 2 root root unconfined_u:object_r:container_var_run_t:s0      60 Jun 25 19:30 /run/aaa

In host:

drwxr-xr-x. 53 root root system_u:object_r:var_run_t:s0     1720 Jun 25 13:15 /run
drwxr-xr-x.  2 root root unconfined_u:object_r:var_run_t:s0   40 Jun 25 13:22 /run/aaa
drwxr-xr-x.  2 root root unconfined_u:object_r:var_run_t:s0   40 Jun 25 13:22 /run/aaa

goochjj · 2020-06-25T19:40:14Z

oh yeah right
ps -Z $(podman inspect -l -f '{{.State.Pid}}')

goochjj · 2020-06-25T19:49:19Z

Well I answered one of my questions.. --systemd=always changes the label from container_t to container_init_t

edsantiago · 2020-06-25T19:58:07Z

Glad you got it. podman inspect is hanging for me while the container is running, and I'm not going to spend time investigating it today

goochjj · 2020-06-25T19:59:56Z

Do this in another terminal window:

for i in /run/crun/*/notify/*; do env NOTIFY_SOCKET=$i systemd-notify --ready; done

Note, I get the same permission denied errors with podman compiled from master, without the --sdnotify branch.

edsantiago · 2020-06-25T20:03:00Z

If it's all the same to you, I'm going to let @giuseppe and @rhatdan do the heavy lifting on figuring this one out. I see that as the best use of resources since they will probably spend O(few minutes) figuring this out.

goochjj · 2020-06-25T20:08:48Z

Sure, thanks for your help!

goochjj · 2020-06-25T20:13:24Z

I think this:
containers/container-selinux@fd55ae0

Should be allowing it...

giuseppe · 2020-06-26T10:03:28Z

So yes, the label ends up being wrong... but it's wrong on /run which breaks everything

thanks for debugging it. The NOTIFY_SOCKET implementation really looked only at what systemd passes in but it would break with a generic path. I've added another patch now, so the socket is created one level below.

Could you please check if it works for you now?

rhatdan · 2020-06-26T10:35:41Z

This should be allowed in container-selinux-2.137.0-1.fc32 But for some reason this has not gone into updates. I asked @lsm5 and @jnovy To look into it.

rhatdan · 2020-06-26T10:37:22Z

This AVC will still not be allowed.
type=AVC msg=audit(1593095163.591:144897): avc: denied { sendto } for pid=3151035 comm="systemd-notify" path="/run/user/14904/crun/fd69a7326c6e96c6c099709fa7ed6613d2c1dc62751869078673f39bd11e987c/notify/notify" scontext=system_u:system_r:container_t:s0:c320,c811 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

Not sure if it should, this looks like podman did not have the correct label on it.

giuseppe · 2020-06-26T10:42:35Z

This AVC will still not be allowed.
type=AVC msg=audit(1593095163.591:144897): avc: denied { sendto } for pid=3151035 comm="systemd-notify" path="/run/user/14904/crun/fd69a7326c6e96c6c099709fa7ed6613d2c1dc62751869078673f39bd11e987c/notify/notify" scontext=system_u:system_r:container_t:s0:c320,c811 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

Not sure if it should, this looks like podman did not have the correct label on it.

With this PR the notify socket will get the same label as the container instead of "unconfined".

giuseppe · 2020-06-26T19:49:20Z

@goochjj does the current version look good to you? I've still marked it as draft, I'll wait for your review before merging

goochjj · 2020-06-28T19:17:40Z

Nope...

# env NOTIFY_SOCKET=/run/notifytest podman run --systemd always --rm -it fedora sh -c 'printenv NOTIFY_SOCKET; ls -lZ /run; ls -lZ $NOTIFY_SOCKET;systemd-notify --ready'
/run/notify/notify
total 0
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 console
drwx------. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 cryptsetup
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 faillock
drwxrwxrwt. 3 root root system_u:object_r:container_file_t:s0:c686,c1016  60 Jun 28 19:12 lock
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 log
-rw-r--r--. 1 root root system_u:object_r:container_file_t:s0:c686,c1016   0 Jun 28 19:12 motd
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 motd.d
drwx------. 2 root root unconfined_u:object_r:container_var_run_t:s0      60 Jun 28 19:12 notifytest
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 secrets
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 sepermit
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 setrans
drwx--x--x. 3 root root system_u:object_r:container_file_t:s0:c686,c1016  60 Jun 28 19:12 sudo
drwxr-xr-x. 9 root root system_u:object_r:container_file_t:s0:c686,c1016 180 Jun 28 19:12 systemd
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c686,c1016  40 Jun 28 19:12 user
-rw-rw-r--. 1 root utmp system_u:object_r:container_file_t:s0:c686,c1016   0 Jun 28 19:12 utmp
ls: cannot access '/run/notify/notify': No such file or directory
Failed to notify init system: No such file or directory

Dir is mounted at /run/notifytest, but the ENV variable says /run/notify/notify
notifytest folder is 0700, should probably be 755 at least
Can't ls into /run/notifytest (avc?)
avc: denied { read } for pid=2457221 comm="ls" name="notify" dev="tmpfs" ino=33017042 scontext=system_u:system_r:container_t:s0:c61,c850 tcontext=unconfined_u:object_r:container_var_run_t:s0 tclass=dir permissive=0

Using /run/notify/notify, it works better.. ENV gets set to /run/notify/notify/notify, and /run/notify/notify/notify is labeled...

srwxrwxrwx. 1 root root unconfined_u:object_r:container_var_run_t:s0 0 Jun 28 19:14 /run/notify/notify/notify
avc:  denied  { sendto } for  pid=2457445 comm="systemd-notify" path="/run/crun/eb34fb5c578694fed43193854dcfaae2dad904134a6bcb973c98dd57865cdb74/notify/notify" scontext=system_u:system_r:container_t:s0:c588,c926 tcontext=unconfined_u:system_r:container_runtime_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

And yes this FCOS machine is using container-selinux 2.132.0 which I guess doesn't work per rhatdan as far as the sendto bits.

I don't think you need to preserve their NOTIFY_SOCKET value at all. You could always mount it to /run/notify/notify.sock or something, and just set the ENV variable appropriately, rather than doing parsing and trying to decide on a convention.

If you are going to do a convention what is it right now?

goochjj · 2020-06-28T19:53:03Z

Yeah w/ NOTIFY_SOCKET=/run/notify/notify.sock to podman
CRUN passes /run/notify/notify/notify
But it's mounted at /run/notify/notify.sock:

total 0
drwx------. 2 root root unconfined_u:object_r:user_tmp_t:s0 60 Jun 28 19:51 notify.sock

with the wrong label.

giuseppe · 2020-06-29T11:05:30Z

If you are going to do a convention what is it right now?

the convention we have now is to create a directory at the specified NOTIFY_SOCKET path and create the socket below it, so it is $NOTIFY_SOCKET/notify. I think it is better than hard-coding the path as we could risk to override another valid mount point.

I've fixed the issue you've reported but I am still seeing on Fedora 32:

Jun 29 12:40:04 lithium audit[25369]: AVC avc:  denied  { write } for  pid=25369 comm="systemd-notify" name="notify" dev="tmpfs" ino=450087 scontext=system_u:system_r:container_t:s0:c96,c910 tcontext=unconfined_u:object_r:user_tmp_t:s0 tclass=sock_file permissive=0

Signed-off-by: Giuseppe Scrivano <[email protected]>

rhatdan · 2020-06-29T12:32:05Z

The process creating the socket is not creating the socket with the correct label.
One thing we could do is setfscreatecon(mount_label) before creating the socket.

rhatdan · 2020-06-29T12:34:44Z

src/libcrun/linux.c

+      cleanup_free char *socket_path_file = NULL;
+      xasprintf (&socket_path_file, "%s/notify", host_notify_socket_path);
+      /* Ignore the error, the worse that can happen is that the container fails to notify it is ready.  */
+      (void) setxattr (socket_path_file, "security.selinux", container->container_def->linux->mount_label,


setfscreatecon(container->container_def->linux->mount_label) Before creating the socket. Should use setfilecon() rather then calling setxattr directly.

giuseppe · 2020-06-29T12:55:02Z

The process creating the socket is not creating the socket with the correct label.
One thing we could do is setfscreatecon(mount_label) before creating the socket.

the socket has the label:

The process creating the socket is not creating the socket with the correct label.
One thing we could do is setfscreatecon(mount_label) before creating the socket.

sorry that was an error in my patch. I was testing with "crun run" but I forgot that podman uses create+start.

It should be fixed now, and I get:

Jun 29 14:52:26 lithium audit[65397]: AVC avc:  denied  { sendto } for  pid=65397 comm="systemd-notify" path="/run/user/1000/crun/339c0c18f473ffaa70e315afcfed961766df49bba6a4cb0f6118c0dd0838ff5d/notify/notify" scontext=system_u:system_r:container_t:s0:c46,c49 tcontext=unconfined_u:system_r:container_runtime_t:s0-s0:c0.c1023 tclass=unix_dgram_socket permissive=0

I've updated container-selinux to: https://bodhi.fedoraproject.org/updates/FEDORA-2020-e6bbbe262b

Signed-off-by: Giuseppe Scrivano <[email protected]>

so there is no risk of overriding another existing mount. If NOTIFY_SOCKET=/run/foo/bar the notify socket will be mounted at /run/foo/bar/notify in the container. Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe · 2020-06-29T14:11:49Z

it works if I use 6b721daa0b9ff46a444e174995e5ac6600604db5 for container-selinux

rhatdan · 2020-06-29T14:17:48Z

That should be allowed in latest policy.

container-selinux-2.137.0-1.fc32.noarch

rhatdan · 2020-06-29T14:19:12Z

https://bodhi.fedoraproject.org/updates/FEDORA-2020-e6bbbe262b

rhatdan · 2020-06-29T14:19:23Z

Please update it's karma

giuseppe · 2020-06-29T16:18:10Z

updated package: https://bodhi.fedoraproject.org/updates/FEDORA-2020-9ec7a517d4

giuseppe · 2020-06-30T13:37:23Z

@goochjj could you give it a last try? As soon as this PR is merged I'll prepare 0.14

goochjj · 2020-06-30T15:48:35Z

checking

goochjj · 2020-06-30T16:34:08Z

Tested, selinux (w/ container-selinux 137) and without selinux, root and rootless, all clear.

Thanks!

goochjj · 2020-06-30T16:35:40Z

@giuseppe LGTM

giuseppe · 2020-06-30T18:28:23Z

@goochjj thanks for checking it out!

edsantiago · 2020-07-01T20:54:31Z

@giuseppe are you planning to do a build? If so I'd like to try another one shortly thereafter, to include a -tests subpackage.

giuseppe · 2020-07-02T06:44:41Z

@giuseppe are you planning to do a build? If so I'd like to try another one shortly thereafter, to include a -tests subpackage.

I am planning to cut a new release: #416

Would that work or do we need to hold it?

giuseppe mentioned this pull request Jun 25, 2020

Implement --sdnotify cmdline option to control sd-notify behavior containers/podman#6693

Merged

giuseppe marked this pull request as draft June 25, 2020 18:48

giuseppe force-pushed the label-notify-socket branch from d330222 to 1a53815 Compare June 26, 2020 10:02

giuseppe force-pushed the label-notify-socket branch 2 times, most recently from 0dbed09 to fa6d316 Compare June 29, 2020 11:58

contrib: add notify-socket-server for testing

0fcf4e4

Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe force-pushed the label-notify-socket branch from fa6d316 to 264b76a Compare June 29, 2020 12:30

rhatdan reviewed Jun 29, 2020

View reviewed changes

giuseppe force-pushed the label-notify-socket branch from 264b76a to 1eddc1d Compare June 29, 2020 12:53

giuseppe added 2 commits June 29, 2020 14:59

linux: honor mount-label for the notify socket

3cd4f0c

Signed-off-by: Giuseppe Scrivano <[email protected]>

linux: create the notify socket one level below

c04fd15

so there is no risk of overriding another existing mount. If NOTIFY_SOCKET=/run/foo/bar the notify socket will be mounted at /run/foo/bar/notify in the container. Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe force-pushed the label-notify-socket branch from 1eddc1d to c04fd15 Compare June 29, 2020 12:59

giuseppe marked this pull request as ready for review June 30, 2020 06:38

giuseppe merged commit 65e35c5 into containers:master Jul 1, 2020

linux: honor mount-label for the notify socket #419

linux: honor mount-label for the notify socket #419

Conversation

giuseppe commented Jun 25, 2020

rhatdan commented Jun 25, 2020

edsantiago commented Jun 25, 2020

giuseppe commented Jun 25, 2020

goochjj commented Jun 25, 2020

goochjj commented Jun 25, 2020

goochjj commented Jun 25, 2020

goochjj commented Jun 25, 2020

edsantiago commented Jun 25, 2020

goochjj commented Jun 25, 2020

edsantiago commented Jun 25, 2020

goochjj commented Jun 25, 2020

goochjj commented Jun 25, 2020

edsantiago commented Jun 25, 2020

goochjj commented Jun 25, 2020

edsantiago commented Jun 25, 2020

goochjj commented Jun 25, 2020

goochjj commented Jun 25, 2020

giuseppe commented Jun 26, 2020

rhatdan commented Jun 26, 2020

rhatdan commented Jun 26, 2020

giuseppe commented Jun 26, 2020

giuseppe commented Jun 26, 2020

goochjj commented Jun 28, 2020

goochjj commented Jun 28, 2020

giuseppe commented Jun 29, 2020

rhatdan commented Jun 29, 2020

rhatdan Jun 29, 2020

Choose a reason for hiding this comment

giuseppe commented Jun 29, 2020

giuseppe commented Jun 29, 2020

rhatdan commented Jun 29, 2020

rhatdan commented Jun 29, 2020

rhatdan commented Jun 29, 2020

giuseppe commented Jun 29, 2020

giuseppe commented Jun 30, 2020

goochjj commented Jun 30, 2020

goochjj commented Jun 30, 2020

goochjj commented Jun 30, 2020

giuseppe commented Jun 30, 2020

edsantiago commented Jul 1, 2020

giuseppe commented Jul 2, 2020