SELinux Archive on lore.kernel.org
 help / color / Atom feed
From: Dominick Grift <dominick.grift@defensec.nl>
To: Aaron Goidel <acgoide@tycho.nsa.gov>
Cc: paul@paul-moore.com, selinux@vger.kernel.org, sds@tycho.nsa.gov
Subject: Re: [PATCH] selinux-testsuite: add tests for fsnotify
Date: Tue, 10 Sep 2019 07:59:26 +0200
Message-ID: <20190910055926.GA963995@brutus.lan> (raw)
In-Reply-To: <20190710133917.1188-1-acgoide@tycho.nsa.gov>

On Wed, Jul 10, 2019 at 09:39:17AM -0400, Aaron Goidel wrote:
> Added a suite to test permissions for setting inotify and fanotify watches
> on filesystem objects. Tests watch, watch_with_perm, and watch_reads permissions.

Ive also "tested" the fsnotify patch. And my tests indicate that this might cause issues:

I added the access vectors to my policy, but on older systems (debian 10) cron fails to start
cron needs the "entrypoint" permission on /etc/crontab but it looks like appending the "watch" access vectors to common-file disrupted the ordering
The result is that now i have to allow cron to "watch_read" /etc/crontab even though the neither kernel nor selinux user space are aware of the fsnotify access vectors
It seems the cron selinux code got confused and now thinks watch_read is entrypoint (its using selinux code to determine whether it can manually transition to cronjob domains on crontabs)

I am hoping this issue with resolve itself on systems with kernels and user spaces that suppose fsnotify.
However unless i am overlooking something this is still likely to disrupt compatibility

