From: pebenito@ieee.org (Chris PeBenito)
Date: Tue, 23 Jan 2018 19:52:54 -0500
Subject: [refpolicy] [PATCH] postgres: Add neccessary map permissions
In-Reply-To: <CAMN686E0NEVLukcsw5NkKG4Zg0i+-RQP0qdxk+Wrs0OnXYgdmA@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>
	<CAMN686E0NEVLukcsw5NkKG4Zg0i+-RQP0qdxk+Wrs0OnXYgdmA@mail.gmail.com>
Message-ID: <e5435b0c-7775-8540-6585-1a9adc7b612c@ieee.org>
To: refpolicy@oss.tresys.com
List-Id: refpolicy.oss.tresys.com

On 01/23/2018 02:51 PM, Mike Palmiotto wrote:
> 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 should have been clearer with my question.  I meant to ask if the mmap 
was intrinsic to hugetlbfs use.  Since postgres is explicitly mmapping 
it, then it would seem the answer is no.


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

I'd like to delay the open removal too, as I would think you normally 
would need that too.



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