Re: [Libguestfs] [PATCH 1/2] Add -u and -g options to febootstrap-supermin-helper
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