From: Richard Haines <richard_c_haines@btinternet.com>
To: Stephen Smalley <sds@tycho.nsa.gov>,
selinux@vger.kernel.org, paul@paul-moore.com
Subject: Re: [PATCH V4 2/3] selinux-testsuite: Add BPF support to fdreceive test
Date: Thu, 19 Sep 2019 14:41:22 +0100 [thread overview]
Message-ID: <a312f76e05d57e8f955a5a4ce0f1f4afbe18465e.camel@btinternet.com> (raw)
In-Reply-To: <e3a7f75d-10bd-8020-477f-39a99973473d@tycho.nsa.gov>
On Wed, 2019-09-18 at 11:12 -0400, Stephen Smalley wrote:
> On 9/17/19 10:56 AM, Richard Haines wrote:
> > Add BPF map & prog functions to test fdreceive
> > security_file_receive path()
> >
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> > ---
> > policy/Makefile | 2 +-
> > policy/test_fdreceive_bpf.te | 60 +++++++++++++++++++++++
> > tests/bpf/Makefile | 7 +++
> > tests/bpf/test | 55 ++++++++++++++++++++-
> > tests/fdreceive/Makefile | 14 +++++-
> > tests/fdreceive/client.c | 93
> > ++++++++++++++++++++++++++++++++----
> > 6 files changed, 218 insertions(+), 13 deletions(-)
> > create mode 100644 policy/test_fdreceive_bpf.te
> >
> > diff --git a/policy/Makefile b/policy/Makefile
> > index 16a4469..4ca5486 100644
> > --- a/policy/Makefile
> > +++ b/policy/Makefile
> > @@ -72,7 +72,7 @@ TARGETS += test_sctp.te
> > endif
> >
> > ifeq ($(shell grep -q bpf $(POLDEV)/include/support/all_perms.spt
> > && echo true),true)
> > -TARGETS += test_bpf.te
> > +TARGETS += test_bpf.te test_fdreceive_bpf.te
> > endif
> >
> > ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
> > diff --git a/policy/test_fdreceive_bpf.te
> > b/policy/test_fdreceive_bpf.te
> > new file mode 100644
> > index 0000000..961de79
> > --- /dev/null
> > +++ b/policy/test_fdreceive_bpf.te
> > @@ -0,0 +1,60 @@
> > +#################################
> > +#
> > +# Policy for testing BPF file descriptor transfer via socket IPC
> > +#
> > +
> > +attribute fdreceivebpfdomain;
> > +
> > +# Domain for bpf client process.
> > +type test_fdreceive_bpf_client_t;
> > +domain_type(test_fdreceive_bpf_client_t)
> > +unconfined_runs_test(test_fdreceive_bpf_client_t)
> > +typeattribute test_fdreceive_bpf_client_t fdreceivebpfdomain;
> > +typeattribute test_fdreceive_bpf_client_t testdomain;
> > +allow test_fdreceive_bpf_client_t test_fdreceive_file_t:file {
> > rw_file_perms };
> > +allow test_fdreceive_bpf_client_t test_file_t:sock_file {
> > rw_sock_file_perms };
> > +allow test_fdreceive_bpf_client_t
> > test_fdreceive_server_t:unix_stream_socket { connectto };
> > +allow test_fdreceive_bpf_client_t self:bpf { map_create map_read
> > map_write prog_load prog_run };
>
> Does the client require all of these bpf permissions?
Yes as this client does all bpf functions to produce the map or prog
fd.
>
> > +allow test_fdreceive_bpf_client_t self:capability { sys_resource
> > };
> > +allow test_fdreceive_bpf_client_t self:process { setrlimit };
> > +# Server side rules:
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client_t:fd { use
> > };
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client_t:bpf {
> > map_read map_write };
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client_t:bpf {
> > prog_run} ;
> > +
> > +# Domain for bpf client2 process - Removes BPF prog_run perm from
> > server.
> > +# Tests security_file_receive flow.
> > +type test_fdreceive_bpf_client2_t;
> > +domain_type(test_fdreceive_bpf_client2_t)
> > +unconfined_runs_test(test_fdreceive_bpf_client2_t)
> > +typeattribute test_fdreceive_bpf_client2_t fdreceivebpfdomain;
> > +typeattribute test_fdreceive_bpf_client2_t testdomain;
> > +allow test_fdreceive_bpf_client2_t test_fdreceive_file_t:file {
> > rw_file_perms };
> > +allow test_fdreceive_bpf_client2_t test_file_t:sock_file {
> > rw_sock_file_perms };
> > +allow test_fdreceive_bpf_client2_t
> > test_fdreceive_server_t:unix_stream_socket { connectto };
> > +allow test_fdreceive_bpf_client2_t self:bpf { prog_load prog_run
> > };
> > +allow test_fdreceive_bpf_client2_t self:capability { sys_resource
> > };
> > +allow test_fdreceive_bpf_client2_t self:process { setrlimit };
> > +# Server side rules:
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client2_t:fd {
> > use };
>
> Seemingly lacks more than just prog_run, e.g. also lacks
> map_read/map_write. Don't know if it matters but just want to be
> sure
> it is intentional and that the test will exercise the desired check
> enforcement unambiguously.These are correct as I only test the prog
> fd transfer as I set the client -p flag when running test.
These are correct as I only test the prog fd transfer as I set the
client -p flag when running test.
>
> > +
> > +# Domain for bpf client3 process - Removes BPF map_read perm from
> > server.
> > +# Tests security_file_receive flow.
> > +type test_fdreceive_bpf_client3_t;
> > +domain_type(test_fdreceive_bpf_client3_t)
> > +unconfined_runs_test(test_fdreceive_bpf_client3_t)
> > +typeattribute test_fdreceive_bpf_client3_t fdreceivebpfdomain;
> > +typeattribute test_fdreceive_bpf_client3_t testdomain;
> > +allow test_fdreceive_bpf_client3_t test_fdreceive_file_t:file {
> > rw_file_perms };
> > +allow test_fdreceive_bpf_client3_t test_file_t:sock_file {
> > rw_sock_file_perms };
> > +allow test_fdreceive_bpf_client3_t
> > test_fdreceive_server_t:unix_stream_socket { connectto };
> > +allow test_fdreceive_bpf_client3_t self:bpf { map_create map_read
> > map_write };
> > +allow test_fdreceive_bpf_client3_t self:capability { sys_resource
> > };
> > +allow test_fdreceive_bpf_client3_t self:process { setrlimit };
> > +# Server side rules:
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client3_t:fd {
> > use };
> > +allow test_fdreceive_server_t test_fdreceive_bpf_client3_t:bpf {
> > map_write };
>
> Similarly lacks prog_run in addition to map_read.
These are correct as I only test the map fd transfer as I set the
client -m flag when running test.
>
> > +
> > +# Allow all of these domains to be entered from the sysadm domain.
> > +miscfiles_domain_entry_test_files(fdreceivebpfdomain)
> > +userdom_sysadm_entry_spec_domtrans_to(fdreceivebpfdomain)
> > diff --git a/tests/bpf/Makefile b/tests/bpf/Makefile
> > index 46817a5..3513179 100644
> > --- a/tests/bpf/Makefile
> > +++ b/tests/bpf/Makefile
> > @@ -2,9 +2,16 @@ TARGETS = bpf_test
> > DEPS = bpf_common.c bpf_common.h
> > LDLIBS += -lselinux -lbpf
> >
> > +# export so that BPF_ENABLED entries get built correctly on local
> > build
> > +export CFLAGS += -DHAVE_BPF
> > +
> > +BPF_ENABLED = ../fdreceive
> > +
> > all: $(TARGETS)
> > + @set -e; for i in $(BPF_ENABLED); do $(MAKE) -C $$i all ; done
> >
> > clean:
> > rm -f $(TARGETS)
> > + @set -e; for i in $(BPF_ENABLED); do $(MAKE) -C $$i clean ;
> > done
> >
> > $(TARGETS): $(DEPS)
> > diff --git a/tests/bpf/test b/tests/bpf/test
> > index ee00a19..36f1f32 100755
> > --- a/tests/bpf/test
> > +++ b/tests/bpf/test
> > @@ -4,8 +4,10 @@ use Test::More;
> > BEGIN {
> > $basedir = $0;
> > $basedir =~ s|(.*)/[^/]*|$1|;
> > + $fdr_basedir = "$basedir/../fdreceive/";
> >
> > - $test_count = 7;
> > + $test_count = 7;
> > + $test_fdreceive = 0;
> >
> > # allow info to be shown during tests
> > $v = $ARGV[0];
> > @@ -18,6 +20,14 @@ BEGIN {
> > $v = " ";
> > }
> >
> > + # Test if fdreceive is BPF enabled
> > + $result = system("$fdr_basedir/client -t $basedir/test_sock
> > 2>/dev/null");
> > +
> > + if ( $result >> 8 eq 0 ) {
> > + $test_fdreceive = 1;
> > + $test_count += 4;
> > + }
> > +
>
> Do we need this to be conditional? Isn't it better to just fail if
> they
> didn't build correctly?
Okay, I'll also remove this test on the Binder patch.
>
> > plan tests => $test_count;
> > }
> >
> > @@ -61,4 +71,47 @@ $result =
> > system "runcon -t test_bpf_deny_prog_run_t $basedir/bpf_test -p
> > $v 2>&1";
> > ok($result);
> >
> > +if ($test_fdreceive) {
> > + #
> > + ################ BPF Tests for fdreceive
> > #######################
> > + #
> > + # Remove any leftover test file from prior failed runs.
> > + system("rm -rf $basedir/test_sock");
> > +
> > + # Start server process in test_fdreceive_server_t.
> > + system("mkfifo $basedir/flag");
> > + if ( ( $pid = fork() ) == 0 ) {
> > + exec
> > +"runcon -t test_fdreceive_server_t $fdr_basedir/server
> > $basedir/flag $basedir/test_sock";
> > + }
> > +
> > + # Wait for it to initialize.
> > + system("read -t 5 <>$basedir/flag");
> > +
> > + # Test BPF map & prog fd on transfer:
> > + $result = system
> > +"runcon -t test_fdreceive_bpf_client_t -- $fdr_basedir/client -m
> > $basedir/test_sock";
> > + ok( $result eq 0 );
> > +
> > + $result = system
> > +"runcon -t test_fdreceive_bpf_client_t -- $fdr_basedir/client -p
> > $basedir/test_sock";
> > + ok( $result eq 0 );
> > +
> > + # Remove BPF prog_run permission from server:
> > + $result = system
> > +"runcon -t test_fdreceive_bpf_client2_t -- $fdr_basedir/client -p
> > $basedir/test_sock";
> > + ok($result);
> > +
> > + # Remove BPF map_read permission from server:
> > + $result = system
> > +"runcon -t test_fdreceive_bpf_client3_t -- $fdr_basedir/client -m
> > $basedir/test_sock";
> > + ok($result);
> > +
> > + # Kill the server.
> > + kill KILL, $pid;
> > +
> > + # Clean up.
> > + system "rm -rf $basedir/test_sock $basedir/flag";
> > +}
> > +
> > exit;
> > diff --git a/tests/fdreceive/Makefile b/tests/fdreceive/Makefile
> > index bc33f1b..895f91c 100644
> > --- a/tests/fdreceive/Makefile
> > +++ b/tests/fdreceive/Makefile
> > @@ -1,3 +1,13 @@
> > -all: client server
> > +TARGETS = client server
> > +
> > +ifneq (,$(findstring -DHAVE_BPF,$(CFLAGS)))
> > + DEPS = ../bpf/bpf_common.c ../bpf/bpf_common.h
> > + LDLIBS += -lbpf
> > +endif
> > +
> > +all: $(TARGETS)
> > +
> > clean:
> > - rm -f client server
> > + rm -f $(TARGETS)
> > +
> > +client: $(DEPS)
> > diff --git a/tests/fdreceive/client.c b/tests/fdreceive/client.c
> > index de40bc7..770cc99 100644
> > --- a/tests/fdreceive/client.c
> > +++ b/tests/fdreceive/client.c
> > @@ -8,11 +8,29 @@
> > #include <stdio.h>
> > #include <stdlib.h>
> >
> > +#if HAVE_BPF
> > +#include "../bpf/bpf_common.h"
> > +#endif
> > +
> > +static void usage(char *progname)
> > +{
> > + fprintf(stderr,
> > + "usage: %s [-m|-p|t] [file] addr\n"
> > + "\nWhere:\n\t"
> > + "-m Create BPF map fd\n\t"
> > + "-p Create BPF prog fd\n\t"
> > + "-t Test if BPF enabled\n\t"
> > + " If -m or -p not supplied, create a file fd
> > using:\n\t"
> > + "file Test file fd sent to server\n\t"
> > + "addr Servers address\n", progname);
> > + exit(-1);
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > struct sockaddr_un sun;
> > - char buf[1024];
> > - int s, sunlen, ret, buflen;
> > + char buf[1024], *addr = NULL;
> > + int opt, s, sunlen, ret, buflen;
> > struct msghdr msg = { 0 };
> > struct iovec iov;
> > struct cmsghdr *cmsg;
> > @@ -20,15 +38,71 @@ int main(int argc, char **argv)
> > char cmsgbuf[CMSG_SPACE(sizeof myfd)];
> > int *fdptr;
> >
> > - if (argc != 3) {
> > - fprintf(stderr, "usage: %s testfile address\n",
> > argv[0]);
> > - exit(-1);
> > + enum {
> > + FILE_FD,
> > + MAP_FD,
> > + PROG_FD,
> > + BPF_TEST
> > + } client_fd_type;
> > +
> > + client_fd_type = FILE_FD;
> > +
> > + while ((opt = getopt(argc, argv, "mpt")) != -1) {
> > + switch (opt) {
> > + case 'm':
> > + client_fd_type = MAP_FD;
> > + break;
> > + case 'p':
> > + client_fd_type = PROG_FD;
> > + break;
> > + case 't':
> > + client_fd_type = BPF_TEST;
> > + break;
> > + }
> > }
> >
> > - myfd = open(argv[1], O_RDWR);
> > - if (myfd < 0) {
> > - perror(argv[1]);
> > + if ((client_fd_type == FILE_FD && (argc - optind) != 2) ||
> > + (client_fd_type > FILE_FD && (argc - optind) != 1))
> > + usage(argv[0]);
> > +
> > + switch (client_fd_type) {
> > + case FILE_FD:
> > + myfd = open(argv[optind], O_RDWR);
> > + if (myfd < 0) {
> > + perror(argv[optind]);
> > + exit(-1);
> > + }
> > +
> > + addr = argv[optind + 1];
> > + printf("client: Using a file fd\n");
> > + break;
> > +#if HAVE_BPF
> > + case MAP_FD:
> > + /* If BPF enabled, then need to set limits */
> > + bpf_setrlimit();
> > + addr = argv[optind];
> > + myfd = create_bpf_map();
> > + printf("client: Using a BPF map fd\n");
> > + break;
> > + case PROG_FD:
> > + bpf_setrlimit();
> > + addr = argv[optind];
> > + myfd = create_bpf_prog();
> > + printf("client: Using a BPF prog fd\n");
> > + break;
> > + case BPF_TEST:
> > + exit(0);
> > + break;
> > +#else
> > + case MAP_FD:
> > + case PROG_FD:
> > + case BPF_TEST:
> > + fprintf(stderr, "BPF not supported by Client\n");
> > exit(-1);
> > + break;
> > +#endif
> > + default:
> > + usage(argv[0]);
> > }
> >
> > s = socket(AF_UNIX, SOCK_STREAM, 0);
> > @@ -38,7 +112,8 @@ int main(int argc, char **argv)
> > }
> >
> > sun.sun_family = AF_UNIX;
> > - strcpy(sun.sun_path, argv[2]);
> > + strcpy(sun.sun_path, addr);
> > +
> > sunlen = strlen(sun.sun_path) + 1 + sizeof(short);
> > ret = connect(s, (struct sockaddr *)&sun, sunlen);
> > if (ret < 0) {
> >
next prev parent reply other threads:[~2019-09-19 13:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-17 14:56 [PATCH V4 0/3] selinux-testsuite: Add BPF tests Richard Haines
2019-09-17 14:56 ` [PATCH V4 1/3] " Richard Haines
2019-09-18 14:58 ` Stephen Smalley
2019-09-19 13:36 ` Richard Haines
2019-09-17 14:56 ` [PATCH V4 2/3] selinux-testsuite: Add BPF support to fdreceive test Richard Haines
2019-09-18 15:12 ` Stephen Smalley
2019-09-19 13:41 ` Richard Haines [this message]
2019-09-17 14:56 ` [PATCH V4 3/3] selinux-testsuite: Add BPF support to binder test Richard Haines
2019-09-18 15:41 ` [PATCH V4 0/3] selinux-testsuite: Add BPF tests Stephen Smalley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a312f76e05d57e8f955a5a4ce0f1f4afbe18465e.camel@btinternet.com \
--to=richard_c_haines@btinternet.com \
--cc=paul@paul-moore.com \
--cc=sds@tycho.nsa.gov \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).