| Message ID | 87361ecm7f.fsf@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#44549] etc: updates for the guix-daemon SELinux policy | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
Marius Bakke <marius@gnu.org> writes: > Hello Daniel, > > Thanks a lot for this. You're welcome. > > Daniel Brooks <db48x@db48x.net> writes: > >>>From 7dd9ed6da01c5bf125c95592f4978b579198731a Mon Sep 17 00:00:00 2001 >> From: Daniel Brooks <db48x@db48x.net> >> Date: Mon, 9 Nov 2020 07:03:42 -0800 >> Subject: [PATCH] etc: updates for the guix-daemon SELinux policy >> >> * etc/guix-daemon.cil.in: I can't promise that this is a complete list of >> everything that guix-daemon needs, but it's probably most of them. It can >> search for, install, upgrade, and remove packages, create virtual machines, >> update itself, and so on. I haven't tried creating containers yet, which might >> reveal more things to add. > > This commit message is somewhat unorthodox. :-) > > Perhaps it can be shortened to: > > * etc/guix-daemon.cil.in (guix_daemon): Specify more permissions for > guix-daemon to account for daemon updates and newer SELinux. I suppose. Personally I dislike the changelog style commit messages, but when in Rome… >> +;; In particular, you can run semanage permissive -a guix_daemon.guix_daemon_t >> +;; to allow guix-daemon to do whatever it wants. SELinux will still check its >> +;; permissions, and when it doesn't have permission it will still send an >> +;; audit message to your system logs. This lets you know what permissions it >> +;; ought to have. Use ausearch --raw to find the permissions violations, then >> +;; pipe that to audit2allow to generate an updated policy. You'll still need >> +;; to translate that policy into CIL in order to update this file, but that's >> +;; fairly straight-forward. Annoying, but easy. > > I'm not sure about the second paragraph. It's mainly a rehash of the > blog post, no? And there are many other ways to go about > troubleshooting SELinux (I did not use ausearch at all). True. I just wanted a quick summary somewhere in the source so that future us won't have to rely on a random blog post, even one from Dan Walsh. > diff --git a/etc/guix-daemon.cil.in b/etc/guix-daemon.cil.in > index 666e5677a3..b5909f1b18 100644 > --- a/etc/guix-daemon.cil.in > +++ b/etc/guix-daemon.cil.in > @@ -84,6 +84,9 @@ > (allow init_t > guix_daemon_t > (process (transition))) > + (allow init_t > + guix_store_content_t > + (lnk_file (read))) This one is a little unusual; is your service file symlinked or something? > (allow init_t > guix_store_content_t > (file (open read execute))) > @@ -166,6 +169,9 @@ > (allow guix_daemon_t > root_t > (dir (mounton))) > + (allow guix_daemon_t > + guix_daemon_socket_t > + (sock_file (unlink))) That shouldn't be a problem, though we don't have any other rules for guix_daemon_socket_t. Possibly that is because my socket file is labeled guix_daemon_conf_t, for unknown reasons. Perhaps it was not labeled correctly when created, and hasn't been relabeled since. > (allow guix_daemon_t > fs_t > (filesystem (getattr))) > @@ -348,7 +354,12 @@ > getopt setopt))) > (allow guix_daemon_t > self > - (tcp_socket (accept listen bind connect create setopt getopt getattr ioctl))) > + (netlink_route_socket (read write))) > + (allow guix_daemon_t > + self > + (tcp_socket (accept > + listen bind connect create read write > + setopt getopt getattr ioctl))) These are fine; in fact I discovered these myself this morning and was going to send a patch. > Can you test these additional changes on Fedora? Yes, I'll let you know if there are any problems. Also, I'll investigate the socket file some more. > > With this, I no longer have to go through 'guix pack' and 'podman' to > run Guix packages on my RHEL workstation! :-) Ideal :) > > Also, is it OK to add you to the list of contributors at the top of the > file with this name and address? Certainly. db48x
Daniel Brooks <db48x@db48x.net> writes: >> Daniel Brooks <db48x@db48x.net> writes: >> >>>>From 7dd9ed6da01c5bf125c95592f4978b579198731a Mon Sep 17 00:00:00 2001 >>> From: Daniel Brooks <db48x@db48x.net> >>> Date: Mon, 9 Nov 2020 07:03:42 -0800 >>> Subject: [PATCH] etc: updates for the guix-daemon SELinux policy >>> >>> * etc/guix-daemon.cil.in: I can't promise that this is a complete list of >>> everything that guix-daemon needs, but it's probably most of them. It can >>> search for, install, upgrade, and remove packages, create virtual machines, >>> update itself, and so on. I haven't tried creating containers yet, which might >>> reveal more things to add. >> >> This commit message is somewhat unorthodox. :-) >> >> Perhaps it can be shortened to: >> >> * etc/guix-daemon.cil.in (guix_daemon): Specify more permissions for >> guix-daemon to account for daemon updates and newer SELinux. > > I suppose. Personally I dislike the changelog style commit messages, but > when in Rome… It's not a very strong opinion. I think it would be fine without the first person style. >>> +;; In particular, you can run semanage permissive -a guix_daemon.guix_daemon_t >>> +;; to allow guix-daemon to do whatever it wants. SELinux will still check its >>> +;; permissions, and when it doesn't have permission it will still send an >>> +;; audit message to your system logs. This lets you know what permissions it >>> +;; ought to have. Use ausearch --raw to find the permissions violations, then >>> +;; pipe that to audit2allow to generate an updated policy. You'll still need >>> +;; to translate that policy into CIL in order to update this file, but that's >>> +;; fairly straight-forward. Annoying, but easy. >> >> I'm not sure about the second paragraph. It's mainly a rehash of the >> blog post, no? And there are many other ways to go about >> troubleshooting SELinux (I did not use ausearch at all). > > True. I just wanted a quick summary somewhere in the source so that > future us won't have to rely on a random blog post, even one from Dan > Walsh. Fair point. I can imagine a scenario when I'm stuck on a SELinux system without an internet connection. >> diff --git a/etc/guix-daemon.cil.in b/etc/guix-daemon.cil.in >> index 666e5677a3..b5909f1b18 100644 >> --- a/etc/guix-daemon.cil.in >> +++ b/etc/guix-daemon.cil.in >> @@ -84,6 +84,9 @@ >> (allow init_t >> guix_daemon_t >> (process (transition))) >> + (allow init_t >> + guix_store_content_t >> + (lnk_file (read))) > > This one is a little unusual; is your service file symlinked or something? Hmm. Could it be because /etc/systemd/system/guix-daemon.service refers to /var/guix/profiles/per-user/root/current-guix/bin/guix-daemon? >> (allow init_t >> guix_store_content_t >> (file (open read execute))) >> @@ -166,6 +169,9 @@ >> (allow guix_daemon_t >> root_t >> (dir (mounton))) >> + (allow guix_daemon_t >> + guix_daemon_socket_t >> + (sock_file (unlink))) > > That shouldn't be a problem, though we don't have any other rules for > guix_daemon_socket_t. Possibly that is because my socket file is labeled > guix_daemon_conf_t, for unknown reasons. Perhaps it was not labeled > correctly when created, and hasn't been relabeled since. It could also be an artifact from my ancient experiments with Guix and SELinux on this system. Perhaps we should test on a "clean" system to verify, I can do that next week. >> (allow guix_daemon_t >> fs_t >> (filesystem (getattr))) >> @@ -348,7 +354,12 @@ >> getopt setopt))) >> (allow guix_daemon_t >> self >> - (tcp_socket (accept listen bind connect create setopt getopt getattr ioctl))) >> + (netlink_route_socket (read write))) >> + (allow guix_daemon_t >> + self >> + (tcp_socket (accept >> + listen bind connect create read write >> + setopt getopt getattr ioctl))) > > These are fine; in fact I discovered these myself this morning and was > going to send a patch. > >> Can you test these additional changes on Fedora? > > Yes, I'll let you know if there are any problems. Also, I'll investigate > the socket file some more. Awesome, thanks a lot! Can you "squash" the relevant changes from my patch and send a new patch when you are done? As a side note, I've seen a couple other audit messages from guix-daemon, although though they don't seem to cause a problem in practice. type=AVC msg=audit(1605189801.627:8637388): avc: denied { read } for pid=2312896 comm="guix-daemon" path="socket:[74336318]" dev="sockfs" ino=74336318 scontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 tcontext=system_u:system_r:init_t:s0 tclass=unix_stream_socket permissive=0 type=AVC msg=audit(1605189801.627:8637388): avc: denied { read } for pid=2312896 comm="guix-daemon" path="socket:[74336318]" dev="sockfs" ino=74336318 scontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 tcontext=system_u:system_r:init_t:s0 tclass=unix_stream_socket permissive=0 type=AVC msg=audit(1605189801.627:8637388): avc: denied { siginh } for pid=2312896 comm="guix-daemon" scontext=system_u:system_r:init_t:s0 tcontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 tclass=process permissive=0 Not sure what that's about.
Marius Bakke <marius@gnu.org> writes: >>> + (allow init_t >>> + guix_store_content_t >>> + (lnk_file (read))) >> >> This one is a little unusual; is your service file symlinked or something? > > Hmm. Could it be because /etc/systemd/system/guix-daemon.service refers > to /var/guix/profiles/per-user/root/current-guix/bin/guix-daemon? That was it. Not sure how I left that one out, in fact. >>> + (allow guix_daemon_t >>> + guix_daemon_socket_t >>> + (sock_file (unlink))) >> >> That shouldn't be a problem, though we don't have any other rules for >> guix_daemon_socket_t. Possibly that is because my socket file is labeled >> guix_daemon_conf_t, for unknown reasons. Perhaps it was not labeled >> correctly when created, and hasn't been relabeled since. > > It could also be an artifact from my ancient experiments with Guix and > SELinux on this system. Perhaps we should test on a "clean" system to > verify, I can do that next week. Ok, I figured this one out. When the socket file is created it is labeled at guix_daemon_conf_t, but the filecon rules will cause that to be relabeled to guix_daemon_socket_t at some point in the future. When the guix-daemon process stops it tries to delete the socket file, but can't. I'll go ahead and include the rule. > Can you "squash" the relevant changes from my patch and send a new patch > when you are done? Will do. > > As a side note, I've seen a couple other audit messages from > guix-daemon, although though they don't seem to cause a problem in > practice. > > type=AVC msg=audit(1605189801.627:8637388): avc: denied { read } for > pid=2312896 comm="guix-daemon" path="socket:[74336318]" dev="sockfs" > ino=74336318 scontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 > tcontext=system_u:system_r:init_t:s0 tclass=unix_stream_socket > permissive=0 > type=AVC msg=audit(1605189801.627:8637388): avc: denied { read } for > pid=2312896 comm="guix-daemon" path="socket:[74336318]" dev="sockfs" > ino=74336318 scontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 > tcontext=system_u:system_r:init_t:s0 tclass=unix_stream_socket > permissive=0 > type=AVC msg=audit(1605189801.627:8637388): avc: denied { siginh } for > pid=2312896 comm="guix-daemon" scontext=system_u:system_r:init_t:s0 > tcontext=system_u:system_r:guix_daemon.guix_daemon_t:s0 tclass=process > permissive=0 The first two are already covered by the new policy, and the third is inconsequential. The kernel checks on our behalf to see if our child processes are allowed to inherit our signal state. That's usually disallowed, so that rule is marked 'dontaudit' so that it doesn't spam the logs; you probably had that disabled. I'm not going to add a rule allowing that one; It would just cause accidents. db48x
diff --git a/etc/guix-daemon.cil.in b/etc/guix-daemon.cil.in index 666e5677a3..b5909f1b18 100644 --- a/etc/guix-daemon.cil.in +++ b/etc/guix-daemon.cil.in @@ -84,6 +84,9 @@ (allow init_t guix_daemon_t (process (transition))) + (allow init_t + guix_store_content_t + (lnk_file (read))) (allow init_t guix_store_content_t (file (open read execute))) @@ -166,6 +169,9 @@ (allow guix_daemon_t root_t (dir (mounton))) + (allow guix_daemon_t + guix_daemon_socket_t + (sock_file (unlink))) (allow guix_daemon_t fs_t (filesystem (getattr))) @@ -348,7 +354,12 @@ getopt setopt))) (allow guix_daemon_t self - (tcp_socket (accept listen bind connect create setopt getopt getattr ioctl))) + (netlink_route_socket (read write))) + (allow guix_daemon_t + self + (tcp_socket (accept + listen bind connect create read write + setopt getopt getattr ioctl))) (allow guix_daemon_t unreserved_port_t (tcp_socket (name_bind name_connect accept listen)))