<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 2014年03月10日 09:52, Sheldon wrote:<br>
</div>
<blockquote cite="mid:531D1AF8.9070002@linux.vnet.ibm.com"
type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div class="moz-cite-prefix">On 03/08/2014 04:20 AM, Christy Perez
wrote:<br>
</div>
<blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
type="cite">
<pre wrap="">Sorry this took me a while to get back to you. Answers below.
Do you want to pick this back up in the overall kimchi thread?
On Thu, 2014-03-06 at 13:29 +0800, Sheldon wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 03/04/2014 06:38 AM, Christy Perez wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Wed, 2014-02-26 at 20:26 +0800, Sheldon wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 02/25/2014 08:23 AM, Christy Perez wrote:
</pre>
<blockquote type="cite">
<pre wrap="">qemu can't use an ISO if 'other' can't execute all the directories
containing the file. Return an error explaning this, and suggest
the way to fix it.
Signed-off-by: Christy Perez <a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:christy@linux.vnet.ibm.com"><christy@linux.vnet.ibm.com></a>
---
po/en_US.po | 6 ++++++
src/kimchi/exception.py | 4 ++++
src/kimchi/i18n.py | 2 ++
src/kimchi/utils.py | 9 +++++++++
src/kimchi/vmtemplate.py | 7 +++++--
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/po/en_US.po b/po/en_US.po
index ca1fe86..38af64c 100644
--- a/po/en_US.po
+++ b/po/en_US.po
@@ -674,6 +674,12 @@ msgid "Bad format while reading volume descriptor in ISO %(filename)s"
msgstr ""
#, python-format
+msgid "The hypervisor doesn't have permission to use this ISO %(filename)s. \
+ Consider moving it under /var/lib/libvirt, or \
+ (not recommended) 'chmod -R o+x 'path_to_iso'."
+msgstr ""
+
+#, python-format
msgid "Virtual machine %(name)s already exists"
msgstr "<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:diff--gita/src/kimchi/exception.pyb/src/kimchi/exception.pyindex2d974a8..263bd0c100644---a/src/kimchi/exception.py+++b/src/kimchi/exception.py@@-75,6+75,10@@classInvalidParameter%28KimchiException%29:pass+classInvalidPermission%28KimchiException%29:+pass++classInvalidOperation%28KimchiException%29:passdiff--gita/src/kimchi/i18n.pyb/src/kimchi/i18n.pyindexfea0184..05b347c100644---a/src/kimchi/i18n.py+++b/src/kimchi/i18n.py@@-57,6+57,8@@messages=%7B">"
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py
index 2d974a8..263bd0c 100644
--- a/src/kimchi/exception.py
+++ b/src/kimchi/exception.py
@@ -75,6 +75,10 @@ class InvalidParameter(KimchiException):
pass
+class InvalidPermission(KimchiException):
+ pass
+
+
class InvalidOperation(KimchiException):
pass
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index fea0184..05b347c 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -57,6 +57,8 @@ messages = {
"</a>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 (not recommended) 'chmod -R o+x 'path_to_iso'." ),
"KCHVM0001E": _("Virtual machine %(name)s already exists"),
"KCHVM0002E": _("Virtual machine %(name)s does not exist<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:%29,diff--gita/src/kimchi/utils.pyb/src/kimchi/utils.pyindex6be1c04..c325be3100644---a/src/kimchi/utils.py+++b/src/kimchi/utils.py@@-27,6+27,7@@importpsutilimportreimportsubprocessimporturllib2+importstatfromthreadingimportTimerfromcherrypy.lib.reprconfimportParser@@-234,3+235,11@@defrun_setfacl_set_attr%28path,attr=">"),
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 6be1c04..c325be3 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -27,6 +27,7 @@ import psutil
import re
import subprocess
import urllib2
+import stat
from threading import Timer
from cherrypy.lib.reprconf import Parser
@@ -234,3 +235,11 @@ def run_setfacl_set_attr(path, attr="</a>r", user=""):
set_user = ["setfacl", "--modify", "user:%s:%s" % (user, attr), path]
out, error, ret = run_command(set_user)
return ret == 0
+
+def check_iso_path_perm(path):
+ """
+ libvirt requires that all parent dirs have o+x
+ """
+ if path == '/': return True
+ return os.stat(path).st_mode & stat.S_IXOTH and \
+ check_iso_path_perm(os.path.dirname(path))
</pre>
</blockquote>
<pre wrap="">just test other's permission is not enough.
1. when the path user is qemu. why we need S_IXOTH?
</pre>
</blockquote>
<pre wrap="">That's what the libvirt doc says, and what I've found is needed: " 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'. "
Source: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/drvqemu.html">http://libvirt.org/drvqemu.html</a>
</pre>
</blockquote>
<pre wrap="">so that's means the disk images without S_IXOTH, qemu still can have
search permissions right?
</pre>
</blockquote>
<pre wrap="">Yep.
The images themselves won't have S_IXOTH. qemu will own them, though:
[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib | grep libvirt
drwxr-xr-x. 9 root root 4.0K Feb 19 11:16 libvirt
[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib/libvirt | grep images
drwx--x--x. 2 root root 4.0K Mar 7 06:22 images
[root@kop4 kimchi-ginger-pkvm]# ls -lah /var/lib/libvirt/images/
total 2.9G
drwx--x--x. 2 root root 4.0K Mar 7 06:22 .
drwxr-xr-x. 9 root root 4.0K Feb 19 11:16 ..
-rw-------. 1 qemu qemu 193K Mar 7 06:22
2f7130eb-539c-4401-82eb-cccd18708f56-0.img
-r--r--r--. 1 qemu qemu 2.9G Mar 7 06:20
RHEL-7.0-20140226.0-Server-ppc64-dvd1.iso</pre>
</blockquote>
</blockquote>
I think Sheldon means besides condition of others have x of ISO's
ancestor directories, directory owner/group is qemu with x access
can also guarantee qemu access ISO directoy success.<br>
<br>
directory owner==qemu AND qemu can x OR directory group==qemu
AND qemu can x OR other can x => qemu can access the iso file
directory(check should pass)<br>
<br>
So based on these complex check personally I favour sudo 'qemu' and
try open iso file to validate access. It is simple and avoid
handling corner case.<br>
<br>
<blockquote cite="mid:531D1AF8.9070002@linux.vnet.ibm.com"
type="cite">
<blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">o+x is sufficient but not necessary condition?
</pre>
</blockquote>
<pre wrap="">o+x is necessary.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">2. as you said "libvirt requires that all parent dirs have o+x"
you has adding 'X' to the ACL for each directory to try it.
I wander have check your ACL enable?
</pre>
</blockquote>
<pre wrap="">I don't think the ACL will be enough. It looks like it has to have o+x
for all parent paths. And yes, my ACLs were enabled on my filesystem.
</pre>
</blockquote>
<pre wrap="">That's strangely. but the docs <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/drvqemu.html">http://libvirt.org/drvqemu.html</a>
says:
it simple to set o+x, means this is the only way.
</pre>
</blockquote>
<pre wrap="">Yes, I think so. </pre>
</blockquote>
it is strange from what I test, it seems o+x is not necessary but
sufficient. <br>
I really want to know why o+x is necessary and why it is the only
way <br>
to check the permission. <br>
<br>
I have try 5 test case with kimchi.<br>
<br>
1. There's a iso on my laptop, with o+x permission <br>
$ ls /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
$ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
class="moz-txt-tag">/</span></i> -l |grep iso <br>
drwxrwxrwx. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso <br>
<br>
I can create a vm and start is successfully. <br>
<br>
<br>
2. Now I remove x permission <br>
$ sudo chmod o-x /home/shhfeng/iso <br>
$ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
class="moz-txt-tag">/</span></i> -l |grep iso <br>
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso <br>
<br>
I failed to start it for permission. <br>
<br>
3. I change the own of this iso, still without x permission. <br>
$ sudo chown qemu:qemu /home/shhfeng/iso <br>
$ ls <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
class="moz-txt-tag">/</span></i> -l |grep iso <br>
drwxrwxrw-. 4 qemu qemu 4096 Oct 3 00:22 iso <br>
<br>
Now I can start it successfully. <br>
<br>
<br>
4. just set the qemu user as search permission. <br>
$ sudo chown shhfeng:shhfeng <i class="moz-txt-slash"><span
class="moz-txt-tag">/</span>home/shhfeng/iso<span
class="moz-txt-tag">/</span></i> <br>
$ ls -l <i class="moz-txt-slash"><span class="moz-txt-tag">/</span>home/shhfeng<span
class="moz-txt-tag">/</span></i> |grep iso <br>
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso <br>
$ setfacl -m u:qemu:x /home/shhfeng/iso <br>
$ getfacl /home/shhfeng/iso <br>
getfacl: Removing leading '/' from absolute path names <br>
# file: home/shhfeng/iso <br>
# owner: shhfeng <br>
# group: shhfeng <br>
user::rwx <br>
user:qemu:--x <br>
group::rwx <br>
mask::rwx <br>
other::rw- <br>
<br>
Now I can also start it successfully. <br>
<br>
<br>
5. remove qemu user as search permission, <br>
add set the 'kvm' group to the path, qemu is in "kvm" group <br>
$ setfacl -x u:qemu /home/shhfeng/iso <br>
$ groups qemu <br>
qemu : qemu kvm <br>
$ sudo chown shhfeng:kvm <i class="moz-txt-slash"><span
class="moz-txt-tag">/</span>home/shhfeng/iso<span
class="moz-txt-tag">/</span></i> <br>
<br>
Now I can also start it successfully. <br>
<br>
<br>
only case 2, the kimchi deny to start vm for permission. <br>
<br>
<br>
<br>
I write a python script checkperm.py <br>
It is the same result of libvirt run qemu. you can have a try. <br>
If others do like like checkperm.py, <br>
I think we can probe the permission by let libvirt to start the
guest with the iso. <br>
<br>
$ sudo python checkperm.py
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso <br>
<br>
# checkperm.py <br>
1 from multiprocessing import Process, Queue <br>
2 import sys <br>
3 import grp <br>
4 import os <br>
5 import pwd <br>
6 <br>
7 <br>
8 def probe_file_permission_as_user(file, user): <br>
9 def probe_permission(q, file, user): <br>
10 uid = pwd.getpwnam(user).pw_uid <br>
11 gid = pwd.getpwnam(user).pw_gid <br>
12 gids = [g.gr_gid for g in grp.getgrall() if user in
g.gr_mem] <br>
13 os.setgid(gid) <br>
14 os.setgroups(gids) <br>
15 os.setuid(uid) <br>
16 try: <br>
17 with open(file): <br>
18 q.put(True) <br>
19 except Exception: <br>
20 q.put(False) <br>
21 <br>
22 queue = Queue() <br>
23 p = Process(target=probe_permission, args=(queue, file,
user)) <br>
24 p.start() <br>
25 p.join() <br>
26 return queue.get() <br>
27 <br>
28 if __name__ == '__main__': <br>
29 print probe_file_permission_as_user(sys.argv[1], 'qemu')
<blockquote cite="mid:1394223617.4878.215.camel@christy-toshiba"
type="cite">
<blockquote type="cite">
<pre wrap="">"""
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.
"""
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">3. have you check add qemu in all the paths group?
and open set g+x?
can you try my patch, seen it can works?
add a method to probe the permission as qemu user
</pre>
</blockquote>
<pre wrap="">I don't see how the patch listed there will apply. Maybe github
reformats it strangely when you paste it in?
Even so, as I pasted from the libvirt doc, I don't think that will be
enough to make libvirt happy.
</pre>
</blockquote>
<pre wrap="">save as, and git apply. but I think it will not successfully, for the
upstream changes so quickly.
Yes, we should make libvirt and kimchi user happy?
</pre>
</blockquote>
<pre wrap="">I think it's a security leak to allow someone to put a file in one
directory, and then we give o+x to every parent of it. If I put an ISO
at /home/christy/example.iso and kimchi changed recursively all parents
to o+x, it would give all users the ability to see what's in /home/.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
index af07ee3..76ac772 100644
--- a/src/kimchi/vmtemplate.py
+++ b/src/kimchi/vmtemplate.py
@@ -29,9 +29,9 @@ import urlparse
from kimchi import isoinfo
from kimchi import osinfo
-from kimchi.exception import InvalidParameter, IsoFormatError
+from kimchi.exception import InvalidParameter, IsoFormatError, InvalidPermission
from kimchi.isoinfo import IsoImage
-
+from kimchi.utils import check_iso_path_perm
QEMU_NAMESPACE = "xmlns:qemu='<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://libvirt.org/schemas/domain/qemu/1.0">http://libvirt.org/schemas/domain/qemu/1.0</a>'"
@@ -55,6 +55,9 @@ class VMTemplate(object):
iso_distro = iso_version = 'unknown'
iso = args.get('cdrom', '')
+ if iso.startswith('/') and \
+ not check_iso_path_perm(os.path.dirname(iso)):
+ raise InvalidPermission("KCHISO0008E", {'filename': iso})
if scan and len(iso) > 0:
iso_distro, iso_version = self.get_iso_info(iso)
if not iso.startswith('/'):
</pre>
</blockquote>
</blockquote>
<pre wrap="">Regards,
- Christy
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<pre wrap="">Regards,
- Christy
</pre>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Thanks and best regards!
Sheldon Feng(冯少合)<a moz-do-not-send="true" class="moz-txt-link-rfc2396E" href="mailto:shaohef@linux.vnet.ibm.com"><shaohef@linux.vnet.ibm.com></a>
IBM Linux Technology Center</pre>
</blockquote>
<br>
</body>
</html>