From: mike.palmiotto@crunchydata.com (Mike Palmiotto)
Date: Tue, 23 Jan 2018 14:51:37 -0500
Subject: [refpolicy] [PATCH] postgres: Add neccessary map permissions
In-Reply-To: <CAMN686EivdfKrsytjny66yHdf+to0vkvsovoTBLE-kFsFDw4ow@mail.gmail.com>
References: <20180121165603.1665-1-aranea@aixah.de>
	<CAMN686FHEWc0z2ZeFnJKgssx2zA5GkEWukis+r_fd0zKLCicxA@mail.gmail.com>
	<732fca65-0736-88e6-2927-48cd95fa4248@ieee.org>
	<CAMN686EivdfKrsytjny66yHdf+to0vkvsovoTBLE-kFsFDw4ow@mail.gmail.com>
Message-ID: <CAMN686E0NEVLukcsw5NkKG4Zg0i+-RQP0qdxk+Wrs0OnXYgdmA@mail.gmail.com>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On Mon, Jan 22, 2018 at 11:12 PM, Mike Palmiotto
<mike.palmiotto@crunchydata.com> wrote:
> On Mon, Jan 22, 2018 at 5:10 PM, Chris PeBenito <pebenito@ieee.org> wrote:
>> On 01/21/2018 02:52 PM, Mike Palmiotto via refpolicy wrote:
>>>
>>> On Sun, Jan 21, 2018 at 11:56 AM, Luis Ressel via refpolicy
>>> <refpolicy@oss.tresys.com> wrote:
>>>>
>>>> I'm also removing pg's permission to open hugetlbfs_t files, since it
>>>> doesn't make any sense.
>>>
>>>
>>> Is this because hugetlbfs_t files are accessed via mmap or read?
>>> Doesn't read require an open file descriptor?
>>>
>>> virtd_t and mysqld_t are also calling the `fs_rw_hugetlbfs_files`
>>> interface. If this is meant to address an issue fundamental to
>>> hugetlbfs and not something postgres-specific, perhaps the commit
>>> should fix the interface itself and/or virtd_t and mysqld_t as well.
>>
>>
>> This seems like a possibility, but I'd like more info.
>
> From looking through the kernel docs
> (https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt), it
> looks like 'read' system calls are perfectly acceptable, which is what
> initially lead me to believe that it's valid to open a hugetblfs_t
> file (fd open => read(fd)).
>
> Candidly, I'm not super familiar with how hugetlbfs works, but the
> only references I see to it in the postgres source
> (https://www.postgresql.org/docs/10/static/runtime-config-resource.html#GUC-HUGE-PAGES)
> involve mmap calls. This might explain Luis's comment about 'open' not
> making sense. If this is the case, my next question is "Are virtd_t
> and mysqld_t the same?" I haven't had a chance to check yet.
>
> I would like to verify that this change doesn't break hugetlbfs
> support on postgres. I'll spin up a Vagrant box this week to test it
> out. In the meantime, maybe we should just go ahead with the added map
> perms and address the open removal in a later commit (once proper
> testing and vetting of other domains has been done)?

I've verified that I can start postgres in Enforcing without the 'open' perm:
https://gist.github.com/anonymous/c8106ba77d50f87a57cbd875bbe84fbb

I would really like to test this more extensively on a Fedora 27 box,
so I can try the full patch -- just haven't had the time yet.
Chris/Luis, how do you feel about just adding the map perm and leaving
the 'open' removal for a future patch?

>
>
>>
>>
>>>> ---
>>>>   policy/modules/kernel/filesystem.if   | 18 ++++++++++++++++++
>>>>   policy/modules/services/postgresql.te |  3 ++-
>>>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/policy/modules/kernel/filesystem.if
>>>> b/policy/modules/kernel/filesystem.if
>>>> index 41f19619..e2a58d8e 100644
>>>> --- a/policy/modules/kernel/filesystem.if
>>>> +++ b/policy/modules/kernel/filesystem.if
>>>> @@ -2322,6 +2322,24 @@ interface(`fs_rw_inherited_hugetlbfs_files',`
>>>>          allow $1 hugetlbfs_t:file rw_inherited_file_perms;
>>>>   ')
>>>>
>>>> +########################################
>>>> +## <summary>
>>>> +##     Read and write inherited hugetlbfs files.
>>>> +## </summary>
>>>> +## <param name="domain">
>>>> +##     <summary>
>>>> +##     Domain allowed access.
>>>> +##     </summary>
>>>> +## </param>
>>>> +#
>>>> +interface(`fs_mmap_rw_inherited_hugetlbfs_files',`
>>>> +       gen_require(`
>>>> +               type hugetlbfs_t;
>>>> +       ')
>>>> +
>>>> +       allow $1 hugetlbfs_t:file mmap_rw_inherited_file_perms;
>>>> +')
>>>> +
>>>>   ########################################
>>>>   ## <summary>
>>>>   ##     Read and write hugetlbfs files.
>>>> diff --git a/policy/modules/services/postgresql.te
>>>> b/policy/modules/services/postgresql.te
>>>> index f118d9d0..97a9d153 100644
>>>> --- a/policy/modules/services/postgresql.te
>>>> +++ b/policy/modules/services/postgresql.te
>>>> @@ -297,6 +297,7 @@ manage_fifo_files_pattern(postgresql_t,
>>>> postgresql_tmp_t, postgresql_tmp_t)
>>>>   manage_sock_files_pattern(postgresql_t, postgresql_tmp_t,
>>>> postgresql_tmp_t)
>>>>   files_tmp_filetrans(postgresql_t, postgresql_tmp_t, { dir file
>>>> sock_file })
>>>>   fs_tmpfs_filetrans(postgresql_t, postgresql_tmp_t, { dir file lnk_file
>>>> sock_file fifo_file })
>>>> +allow postgresql_t postgresql_tmp_t:file map;
>>>>
>>>>   manage_dirs_pattern(postgresql_t, postgresql_var_run_t,
>>>> postgresql_var_run_t)
>>>>   manage_files_pattern(postgresql_t, postgresql_var_run_t,
>>>> postgresql_var_run_t)
>>>> @@ -330,7 +331,7 @@ dev_read_urand(postgresql_t)
>>>>
>>>>   fs_getattr_all_fs(postgresql_t)
>>>>   fs_search_auto_mountpoints(postgresql_t)
>>>> -fs_rw_hugetlbfs_files(postgresql_t)
>>>> +fs_mmap_rw_inherited_hugetlbfs_files(postgresql_t)
>>>>
>>>>   selinux_get_enforce_mode(postgresql_t)
>>>>   selinux_validate_context(postgresql_t)
>>
>>
>> --
>> Chris PeBenito
>
>
>
> --
> Mike Palmiotto
> Software Engineer
> Crunchy Data Solutions
> https://crunchydata.com


-- 
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com