From: jason@perfinion.com (Jason Zaman)
Date: Mon, 11 Sep 2017 00:29:51 +0800
Subject: [refpolicy] file map perm issues
In-Reply-To: <20170910134528.GA3453@julius.enp8s0.d30>
References: <20170910124023.GA29705@meriadoc.perfinion.com>
<20170910134528.GA3453@julius.enp8s0.d30>
Message-ID: <20170910162951.GB27516@meriadoc.perfinion.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com
On Sun, Sep 10, 2017 at 03:45:28PM +0200, Dominick Grift via refpolicy wrote:
> 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;
> ')
okay works for me. one more question then.
files_mmap_etc_files or files_map_etc_files?
> > 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?
> >
> > ########################################
> > ##
> > ## Map user tmpfs files.
> > ##
> > ##
> > ##
> > ## Domain allowed access.
> > ##
> > ##
> > #
> > 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
No need for search or anything right? it's assumed some other interface
will handle that?
>
> >
> >
> > 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
okay thats super weird. I have every single libnss_files doing a mmap
and yours dont... why?
Can someone test on other distros?
-- Jason