[RFC][PATCH V2 0/3] Issue #322
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V1 -> V2 use debug to log the open exception trace stack. It is complex to check the permission for qemu user. There are too many access controlling mechanisms in Linux, such as ACL, traditional ugo+-rwx, SELinux and AppArmor. It's not possible to enumerate and check every access mechanisms if it allows QEMU to access a file. So now I'm simply trying to access the file with qemu user and see if it's OK It is simple and avoid handling corner case. Also I can try to start a simple guest with the given iso. ShaoHe Feng (3): add a method to probe the permission as qemu user qemu user tests: probe the username of qemu process started by libvirt Don't allow templates to be created with ISOs that won't be usable. Makefile.am | 1 + src/kimchi/i18n.py | 5 ++++ src/kimchi/kvmusertests.py | 62 +++++++++++++++++++++++++++++++++++++++++++ src/kimchi/model/templates.py | 13 ++++++++- src/kimchi/utils.py | 26 ++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 src/kimchi/kvmusertests.py -- 1.8.4.2
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Now we need to check the 'qemu' user can open an iso files. This patch is used to check 'qemu' user has permission to open a file. Test this patch: $ mkdir -p a/b/c $ touch a/b/c/f $ chmod o-x a/b/c $ sudo PYTHONPATH=src python -c ' from kimchi.utils import probe_file_permission_as_user print probe_file_permission_as_user("a/b/c/f", "qemu")' It will return False change another user, it may return True Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/utils.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index 7b15d7f..763a01e 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -19,12 +19,16 @@ # import cherrypy +import grp +from multiprocessing import Process, Queue import os import psutil +import pwd import re import subprocess import urllib2 from threading import Timer +import traceback from cherrypy.lib.reprconf import Parser @@ -236,3 +240,25 @@ def run_setfacl_set_attr(path, attr="r", user=""): set_user = ["setfacl", "--modify", "user:%s:%s" % (user, attr), path] out, error, ret = run_command(set_user) return ret == 0 + + +def probe_file_permission_as_user(file, user): + def probe_permission(q, file, user): + uid = pwd.getpwnam(user).pw_uid + gid = pwd.getpwnam(user).pw_gid + gids = [g.gr_gid for g in grp.getgrall() if user in g.gr_mem] + os.setgid(gid) + os.setgroups(gids) + os.setuid(uid) + try: + with open(file): + q.put((True, None)) + except Exception as e: + kimchi_log.debug(traceback.format_exc()) + q.put((False, e)) + + queue = Queue() + p = Process(target=probe_permission, args=(queue, file, user)) + p.start() + p.join() + return queue.get() -- 1.8.4.2
import cherrypy +import grp +from multiprocessing import Process, Queue import os import psutil +import pwd import re import subprocess import urllib2 from threading import Timer +import traceback
from cherrypy.lib.reprconf import Parser The statements "from <foo> import <bar>" should be placed at the end of
Am 10-03-2014 05:23, schrieb shaohef@linux.vnet.ibm.com: their corresponding blocks. So, the statement "from multiprocessing..." should be moved to before "from threading..." (alphabetical order) and "import traceback" should be moved to after "import urllib2" (before the statements "from <foo> import <bar>").
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> We want to fix the searchable permission for qemu user. But we should find the username of qemu process firstly. searchable permission is a known problem. We have discussed several times on IRC. Royce reports the qemu username is different on different distros Zhou Zheng Sheng gives a better way to find the qemu process ID Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- Makefile.am | 1 + src/kimchi/kvmusertests.py | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/kimchi/kvmusertests.py diff --git a/Makefile.am b/Makefile.am index 0fd019e..ddc39ac 100644 --- a/Makefile.am +++ b/Makefile.am @@ -50,6 +50,7 @@ PEP8_WHITELIST = \ src/kimchi/featuretests.py \ src/kimchi/iscsi.py \ src/kimchi/isoinfo.py \ + src/kimchi/kvmusertests.py \ src/kimchi/mockmodel.py \ src/kimchi/model/*.py \ src/kimchi/repositories.py \ diff --git a/src/kimchi/kvmusertests.py b/src/kimchi/kvmusertests.py new file mode 100644 index 0000000..1da3ddc --- /dev/null +++ b/src/kimchi/kvmusertests.py @@ -0,0 +1,62 @@ +# Project Kimchi +# +# Copyright IBM, Corp. 2013 +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +import libvirt +import psutil +import uuid + + +from kimchi.rollbackcontext import RollbackContext + + +class UserTests(object): + SIMPLE_VM_XML = """ + <domain type='kvm'> + <name>%s</name> + <uuid>%s</uuid> + <memory unit='KiB'>10240</memory> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + </domain>""" + + def __init__(self): + self.vm_uuid = uuid.uuid3(uuid.NAMESPACE_DNS, 'vm-test.kimchi.org') + self.vm_name = "kimchi_test_%s" % self.vm_uuid + + def probe_user(self): + xml = self.SIMPLE_VM_XML % (self.vm_name, self.vm_uuid) + user = None + with RollbackContext() as rollback: + conn = libvirt.open('qemu:///system') + rollback.prependDefer(conn.close) + dom = conn.defineXML(xml) + rollback.prependDefer(dom.undefine) + dom.create() + rollback.prependDefer(dom.destroy) + with open('/var/run/libvirt/qemu/%s.pid' % self.vm_name) as f: + pidStr = f.read() + p = psutil.Process(int(pidStr)) + user = p.username + return user + + +if __name__ == '__main__': + ut = UserTests() + print ut.probe_user() -- 1.8.4.2
Am 10-03-2014 05:23, schrieb shaohef@linux.vnet.ibm.com:
+import libvirt +import psutil +import uuid + + +from kimchi.rollbackcontext import RollbackContext
"libvirt" is a third-party, non-kimchi library, so it should be placed in a separate block: import psutil import uuid import libvirt from kimchi.rollbackcontext import RollbackContext
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> qemu can't use an ISO if it has no search permission on the directories containing the file. Return an error explaning this, and suggest the way to fix it. http://libvirt.org/drvqemu.html#securitydac In the "session" instance, the POSIX users/groups model restricts QEMU virtual machines (and libvirtd in general) to only have access to resources with the same user/group ID as the client application. There is no finer level of configuration possible for the "session" instances. If QEMU virtual machines from the "system" instance are being run as non-root, there will be greater restrictions on what host resources the QEMU process will be able to access. The libvirtd daemon will attempt to manage permissions on resources to minimise the likelihood of unintentional security denials Any files/devices used as guest disk images must be accessible to the user/group ID that QEMU guests are configured to run as. The libvirtd daemon will automatically set the ownership of the file/device path to the correct user/group ID. Applications / administrators must be aware though that the parent directory permissions may still deny access. The directories containing disk images must either have their ownership set to match the user/group configured for QEMU, or their UNIX file permissions must have the 'execute/search' bit enabled for 'others'. The simplest option is the latter one, of just enabling the 'execute/search' bit. For any directory to be used for storing disk images, this can be achieved by running the following command on the directory itself, and any parent directories chmod o+x /path/to/directory In particular note that if using the "system" instance and attempting to store disk images in a user home directory, the default permissions on $HOME are typically too restrictive to allow access. Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com> Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 5 +++++ src/kimchi/model/templates.py | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index cbfcf5d..daf5937 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -54,6 +54,11 @@ messages = { "KCHISO0005E": _("Invalid El Torito boot indicator in ISO %(filename)s"), "KCHISO0006E": _("Unexpected volume type for primary volume in ISO %(filename)s"), "KCHISO0007E": _("Bad format while reading volume descriptor in ISO %(filename)s"), + "KCHISO0008E": _("The hypervisor doesn't have permission to use this ISO %(filename)s. " + "Consider moving it under /var/lib/libvirt, or set the search permission " + "to file access control lists for '%(user)s' user if possible, or add the " + "'%(user)s' to the ISO path group, or (not recommended) 'chmod -R o+x 'path_to_iso'." + "Details: %(err)s" ), "KCHVM0001E": _("Virtual machine %(name)s already exists"), "KCHVM0002E": _("Virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/templates.py b/src/kimchi/model/templates.py index 52e7eab..d294def 100644 --- a/src/kimchi/model/templates.py +++ b/src/kimchi/model/templates.py @@ -25,7 +25,9 @@ import libvirt from kimchi import xmlutils from kimchi.exception import InvalidOperation, InvalidParameter +from kimchi.kvmusertests import UserTests from kimchi.utils import pool_name_from_uri +from kimchi.utils import probe_file_permission_as_user from kimchi.vmtemplate import VMTemplate @@ -36,8 +38,17 @@ class TemplatesModel(object): def create(self, params): name = params.get('name', '').strip() + iso = params['cdrom'] + # check search permission + if iso.startswith('/') and os.path.isfile(iso): + user = UserTests().probe_user() + ret, excp = probe_file_permission_as_user(iso, user) + if ret is False: + raise InvalidParameter('KCHISO0008E', + {'filename': iso, 'user': user, + 'err': excp}) + if not name: - iso = params['cdrom'] iso_name = os.path.splitext(iso[iso.rfind('/') + 1:])[0] name = iso_name + str(int(time.time()*1000)) params['name'] = name -- 1.8.4.2
participants (2)
-
Crístian Viana -
shaohef＠linux.vnet.ibm.com