From: pebenito@ieee.org (Chris PeBenito)
Date: Mon, 11 Sep 2017 19:58:54 -0400
Subject: [refpolicy] file map perm issues
In-Reply-To: <20170910124023.GA29705@meriadoc.perfinion.com>
References: <20170910124023.GA29705@meriadoc.perfinion.com>
Message-ID: <677be737-abd9-eef4-8d57-086477802818@ieee.org>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On 09/10/2017 08:40 AM, Jason Zaman via refpolicy wrote:
> Hey all,
> 
> So kernel 4.13 was just released which contains the file map stuff and
> I've run into a fair few issues. I'd like some discussion about naming
> and how to best apply the perm.
> 
> we currently have a bunch of interfaces like "*_read_*_files", eg:
> files_read_etc_files, files_rw_etc_files, files_manage_etc_files.
> should interfaces like this include the map perm? I am thinking no?
> Or maybe included for domains that seem to always need it (eg etc_t) and
> not by default?

In general, the map permission should live by itself, especially on 
generic types like etc_t.  The exception being execs on binaries, as you 
have to map to execute a binary.

If you have a specific type who is always associated with mmaping, then 
map can be lumped in with other interfaces, for example, lib_t.  Using 
shared libs requires map and execute.

The main idea being, with mmap, you can mmap read, write, or executable, 
so the map permission by itself doesn't tell the whole idea (just like 
the open perm).  The main danger with map is that it's not revocable, so 
once a file is mmaped, if you lose access in the policy (e.g. due to a 
boolean change), the mmap isn't revoked.


> we also have these defs file_read_perms file_mmap_perms. so since those
> are different I'm thinking that files_read_etc_files should also be
> separated like that?
> 
> If they are to be separated, should things be just plain
> allow ...:file map; or should it just use file mmap_file_perms;? The
> issue with this one is that mmap_file_perms includes the execute perm.
> If we should just use mmap_file_perms, then maybe the defs should be
> changed like this:
> 
> -define(`mmap_file_perms',`{ getattr open map read execute ioctl }')
> +define(`mmap_file_perms',`{ getattr open map read ioctl }')
>   define(`exec_file_perms',`{ getattr open map read execute ioctl execute_no_trans }')
> 
> Isnt execute generally a more risky perm? also manage_file_perms and
> write_ and rw_ defs currently dont give the map perm, should they?

This was an unfortunate mistake with naming, especially now with the map 
permission being defined.  It is more accurate that the permission set 
be "mmap as executable" or "mmap as rx".  I'll have to revisit the 
permission set usage and the interfaces that use it to come up with a 
revised vocabulary for the permission set name(s) and interface verbs.


> 
> Once that is decided, what should interfaces for map interfaces look
> like? just map? should *_map_*_files interfaces include include read and
> open and stuff too ro are people expected to just use both interfaces
> together?
> 
> ########################################
> ## <summary>
> ##     Map user tmpfs files.
> ## </summary>
> ## <param name="domain">
> ##     <summary>
> ##     Domain allowed access.
> ##     </summary>
> ## </param>
> #
> interface(`userdom_map_user_tmpfs_files',`
>         gen_require(`
>                 type user_tmpfs_t;
>         ')
> 
>         allow $1 user_tmpfs_t:file map;
> ')

Yes, this is the general pattern: a map interface + one or more 
interfaces to comprise the actual read, write, etc. access.

-- 
Chris PeBenito