[systemd-devel] [PATCH] nspawn: spawn shell under specified --user

[Michal Vyskocil]

On Mon, Jun 27, 2011 at 02:01:27PM +0200, Lennart Poettering wrote:
> On Fri, 24.06.11 14:39, Michal Vyskocil (mvyskocil at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20110627/1a5565ad/attachment.pgp>