On Mon, Jun 27, 2011 at 02:01:27PM +0200, Lennart Poettering wrote: > On Fri, 24.06.11 14:39, Michal Vyskocil (mvysko...@suse.cz) wrote: > > > Add -u/--user option, which changes the effective and real user and > > group id to the new value. The user must exists in the chroot, otherwise > > it will fail. Both username and user id are accepted. > > Sounds sensible, though I do wonder about the ultimate usefulness of > this given that this requires user settings configured on the host > systems in a way that makes sense in the container too. (i.e. the $HOME > and UID/GID of the user must be in sync in host and in container). Or am > I missing something?
Yes, that's the requirements - user must exists in chroot. But I don't
see any need why the uid/gid must be the same. All things are done after
chroot("."), so in the context of container.
The original idea behind was user systemd-nspawn instead of chroot for
local builds of our packages on systemd running with systemd. So instead
of chroot su -c $BUILD_COMMAND - $BUILD_USER simply call systemd-nspawn
-u $BUILD_USER $BUILD_COMMAND. I assume something similar have mock
(chroot + special user used for build) used by Fedora, so it might
benefit from that change as well.
I don't know it there's an another usecase, because even if it runs with
different user, it still have a lot of powerfull capabilities.
>
> > +static struct passwd *getpwun(const char* user) {
> > +
> > + struct passwd *pw;
> > +
> > + pw = getpwnam(user);
> > +
> > + if (!pw && isdigits(user)) {
> > + pw = getpwuid((uid_t)atoi(user));
> > + }
> > +
> > + if (! (pw && pw->pw_name && pw->pw_name[0] && pw->pw_dir &&
> > pw->pw_dir[0]
> > + && pw->pw_passwd)) {
> > + log_error("user name or id %s does not exist: %m", user);
> > + return NULL;
> > + }
>
> Please work the other way here. Use "safe_atou()" first on the
> username, and if that works it's a numeric uid. If it doesn't try
> getpwnam(). Code that already does this you find in get_user_creds() in
> execute.c.
Reading your code the get_user_creds seems to be a perfect for the
user-switching in
nspawn as well. What about move it to another location like src/util.c
and use it from both execute.c and nspawn.c?
>
> > + mkdir_p(pw->pw_dir, 0755);
> > + if (chown(pw->pw_dir, pw->pw_uid, pw->pw_gid) < 0)
> > {
> > + log_error("chown(%s) failed: %m",
> > pw->pw_dir);
> > + goto child_fail;
> > + }
>
> Please use safe_mkdir() here.
Will do
Thanks for the review.
Michal Vyskocil
pgpQaxzfxCk8z.pgp
Description: PGP signature
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
