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

[Nicolas Iooss <nicolas.iooss@m4x.org>]

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