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