[lxc-devel] [PATCH 1/3] lxc-attach: Try really hard to determine login shell
Serge Hallyn
serge.hallyn at ubuntu.com
Wed Mar 6 15:20:53 UTC 2013
Quoting Christian Seiler (christian at iwakd.de):
> If no command is specified, and using getpwuid() to determine the login
> shell fails, try to spawn a process that executes the utility 'getent'.
> getpwuid() may fail because of incompatibilities between the NSS
> implementations on the host and in the container.
>
> Signed-off-by: Christian Seiler <christian at iwakd.de>
> ---
> src/lxc/attach.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/lxc/attach.h | 3 +
> src/lxc/lxc_attach.c | 11 +++
> 3 files changed, 218 insertions(+)
>
> diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> index af3d7a0..88356e1 100644
> --- a/src/lxc/attach.c
> +++ b/src/lxc/attach.c
> @@ -32,7 +32,9 @@
> #include <sys/prctl.h>
> #include <sys/mount.h>
> #include <sys/syscall.h>
> +#include <sys/wait.h>
> #include <linux/unistd.h>
> +#include <pwd.h>
>
> #if !HAVE_DECL_PR_CAPBSET_DROP
> #define PR_CAPBSET_DROP 24
> @@ -275,3 +277,205 @@ int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx)
>
> return 0;
> }
> +
> +struct passwd *lxc_attach_getpwuid(uid_t uid)
> +{
> + /* static variables for result, we assume that
> + * 256 is large enough to hold the information
I don't see anything in getpwent or passd(5) man pages
suggesting that's enough, though. We don't want to
risk overflows or funkiness in a host's privileged
attach user due to malice in a container.
Is there any reason not to just keep char*s for each
field you find in line until you verify pw_uid matches,
and when it does then just strdup the values for the
struct passwd? (You can set the ':' locations to '\0'
to make strdup work fine). That should leave the code
a lot simpler.
If we have an actual guarantee that 256 will be enough
then ignore the above - I just don't know where such a
guarantee comes from.
> + * on username, passwd and gecos. for paths we
> + * use MAXPATHLEN instead
> + */
> + static char pwstruct_name[256];
> + static char pwstruct_passwd[256];
> + static char pwstruct_gecos[256];
> + static char pwstruct_dir[MAXPATHLEN];
> + static char pwstruct_shell[MAXPATHLEN];
> + static struct passwd result = {
> + .pw_name = pwstruct_name,
> + .pw_passwd = pwstruct_passwd,
> + .pw_uid = (uid_t) -1,
> + .pw_gid = (gid_t) -1,
> + .pw_gecos = pwstruct_gecos,
> + .pw_dir = pwstruct_dir,
> + .pw_shell = pwstruct_shell,
> + };
> +
> + /* local variables */
> + pid_t pid;
> + int pipes[2];
> + int ret;
> + int fd;
> +
> + /* we need to fork off a process that runs the
> + * getent program, and we need to capture its
> + * output, so we use a pipe for that purpose
> + */
> + ret = pipe(pipes);
> + if (ret < 0)
> + return NULL;
> +
> + pid = fork();
> + if (pid < 0) {
> + close(pipes[0]);
> + close(pipes[1]);
> + return NULL;
> + }
> +
> + if (pid) {
> + /* parent process */
> + FILE *pipe_f;
> + char *line = NULL;
> + size_t line_bufsz = 0;
> + int found = 0;
> + int status;
> +
> + close(pipes[1]);
> +
> + pipe_f = fdopen(pipes[0], "r");
> + while (getline(&line, &line_bufsz, pipe_f) != -1) {
> + char *token;
> + char *saveptr = NULL;
> + long value;
> + char *endptr = NULL;
> + int i;
> +
> + /* if we already found something, just continue
> + * to read until the pipe doesn't deliver any more
> + * data, but don't modify the existing data
> + * structure
> + */
> + if (found)
> + continue;
> +
> + /* trim line on the right hand side */
> + for (i = strlen(line); line && i > 0 && (line[i - 1] == '\n' || line[i - 1] == '\r'); --i)
is the check for 'line' necessary there?
> + line[i - 1] = '\0';
> +
> + /* split into tokens: first user name */
> + token = strtok_r(line, ":", &saveptr);
> + if (!token)
> + continue;
> + snprintf(pwstruct_name, sizeof(pwstruct_name), "%s", token);
> +
> + /* next: dummy password field */
> + token = strtok_r(NULL, ":", &saveptr);
> + if (!token)
> + continue;
> + snprintf(pwstruct_passwd, sizeof(pwstruct_passwd), "%s", token);
> +
> + /* next: user id */
> + token = strtok_r(NULL, ":", &saveptr);
> + value = token ? strtol(token, &endptr, 10) : 0;
> + if (!token || !endptr || *endptr || value == LONG_MIN || value == LONG_MAX)
> + continue;
> + result.pw_uid = (uid_t) value;
> + /* dummy sanity check: user id matches */
> + if (result.pw_uid != uid)
> + continue;
> +
> + /* next: gid */
> + token = strtok_r(NULL, ":", &saveptr);
> + value = token ? strtol(token, &endptr, 10) : 0;
> + if (!token || !endptr || *endptr || value == LONG_MIN || value == LONG_MAX)
> + continue;
> + result.pw_gid = (gid_t) value;
> +
> + /* next: gecos */
> + token = strtok_r(NULL, ":", &saveptr);
> + if (!token)
> + continue;
> + snprintf(pwstruct_gecos, sizeof(pwstruct_gecos), "%s", token);
> +
> + /* next: dir */
> + token = strtok_r(NULL, ":", &saveptr);
> + if (!token)
> + continue;
> + snprintf(pwstruct_dir, sizeof(pwstruct_dir), "%s", token);
> +
> + /* next: shell */
> + token = strtok_r(NULL, ":", &saveptr);
> + if (!token)
> + continue;
> + snprintf(pwstruct_shell, sizeof(pwstruct_shell), "%s", token);
> +
> + /* sanity check */
> + token = strtok_r(NULL, ":", &saveptr);
> + if (token)
> + continue;
> +
> + /* we successfully parsed an entry that matched, we're done,
> + * just make sure the structure was not modified in the caller
> + * and make it point back to our own static fields
> + */
> + result.pw_name = pwstruct_name;
> + result.pw_passwd = pwstruct_passwd;
> + result.pw_gecos = pwstruct_gecos;
> + result.pw_dir = pwstruct_dir;
> + result.pw_shell = pwstruct_shell;
If you're going to do it this way, you don't need to assign pw_name etc
here.
> +
> + found = 1;
> + }
> +
> + free(line);
> + fclose(pipe_f);
> + again:
> + if (waitpid(pid, &status, 0) < 0) {
> + if (errno == EINTR)
> + goto again;
> + return NULL;
> + }
> +
> + /* some sanity checks: if anything even hinted at going
> + * wrong: we can't be sure we have a valid result, so
> + * we assume we don't
> + */
> +
> + if (!WIFEXITED(status))
> + return NULL;
> +
> + if (WEXITSTATUS(status) != 0)
> + return NULL;
> +
> + if (!found)
> + return NULL;
> +
> + return &result;
> + } else {
> + /* child process */
> + char uid_buf[32];
> + char *arguments[] = {
> + "getent",
> + "passwd",
> + uid_buf,
> + NULL
> + };
> +
> + close(pipes[0]);
> +
> + /* we want to capture stdout */
> + dup2(pipes[1], 1);
> + close(pipes[1]);
> +
> + /* get rid of stdin/stderr, so we try to associate it
> + * with /dev/null
> + */
> + fd = open("/dev/null", O_RDWR);
> + if (fd < 0) {
So if the container's /dev is messed up (no null), lxc-attach will
run with closed fds 0 and 2. I think it might be better to keep
at least stderr open. Do you agree?
> + close(0);
> + close(2);
> + } else {
> + dup2(fd, 0);
> + dup2(fd, 2);
> + close(fd);
> + }
> +
> + /* finish argument list */
> + ret = snprintf(uid_buf, sizeof(uid_buf), "%ld", (long) uid);
> + if (ret <= 0)
> + exit(-1);
> +
> + /* try to run getent program */
> + (void) execvp("getent", arguments);
> + exit(-1);
> + }
> +}
> diff --git a/src/lxc/attach.h b/src/lxc/attach.h
> index 4d4f719..90e693a 100644
> --- a/src/lxc/attach.h
> +++ b/src/lxc/attach.h
> @@ -38,4 +38,7 @@ extern int lxc_attach_to_ns(pid_t other_pid, int which);
> extern int lxc_attach_remount_sys_proc();
> extern int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx);
>
> +struct passwd;
> +extern struct passwd *lxc_attach_getpwuid(uid_t uid);
> +
> #endif
> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> index 1f60266..d84c3d8 100644
> --- a/src/lxc/lxc_attach.c
> +++ b/src/lxc/lxc_attach.c
> @@ -438,6 +438,17 @@ int main(int argc, char *argv[])
> uid = getuid();
>
> passwd = getpwuid(uid);
> +
> + /* this probably happens because of incompatible nss
> + * implementations in host and container (remember, this
> + * code is still using the host's glibc but our mount
> + * namespace is in the container)
> + * we may try to get the information by spawning a
> + * [getent passwd uid] process and parsing the result
> + */
> + if (!passwd)
> + passwd = lxc_attach_getpwuid(uid);
> +
> if (!passwd) {
> SYSERROR("failed to get passwd " \
> "entry for uid '%d'", uid);
> --
> 1.7.10.4
>
More information about the lxc-devel
mailing list