2017-05-19 16:16:52

by Stephen Smalley

[permalink] [raw]
Subject: [refpolicy] [PATCH] refpolicy: Define and allow map permission

Kernel commit 6941857e82ae ("selinux: add a map permission check
for mmap") added a map permission check on mmap so that we can
distinguish memory mapped access (since it has different implications
for revocation). The purpose of a separate map permission check on
mmap(2) is to permit policy to prohibit memory mapping of specific files
for which we need to ensure that every access is revalidated, particularly
useful for scenarios where we expect the file to be relabeled at runtime
in order to reflect state changes (e.g. cross-domain solution, assured
pipeline without data copying). The kernel commit is anticipated to
be included in Linux 4.13.

This refpolicy change defines map permission for refpolicy. It mirrors
the definition in the kernel classmap by adding it to the common
definitions for files and sockets. This will break compatibility for
kernels that predate the dynamic class/perm mapping support (< 2.6.33,
< RHEL 6); on such kernels, one would instead need to add map permission
to the end of each file and socket access vector.

This change also adds map permission to the object permission sets for
regular files and character devices that already include open permission,
under the rationale that any process that could previously open a file
could also mmap it. Map permission is only included in the sets for
regular files and character devices since mmap does not seem to be widely
used on other file types (and is often not supported at all). Technically,
this is still more restrictive than before map permission was defined,
since previously any process with read permission to a file could
potentially mmap it, even if it only inherited or received an already
opened file descriptor, and this change further does not address allow
rules that do not use object permission sets so this may not be 100%
backward compatible.

An alternative approach would be to only allow map permission as needed,
e.g. only in the mmap_file_perms and exec_file_perms object permission
sets (since map is always required there) and only in specific interfaces
or modules. This would provide greater least privilege but is less
compatible and seems unwarranted given the limited benefits of the
permission and its narrow use case.

It is important to note that effective use of this permission requires
complete removal of unconfined, as otherwise unconfined domains will be
able to map all file types and therefore bypass the intended protection.
This is true regardless of whether map permission is added to most
object permission sets due to the manner in which unconfined policy
is written (complemented permission sets).

Further, domains that rely on this permission must be carefully written
to NOT use the object permission sets that allow map permission.
Possibly _nomap variants of common object permission sets would be useful
for this purpose.

Signed-off-by: Stephen Smalley <[email protected]>
---
policy/flask/access_vectors | 2 ++
policy/support/obj_perm_sets.spt | 26 +++++++++++++-------------
2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/policy/flask/access_vectors b/policy/flask/access_vectors
index 7652a31..b0f1354 100644
--- a/policy/flask/access_vectors
+++ b/policy/flask/access_vectors
@@ -20,6 +20,7 @@ common file
relabelfrom
relabelto
append
+ map
unlink
link
rename
@@ -47,6 +48,7 @@ common socket
relabelfrom
relabelto
append
+ map
# socket-specific
bind
connect
diff --git a/policy/support/obj_perm_sets.spt b/policy/support/obj_perm_sets.spt
index 872ca1d..d597f37 100644
--- a/policy/support/obj_perm_sets.spt
+++ b/policy/support/obj_perm_sets.spt
@@ -153,17 +153,17 @@ define(`relabel_dir_perms',`{ getattr relabelfrom relabelto }')
#
define(`getattr_file_perms',`{ getattr }')
define(`setattr_file_perms',`{ setattr }')
-define(`read_file_perms',`{ getattr open read lock ioctl }')
-define(`mmap_file_perms',`{ getattr open read execute ioctl }')
-define(`exec_file_perms',`{ getattr open read execute ioctl execute_no_trans }')
-define(`append_file_perms',`{ getattr open append lock ioctl }')
-define(`write_file_perms',`{ getattr open write append lock ioctl }')
+define(`read_file_perms',`{ getattr open map read lock ioctl }')
+define(`mmap_file_perms',`{ getattr open map read execute ioctl }')
+define(`exec_file_perms',`{ getattr open map read execute ioctl execute_no_trans }')
+define(`append_file_perms',`{ getattr open map append lock ioctl }')
+define(`write_file_perms',`{ getattr open map write append lock ioctl }')
define(`rw_inherited_file_perms',`{ getattr read write append ioctl lock }')
-define(`rw_file_perms',`{ open rw_inherited_file_perms }')
-define(`create_file_perms',`{ getattr create open }')
+define(`rw_file_perms',`{ open map rw_inherited_file_perms }')
+define(`create_file_perms',`{ getattr create open map }')
define(`rename_file_perms',`{ getattr rename }')
define(`delete_file_perms',`{ getattr unlink }')
-define(`manage_file_perms',`{ create open getattr setattr read write append rename link unlink ioctl lock }')
+define(`manage_file_perms',`{ create open map getattr setattr read write append rename link unlink ioctl lock }')
define(`relabelfrom_file_perms',`{ getattr relabelfrom }')
define(`relabelto_file_perms',`{ getattr relabelto }')
define(`relabel_file_perms',`{ getattr relabelfrom relabelto }')
@@ -241,14 +241,14 @@ define(`relabel_blk_file_perms',`{ getattr relabelfrom relabelto }')
#
define(`getattr_chr_file_perms',`{ getattr }')
define(`setattr_chr_file_perms',`{ setattr }')
-define(`read_chr_file_perms',`{ getattr open read lock ioctl }')
-define(`append_chr_file_perms',`{ getattr open append lock ioctl }')
-define(`write_chr_file_perms',`{ getattr open write append lock ioctl }')
-define(`rw_chr_file_perms',`{ getattr open read write append ioctl lock }')
+define(`read_chr_file_perms',`{ getattr open map read lock ioctl }')
+define(`append_chr_file_perms',`{ getattr open map append lock ioctl }')
+define(`write_chr_file_perms',`{ getattr open map write append lock ioctl }')
+define(`rw_chr_file_perms',`{ getattr open map read write append ioctl lock }')
define(`create_chr_file_perms',`{ getattr create }')
define(`rename_chr_file_perms',`{ getattr rename }')
define(`delete_chr_file_perms',`{ getattr unlink }')
-define(`manage_chr_file_perms',`{ create open getattr setattr read write append rename link unlink ioctl lock }')
+define(`manage_chr_file_perms',`{ create open map getattr setattr read write append rename link unlink ioctl lock }')
define(`relabelfrom_chr_file_perms',`{ getattr relabelfrom }')
define(`relabelto_chr_file_perms',`{ getattr relabelto }')
define(`relabel_chr_file_perms',`{ getattr relabelfrom relabelto }')
--
2.9.3


2017-05-22 23:15:20

by Chris PeBenito

[permalink] [raw]
Subject: [refpolicy] [PATCH] refpolicy: Define and allow map permission

On 05/19/2017 12:16 PM, Stephen Smalley via refpolicy wrote:
> Kernel commit 6941857e82ae ("selinux: add a map permission check
> for mmap") added a map permission check on mmap so that we can
> distinguish memory mapped access (since it has different implications
> for revocation). The purpose of a separate map permission check on
> mmap(2) is to permit policy to prohibit memory mapping of specific files
> for which we need to ensure that every access is revalidated, particularly
> useful for scenarios where we expect the file to be relabeled at runtime
> in order to reflect state changes (e.g. cross-domain solution, assured
> pipeline without data copying). The kernel commit is anticipated to
> be included in Linux 4.13.
>
> This refpolicy change defines map permission for refpolicy. It mirrors
> the definition in the kernel classmap by adding it to the common
> definitions for files and sockets. This will break compatibility for
> kernels that predate the dynamic class/perm mapping support (< 2.6.33,
> < RHEL 6); on such kernels, one would instead need to add map permission
> to the end of each file and socket access vector.
>
> This change also adds map permission to the object permission sets for
> regular files and character devices that already include open permission,
> under the rationale that any process that could previously open a file
> could also mmap it. Map permission is only included in the sets for
> regular files and character devices since mmap does not seem to be widely
> used on other file types (and is often not supported at all). Technically,
> this is still more restrictive than before map permission was defined,
> since previously any process with read permission to a file could
> potentially mmap it, even if it only inherited or received an already
> opened file descriptor, and this change further does not address allow
> rules that do not use object permission sets so this may not be 100%
> backward compatible.
>
> An alternative approach would be to only allow map permission as needed,
> e.g. only in the mmap_file_perms and exec_file_perms object permission
> sets (since map is always required there) and only in specific interfaces
> or modules. This would provide greater least privilege but is less
> compatible and seems unwarranted given the limited benefits of the
> permission and its narrow use case.

My inclination is that map not be added to most permission set macros.
My suspicion is that other than for shared objects and executables,
there is very little mmapping that happens in most systems; hopefully
few enough that not having it in the permission set macros wouldn't be
too painful to resolve.

That being said, I don't have any hard evidence to back up the above
suspicion. However, if we did put the perm in the macros, then it would
be difficult for those who do care to undo the permission.


> It is important to note that effective use of this permission requires
> complete removal of unconfined, as otherwise unconfined domains will be
> able to map all file types and therefore bypass the intended protection.
> This is true regardless of whether map permission is added to most
> object permission sets due to the manner in which unconfined policy
> is written (complemented permission sets).
>
> Further, domains that rely on this permission must be carefully written
> to NOT use the object permission sets that allow map permission.
> Possibly _nomap variants of common object permission sets would be useful
> for this purpose.
>
> Signed-off-by: Stephen Smalley <[email protected]>
> ---
> policy/flask/access_vectors | 2 ++
> policy/support/obj_perm_sets.spt | 26 +++++++++++++-------------
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/policy/flask/access_vectors b/policy/flask/access_vectors
> index 7652a31..b0f1354 100644
> --- a/policy/flask/access_vectors
> +++ b/policy/flask/access_vectors
> @@ -20,6 +20,7 @@ common file
> relabelfrom
> relabelto
> append
> + map
> unlink
> link
> rename
> @@ -47,6 +48,7 @@ common socket
> relabelfrom
> relabelto
> append
> + map
> # socket-specific
> bind
> connect
> diff --git a/policy/support/obj_perm_sets.spt b/policy/support/obj_perm_sets.spt
> index 872ca1d..d597f37 100644
> --- a/policy/support/obj_perm_sets.spt
> +++ b/policy/support/obj_perm_sets.spt
> @@ -153,17 +153,17 @@ define(`relabel_dir_perms',`{ getattr relabelfrom relabelto }')
> #
> define(`getattr_file_perms',`{ getattr }')
> define(`setattr_file_perms',`{ setattr }')
> -define(`read_file_perms',`{ getattr open read lock ioctl }')
> -define(`mmap_file_perms',`{ getattr open read execute ioctl }')
> -define(`exec_file_perms',`{ getattr open read execute ioctl execute_no_trans }')
> -define(`append_file_perms',`{ getattr open append lock ioctl }')
> -define(`write_file_perms',`{ getattr open write append lock ioctl }')
> +define(`read_file_perms',`{ getattr open map read lock ioctl }')
> +define(`mmap_file_perms',`{ getattr open map read execute ioctl }')
> +define(`exec_file_perms',`{ getattr open map read execute ioctl execute_no_trans }')
> +define(`append_file_perms',`{ getattr open map append lock ioctl }')
> +define(`write_file_perms',`{ getattr open map write append lock ioctl }')
> define(`rw_inherited_file_perms',`{ getattr read write append ioctl lock }')
> -define(`rw_file_perms',`{ open rw_inherited_file_perms }')
> -define(`create_file_perms',`{ getattr create open }')
> +define(`rw_file_perms',`{ open map rw_inherited_file_perms }')
> +define(`create_file_perms',`{ getattr create open map }')
> define(`rename_file_perms',`{ getattr rename }')
> define(`delete_file_perms',`{ getattr unlink }')
> -define(`manage_file_perms',`{ create open getattr setattr read write append rename link unlink ioctl lock }')
> +define(`manage_file_perms',`{ create open map getattr setattr read write append rename link unlink ioctl lock }')
> define(`relabelfrom_file_perms',`{ getattr relabelfrom }')
> define(`relabelto_file_perms',`{ getattr relabelto }')
> define(`relabel_file_perms',`{ getattr relabelfrom relabelto }')
> @@ -241,14 +241,14 @@ define(`relabel_blk_file_perms',`{ getattr relabelfrom relabelto }')
> #
> define(`getattr_chr_file_perms',`{ getattr }')
> define(`setattr_chr_file_perms',`{ setattr }')
> -define(`read_chr_file_perms',`{ getattr open read lock ioctl }')
> -define(`append_chr_file_perms',`{ getattr open append lock ioctl }')
> -define(`write_chr_file_perms',`{ getattr open write append lock ioctl }')
> -define(`rw_chr_file_perms',`{ getattr open read write append ioctl lock }')
> +define(`read_chr_file_perms',`{ getattr open map read lock ioctl }')
> +define(`append_chr_file_perms',`{ getattr open map append lock ioctl }')
> +define(`write_chr_file_perms',`{ getattr open map write append lock ioctl }')
> +define(`rw_chr_file_perms',`{ getattr open map read write append ioctl lock }')
> define(`create_chr_file_perms',`{ getattr create }')
> define(`rename_chr_file_perms',`{ getattr rename }')
> define(`delete_chr_file_perms',`{ getattr unlink }')
> -define(`manage_chr_file_perms',`{ create open getattr setattr read write append rename link unlink ioctl lock }')
> +define(`manage_chr_file_perms',`{ create open map getattr setattr read write append rename link unlink ioctl lock }')
> define(`relabelfrom_chr_file_perms',`{ getattr relabelfrom }')
> define(`relabelto_chr_file_perms',`{ getattr relabelto }')
> define(`relabel_chr_file_perms',`{ getattr relabelfrom relabelto }')
>


--
Chris PeBenito