Re: [PATCH v3] scripts/selinux: modernize mdp

[Dominick Grift <dominick.grift@defensec.nl>]

On Wed, Feb 20, 2019 at 02:34:16PM -0500, Stephen Smalley wrote:
> On Wed, Feb 20, 2019 at 10:34 AM Dominick Grift
> <dominick.grift@defensec.nl> wrote:
> >
> > The MDP example no longer works on modern systems.
> >
> > Add support for devtmpfs. This is required by login programs to relabel terminals.
> > Compile the policy with deny_unknown allow status to anticipate user space object managers in core components such as systemd.
> > Add default seusers mapping and failsafe context for the SELinux PAM module.
> >
> > V2:
> > Fix existing file test for setfiles.
> > Add a file test for checkpolicy similar to the test for setfiles for consistency.
> > Execute setfiles with -F to ensure that customizables are relabeled as well in scenarios where filesystems are labeled but where SELinux is disabled.
> >
> > V3: Fixes file test that was introduced in V2.
> >
> > Signed-off-by: Dominick Grift <dominick.grift@defensec.nl>
> > ---
> >  scripts/selinux/install_policy.sh | 21 ++++++++++++++++-----
> >  scripts/selinux/mdp/mdp.c         |  1 +
> >  2 files changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/selinux/install_policy.sh b/scripts/selinux/install_policy.sh
> > index 0b86c47baf7d..e32f333f14cc 100755
> > --- a/scripts/selinux/install_policy.sh
> > +++ b/scripts/selinux/install_policy.sh
> > @@ -6,7 +6,7 @@ if [ `id -u` -ne 0 ]; then
> >  fi
> >  SF=`which setfiles`
> >  if [ $? -eq 1 ]; then
> > -       if [ -f /sbin/setfiles ]; then
> > +       if [ -f /usr/setfiles ]; then
> >                 SF="/usr/setfiles"
> 
> I don't believe we've ever installed setfiles anywhere other than
> /sbin/setfile or /usr/sbin/setfiles so no other locations should be
> used.  Maybe just drop the inner if statement entirely.
> 
> >         else
> >                 echo "no selinux tools installed: setfiles"
> > @@ -17,14 +17,25 @@ fi
> >  cd mdp
> >
> >  CP=`which checkpolicy`
> > +if [ $? -eq 1 ]; then
> > +       if [ -f /usr/checkpolicy ]; then
> > +               CP="/usr/checkpolicy"
> 
> Similarly for /usr/bin/checkpolicy or /bin/checkpolicy.
> 
> > +       else
> > +               echo "no selinux tools installed: checkpolicy"
> > +               exit 1
> > +       fi
> > +fi
> >  VERS=`$CP -V | awk '{print $1}'`
> >
> >  ./mdp policy.conf file_contexts
> > -$CP -o policy.$VERS policy.conf
> > +$CP -U allow -o policy.$VERS policy.conf
> >
> >  mkdir -p /etc/selinux/dummy/policy
> >  mkdir -p /etc/selinux/dummy/contexts/files
> >
> > +echo "__default__:user_u" > /etc/selinux/dummy/seusers
> > +echo "base_r:base_t" > /etc/selinux/dummy/contexts/failsafe_context
> > +
> >  cp file_contexts /etc/selinux/dummy/contexts/files
> >  cp dbus_contexts /etc/selinux/dummy/contexts
> >  cp policy.$VERS /etc/selinux/dummy/policy
> > @@ -55,15 +66,15 @@ else
> >  fi
> >
> >  cd /etc/selinux/dummy/contexts/files
> > -$SF file_contexts /
> > +$SF -F file_contexts /
> >
> >  mounts=`cat /proc/$$/mounts | egrep "ext2|ext3|xfs|jfs|ext4|ext4dev|gfs2" | awk '{ print $2 '}`
> > -$SF file_contexts $mounts
> > +$SF -F file_contexts $mounts
> >
> >
> >  dodev=`cat /proc/$$/mounts | grep "/dev "`
> >  if [ "eq$dodev" != "eq" ]; then
> >         mount --move /dev /mnt
> 
> I recall seeing errors from mount when running this script about
> invalid usage of mount --move?

Yes but we may want this for (backwards) compatibility? I suspect that this might work in some scenario's (just not ours)

> 
> > -       $SF file_contexts /dev
> > +       $SF -F file_contexts /dev
> >         mount --move /mnt /dev
> >  fi
> > diff --git a/scripts/selinux/mdp/mdp.c b/scripts/selinux/mdp/mdp.c
> > index 073fe7537f6c..cf06d5694cbc 100644
> > --- a/scripts/selinux/mdp/mdp.c
> > +++ b/scripts/selinux/mdp/mdp.c
> > @@ -131,6 +131,7 @@ int main(int argc, char *argv[])
> >
> >         fprintf(fout, "fs_use_trans mqueue user_u:base_r:base_t;\n");
> >         fprintf(fout, "fs_use_trans devpts user_u:base_r:base_t;\n");
> > +       fprintf(fout, "fs_use_trans devtmpfs user_u:base_r:base_t;\n");
> 
> This will conflict with my patch but probably your patch should be
> applied first since it fixes current usage of mdp, and then mine can
> be re-based and introduce the MLS support (which hopefully can now be
> tested on Fedora). Ideally we'd add fs_use* and genfscon statements
> for all filesystems supported by the kernel but that can come later.

Probably yes, but you can also just take my changes and add them to your patch.

BTW: we also might want to use object_r instead of base_r for object contexts for consistency?

> 
> >         fprintf(fout, "fs_use_trans hugetlbfs user_u:base_r:base_t;\n");
> >         fprintf(fout, "fs_use_trans tmpfs user_u:base_r:base_t;\n");
> >         fprintf(fout, "fs_use_trans shm user_u:base_r:base_t;\n");
> > --
> > 2.21.0.rc1
> >

-- 
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift