[Libguestfs] [PATCH 1/2] Add -u and -g options to febootstrap-supermin-helper
Richard W.M. Jones
rjones at redhat.com
Fri Oct 1 16:33:06 UTC 2010
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
More information about the Libguestfs
mailing list