[Libguestfs] [PATCH v2 2/7] New API: setfiles - SELinux relabel parts of the filesystem.

[Pino Toscano]

On Thursday, 14 July 2016 09:49:56 CEST Richard W.M. Jones wrote:
> ---
>  appliance/packagelist.in |  1 +
>  daemon/Makefile.am       |  1 +
>  daemon/setfiles.c        | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
>  generator/actions.ml     | 22 ++++++++++++
>  gobject/Makefile.inc     |  2 ++
>  src/MAX_PROC_NR          |  2 +-
>  6 files changed, 120 insertions(+), 1 deletion(-)
>  create mode 100644 daemon/setfiles.c
> 
> diff --git a/appliance/packagelist.in b/appliance/packagelist.in
> index 5f04c1c..3a4790b 100644
> --- a/appliance/packagelist.in
> +++ b/appliance/packagelist.in
> @@ -43,6 +43,7 @@ ifelse(REDHAT,1,
>    ntfs-3g
>    openssh-clients
>    pcre
> +  policycoreutils
>    reiserfs-utils
>    libselinux
>    syslinux-extlinux
> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
> index b77d1e7..9bd495f 100644
> --- a/daemon/Makefile.am
> +++ b/daemon/Makefile.am
> @@ -168,6 +168,7 @@ guestfsd_SOURCES = \
>  	rsync.c \
>  	scrub.c \
>  	selinux.c \
> +	setfiles.c \

Why not directly in selinux.c? IMHO would be more logic (all the
SELinux stuff in the daemon grouped there).

>  	sfdisk.c \
>  	sh.c \
>  	sleep.c \
> diff --git a/daemon/setfiles.c b/daemon/setfiles.c
> new file mode 100644
> index 0000000..3f249c3
> --- /dev/null
> +++ b/daemon/setfiles.c
> @@ -0,0 +1,93 @@
> +/* libguestfs - the guestfsd daemon
> + * Copyright (C) 2016 Red Hat Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "guestfs_protocol.h"
> +#include "daemon.h"
> +#include "actions.h"
> +
> +GUESTFSD_EXT_CMD(str_setfiles, setfiles);
> +
> +#define MAX_ARGS 64
> +
> +/* Takes optional arguments, consult optargs_bitmask. */
> +int
> +do_setfiles (const char *specfile, const char *path,
> +             int force)
> +{
> +  const char *argv[MAX_ARGS];
> +  CLEANUP_FREE char *s_dev = NULL, *s_proc = NULL, *s_selinux = NULL,
> +    *s_sys = NULL, *s_specfile = NULL, *s_path = NULL;
> +  CLEANUP_FREE char *err = NULL;
> +  size_t i = 0;
> +
> +  s_dev = sysroot_path ("/dev");
> +  if (!s_dev) {
> +  malloc_error:
> +    reply_with_perror ("malloc");
> +    return -1;
> +  }
> +  s_proc = sysroot_path ("/proc");       if (!s_proc) goto malloc_error;
> +  s_selinux = sysroot_path ("/selinux"); if (!s_selinux) goto malloc_error;
> +  s_sys = sysroot_path ("/sys");         if (!s_sys) goto malloc_error;
> +  s_specfile = sysroot_path (specfile);  if (!s_specfile) goto malloc_error;
> +  s_path = sysroot_path (path);          if (!s_path) goto malloc_error;

I'd simpy do all the sysroot_path calls one after each other, and then
check all the results at once:

  if (!s_dev || !s_proc ...)

All these buffers are CLEANUP_FREE anyway, and this could IMHO simplify
the reading/organization of the code.

> +  /* Default settings if not selected. */
> +  if (!(optargs_bitmask & GUESTFS_SETFILES_FORCE_BITMASK))
> +    force = 0;
> +
> +  ADD_ARG (argv, i, str_setfiles);
> +  if (force)
> +    ADD_ARG (argv, i, "-F");
> +
> +  /* Exclude some directories that should never be relabelled in
> +   * ordinary Linux guests.  These won't be mounted anyway.  We have
> +   * to prefix all these with the sysroot path.
> +   */
> +  ADD_ARG (argv, i, "-e"); ADD_ARG (argv, i, s_dev);
> +  ADD_ARG (argv, i, "-e"); ADD_ARG (argv, i, s_proc);
> +  ADD_ARG (argv, i, "-e"); ADD_ARG (argv, i, s_selinux);
> +  ADD_ARG (argv, i, "-e"); ADD_ARG (argv, i, s_sys);
> +
> +  /* Relabelling in a chroot. */
> +  if (STRNEQ (sysroot, "/")) {
> +    ADD_ARG (argv, i, "-r");
> +    ADD_ARG (argv, i, sysroot);
> +  }
> +
> +  /* Suppress non-error output. */
> +  ADD_ARG (argv, i, "-q");
> +
> +  /* Add parameters. */
> +  ADD_ARG (argv, i, s_specfile);
> +  ADD_ARG (argv, i, s_path);
> +  ADD_ARG (argv, i, NULL);
> +
> +  if (commandv (NULL, &err, argv) == -1) {
> +    reply_with_perror ("%s", err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 25108a2..49c360c 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -13149,6 +13149,28 @@ fails and the C<errno> is set to C<ENODEV>." };
>      shortdesc = "walk through the filesystem content";
>      longdesc = "Internal function for filesystem_walk." };
>  
> +  { defaults with
> +    name = "setfiles"; added = (1, 33, 43);
> +    style = RErr, [String "specfile"; Pathname "path"], [OBool "force"];
> +    proc_nr = Some 467;

This needs to be feature-dependent IMHO, so it will fail gracefully on
non-SELinux hosts.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160714/e236901a/attachment.sig>