MIME-Version: 1.0
References: <23856.1563381159@sss.pgh.pa.us>
In-Reply-To: <23856.1563381159@sss.pgh.pa.us>
From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
Date: Thu, 18 Jul 2019 19:23:08 -0400
Message-ID: 
 <CAMN686Ew55FmGAZMsin_r2GKOpTwNwGS2PJ2v+KX8jO2yU3Sqw@mail.gmail.com>
Subject: Re: sepgsql seems rather thoroughly broken on Fedora 30
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org, Joe Conway <mail@joeconway.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jul 17, 2019 at 12:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I tried to run the contrib/sepgsql tests, following the instructions,
> on a recently-set-up Fedora 30 machine.  I've done that successfully
> on previous Fedora releases, but it's no go with F30.
>
> First off, building the sepgsql-regtest.pp policy file spews
> a bunch of complaints that I don't recall having seen before:
>
> $ make -f /usr/share/selinux/devel/Makefile
> /usr/share/selinux/devel/include/services/container.if:14: Error: duplicate definition of container_runtime_domtrans(). Original definition on 14.
> /usr/share/selinux/devel/include/services/container.if:41: Error: duplicate definition of container_runtime_run(). Original definition on 41.
> /usr/share/selinux/devel/include/services/container.if:61: Error: duplicate definition of container_runtime_exec(). Original definition on 61.
> /usr/share/selinux/devel/include/services/container.if:80: Error: duplicate definition of container_read_state(). Original definition on 80.
> ... more of the same ...
> /usr/share/selinux/devel/include/services/container.if:726: Error: duplicate definition of docker_stream_connect(). Original definition on 726.
> /usr/share/selinux/devel/include/services/container.if:730: Error: duplicate definition of docker_spc_stream_connect(). Original definition on 730.
> /usr/share/selinux/devel/include/services/container.if:744: Error: duplicate definition of container_spc_read_state(). Original definition on 744.
> /usr/share/selinux/devel/include/services/container.if:763: Error: duplicate definition of container_domain_template(). Original definition on 763.
> /usr/share/selinux/devel/include/services/container.if:791: Error: duplicate definition of container_spc_rw_pipes(). Original definition on 791.

These errors are due to a conflict between "container-selinux" and
"selinux-policy-devel." With both packages installed, you will see the
container interface file in both
/usr/share/selinux/devel/include/contrib and
/usr/share/selinux/devel/include/services:

% sudo find /usr/share/selinux/devel -type f -name "container.if"
/usr/share/selinux/devel/include/contrib/container.if
/usr/share/selinux/devel/include/services/container.if

This is likely a bug that should be fixed by "container-selinux." I'll
see what I can do about getting that fixed upstream.
As you noted, the build errors are likely a red herring, since the .pp
still installs and the test script recognizes the module as loaded. If
you want to get rid of these for now and you aren't particularly
concerned about your container policy module, you can just uninstall
the "container-selinux" package.

>
> ============== checking selinux environment           ==============
> checking for matchpathcon           ... ok
> checking for runcon                 ... ok
> checking for sestatus               ... ok
> checking current user domain        ... unconfined_t
> checking selinux operating mode     ... enforcing
> checking for sepgsql-regtest policy ... ok
> checking whether policy is enabled  ... on
> on
> checking whether we can run psql    ... failed
>
> <snip>
> (BTW, what's that extra "on" after "checking whether policy is enabled"?)

The second "on" is from the `getsebool sepgsql_enable_users_ddl`
check, which has no associated "checking policy boolean" message.
We'll minimally want to add more specific messages for the two
`getsebool` checks.

> $ runcon -t sepgsql_regtest_user_t psql --help
> psql: fatal: could not look up effective user ID 1000: user does not exist
>
> But uid 1000 is me according to /etc/passwd and according to "id":
>
> $ id
> uid=1000(tgl) gid=1000(tgl) groups=1000(tgl),10(wheel) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
>
> so there's nothing much wrong with having that as euid.
>
> I speculate that the policy is forbidding sepgsql_regtest_user_t
> from reading /etc/passwd.  Perhaps this is fallout from the
> compile problems reported for the policy module?  But I'm way
> out of my depth here.

I wonder what your password file is labeled. It ought to be:
% ls -Z /etc/passwd
system_u:object_r:passwd_file_t:s0 /etc/passwd

The sepgsql_regtest_user_t domain should be allowed to read any file
labeled "passwd_file_t". We can check that with the `sesearch` tool,
provided by the "setools-console" package on F30:

% sudo sesearch -A -s sepgsql_regtest_user_t -t passwd_file_t
allow domain file_type:blk_file map; [ domain_can_mmap_files ]:True
allow domain file_type:chr_file map; [ domain_can_mmap_files ]:True
allow domain file_type:file map; [ domain_can_mmap_files ]:True
allow nsswitch_domain passwd_file_t:file { getattr ioctl lock map open read };

If your /etc/passwd label is not correct, you can try just running
`restorecon -RF /` to fix it.

In any case, it looks like this entire test script and policy could
use another layer of varnish, so I'll work on fixing up the
messages/functionality and post a patch which makes this a bit more
robust (hopefully a bit later tonight).
Sorry for the delayed response. Hopefully the band-aid fixes I
provided get you going for now.

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