On Thu, 2020-05-14 at 10:50 +0200, Ondrej Mosnacek wrote:
> On Fri, Apr 17, 2020 at 6:38 PM Richard Haines
> <richard_c_haines@xxxxxxxxxxxxxx> wrote:
> > Kernel 5.7 introduced the watch_queue service that allows watching
> > for
> > key changes. This requires key { view } permission, therefore check
> > if
> > allowed or not.
> >
> > Note that the keyctl_watch_key() function is not yet built into the
> > keyutils library, therefore a syscall() is used.
> >
> > Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
>
> OK, so I tried to build kernel RPMs(*) from the linux-next tree and
> try out this patch, but I ran into a few problems:
> 1. <linux/watch_queue.h> includes <linux/fcntl.h>, which conflicts
> with glibc's <sys/fcntl.h>. This will maybe get fixed in glibc/kernel
> eventually, but I can't apply the patch as-is (maybe we can #define
> _LINUX_FCNTL_H before including <linux/watch_queue.h> as a temporary
> workaround?).
> 2. Even when I got the test program to build, the test failed. I ran
> out of time to dig further, but it may be due to the issues I spotted
> later in the test policy (see below).
>
> (*) I wanted to try out the new Fedora's kernel build machinery, so I
> burnt too much time on this... but I now have a semi-automated way to
> build linux-next kernels as RPMs, which might come in handy in the
> future :)
Thanks for the feedback I'll see about fixing the issues.
>
> > ---
> > defconfig | 5 ++
> > policy/Makefile | 2 +-
> > policy/test_watchkey.te | 34 ++++++++++++
> > tests/Makefile | 4 ++
> > tests/watchkey/.gitignore | 1 +
> > tests/watchkey/Makefile | 7 +++
> > tests/watchkey/test | 29 ++++++++++
> > tests/watchkey/watchkey.c | 113
> > ++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 194 insertions(+), 1 deletion(-)
> > create mode 100644 policy/test_watchkey.te
> > create mode 100644 tests/watchkey/.gitignore
> > create mode 100644 tests/watchkey/Makefile
> > create mode 100755 tests/watchkey/test
> > create mode 100644 tests/watchkey/watchkey.c
> >
> > diff --git a/defconfig b/defconfig
> > index 0574f1d..9afbc2f 100644
> > --- a/defconfig
> > +++ b/defconfig
> > @@ -78,3 +78,8 @@ CONFIG_KEY_DH_OPERATIONS=y
> > # Test key management socket.
> > # This is not required for SELinux operation itself.
> > CONFIG_NET_KEY=m
> > +
> > +# watch_queue for key changes.
> > +# They are not required for SELinux operation itself.
> > +CONFIG_WATCH_QUEUE=y
> > +CONFIG_KEY_NOTIFICATIONS=y
> > diff --git a/policy/Makefile b/policy/Makefile
> > index 87b2856..b3f5e3d 100644
> > --- a/policy/Makefile
> > +++ b/policy/Makefile
> > @@ -86,7 +86,7 @@ TARGETS += test_bpf.te test_fdreceive_bpf.te
> > test_binder_bpf.te
> > endif
> >
> > ifeq ($(shell grep -q all_key_perms
> > $(POLDEV)/include/support/all_perms.spt && echo true),true)
> > -TARGETS += test_keys.te
> > +TARGETS += test_keys.te test_watchkey.te
> > endif
> >
> > ifeq ($(shell grep -q all_file_perms.*watch
> > $(POLDEV)/include/support/all_perms.spt && echo true),true)
> > diff --git a/policy/test_watchkey.te b/policy/test_watchkey.te
> > new file mode 100644
> > index 0000000..e1d4c78
> > --- /dev/null
> > +++ b/policy/test_watchkey.te
> > @@ -0,0 +1,34 @@
> > +#
> > +######### Check watch_queue for key changes policy module
> > ##########
> > +#
> > +attribute watchkeydomain;
> > +
> > +################# Allow watch_queue key { view }
> > ##########################
> > +type test_watchkey_t;
> > +domain_type(test_watchkey_t)
> > +unconfined_runs_test(test_watchkey_t)
> > +typeattribute test_watchkey_t testdomain;
> > +typeattribute test_watchkey_t keydomain;
>
> You declare attribute "watchkeydomain" above and later call some
> interfaces on it, but assign all the types to "keydomain". I assume
> you meant to assign them to "watchkeydomain"?
>
> > +
> > +allow test_watchkey_t self:capability { ipc_lock };
> > +allow test_watchkey_t device_t:chr_file { ioctl open read write };
> > +allow test_watchkey_t self:key { view };
> > +allow_map(test_watchkey_t, device_t, chr_file)
> > +
> > +################# Deny watch_queue key { view }
> > ##########################
> > +type test_watchkey_no_view_t;
> > +domain_type(test_watchkey_no_view_t)
> > +unconfined_runs_test(test_watchkey_no_view_t)
> > +typeattribute test_watchkey_no_view_t testdomain;
> > +typeattribute test_watchkey_no_view_t keydomain;
> > +
> > +allow test_watchkey_no_view_t self:capability { ipc_lock };
> > +allow test_watchkey_no_view_t device_t:chr_file { ioctl open read
> > write };
> > +neverallow test_watchkey_no_view_t self:key { view };
> > +allow_map(test_watchkey_no_view_t, device_t, chr_file)
> > +
> > +#
> > +########### Allow these domains to be entered from sysadm domain
> > ############
> > +#
> > +miscfiles_domain_entry_test_files(watchkeydomain)
> > +userdom_sysadm_entry_spec_domtrans_to(watchkeydomain)
> [...]
>
