From 619c1ef2ac581fe6a3d628ee013fc3ec36b8dc07 Mon Sep 17 00:00:00 2001
From: Nick Kralevich <nnk@google.com>
Date: Tue, 30 Oct 2018 20:12:41 -0700
Subject: [PATCH] tun_device: enforce ioctl restrictions
Require all SELinux domains which have permission to perform ioctls on
/dev/tun explicitly specify what ioctls they perform. Only allow the
safe defaults FIOCLEX and FIONCLEX, which are alternate, uncommon ways
to set and unset the O_CLOEXEC flag.
Remove app's ability to issue *any* ioctls on /dev/tun, period. Add
neverallow assertions (compile time assertion + CTS test) to prevent
regressions.
Limit system_server's ability to perform ioctls on /dev/tun to FIOCLEX,
FIONCLEX, TUNGETIFF, and TUNSETIFF. Testing and source code examination
shows that only TUNGETIFF and TUNSETIFF are used by system_server.
The goal of this change is to put SELinux ioctl controls in place for
/dev/tun, so we don't have to maintain the custom kernel patch at
https://android.googlesource.com/kernel/common/+/11cee2be0c2062ba88f04eb51196506f870a3b5d%5E%21
Delete the neverallow assertion in isolated_app.te. This is already
covered by the assertion present in app_neverallows.te.
Test: cts-tradefed run cts -m CtsHostsideNetworkTests -t com.android.cts.net.HostsideVpnTests
Test: cts-tradefed run cts -m CtsHostsideNetworkTests
Test: cts-tradefed run cts -m CtsNetTestCases
Bug: 111560739
Bug: 111560570
Change-Id: Ibe1c3a9e880db0bee438535554abdbc6d84eec45
---
private/app_neverallows.te | 31 +++++++++++++++++++++++++++++--
private/isolated_app.te | 3 ---
private/system_server.te | 1 +
public/app.te | 2 +-
public/domain.te | 4 ++++
5 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/private/app_neverallows.te b/private/app_neverallows.te
index c1f9a2b01..46b49c274 100644
--- a/private/app_neverallows.te
+++ b/private/app_neverallows.te
@@ -112,8 +112,35 @@ neverallow { all_untrusted_apps -mediaprovider } {
# No untrusted component should be touching /dev/fuse
neverallow all_untrusted_apps fuse_device:chr_file *;
-# Do not allow untrusted apps to directly open tun_device
-neverallow all_untrusted_apps tun_device:chr_file open;
+# Do not allow untrusted apps to directly open or
+# issue ioctls to the tun_device
+neverallow all_untrusted_apps tun_device:chr_file { open ioctl };
+# Additionally, assert that the following ioctls are never reachable.
+# This should already be blocked by the neverallow rule above, but this
+# is added for robustness, and to prove equivalence to the kernel patch at
+# https://android.googlesource.com/kernel/common/+/11cee2be0c2062ba88f04eb51196506f870a3b5d%5E%21
+neverallowxperm all_untrusted_apps tun_device:chr_file ioctl {
+ SIOCGIFHWADDR
+ SIOCSIFHWADDR
+ TUNATTACHFILTER
+ TUNDETACHFILTER
+ TUNGETFEATURES
+ TUNGETFILTER
+ TUNGETSNDBUF
+ TUNGETVNETHDRSZ
+ TUNSETDEBUG
+ TUNSETGROUP
+ TUNSETIFF
+ TUNSETLINK
+ TUNSETNOCSUM
+ TUNSETOFFLOAD
+ TUNSETOWNER
+ TUNSETPERSIST
+ TUNSETQUEUE
+ TUNSETSNDBUF
+ TUNSETTXFILTER
+ TUNSETVNETHDRSZ
+};
# Only allow appending to /data/anr/traces.txt (b/27853304, b/18340553)
neverallow all_untrusted_apps anr_data_file:file ~{ open append };
diff --git a/private/isolated_app.te b/private/isolated_app.te
index a17f22a4c..1b56c5cf8 100644
--- a/private/isolated_app.te
+++ b/private/isolated_app.te
@@ -57,9 +57,6 @@ unix_socket_connect(isolated_app, traced_producer, traced)
##### Neverallow
#####
-# Do not allow isolated_app to directly open tun_device
-neverallow isolated_app tun_device:chr_file open;
-
# Isolated apps should not directly open app data files themselves.
neverallow isolated_app { app_data_file privapp_data_file }:file open;
diff --git a/private/system_server.te b/private/system_server.te
index 506378e46..a96b82be6 100644
--- a/private/system_server.te
+++ b/private/system_server.te
@@ -339,6 +339,7 @@ allow system_server audio_device:chr_file rw_file_perms;
# tun device used for 3rd party vpn apps
allow system_server tun_device:chr_file rw_file_perms;
+allowxperm system_server tun_device:chr_file ioctl { TUNGETIFF TUNSETIFF };
# Manage system data files.
allow system_server system_data_file:dir create_dir_perms;
diff --git a/public/app.te b/public/app.te
index 7f0d5548e..549930291 100644
--- a/public/app.te
+++ b/public/app.te
@@ -334,7 +334,7 @@ allow appdomain runas_exec:file getattr;
# Apps receive an open tun fd from the framework for
# device traffic. Do not allow untrusted app to directly open tun_device
-allow { appdomain -isolated_app -ephemeral_app } tun_device:chr_file { read write getattr ioctl append };
+allow { appdomain -isolated_app -ephemeral_app } tun_device:chr_file { read write getattr append };
# Connect to adbd and use a socket transferred from it.
# This is used for e.g. adb backup/restore.
diff --git a/public/domain.te b/public/domain.te
index 0a838a3d4..0244b7a45 100644
--- a/public/domain.te
+++ b/public/domain.te
@@ -300,6 +300,10 @@ allowxperm domain devpts:chr_file ioctl unpriv_tty_ioctls;
# 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 blk_file } ioctl { FIOCLEX FIONCLEX };
+# If a domain has ioctl access to tun_device, it must clearly enumerate the
+# ioctls used. Safe defaults are listed below.
+allowxperm domain tun_device:chr_file ioctl { FIOCLEX FIONCLEX };
+
# Allow a process to make a determination whether a file descriptor
# for a plain file or pipe (fifo_file) is a tty. Note that granting
# this whitelist to domain does not grant the ioctl permission to
--
GitLab