Libguestfs - Oct 2010 - [PATCH 1/2] Add -u and -g options to febootstrap-supermin-helper

Bash automatically resets euid to uid when it executes. This can mean that the
effective user id of a program at the point it calls febootstrap-supermin-helper
can be lost if any part of execution chain involved bash. This in turn can
result in:

* the generation of an incorrect checksum, which contains the uid.
* the generation of supermin files with differing owners

The -u and -g options allow the caller to pass in an explicit user and group to
run as. These will be used when generating a checksum. Additionally,
febootstrap-supermin-helper will set(u|g)id as appropriate if they are given for
non-checksum output. This ensures all generated files will have the correct
ownership, regardless of how they are created.
---
 helper/checksum.c |    2 +-
 helper/helper.h   |    2 +
 helper/main.c     |  145 ++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 130 insertions(+), 19 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-u-and-g-options-to-febootstrap-supermin-helper.patch
Type: text/x-patch
Size: 6426 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20101001/f90fd042/attachment.bin>

If usage information is displayed because of an error, it should go to stderr.
If it is displayed because the -h option was given it should go to stdout.
---
 helper/main.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Send-usage-output-to-stdout-or-stderr-depending-on-c.patch
Type: text/x-patch
Size: 1185 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20101001/e275ebad/attachment.bin>

On Fri, Oct 01, 2010 at 04:50:32PM +0100, Matthew Booth
wrote:
> 
> Bash automatically resets euid to uid when it executes. This can mean that
the
> effective user id of a program at the point it calls
febootstrap-supermin-helper
> can be lost if any part of execution chain involved bash. This in turn can
> result in:
> 
> * the generation of an incorrect checksum, which contains the uid.
> * the generation of supermin files with differing owners
> 
> The -u and -g options allow the caller to pass in an explicit user and
group to
> run as. These will be used when generating a checksum. Additionally,
> febootstrap-supermin-helper will set(u|g)id as appropriate if they are
given for
> non-checksum output. This ensures all generated files will have the correct
> ownership, regardless of how they are created.

There are a few problems.

The documentation needs to be updated (helper/febootstrap-super-helper.pod).
> -           PACKAGE_STRING, hostcpu, modpath, geteuid ());
> +           PACKAGE_STRING, hostcpu, modpath, run_uid);
I think you should just use geteuid() here, and change the code below
so that -u and -g perform the setuid/setgid unconditionally if they
are set.

The only thing this would stop you from doing is using the -u option
as non-root to generate a checksum for some other user, but I can't
see how that is helpful to anyone.

This makes the patch much smaller and simpler.
> +uid_t run_uid;
> +uid_t run_gid;
So you won't need these ...
>  enum { HELP_OPTION = CHAR_MAX + 1 };
>  
> -static const char *options = "f:k:vV";
> +static const char *options = "f:u:g:k:vV";
These should be in alnum order, same as for the current options.
> @@ -84,12 +96,70 @@ usage (const char *progname)
>            progname, progname, progname, progname, progname, progname);
>  }
>  
> +static int
> +parseint (const char *id, const char *progname)
> +{
> +  char *invalid;
> +  long int val = strtol (id, &invalid, 10);
> +  if (*invalid != '\0') {
> +    fprintf (stderr, "%s is not a valid uid\n", id);
> +    usage (progname);
> +    exit (EXIT_FAILURE);
> +  }
> +  return val;
> +}
Not sure if this is correct.  The manpage for strtol checks for a
bunch of other conditions.  Could be better to use xstrtol from gnulib.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

Bash automatically resets euid to uid when it executes. This means that the
effective user id of a program at the point it calls febootstrap-supermin-helper
will be lost if any part of execution chain involved bash. This in turn can
result in:

* the generation of an incorrect checksum, which contains the uid.
* the generation of supermin files with a mixture of owners

The -u and -g options allow the caller to pass in an explicit user and group to
run as. febootstrap-supermin-helper will set(u|g)id as appropriate.
---
 helper/febootstrap-supermin-helper.pod |   13 ++++
 helper/main.c                          |  116 +++++++++++++++++++++++++++++++-
 lib/.gitignore                         |   10 +++
 m4/.gitignore                          |    4 +
 m4/gnulib-cache.m4                     |    3 +-
 5 files changed, 142 insertions(+), 4 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-u-and-g-options-to-febootstrap-supermin-helper.patch
Type: text/x-patch
Size: 7050 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20101028/efdac26a/attachment.bin>