From 8ee8e26355ebdc70febf40cbf2be47cf1ebb3fcb Mon Sep 17 00:00:00 2001
From: Nick Kralevich <nnk@google.com>
Date: Wed, 17 Oct 2018 11:04:06 -0700
Subject: [PATCH] more ioctl work

Add a neverallow rule requiring fine-grain ioctl filtering for most file
and socket object classes. Only chr_file and blk_file are excluded. The
goal is to ensure that any file descriptor which supports ioctl commands
uses a whitelist.

Further refine the list of file / socket objects which require ioctl
filtering. The previous ioctl filtering did not cover the following:

1) ioctls on /proc/PID files
2) ioctls on directories in /dev
3) PDX unix domain sockets

Add FIONCLEX to the list of globally safe ioctls. FIOCLEX and FIONCLEX
are alternate, uncommon ways to set the O_CLOEXEC flag on a file
descriptor, which is a harmless operation.

Test: device boots and no problems.
Change-Id: I6ba31fbe2f21935243a344d33d67238d72a8e618
---
 public/domain.te    | 11 ++++++-----
 public/ioctl_macros |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/public/domain.te b/public/domain.te
index 680d5e48b..2bdc53c56 100644
--- a/public/domain.te
+++ b/public/domain.te
@@ -298,7 +298,7 @@ allow domain fs_type:dir getattr;
 allowxperm domain domain:{ icmp_socket rawip_socket tcp_socket udp_socket }
   ioctl { unpriv_sock_ioctls unpriv_tty_ioctls };
 # default whitelist for unix sockets.
-allowxperm domain domain:{ unix_dgram_socket unix_stream_socket }
+allowxperm domain { domain pdx_channel_socket_type }:{ unix_dgram_socket unix_stream_socket }
   ioctl unpriv_unix_sock_ioctls;
 
 # Restrict PTYs to only whitelisted ioctls.
@@ -309,8 +309,8 @@ allowxperm domain devpts:chr_file ioctl unpriv_tty_ioctls;
 
 # All domains must clearly enumerate what ioctls they use
 # on filesystem objects (plain files, directories, symbolic links,
-# named pipes, and named sockets)
-allowxperm domain { file_type fs_type }:{ dir notdevfile_class_set } ioctl { 0 };
+# named pipes, and named sockets). We start off with a safe set.
+allowxperm domain { file_type fs_type domain dev_type }:{ dir notdevfile_class_set } ioctl { FIOCLEX FIONCLEX };
 
 # Allow a process to make a determination whether a file descriptor
 # for a plain file is a tty. Note that granting this whitelist to domain
@@ -351,8 +351,9 @@ allow domain apex_mnt_dir:lnk_file r_file_perms;
 ### neverallow rules
 ###
 
-# All socket ioctls must be restricted to a whitelist.
-neverallowxperm domain domain:socket_class_set ioctl { 0 };
+# All ioctls on file-like objects (except chr_file and blk_file) and
+# sockets must be restricted to a whitelist.
+neverallowxperm * *:{ dir notdevfile_class_set socket_class_set } ioctl { 0 };
 
 # b/68014825 and https://android-review.googlesource.com/516535
 # rfc6093 says that processes should not use the TCP urgent mechanism
diff --git a/public/ioctl_macros b/public/ioctl_macros
index f7081d576..5cbfae53f 100644
--- a/public/ioctl_macros
+++ b/public/ioctl_macros
@@ -43,14 +43,14 @@ SIOCIWFIRSTPRIV-SIOCIWLASTPRIV
 
 # commonly used ioctls on unix sockets
 define(`unpriv_unix_sock_ioctls', `{
-  TIOCOUTQ FIOCLEX TCGETS TIOCGWINSZ TIOCSWINSZ FIONREAD
+  TIOCOUTQ FIOCLEX FIONCLEX TCGETS TIOCGWINSZ TIOCSWINSZ FIONREAD
 }')
 
 # commonly used TTY ioctls
 # merge with unpriv_unix_sock_ioctls?
 define(`unpriv_tty_ioctls', `{
-  TIOCOUTQ FIOCLEX TCGETS TCSETS TIOCGWINSZ TIOCSWINSZ TIOCSCTTY TCSETSW
-  TCFLSH TIOCSPGRP TIOCGPGRP
+  TIOCOUTQ FIOCLEX FIONCLEX TCGETS TCSETS TIOCGWINSZ TIOCSWINSZ TIOCSCTTY
+  TCSETSW TCFLSH TIOCSPGRP TIOCGPGRP
 }')
 
 # point to point ioctls
-- 
GitLab