From: dac.override@gmail.com (Dominick Grift)
Date: Sun, 10 Sep 2017 15:45:28 +0200
Subject: [refpolicy] file map perm issues
In-Reply-To: <20170910124023.GA29705@meriadoc.perfinion.com>
References: <20170910124023.GA29705@meriadoc.perfinion.com>
Message-ID: <20170910134528.GA3453@julius.enp8s0.d30>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Sun, Sep 10, 2017 at 08:40:23PM +0800, 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?

Most of the time you can't really tie the map to the file. one domain might want to map the file, the other might not

the exception are binary files in my opinion. binaries usually get mapped it seems

> 
> 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?

I would just create a files_map_etc_files() and call that only where needed

> 
> 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 }')

No the mmap_file_perms should be left alone.

a map_file_perms and map_sock_file_perms are probably not worth it (although in dssp2 i did create them)

in other words you should probably just use raw rules

files_map_etc_files(`,'
    gen_require(` type etc_t; ')

    allow ARG1 etc_t:file map;
')

>  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?

no because map usually isnt specific to a file (unless its a binary maybe)

one domain might want map on the file and the other might not want map on the file

> 
> 
> 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 the above looks good to me

> 
> 
> Lastly, Ive seen a whole ton of domains need allow foo etc_t:file map;
> and the audit logs show /etc/passwd as the file being accessed.  I'm
> fairly certain this is from nsswitch. Can someone else verify too?
> strace (below) and the fact that there is a very strong correlation with
> domains that contain nsswitch_domain.
> 
> authlogin.te already contains: files_read_etc_files(nsswitch_domain), so
> if we just add file map to the _read_ interfaces then it'll just work.
> otherwise adding a files_mmap_etc_files(nsswitch_domain) would work.
> 
> excerpt of relevant lines:
> $ strace whoami
> read(3, "# /etc/nsswitch.conf:\n# $Header:"..., 512) = 508
> open("/lib64/libnss_nis.so.2", O_RDONLY|O_CLOEXEC) = 3
> open("/lib64/libnss_files.so.2", O_RDONLY|O_CLOEXEC) = 3
> open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 3
> mmap(NULL, 2305, PROT_READ, MAP_SHARED, 3, 0) = 0x7fa04654f000
> 
> I'm going to test out these fixes more and will send patches once style
> has been decided.

for what its worth. i don't allow nss clients to map by default


> 
> -- Jason
> _______________________________________________
> refpolicy mailing list
> refpolicy at oss.tresys.com
> http://oss.tresys.com/mailman/listinfo/refpolicy

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8  02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
Url : http://oss.tresys.com/pipermail/refpolicy/attachments/20170910/805fe1f0/attachment.bin