[libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> If we enable userns, the process with uid/gid in idmap should have enough permission to access dir we provided for containers. Currently, the debug log is very implicit or misleading sometimes. This patch will help clarify this for us when using debug log or virsh. v2: syntax-check clean v3: reliable method for checking permission of dir Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8abaea0..9a05e30 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -110,6 +110,13 @@ struct __lxc_child_argv { int handshakefd; }; +typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t; +struct __lxc_userns_DirPermCheck_argv { + uid_t uid; + gid_t gid; + virDomainDefPtr vmDef; +}; + static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, const char *srcprefix); @@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) return false; } +static +int lxcContainerCheckDirPermissionChild(void *argv) +{ + size_t i; + lxc_userns_DirPermCheck_argv_t *args = argv; + uid_t uid = args->uid; + uid_t gid = args->gid; + virDomainDefPtr vmDef = args->vmDef; + char *path; + + if (virSetUIDGID(uid, gid, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("setuid or setgid failed")); + _exit(-1); + } + + for (i = 0; i < vmDef->nfss; i++) { + path = vmDef->fss[i]->src; + if (access(path, R_OK) || access(path, W_OK) || virFileIsExecutable(path)) { + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d", + vmDef->fss[i]->src, uid, gid); + _exit(-1); + } + } + + _exit(0); +} + +/* + * Helper function for helping check + * whether we have enough privilege + * to operate the source dir when userns enabled + * @vmDef: pointer to vm definition structure + * Returns 0 on success or -1 in case of error + */ +static int +lxcContainerCheckDirPermission(virDomainDefPtr vmDef) +{ + uid_t uid; + gid_t gid; + int cpid = 0; + int status; + char *childStack; + char *stack; + int flags = SIGCHLD; + + uid = vmDef->idmap.uidmap[0].target; + gid = vmDef->idmap.gidmap[0].target; + + lxc_userns_DirPermCheck_argv_t args = { + .uid = uid, + .gid = gid, + .vmDef = vmDef + }; + + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0) + return -1; + + childStack = stack + (getpagesize() * 4); + cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, &args); + VIR_FREE(stack); + if (cpid < 0) { + virReportSystemError(errno, "%s", + _("Unable to clone to check permission of directory")); + return -1; + } else if (virProcessWait(cpid, &status) < 0) { + return -1; + } + + if (WEXITSTATUS(status) != 0) { + virReportSystemError(errno, "%s", + _("Check the permission of source dir provided for container")); + return -1; + } + + return 0; +} + /** * lxcContainerStart: * @def: pointer to virtual machine structure @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def, if (userns_supported()) { VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; + if (lxcContainerCheckDirPermission(def) < 0) { + return -1; + } } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); -- 1.8.2.1
ping?
-----Original Message----- From: Chen Hanxiao [mailto:chenhanxiao@cn.fujitsu.com] Sent: Tuesday, September 10, 2013 4:18 PM To: libvir-list@redhat.com Cc: chenhanxiao@cn.fujitsu.com Subject: [libvirt][PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
If we enable userns, the process with uid/gid in idmap should have enough permission to access dir we provided for containers. Currently, the debug log is very implicit or misleading sometimes. This patch will help clarify this for us when using debug log or virsh.
v2: syntax-check clean
v3: reliable method for checking permission of dir
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8abaea0..9a05e30 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -110,6 +110,13 @@ struct __lxc_child_argv { int handshakefd; };
+typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t; +struct __lxc_userns_DirPermCheck_argv { + uid_t uid; + gid_t gid; + virDomainDefPtr vmDef; +}; + static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, const char *srcprefix);
@@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) return false; }
+static +int lxcContainerCheckDirPermissionChild(void *argv) +{ + size_t i; + lxc_userns_DirPermCheck_argv_t *args = argv; + uid_t uid = args->uid; + uid_t gid = args->gid; + virDomainDefPtr vmDef = args->vmDef; + char *path; + + if (virSetUIDGID(uid, gid, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("setuid or setgid failed")); + _exit(-1); + } + + for (i = 0; i < vmDef->nfss; i++) { + path = vmDef->fss[i]->src; + if (access(path, R_OK) || access(path, W_OK) || virFileIsExecutable(path)) { + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d", + vmDef->fss[i]->src, uid, gid); + _exit(-1); + } + } + + _exit(0); +} + +/* + * Helper function for helping check + * whether we have enough privilege + * to operate the source dir when userns enabled + * @vmDef: pointer to vm definition structure + * Returns 0 on success or -1 in case of error + */ +static int +lxcContainerCheckDirPermission(virDomainDefPtr vmDef) +{ + uid_t uid; + gid_t gid; + int cpid = 0; + int status; + char *childStack; + char *stack; + int flags = SIGCHLD; + + uid = vmDef->idmap.uidmap[0].target; + gid = vmDef->idmap.gidmap[0].target; + + lxc_userns_DirPermCheck_argv_t args = { + .uid = uid, + .gid = gid, + .vmDef = vmDef + }; + + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0) + return -1; + + childStack = stack + (getpagesize() * 4); + cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, &args); + VIR_FREE(stack); + if (cpid < 0) { + virReportSystemError(errno, "%s", + _("Unable to clone to check permission of directory")); + return -1; + } else if (virProcessWait(cpid, &status) < 0) { + return -1; + } + + if (WEXITSTATUS(status) != 0) { + virReportSystemError(errno, "%s", + _("Check the permission of source dir provided for container")); + return -1; + } + + return 0; +} + /** * lxcContainerStart: * @def: pointer to virtual machine structure @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def, if (userns_supported()) { VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; + if (lxcContainerCheckDirPermission(def) < 0) { + return -1; + } } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); -- 1.8.2.1
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Chen Hanxiao Sent: Wednesday, October 09, 2013 6:03 PM To: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH v3]LXC: Helper function for checking
ping... permission of
dir when userns enabled
ping?
-----Original Message----- From: Chen Hanxiao [mailto:chenhanxiao@cn.fujitsu.com] Sent: Tuesday, September 10, 2013 4:18 PM To: libvir-list@redhat.com Cc: chenhanxiao@cn.fujitsu.com Subject: [libvirt][PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
If we enable userns, the process with uid/gid in idmap should have enough permission to access dir we provided for containers. Currently, the debug log is very implicit or misleading sometimes. This patch will help clarify this for us when using debug log or virsh.
v2: syntax-check clean
v3: reliable method for checking permission of dir
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8abaea0..9a05e30 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -110,6 +110,13 @@ struct __lxc_child_argv { int handshakefd; };
+typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t; +struct __lxc_userns_DirPermCheck_argv { + uid_t uid; + gid_t gid; + virDomainDefPtr vmDef; +}; + static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, const char *srcprefix);
@@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) return false; }
+static +int lxcContainerCheckDirPermissionChild(void *argv) +{ + size_t i; + lxc_userns_DirPermCheck_argv_t *args = argv; + uid_t uid = args->uid; + uid_t gid = args->gid; + virDomainDefPtr vmDef = args->vmDef; + char *path; + + if (virSetUIDGID(uid, gid, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("setuid or setgid failed")); + _exit(-1); + } + + for (i = 0; i < vmDef->nfss; i++) { + path = vmDef->fss[i]->src; + if (access(path, R_OK) || access(path, W_OK) || virFileIsExecutable(path)) { + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d", + vmDef->fss[i]->src, uid, gid); + _exit(-1); + } + } + + _exit(0); +} + +/* + * Helper function for helping check + * whether we have enough privilege + * to operate the source dir when userns enabled + * @vmDef: pointer to vm definition structure + * Returns 0 on success or -1 in case of error + */ +static int +lxcContainerCheckDirPermission(virDomainDefPtr vmDef) +{ + uid_t uid; + gid_t gid; + int cpid = 0; + int status; + char *childStack; + char *stack; + int flags = SIGCHLD; + + uid = vmDef->idmap.uidmap[0].target; + gid = vmDef->idmap.gidmap[0].target; + + lxc_userns_DirPermCheck_argv_t args = { + .uid = uid, + .gid = gid, + .vmDef = vmDef + }; + + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0) + return -1; + + childStack = stack + (getpagesize() * 4); + cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, &args); + VIR_FREE(stack); + if (cpid < 0) { + virReportSystemError(errno, "%s", + _("Unable to clone to check permission of directory")); + return -1; + } else if (virProcessWait(cpid, &status) < 0) { + return -1; + } + + if (WEXITSTATUS(status) != 0) { + virReportSystemError(errno, "%s", + _("Check the permission of source dir provided for container")); + return -1; + } + + return 0; +} + /** * lxcContainerStart: * @def: pointer to virtual machine structure @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def, if (userns_supported()) { VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; + if (lxcContainerCheckDirPermission(def) < 0) { + return -1; + } } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); -- 1.8.2.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Oct 15, 2013 at 06:41:28PM +0800, Chen Hanxiao wrote:
ping...
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Chen Hanxiao Sent: Wednesday, October 09, 2013 6:03 PM To: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
ping?
-----Original Message----- From: Chen Hanxiao [mailto:chenhanxiao@cn.fujitsu.com] Sent: Tuesday, September 10, 2013 4:18 PM To: libvir-list@redhat.com Cc: chenhanxiao@cn.fujitsu.com Subject: [libvirt][PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
From: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
If we enable userns, the process with uid/gid in idmap should have enough permission to access dir we provided for containers. Currently, the debug log is very implicit or misleading sometimes. This patch will help clarify this for us when using debug log or virsh.
v2: syntax-check clean
v3: reliable method for checking permission of dir
Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> --- src/lxc/lxc_container.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8abaea0..9a05e30 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -110,6 +110,13 @@ struct __lxc_child_argv { int handshakefd; };
+typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t; +struct __lxc_userns_DirPermCheck_argv { + uid_t uid; + gid_t gid; + virDomainDefPtr vmDef; +}; + static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, const char *srcprefix);
@@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) return false; }
+static +int lxcContainerCheckDirPermissionChild(void *argv) +{ + size_t i; + lxc_userns_DirPermCheck_argv_t *args = argv; + uid_t uid = args->uid; + uid_t gid = args->gid; + virDomainDefPtr vmDef = args->vmDef; + char *path; + + if (virSetUIDGID(uid, gid, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("setuid or setgid failed")); + _exit(-1); + } + + for (i = 0; i < vmDef->nfss; i++) { + path = vmDef->fss[i]->src; + if (access(path, R_OK) || access(path, W_OK) || virFileIsExecutable(path)) { + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d", + vmDef->fss[i]->src, uid, gid); + _exit(-1); + } + } + + _exit(0); +} + +/* + * Helper function for helping check + * whether we have enough privilege + * to operate the source dir when userns enabled + * @vmDef: pointer to vm definition structure + * Returns 0 on success or -1 in case of error + */ +static int +lxcContainerCheckDirPermission(virDomainDefPtr vmDef) +{ + uid_t uid; + gid_t gid; + int cpid = 0; + int status; + char *childStack; + char *stack; + int flags = SIGCHLD; + + uid = vmDef->idmap.uidmap[0].target; + gid = vmDef->idmap.gidmap[0].target; + + lxc_userns_DirPermCheck_argv_t args = { + .uid = uid, + .gid = gid, + .vmDef = vmDef + }; + + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0) + return -1; + + childStack = stack + (getpagesize() * 4); + cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, &args); + VIR_FREE(stack); + if (cpid < 0) { + virReportSystemError(errno, "%s", + _("Unable to clone to check permission of directory")); + return -1; + } else if (virProcessWait(cpid, &status) < 0) { + return -1; + } + + if (WEXITSTATUS(status) != 0) { + virReportSystemError(errno, "%s", + _("Check the permission of source dir provided for container")); + return -1; + } + + return 0; +} + /** * lxcContainerStart: * @def: pointer to virtual machine structure @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def, if (userns_supported()) { VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; + if (lxcContainerCheckDirPermission(def) < 0) { + return -1; + } } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); -- 1.8.2.1
This patch is basically about improving the error message from LXC when permissions are inadequate. Over the past week I've pushed a large number of patches to fix LXC error reporting during startup. As such, I'm not sure that we still need to have this patch - can you try latest GIT master and show what error message you get with existing code. I'd really rather avoid checking permissions ourselves, if we have got error reporting at time of use working properly Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, October 15, 2013 7:07 PM To: Chen Hanxiao Cc: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
On Tue, Oct 15, 2013 at 06:41:28PM +0800, Chen Hanxiao wrote:
ping...
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Chen Hanxiao Sent: Wednesday, October 09, 2013 6:03 PM
_("Kernel doesn't support user namespace")); -- 1.8.2.1
This patch is basically about improving the error message from LXC when permissions are inadequate. Over the past week I've pushed a large number of patches to fix LXC error reporting during startup. As such, I'm not sure that we still need to have this patch - can you try latest GIT master and show what error message you get with existing code.
I'd really rather avoid checking permissions ourselves, if we have got error reporting at time of use working properly
I'll get the latest upstream code and see whether we still need this patch according to the error reporting. Thanks!
Daniel --
participants (2)
-
Chen Hanxiao -
Daniel P. Berrange