[RFC] ioctl: add test for conditional xperms

Commit Message

Christian Göttsche Nov. 28, 2024, 3:28 p.m. UTC

From: Christian Göttsche <[e-mail]>

Add checks for extended permission av rules in conditional blocks.
Requires policy version 34.

Signed-off-by: Christian Göttsche <[e-mail]>
---
 policy/Makefile                  |  5 ++
 policy/test_ioctl_cond_xperms.te | 26 ++++++++++
 tests/ioctl/.gitignore           |  2 +
 tests/ioctl/test                 | 82 +++++++++++++++++++++++++++++++-
 tests/ioctl/test_siocgifindex.c  | 32 +++++++++++++
 tests/ioctl/test_siocgifname.c   | 32 +++++++++++++
 6 files changed, 177 insertions(+), 2 deletions(-)
 create mode 100644 policy/test_ioctl_cond_xperms.te
 create mode 100644 tests/ioctl/test_siocgifindex.c
 create mode 100644 tests/ioctl/test_siocgifname.c

Comments

Stephen Smalley Dec. 12, 2024, 4:02 p.m. UTC | #1

On Fri, Nov 29, 2024 at 5:56 AM Christian Göttsche
<[e-mail]> wrote:
>
> From: Christian Göttsche <[e-mail]>
>
> Add checks for extended permission av rules in conditional blocks.
> Requires policy version 34.
>
> Signed-off-by: Christian Göttsche <[e-mail]>

One minor note below but otherwise you can add:
Acked-by: Stephen Smalley <[e-mail]>
Tested-by: Stephen Smalley <[e-mail]>

> diff --git a/tests/ioctl/test b/tests/ioctl/test
> index f313f06..6c33bf5 100755
> --- a/tests/ioctl/test
> +++ b/tests/ioctl/test
> @@ -62,6 +68,78 @@ if ($test_xperms) {
>      ok($result);
>  }
>
> +if ($test_cond_xperms) {
> +    #
> +    # Attempt to perform the ioctls in the false configuration
> +    #
> +
> +    #
> +    # First round with boolean set to false
> +    #
> +    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result >> 8, 7 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result >> 8, 7 );
> +
> +    #
> +    # Second round with boolean set to true
> +    #
> +    $result = system "setsebool test_ioctl_cond_xperm_switch on 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result >> 8, 7 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result >> 8, 7 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result >> 8, 7 );
> +
> +    #
> +    # Third (control) round with boolean set to false
> +    #

Shouldn't this comment be moved up before the setsebool and test above?

> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> +    ok( $result, 0 );
> +
> +    $result = system
> +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> +    ok( $result >> 8, 7 );
> +}
> +
>  system "rm -f $basedir/temp_file 2>&1";
>
>  exit;

Ondrej Mosnacek Jan. 6, 2025, 11:17 a.m. UTC | #2

On Thu, Dec 12, 2024 at 5:16 PM Stephen Smalley
<[e-mail]> wrote:
>
> On Fri, Nov 29, 2024 at 5:56 AM Christian Göttsche
> <[e-mail]> wrote:
> >
> > From: Christian Göttsche <[e-mail]>
> >
> > Add checks for extended permission av rules in conditional blocks.
> > Requires policy version 34.
> >
> > Signed-off-by: Christian Göttsche <[e-mail]>
>
> One minor note below but otherwise you can add:
> Acked-by: Stephen Smalley <[e-mail]>
> Tested-by: Stephen Smalley <[e-mail]>

Thanks, I applied this patch now, including the comment move:
https://github.com/SELinuxProject/selinux-testsuite/commit/e8ed44c4ca6cce673529e206e6dfed15bff22af4

...along with a patch that allows for the new tests to be run in the
CI already now under the secnext kernel:
https://github.com/SELinuxProject/selinux-testsuite/commit/86d604261a29035ba324ef1ccec1b26a2ae56fcf

> > diff --git a/tests/ioctl/test b/tests/ioctl/test
> > index f313f06..6c33bf5 100755
> > --- a/tests/ioctl/test
> > +++ b/tests/ioctl/test
> > @@ -62,6 +68,78 @@ if ($test_xperms) {
> >      ok($result);
> >  }
> >
> > +if ($test_cond_xperms) {
> > +    #
> > +    # Attempt to perform the ioctls in the false configuration
> > +    #
> > +
> > +    #
> > +    # First round with boolean set to false
> > +    #
> > +    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result >> 8, 7 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result >> 8, 7 );
> > +
> > +    #
> > +    # Second round with boolean set to true
> > +    #
> > +    $result = system "setsebool test_ioctl_cond_xperm_switch on 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result >> 8, 7 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result >> 8, 7 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result >> 8, 7 );
> > +
> > +    #
> > +    # Third (control) round with boolean set to false
> > +    #
>
> Shouldn't this comment be moved up before the setsebool and test above?
>
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
> > +    ok( $result, 0 );
> > +
> > +    $result = system
> > +      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
> > +    ok( $result >> 8, 7 );
> > +}
> > +
> >  system "rm -f $basedir/temp_file 2>&1";
> >
> >  exit;
>

Patch

diff --git a/policy/Makefile b/policy/Makefile
index 32d7ede..46e51f3 100644
--- a/policy/Makefile
+++ b/policy/Makefile
@@ -175,6 +175,11 @@  ifeq ($(shell grep -q user_namespace $(POLDEV)/include/support/all_perms.spt &&
 export M4PARAM += -Duser_namespace_defined
 endif
 
+# conditional xperm dependency: policy >= 34
+ifeq ($(shell [ $(POL_VERS) -ge 34 -a $(MAX_KERNEL_POLICY) -ge 34 ] && echo true),true)
+TARGETS += test_ioctl_cond_xperms.te
+endif
+
 all: build
 
 expand_check:
diff --git a/policy/test_ioctl_cond_xperms.te b/policy/test_ioctl_cond_xperms.te
new file mode 100644
index 0000000..03e6c0f
--- /dev/null
+++ b/policy/test_ioctl_cond_xperms.te
@@ -0,0 +1,26 @@ 
+define(`SIOCGIFNAME', `{ 0x8910 }')
+define(`SIOCGIFINDEX', `{ 0x8933 }')
+
+bool test_ioctl_cond_xperm_switch false;
+
+# Domain for process that is allowed SIOCGIFNAME on true and SIOCGIFINDEX on false.
+type test_ioctl_cond_xperm_a_t;
+testsuite_domain_type(test_ioctl_cond_xperm_a_t)
+typeattribute test_ioctl_cond_xperm_a_t ioctldomain;
+allow test_ioctl_cond_xperm_a_t self:udp_socket { create ioctl };
+if (test_ioctl_cond_xperm_switch) {
+  allowxperm test_ioctl_cond_xperm_a_t self:udp_socket ioctl SIOCGIFNAME;
+} else {
+  allowxperm test_ioctl_cond_xperm_a_t self:udp_socket ioctl SIOCGIFINDEX;
+}
+
+# Domain for process that is allowed SIOCGIFINDEX on true and SIOCGIFNAME on false.
+type test_ioctl_cond_xperm_b_t;
+testsuite_domain_type(test_ioctl_cond_xperm_b_t)
+typeattribute test_ioctl_cond_xperm_b_t ioctldomain;
+allow test_ioctl_cond_xperm_b_t self:udp_socket { create ioctl };
+if (test_ioctl_cond_xperm_switch) {
+  allowxperm test_ioctl_cond_xperm_b_t self:udp_socket ioctl SIOCGIFINDEX;
+} else {
+  allowxperm test_ioctl_cond_xperm_b_t self:udp_socket ioctl SIOCGIFNAME;
+}
diff --git a/tests/ioctl/.gitignore b/tests/ioctl/.gitignore
index e89db81..a4e44e7 100644
--- a/tests/ioctl/.gitignore
+++ b/tests/ioctl/.gitignore
@@ -1,2 +1,4 @@ 
 test_ioctl
 test_noioctl
+test_siocgifindex
+test_siocgifname
diff --git a/tests/ioctl/test b/tests/ioctl/test
index f313f06..6c33bf5 100755
--- a/tests/ioctl/test
+++ b/tests/ioctl/test
@@ -6,9 +6,11 @@ 
 use Test;
 
 BEGIN {
-    $test_count  = 2;
-    $test_xperms = 0;
+    $test_count       = 2;
+    $test_xperms      = 0;
+    $test_cond_xperms = 0;
 
+    $polver    = `checkpolicy -V | cut -f 1 -d ' '`;
     $modver    = `checkmodule -V | cut -f 2 -d -`;
     $selinuxfs = `cat /proc/mounts | grep selinuxfs | cut -f 2 -d ' '`;
     chomp($selinuxfs);
@@ -17,6 +19,10 @@  BEGIN {
         $test_xperms = 1;
         $test_count += 2;
     }
+    if ( $polver >= 34 && $kernver >= 34 ) {
+        $test_cond_xperms = 1;
+        $test_count += 15;
+    }
 
     plan tests => $test_count;
 }
@@ -62,6 +68,78 @@  if ($test_xperms) {
     ok($result);
 }
 
+if ($test_cond_xperms) {
+    #
+    # Attempt to perform the ioctls in the false configuration
+    #
+
+    #
+    # First round with boolean set to false
+    #
+    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result >> 8, 7 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result >> 8, 7 );
+
+    #
+    # Second round with boolean set to true
+    #
+    $result = system "setsebool test_ioctl_cond_xperm_switch on 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result >> 8, 7 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result >> 8, 7 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result, 0 );
+
+    $result = system "setsebool test_ioctl_cond_xperm_switch off 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result >> 8, 7 );
+
+    #
+    # Third (control) round with boolean set to false
+    #
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_a_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifname 2>&1";
+    ok( $result, 0 );
+
+    $result = system
+      "runcon -t test_ioctl_cond_xperm_b_t -- $basedir/test_siocgifindex 2>&1";
+    ok( $result >> 8, 7 );
+}
+
 system "rm -f $basedir/temp_file 2>&1";
 
 exit;
diff --git a/tests/ioctl/test_siocgifindex.c b/tests/ioctl/test_siocgifindex.c
new file mode 100644
index 0000000..ab21ad4
--- /dev/null
+++ b/tests/ioctl/test_siocgifindex.c
@@ -0,0 +1,32 @@ 
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <errno.h>
+#include <net/if.h>
+
+int main(int argc, char **argv)
+{
+	int sock;
+	int rc;
+	struct ifreq val = {};
+
+	sock = socket(AF_INET, SOCK_DGRAM, 0);
+	if(sock == -1) {
+		perror("test_siocgifindex:open");
+		exit(1);
+	}
+
+	rc = ioctl(sock, SIOCGIFINDEX, &val);
+	if (rc < 0 && errno != ENODEV) {
+		perror("test_siocgifindex:SIOCGIFINDEX");
+		close(sock);
+		exit(7);
+	}
+
+	close(sock);
+	exit(0);
+}
diff --git a/tests/ioctl/test_siocgifname.c b/tests/ioctl/test_siocgifname.c
new file mode 100644
index 0000000..5308bfe
--- /dev/null
+++ b/tests/ioctl/test_siocgifname.c
@@ -0,0 +1,32 @@ 
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <errno.h>
+#include <net/if.h>
+
+int main(int argc, char **argv)
+{
+	int sock;
+	int rc;
+	struct ifreq val = {};
+
+	sock = socket(AF_INET, SOCK_DGRAM, 0);
+	if(sock == -1) {
+		perror("test_siocgifname:socket");
+		exit(1);
+	}
+
+	rc = ioctl(sock, SIOCGIFNAME, &val);
+	if (rc < 0 && errno != ENODEV) {
+		perror("test_siocgifname:SIOCGIFNAME");
+		close(sock);
+		exit(7);
+	}
+
+	close(sock);
+	exit(0);
+}