[Libguestfs] [PATCH] NEW API: add a new api restorecon

[Richard W.M. Jones]

On Wed, Oct 24, 2012 at 02:04:47PM +0800, Wanlong Gao wrote:
> Add a new api restorecon to restore file(s) default
> SELinux security contexts.

In general terms, yes.  However there are some specific problems with
this patch, see below.

> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/selinux.c          |   69 +
>  generator/actions.ml      |   25 +
>  gobject/Makefile.inc      |    6 +-
>  po/POTFILES               |    2 +
>  src/MAX_PROC_NR           |    2 +-
>  21 files changed, 13282 insertions(+), 28030 deletions(-)
> 
> diff --git a/daemon/selinux.c b/daemon/selinux.c
> index 40590e1..14bc666 100644
> --- a/daemon/selinux.c
> +++ b/daemon/selinux.c
> @@ -31,6 +31,10 @@
>  #include "actions.h"
>  #include "optgroups.h"
>  
> +#define MAX_ARGS 128
> +
> +GUESTFSD_EXT_CMD(str_restorecon, restorecon);
> +
>  #if defined(HAVE_LIBSELINUX)
>  
>  int
> @@ -106,3 +110,68 @@ do_getcon (void)
>  }
>  
>  #endif /* !HAVE_LIBSELINUX */
> +
> +int
> +do_restorecon (const char *pathname,
> +               const char *excludedir,
> +               const char *labelprefix,
> +               int recursion,
> +               int force)
> +{
> +  int r;
> +  size_t i = 0;
> +  char *buf;
> +  char *exdir;
> +  char *err;
> +  const char *argv[MAX_ARGS];
> +
> +  buf = sysroot_path (pathname);
> +  if (!buf) {
> +    reply_with_error ("malloc");
> +    return -1;
> +  }
> +
> +  ADD_ARG (argv, i, str_restorecon);
> +
> +  if (optargs_bitmask & GUESTFS_RESTORECON_EXCLUDEDIR_BITMASK) {
> +    if (excludedir) {
> +      exdir = sysroot_path (excludedir);
> +      if (!exdir) {
> +        reply_with_error ("malloc");
> +        return -1;
> +      }
> +      ADD_ARG (argv, i, "-e");
> +      ADD_ARG (argv, i, exdir);
> +    }
> +  }

Note that the -e option can be specified multiple times (see
the man page).

It's possible to encode this using OStringList.  On the other hand
that is complicated and I don't mind if you just drop the excludedir
optional argument, and we'll wait and see if anyone asks for it.

> +  if (optargs_bitmask & GUESTFS_RESTORECON_LABELPREFIX_BITMASK) {
> +    if (labelprefix) {
> +      ADD_ARG (argv, i, "-L");
> +      ADD_ARG (argv, i, labelprefix);
> +    }
> +  }
> +
> +  if (optargs_bitmask & GUESTFS_RESTORECON_RECURSION_BITMASK)
> +    if (recursion)
> +      ADD_ARG (argv, i, "-R");
> +
> +  if (optargs_bitmask & GUESTFS_RESTORECON_FORCE_BITMASK)
> +    if (force)
> +      ADD_ARG (argv, i, "-F");
> +
> +  ADD_ARG (argv, i, buf);
> +  ADD_ARG (argv, i, NULL);
> +
> +  r = commandv (NULL, &err, argv);
> +  free (buf);
> +  if (exdir) free (exdir);
> +  if (r == -1) {
> +    reply_with_error ("%s: %s", pathname, err);
> +    free (err);
> +    return -1;
> +  }
> +
> +  free (err);
> +  return 0;
> +}
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 71aee37..12796a7 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -10241,6 +10241,31 @@ If the optional C<suffix> parameter is given, then the suffix
>  
>  See also: C<guestfs_mkdtemp>." };
>  
> +  { defaults with
> +    name = "restorecon";
> +    style = RErr, [Pathname "pathname"], [OString "excludedir"; OString "labelprefix"; OBool "recursion"; OBool "force"];
> +    proc_nr = Some 374;
> +    tests = [
> +      InitScratchFS, Always, TestRun (

This test shouldn't be 'Always'.  It should be conditional on having
selinux.

> +        [["mkdir"; "/a"];
> +         ["mkdir"; "/a/b"];
> +         ["touch"; "/a/b/c"];
> +         ["mkdir"; "/a/d"];
> +         ["touch"; "/a/d/e"];
> +         ["restorecon"; "/a"; "/a/d"; "NOARG"; "true"; "true"]])
> +    ];
> +    shortdesc = "restore file(s) default SELinux security contexts";
> +    longdesc = "\
> +This program is primarily used to reset the security context (type)
> +(extended attributes) on one or more files.
> +
> +It can be run at any time to correct errors, to add support for new policy.
> +
> +If a file object does not have a context, restorecon will write the default
> +context to the file object's extended attributes. If a file object has a
> +context, C<restorecon> will only modify the type portion of the security
> +context.  The C<force> option will force a replacement of the entire context."};
> diff --git a/po/POTFILES b/po/POTFILES
> index a73377d..5cded4e 100644
> --- a/po/POTFILES
> +++ b/po/POTFILES
> @@ -138,6 +138,7 @@ fish/tilde.c
>  fish/time.c
>  format/format.c
>  fuse/guestmount.c
> +gobject/docs/guestfs-scan.c
>  gobject/src/optargs-add_domain.c
>  gobject/src/optargs-add_drive.c
>  gobject/src/optargs-btrfs_filesystem_resize.c

This hunk is wrong.  I think you've got a rogue file which needs to be
deleted.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw