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

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


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