> 
> Signed-off-by: Aaron Goidel <acgoide@tycho.nsa.gov>
> ---
>  policy/Makefile              |   4 ++
>  policy/test_notify.te        |  74 ++++++++++++++++++++++++
>  tests/Makefile               |   4 ++
>  tests/notify/Makefile        |   5 ++
>  tests/notify/test            | 101 +++++++++++++++++++++++++++++++++
>  tests/notify/test_fanotify.c | 105 +++++++++++++++++++++++++++++++++++
>  tests/notify/test_inotify.c  |  43 ++++++++++++++
>  7 files changed, 336 insertions(+)
>  create mode 100644 policy/test_notify.te
>  create mode 100644 tests/notify/Makefile
>  create mode 100755 tests/notify/test
>  create mode 100644 tests/notify/test_fanotify.c
>  create mode 100644 tests/notify/test_inotify.c
> 
> diff --git a/policy/Makefile b/policy/Makefile
> index 305b572..65f88c5 100644
> --- a/policy/Makefile
> +++ b/policy/Makefile
> @@ -71,6 +71,10 @@ ifeq ($(shell grep -q corenet_sctp_bind_all_nodes $(POLDEV)/include/kernel/coren
>  TARGETS += test_sctp.te
>  endif
>  
> +ifeq ($(shell grep -q all_file_perms.*watch $(POLDEV)/include/support/all_perms.spt && echo true),true)
> +TARGETS+=test_notify.te
> +endif
> +
>  ifeq (x$(DISTRO),$(filter x$(DISTRO),xRHEL4 xRHEL5 xRHEL6))
>  TARGETS:=$(filter-out test_overlayfs.te test_mqueue.te, $(TARGETS))
>  endif
> diff --git a/policy/test_notify.te b/policy/test_notify.te
> new file mode 100644
> index 0000000..8ba6f1a
> --- /dev/null
> +++ b/policy/test_notify.te
> @@ -0,0 +1,74 @@
> +####################################################
> +# Policy for testing inoftify and fanotify watches #
> +####################################################
> +
> +attribute test_notify_domain;
> +
> +# Type for the file on which we want to set a watch
> +type test_notify_file_t;
> +files_type(test_notify_file_t);
> +
> +# Domain for the process which CAN set a non-permission watch
> +type test_watch_t;
> +domain_type(test_watch_t);
> +unconfined_runs_test(test_watch_t);
> +
> +typeattribute test_watch_t test_notify_domain;
> +typeattribute test_watch_t testdomain;
> +
> +allow test_notify_domain self:capability sys_admin;
> +
> +allow test_watch_t test_notify_file_t:file { read write open watch };
> +
> +# Domain for the process which CAN set a NON-access watch on a file
> +type test_perm_watch_t;
> +domain_type(test_perm_watch_t);
> +unconfined_runs_test(test_perm_watch_t);
> +
> +typeattribute test_perm_watch_t test_notify_domain;
> +typeattribute test_perm_watch_t testdomain;
> +
> +allow test_perm_watch_t test_notify_file_t:file { read write open watch watch_with_perm };
> +
> +# Domain which CAN set a NON-perm watch which MAY read accesses
> +type test_read_watch_t;
> +domain_type(test_read_watch_t);
> +unconfined_runs_test(test_read_watch_t);
> +
> +typeattribute test_read_watch_t test_notify_domain;
> +typeattribute test_read_watch_t testdomain;
> +
> +allow test_read_watch_t test_notify_file_t:file { read write open watch watch_reads };
> +
> +# Domain which CAN set any watch which CAN read accesses
> +type test_perm_read_watch_t;
> +domain_type(test_perm_read_watch_t);
> +unconfined_runs_test(test_perm_read_watch_t);
> +
> +typeattribute test_perm_read_watch_t test_notify_domain;
> +typeattribute test_perm_read_watch_t testdomain;
> +
> +allow test_perm_read_watch_t test_notify_file_t:file { read write open watch watch_with_perm watch_reads };
> +
> +# Domain which CANNOT set any watches
> +type test_no_watch_t;
> +domain_type(test_no_watch_t);
> +unconfined_runs_test(test_no_watch_t);
> +
> +typeattribute test_no_watch_t test_notify_domain;
> +typeattribute test_no_watch_t testdomain;
> +
> +allow test_no_watch_t test_notify_file_t:file { read write open };
> +
> +# Domain which has no write access but can watch
> +type test_rdonly_t;
> +domain_type(test_rdonly_t);
> +unconfined_runs_test(test_rdonly_t);
> +
> +typeattribute test_rdonly_t test_notify_domain;
> +typeattribute test_rdonly_t testdomain;
> +
> +allow test_rdonly_t test_notify_file_t:file { read open watch };
> +
> +miscfiles_domain_entry_test_files(test_notify_domain);
> +userdom_sysadm_entry_spec_domtrans_to(test_notify_domain);
> diff --git a/tests/Makefile b/tests/Makefile
> index 63aa325..b99c96e 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -50,6 +50,10 @@ ifeq ($(shell grep "^SELINUX_INFINIBAND_PKEY_TEST=" infiniband_pkey/ibpkey_test.
>  SUBDIRS += infiniband_pkey
>  endif
>  
> +ifeq ($(shell grep -q all_file_perms.*watch $(POLDEV)/include/support/all_perms.spt && echo true),true)
> +SUBDIRS+=notify
> +endif
> +
>  ifeq ($(DISTRO),RHEL4)
>      SUBDIRS:=$(filter-out bounds dyntrace dyntrans inet_socket mmap nnp_nosuid overlay unix_socket, $(SUBDIRS))
>  endif
> diff --git a/tests/notify/Makefile b/tests/notify/Makefile
> new file mode 100644
> index 0000000..78c4b3b
> --- /dev/null
> +++ b/tests/notify/Makefile
> @@ -0,0 +1,5 @@
> +TARGETS=test_inotify test_fanotify
> +
> +all: $(TARGETS)
> +clean: 
> +	rm -f $(TARGETS)
> diff --git a/tests/notify/test b/tests/notify/test
> new file mode 100755
> index 0000000..21f03de
> --- /dev/null
> +++ b/tests/notify/test
> @@ -0,0 +1,101 @@
> +#!/usr/bin/perl
> +
> +use Test;
> +BEGIN { plan tests => 14 }    # number of tests to run
> +
> +# help the test script locate itself
> +$basedir = $0;
> +$basedir =~ s|(.*)/[^/]*|$1|;
> +
> +# Get rid of a testfile from last run if it's there (just in case)
> +system("rm -f $basedir/watch_me");
> +
> +# Create a new test file
> +system("touch $basedir/watch_me");
> +system("chcon -t test_notify_file_t $basedir/watch_me");
> +
> +## TESTS
> +
> +## TEST BASIC WATCH PERMISSION
> +# Should be able to set inotify watch
> +$exit_val =
> +  system("runcon -t test_watch_t $basedir/test_inotify $basedir/watch_me 2>&1");
> +ok( $exit_val, 0 );
> +
> +# Should be able to set non-permissions based fanotify watch
> +$exit_val = system(
> +    "runcon -t test_watch_t $basedir/test_fanotify $basedir/watch_me 2>&1");
> +ok( $exit_val, 0 );
> +
> +# Should NOT be able to set permission based fanotify watch
> +$exit_val = system(
> +    "runcon -t test_watch_t $basedir/test_fanotify -p $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +# Should NOT be able to set read based fanotify watch
> +$exit_val = system(
> +    "runcon -t test_watch_t $basedir/test_fanotify -r $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +# Should NOT be able to set read based inotify watch
> +$exit_val = system(
> +    "runcon -t test_watch_t $basedir/test_inotify -r $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +## TEST PERM WATCH
> +# Should be able to set permission based fanotify watch
> +$exit_val = system(
> +"runcon -t test_perm_watch_t $basedir/test_fanotify -p $basedir/watch_me 2>&1"
> +);
> +ok( $exit_val, 0 );
> +
> +# Should NOT be able to set watch of accesses
> +$exit_val = system(
> +"runcon -t test_perm_watch_t $basedir/test_fanotify -r $basedir/watch_me 2>&1"
> +);
> +ok($exit_val);    # this should fail
> +
> +## TEST READ NO PERM WATCH PERMSISSIONS
> +# Should NOT be able to set read and perm watch
> +$exit_val = system(
> +"runcon -t test_read_watch_t $basedir/test_fanotify -p -r $basedir/watch_me 2>&1"
> +);
> +ok($exit_val);    # should fail
> +
> +# Should be able to set read inotify watch
> +$exit_val = system(
> +"runcon -t test_read_watch_t $basedir/test_inotify -r $basedir/watch_me 2>&1"
> +);
> +ok( $exit_val, 0 );
> +
> +## TEST READ WITH PERM WATCH PERMSISSIONS
> +# Should be able to set read and perm watch
> +$exit_val = system(
> +"runcon -t test_perm_read_watch_t $basedir/test_fanotify -p -r $basedir/watch_me 2>&1"
> +);
> +ok( $exit_val, 0 );
> +
> +## TEST NO WATCH PERMSISSIONS
> +# Should NOT be able to set inotify watch
> +$exit_val = system(
> +    "runcon -t test_no_watch_t $basedir/test_inotify $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +# Should NOT be able to set any fanotify watch
> +$exit_val = system(
> +    "runcon -t test_no_watch_t $basedir/test_fanotify $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +## TEST READ ONLY
> +# Should NOT be able to get read-write descriptor
> +$exit_val = system(
> +    "runcon -t test_rdonly_t $basedir/test_fanotify -l $basedir/watch_me 2>&1");
> +ok($exit_val);    # this should fail
> +
> +# Should be able to get read-write descriptor
> +$exit_val = system(
> +    "runcon -t test_watch_t $basedir/test_fanotify -l $basedir/watch_me 2>&1");
> +ok( $exit_val, 0 );
> +
> +# Clean up test file
> +system("rm -f $basedir/watch_me");
> diff --git a/tests/notify/test_fanotify.c b/tests/notify/test_fanotify.c
> new file mode 100644
> index 0000000..fff773f
> --- /dev/null
> +++ b/tests/notify/test_fanotify.c
> @@ -0,0 +1,105 @@
> +#define _GNU_SOURCE 1
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include <getopt.h>
> +
> +#include <fcntl.h>
> +#include <poll.h>
> +#include <sys/fanotify.h>
> +#include <unistd.h>
> +
> +void printUsage()
> +{
> +	fprintf(stderr, "Usage: test_fanotify [-p] [-r] [-l] file_name\n");
> +	exit(1);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	if (argc < 2) {
> +		printUsage();
> +	}
> +
> +	int fd, ret, arg;
> +	int mask = FAN_OPEN;  // default mask
> +	int listening = 0;
> +
> +	// the -p flag will test for watch_with_perm
> +	// the mask used at mark will contain FAN_OPEN_PERM
> +	//
> +	// the -r flag will test for watching accesses to files for reads
> +	// the mask will contain FAN_ACCESS
> +	while ((arg = getopt(argc, argv, "prl")) != -1) {
> +		switch (arg) {
> +		case 'p':
> +			mask |= FAN_OPEN_PERM;
> +			break;
> +		case 'r':
> +			mask |= FAN_ACCESS;
> +			break;
> +		case 'l':
> +			listening = 1;
> +			break;
> +		default:
> +			printUsage();
> +		}
> +	}
> +
> +	// get file descriptor for new fanotify event queue
> +	fd = fanotify_init(FAN_CLASS_CONTENT, O_RDWR);
> +	if (fd < 0) {
> +		perror("fanotify_init:bad file descriptor");
> +		exit(1);
> +	}
> +
> +	// mark a filesystem object and add mark to event queue
> +	// get notifications on file opens, accesses, and closes
> +	// use current working directory as base dir
> +	ret = fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, argv[optind]);
> +
> +	if (ret < 0) {
> +		perror("test_fanotify:watch denied");
> +		exit(1);
> +	}
> +
> +	// logic to actually listen for an event if the -l flag is passed
> +	// this is used to test if an app with read-only access can get a read/write
> +	// handle to the watched file
> +	if (listening) {
> +		if (fork() == 0) {  // fork a child process to cause an event on the file
> +			FILE *f;
> +
> +			f = fopen(argv[optind], "r");  // open file for reading
> +			fgetc(f);                      // read char from file
> +
> +			fclose(f);
> +		} else {  // logic to watch for events and try to access file read/write
> +			struct pollfd fds;
> +			fds.fd = fd;
> +			fds.events = POLLIN;
> +
> +			while (listening) {
> +				int polled = poll(&fds, 1, 1);
> +				if (polled > 0) {
> +					if (fds.revents & POLLIN) {
> +						struct fanotify_event_metadata buff[200];
> +
> +						size_t len = read(fd, (void *)&buff, sizeof(buff));
> +						if (len == -1) {
> +							perror("test_fanotify:can't open file");
> +							exit(1);
> +						} else {
> +							listening = 0;
> +							break;
> +						}
> +					}
> +				} else if (polled == -1) {
> +					listening = 0;
> +				}
> +			}
> +		}
> +	}
> +	exit(0);
> +}
> diff --git a/tests/notify/test_inotify.c b/tests/notify/test_inotify.c
> new file mode 100644
> index 0000000..17c3565
> --- /dev/null
> +++ b/tests/notify/test_inotify.c
> @@ -0,0 +1,43 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/inotify.h>
> +#include <getopt.h>
> +
> +int main(int argc, char *argv[])
> +{
> +	if (argc < 2) {
> +		fprintf(stderr, "Usage: test_inotify [-r] file_name\n");
> +		exit(1);
> +	}
> +
> +	int fd, wd, arg;
> +	int mask = IN_MODIFY;
> +
> +	while ((arg = getopt(argc, argv, "pr")) != -1) {
> +		switch (arg) {
> +		case 'r':
> +			mask |= IN_ACCESS;
> +			break;
> +		default:
> +			fprintf(stderr, "Usage: test_inotify [-r] file_name\n");
> +			exit(1);
> +		}
> +	}
> +
> +	// get new file descriptor for inotify access
> +	fd = inotify_init();
> +	if (fd < 0) {
> +		perror("inotify_init:bad file descriptor");
> +		exit(1);
> +	}
> +
> +	// set watch on file and get watch descriptor for accessing events on it
> +	wd = inotify_add_watch(fd, argv[optind], mask);
> +
> +	if (wd < 0) {
> +		perror("test_inotify:watch denied");
> +		exit(1);
> +	}
> +
> +	exit(0);
> +}
> -- 
> 2.21.0
> 

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift

  parent reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10 13:39 Aaron Goidel
2019-07-10 13:50 ` Aaron Goidel
2019-08-27 17:14 ` Paul Moore
2019-09-16 19:50   ` Stephen Smalley
2019-09-10  5:59 ` Dominick Grift [this message]
2019-09-10 12:26   ` Stephen Smalley
2019-09-10 14:15     ` Dominick Grift
2019-09-10 14:36       ` Dominick Grift
2019-09-10 14:40       ` Stephen Smalley
2019-09-10 14:54         ` Stephen Smalley
2019-09-10 14:59           ` Stephen Smalley
2019-09-10 15:02             ` [Non-DoD Source] " Stephen Smalley
2019-09-10 15:22               ` Dominick Grift
2019-09-10 16:25                 ` Stephen Smalley
2019-09-10 16:32                   ` Dominick Grift
2019-09-10 18:13                     ` Stephen Smalley
2019-09-10 18:20                       ` Dominick Grift
2019-09-10 19:45                         ` Stephen Smalley
2019-09-10 19:55                           ` Stephen Smalley
2019-09-10 20:00                             ` Dominick Grift
2019-08-01 11:35 Aaron Goidel

Reply instructions:

You may reply publically 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=20190910055926.GA963995@brutus.lan \
    --to=dominick.grift@defensec.nl \
    --cc=acgoide@tycho.nsa.gov \
    --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

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org selinux@archiver.kernel.org
	public-inbox-index selinux

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox