[libvirt] [PATCH] util: Make failure to get suplementary group list for a uid non-fatal
Peter Krempa
pkrempa at redhat.com
Tue Jun 7 16:04:24 UTC 2016
Since introduction of the DAC security driver we've documented that
seclabels with a leading + can be used with numerical uid. This would
not work though with the rest of libvirt if the uid was not actually
used in the system as we'd fail when trying to get a list of
suplementary groups for the given uid. Since a uid without entry in
/etc/passwd (or other user database) will not have any suppolementary
groups we can treat the failure to obtain them as such.
This patch modifies virGetGroupList to not report the error of missing
user and tweaks callers to treat the missing list as having 0
supplementary groups.
The only place reporting errors is virt-login-shell as it's used to
determine whether the given user is allowed to access the shell.
---
With this I'm able to run the VM with any arbitrary UID/GID.
CC: Roy Keene <rkeene at knightpoint.com>
CC: "Daniel P. Berrange" <berrange at redhat.com>
src/security/security_dac.c | 12 +++++++-----
src/util/vircommand.c | 4 +++-
src/util/virfile.c | 28 ++++++++++++++++------------
src/util/virutil.c | 25 ++++++++++++++++---------
tools/virt-login-shell.c | 6 +++++-
5 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 442ce70..e8af093 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -269,11 +269,13 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
int ngroups;
VIR_FREE(priv->groups);
- priv->ngroups = 0;
- if ((ngroups = virGetGroupList(priv->user, priv->group,
- &priv->groups)) < 0)
- return -1;
- priv->ngroups = ngroups;
+
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
+ if ((priv->ngroups = virGetGroupList(priv->user, priv->group,
+ &priv->groups)) < 0)
+ priv->ngroups = 0;
+
return 0;
}
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index f5bd7af..58af06a 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -554,8 +554,10 @@ virExec(virCommandPtr cmd)
childerr = null;
}
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0)
- goto cleanup;
+ ngroups = 0;
pid = virFork();
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 9d460b9..4298ec5 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1978,9 +1978,10 @@ virFileAccessibleAs(const char *path, int mode,
gid == getegid())
return access(path, mode);
- ngroups = virGetGroupList(uid, gid, &groups);
- if (ngroups < 0)
- return -1;
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
+ if ((ngroups = virGetGroupList(uid, gid, &groups)) == -1)
+ ngroups = 0;
pid = virFork();
@@ -2104,9 +2105,10 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
* following dance avoids problems caused by root-squashing
* NFS servers. */
- ngroups = virGetGroupList(uid, gid, &groups);
- if (ngroups < 0)
- return -errno;
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
+ if ((ngroups = virGetGroupList(uid, gid, &groups)) == -1)
+ ngroups = 0;
if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
ret = -errno;
@@ -2407,9 +2409,10 @@ virFileRemove(const char *path,
if (gid == (gid_t) -1)
gid = getegid();
- ngroups = virGetGroupList(uid, gid, &groups);
- if (ngroups < 0)
- return -errno;
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
+ if ((ngroups = virGetGroupList(uid, gid, &groups)) == -1)
+ ngroups = 0;
pid = virFork();
@@ -2583,9 +2586,10 @@ virDirCreate(const char *path,
if (gid == (gid_t) -1)
gid = getegid();
- ngroups = virGetGroupList(uid, gid, &groups);
- if (ngroups < 0)
- return -errno;
+ /* ignore a possible problem in getting supplementary groups just assume
+ * we have none and continue with uid/gid only */
+ if ((ngroups = virGetGroupList(uid, gid, &groups)) == -1)
+ ngroups = 0;
pid = virFork();
diff --git a/src/util/virutil.c b/src/util/virutil.c
index d80d994..cd9c3f2 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -755,9 +755,10 @@ virGetUserDirectory(void)
#ifdef HAVE_GETPWUID_R
/* Look up fields from the user database for the given user. On
- * error, set errno, report the error, and return -1. */
+ * error, set errno, report the error if not instructed otherwise via @quiet,
+ * and return -1. */
static int
-virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir)
+virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, bool quiet)
{
char *strbuf;
struct passwd pwbuf;
@@ -790,12 +791,19 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir)
if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0)
goto cleanup;
}
+
if (rc != 0) {
+ if (quiet)
+ goto cleanup;
+
virReportSystemError(rc,
_("Failed to find user record for uid '%u'"),
(unsigned int) uid);
goto cleanup;
} else if (pw == NULL) {
+ if (quiet)
+ goto cleanup;
+
virReportError(VIR_ERR_SYSTEM_ERROR,
_("Failed to find user record for uid '%u'"),
(unsigned int) uid);
@@ -873,7 +881,7 @@ char *
virGetUserDirectoryByUID(uid_t uid)
{
char *ret;
- virGetUserEnt(uid, NULL, NULL, &ret);
+ virGetUserEnt(uid, NULL, NULL, &ret, false);
return ret;
}
@@ -923,7 +931,7 @@ char *virGetUserRuntimeDirectory(void)
char *virGetUserName(uid_t uid)
{
char *ret;
- virGetUserEnt(uid, &ret, NULL, NULL);
+ virGetUserEnt(uid, &ret, NULL, NULL, false);
return ret;
}
@@ -1096,8 +1104,9 @@ virGetGroupID(const char *group, gid_t *gid)
/* Compute the list of primary and supplementary groups associated
* with @uid, and including @gid in the list (unless it is -1),
* storing a malloc'd result into @list. Return the size of the list
- * on success, or -1 on failure with error reported and errno set. May
- * not be called between fork and exec. */
+ * on success, or -1 on failure with no error reported as this usually isn't
+ * a fatal problem for callers. errno is set on error. May not be called
+ * between fork and exec. */
int
virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
{
@@ -1109,14 +1118,12 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
if (uid == (uid_t)-1)
return 0;
- if (virGetUserEnt(uid, &user, &primary, NULL) < 0)
+ if (virGetUserEnt(uid, &user, &primary, NULL, true) < 0)
return -1;
ret = mgetgroups(user, primary, list);
if (ret < 0) {
sa_assert(!*list);
- virReportSystemError(errno,
- _("cannot get group list for '%s'"), user);
goto cleanup;
}
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 8f87227..b63bf6a 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -249,8 +249,12 @@ main(int argc, char **argv)
if (!(conf = virConfReadFile(login_shell_path, 0)))
goto cleanup;
- if ((ngroups = virGetGroupList(uid, gid, &groups)) < 0)
+ if ((ngroups = virGetGroupList(uid, gid, &groups)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to query supplementary group list for uid '%u'")
+ (unsigned int) uid);
goto cleanup;
+ }
if (virLoginShellAllowedUser(conf, name, groups) < 0)
goto cleanup;
--
2.8.3
More information about the libvir-list
mailing list