Re: [PATCH 2/2] libselinux: update getseuser

From: Nicolas Iooss <nicolas.iooss@m4x.org>
To: "Christian Göttsche" <cgzones@googlemail.com>
Cc: SElinux list <selinux@vger.kernel.org>
Subject: Re: [PATCH 2/2] libselinux: update getseuser
Date: Thu, 21 Jan 2021 22:24:26 +0100	[thread overview]
Message-ID: <CAJfZ7==JzeBdo=H6=ZaeiTL2GX4rmimLV5v5jay2p5_HeDWd1A@mail.gmail.com> (raw)
In-Reply-To: <CAJfZ7=mWSXAEsdzK1D0zdyd7JX=Pj+mdzNsata611gmfFngnTg@mail.gmail.com>

On Wed, Jan 20, 2021 at 5:06 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Fri, Jan 8, 2021 at 5:02 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > - Bail out if not running on a SELinux enabled system
> > - Check whether the passed context is valid
> > - Do not report a get_ordered_context_list_with_level failure on zero
> >   found contexts
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  libselinux/utils/getseuser.c | 47 +++++++++++++++++++++++++-----------
> >  1 file changed, 33 insertions(+), 14 deletions(-)
> >
> > diff --git a/libselinux/utils/getseuser.c b/libselinux/utils/getseuser.c
> > index 9193fe0a..ce1b7b27 100644
> > --- a/libselinux/utils/getseuser.c
> > +++ b/libselinux/utils/getseuser.c
> > @@ -9,32 +9,51 @@ int main(int argc, char **argv)
> >  {
> >         char *seuser = NULL, *level = NULL;
> >         char **contextlist;
> > -       int rc, n, i;
> > +       int rc, n;
> >
> >         if (argc != 3) {
> >                 fprintf(stderr, "usage:  %s linuxuser fromcon\n", argv[0]);
> > -               exit(1);
> > +               return 1;
> > +       }
> > +
> > +       if (!is_selinux_enabled()) {
> > +               fprintf(stderr, "%s may be used only on a SELinux enabled kernel.\n", argv[0]);
> > +               return 4;
> >         }
> >
> >         rc = getseuserbyname(argv[1], &seuser, &level);
> >         if (rc) {
> > -               fprintf(stderr, "getseuserbyname failed:  %s\n",
> > -                       strerror(errno));
> > -               exit(2);
> > +               fprintf(stderr, "getseuserbyname failed:  %s\n", strerror(errno));
> > +               return 2;
> >         }
> >         printf("seuser:  %s, level %s\n", seuser, level);
> > -       n = get_ordered_context_list_with_level(seuser, level, argv[2],
> > -                                               &contextlist);
> > -       if (n <= 0) {
> > -               fprintf(stderr,
> > -                       "get_ordered_context_list_with_level failed:  %s\n",
> > -                       strerror(errno));
> > -               exit(3);
> > +
> > +       rc = security_check_context(argv[2]);
> > +       if (rc) {
> > +               fprintf(stderr, "context '%s' is invalid\n", argv[2]);
> > +               free(seuser);
> > +               free(level);
> > +               return 5;
> > +       }
> > +
> > +       n = get_ordered_context_list_with_level(seuser, level, argv[2], &contextlist);
> > +       if (n < 0) {
> > +               fprintf(stderr, "get_ordered_context_list_with_level failed:  %s\n", strerror(errno));
> > +               free(seuser);
> > +               free(level);
> > +               return 3;
> >         }
> > +
> >         free(seuser);
> >         free(level);
> > -       for (i = 0; i < n; i++)
> > +
> > +       if (n == 0)
> > +               printf("no valid context found\n");
> > +
> > +       for (int i = 0; i < n; i++)
> >                 printf("Context %d\t%s\n", i, contextlist[i]);
> > +
> >         freeconary(contextlist);
> > -       exit(EXIT_SUCCESS);
> > +
> > +       return EXIT_SUCCESS;
> >  }
> > --
> > 2.30.0
>
> Thanks for your patch! Sorry for the delay: I wanted to test things
> and I have been to busy in the past few days to boot my test machine,
> and now I see that your patch greatly improves the usability of
> getseuser! Many thanks!
>
> As an example of the improvement, before (on a non-MLS system):
>
> $ getseuser myadmin system_u:system_r:sshd_t
> seuser:  unconfined_u, level (null)
> Context 0 unconfined_u:unconfined_r:unconfined_t
> $ getseuser myadmin sshd_t
> seuser:  unconfined_u, level (null)
> get_ordered_context_list:  error in processing configuration file
> /etc/selinux/refpolicy-git/contexts/users/unconfined_u
> get_ordered_context_list:  error in processing configuration file
> /etc/selinux/refpolicy-git/contexts/default_contexts
> get_ordered_context_list_with_level failed:  Invalid argument
>
> With your patch, the first command did not change, but the second one
> now returns:
>
> $ getseuser myadmin sshd_t
> seuser:  unconfined_u, level (null)
> context 'sshd_t' is invalid
>
> ... which is much more helpful, in my humble opinion.
>
> So for both your patches: Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> I will merge the 2 patches tomorrow if nobody complains.
>
> Nicolas

Merged. Thanks!
Nicolas