Re: [RFC PATCH] selinux-testsuite: Add TUN/TAP driver tests

From: Stephen Smalley <sds@tycho.nsa.gov>
To: Richard Haines <richard_c_haines@btinternet.com>,
	selinux@vger.kernel.org
Subject: Re: [RFC PATCH] selinux-testsuite: Add TUN/TAP driver tests
Date: Wed, 27 Nov 2019 09:59:05 -0500	[thread overview]
Message-ID: <41aca6c1-7284-0226-3f20-6eb15f437c48@tycho.nsa.gov> (raw)
In-Reply-To: <20191127094702.3418-1-richard_c_haines@btinternet.com>

On 11/27/19 4:47 AM, Richard Haines wrote:
> Test TUN/TAP tun_socket permissions.
> 
> I've made this an RFC patch as TAP supports adding BPF prog file
> descriptors, therefore I've added a simple test that just checks that it
> works. However, there does not seem to be a requirement to test any
> additional SELinux functionality that the basic BPF tests do not
> already cover. Any views !!!

That seems reasonable to me.  Generally our focus is on ensuring test 
coverage of the LSM/SELinux hooks and SELinux kernel APIs, so anything 
that is already covered by an existing test doesn't need to be repeated. 
  In the case of tun/tap, likely only the tun_socket checks themselves 
are unique to these tests.

I would prioritize on 1) ensuring that we have test coverage of new 
security hooks, new functionality within existing hooks, or new SELinux 
kernel APIs as they get added to the kernel, e.g. the perf permission 
checks (github issue #71), the socketpair peer labeling support (#63), 
etc, and then 2) gradually expanding our test coverage of existing 
security hooks and kernel APIs to ensure that future kernels do not 
regress, e.g. it would have been nice to have had explicit tests for 
context mounts as per issue #20 to ensure that we didn't regress during 
the vfs new mount API overhaul (we happen to exercise context mounts as 
part of overlay testing and as part of binderfs testing but not as a 
separate test, not in a comprehensive way, and not for anything other 
than overlay mounts), and likewise for many of the tests identified as 
open github issues.

I haven't looked too closely at this yet, but I did have one minor 
comment on your test policy below.

<snip>
> diff --git a/policy/test_tap_bpf.te b/policy/test_tap_bpf.te
> new file mode 100644
> index 0000000..f921a5a
> --- /dev/null
> +++ b/policy/test_tap_bpf.te
> @@ -0,0 +1,30 @@
> +#
> +########### Test TAP/BPF - 'tun_socket' #################
> +#
> +attribute tapbpfdomain;
> +
> +# For CONFIG_TUN=m
> +kernel_request_load_module(tapbpfdomain)
> +
> +gen_require(`
> +	type tun_tap_device_t;
> +')

Any time you find yourself needing a require, look to see if there is a 
refpolicy interface you could use instead that would provide that 
require and the necessary rules.  In this case, I think you could have 
used corenet_rw_tun_tap_dev().