Re: [Kimchi-devel] [PATCH] Don't allow templates to be created with ISOs that won't be usable.
10 Mar
2014
10 Mar
'14
3:52 a.m.
On 03/08/2014 04:20 AM, Christy Perez wrote:
> 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:
>> On 03/04/2014 06:38 AM, Christy Perez wrote:
>>> On Wed, 2014-02-26 at 20:26 +0800, Sheldon wrote:
>>>> On 02/25/2014 08:23 AM, Christy Perez wrote:
>>>>> 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 <christy@linux.vnet.ibm.com>
>>>>> ---
>>>>> 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 ""
>>>>>
>>>>> 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 = {
>>>>> "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"),
>>>>> 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="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))
>>>> just test other's permission is not enough.
>>>> 1. when the path user is qemu. why we need S_IXOTH?
>>> 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: http://libvirt.org/drvqemu.html
>> so that's means the disk images without S_IXOTH, qemu still can have
>> search permissions right?
> 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
>
>> o+x is sufficient but not necessary condition?
> o+x is necessary.
>>>> 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?
>>> 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.
>> That's strangely. but the docs http://libvirt.org/drvqemu.html
>> says:
>> it simple to set o+x, means this is the only way.
> Yes, I think so.
it is strange from what I test, it seems o+x is not necessary but
sufficient.
I really want to know why o+x is necessary and why it is the only way
to check the permission.
I have try 5 test case with kimchi.
1. There's a iso on my laptop, with o+x permission
$ ls /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrwx. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
I can create a vm and start is successfully.
2. Now I remove x permission
$ sudo chmod o-x /home/shhfeng/iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
I failed to start it for permission.
3. I change the own of this iso, still without x permission.
$ sudo chown qemu:qemu /home/shhfeng/iso
$ ls /home/shhfeng/ -l |grep iso
drwxrwxrw-. 4 qemu qemu 4096 Oct 3 00:22 iso
Now I can start it successfully.
4. just set the qemu user as search permission.
$ sudo chown shhfeng:shhfeng /home/shhfeng/iso/
$ ls -l /home/shhfeng/ |grep iso
drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
$ setfacl -m u:qemu:x /home/shhfeng/iso
$ getfacl /home/shhfeng/iso
getfacl: Removing leading '/' from absolute path names
# file: home/shhfeng/iso
# owner: shhfeng
# group: shhfeng
user::rwx
user:qemu:--x
group::rwx
mask::rwx
other::rw-
Now I can also start it successfully.
5. remove qemu user as search permission,
add set the 'kvm' group to the path, qemu is in "kvm" group
$ setfacl -x u:qemu /home/shhfeng/iso
$ groups qemu
qemu : qemu kvm
$ sudo chown shhfeng:kvm /home/shhfeng/iso/
Now I can also start it successfully.
only case 2, the kimchi deny to start vm for permission.
I write a python script checkperm.py
It is the same result of libvirt run qemu. you can have a try.
If others do like like checkperm.py,
I think we can probe the permission by let libvirt to start the guest
with the iso.
$ sudo python checkperm.py
/home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
# checkperm.py
1 from multiprocessing import Process, Queue
2 import sys
3 import grp
4 import os
5 import pwd
6
7
8 def probe_file_permission_as_user(file, user):
9 def probe_permission(q, file, user):
10 uid = pwd.getpwnam(user).pw_uid
11 gid = pwd.getpwnam(user).pw_gid
12 gids = [g.gr_gid for g in grp.getgrall() if user in g.gr_mem]
13 os.setgid(gid)
14 os.setgroups(gids)
15 os.setuid(uid)
16 try:
17 with open(file):
18 q.put(True)
19 except Exception:
20 q.put(False)
21
22 queue = Queue()
23 p = Process(target=probe_permission, args=(queue, file, user))
24 p.start()
25 p.join()
26 return queue.get()
27
28 if __name__ == '__main__':
29 print probe_file_permission_as_user(sys.argv[1], 'qemu')
>> """
>> 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.
>> """
>>
>>
>>>> 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
>>> 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.
>> 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?
> 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/.
>>>>> 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='http://libvirt.org/schemas/domain/qemu/1.0'"
>>>>>
>>>>> @@ -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('/'):
>>> Regards,
>>>
>>> - Christy
>>>
>>
> Regards,
>
> - Christy
>
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com>
IBM Linux Technology Center
10 Mar
10 Mar
5:03 a.m.
New subject: [PATCH] Don't allow templates to be created with ISOs that won't be usable.
On 2014年03月10日 09:52, Sheldon wrote:
> On 03/08/2014 04:20 AM, Christy Perez wrote:
>> 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:
>>> On 03/04/2014 06:38 AM, Christy Perez wrote:
>>>> On Wed, 2014-02-26 at 20:26 +0800, Sheldon wrote:
>>>>> On 02/25/2014 08:23 AM, Christy Perez wrote:
>>>>>> 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<christy@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> 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 ""
>>>>>>
>>>>>> 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 = {
>>>>>> "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"),
>>>>>> 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="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))
>>>>> just test other's permission is not enough.
>>>>> 1. when the path user is qemu. why we need S_IXOTH?
>>>> 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:http://libvirt.org/drvqemu.html
>>> so that's means the disk images without S_IXOTH, qemu still can have
>>> search permissions right?
>> 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
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.
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)
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.
>>
>>> o+x is sufficient but not necessary condition?
>> o+x is necessary.
>>>>> 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?
>>>> 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.
>>> That's strangely. but the docshttp://libvirt.org/drvqemu.html
>>> says:
>>> it simple to set o+x, means this is the only way.
>> Yes, I think so.
> it is strange from what I test, it seems o+x is not necessary but
> sufficient.
> I really want to know why o+x is necessary and why it is the only way
> to check the permission.
>
> I have try 5 test case with kimchi.
>
> 1. There's a iso on my laptop, with o+x permission
> $ ls /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
> /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
> $ ls /home/shhfeng/ -l |grep iso
> drwxrwxrwx. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
>
> I can create a vm and start is successfully.
>
>
> 2. Now I remove x permission
> $ sudo chmod o-x /home/shhfeng/iso
> $ ls /home/shhfeng/ -l |grep iso
> drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
>
> I failed to start it for permission.
>
> 3. I change the own of this iso, still without x permission.
> $ sudo chown qemu:qemu /home/shhfeng/iso
> $ ls /home/shhfeng/ -l |grep iso
> drwxrwxrw-. 4 qemu qemu 4096 Oct 3 00:22 iso
>
> Now I can start it successfully.
>
>
> 4. just set the qemu user as search permission.
> $ sudo chown shhfeng:shhfeng /home/shhfeng/iso/
> $ ls -l /home/shhfeng/ |grep iso
> drwxrwxrw-. 4 shhfeng shhfeng 4096 Oct 3 00:22 iso
> $ setfacl -m u:qemu:x /home/shhfeng/iso
> $ getfacl /home/shhfeng/iso
> getfacl: Removing leading '/' from absolute path names
> # file: home/shhfeng/iso
> # owner: shhfeng
> # group: shhfeng
> user::rwx
> user:qemu:--x
> group::rwx
> mask::rwx
> other::rw-
>
> Now I can also start it successfully.
>
>
> 5. remove qemu user as search permission,
> add set the 'kvm' group to the path, qemu is in "kvm" group
> $ setfacl -x u:qemu /home/shhfeng/iso
> $ groups qemu
> qemu : qemu kvm
> $ sudo chown shhfeng:kvm /home/shhfeng/iso/
>
> Now I can also start it successfully.
>
>
> only case 2, the kimchi deny to start vm for permission.
>
>
>
> I write a python script checkperm.py
> It is the same result of libvirt run qemu. you can have a try.
> If others do like like checkperm.py,
> I think we can probe the permission by let libvirt to start the guest
> with the iso.
>
> $ sudo python checkperm.py
> /home/shhfeng/iso/RHEL6.4-20130123.0-Server-x86_64-DVD1.iso
>
> # checkperm.py
> 1 from multiprocessing import Process, Queue
> 2 import sys
> 3 import grp
> 4 import os
> 5 import pwd
> 6
> 7
> 8 def probe_file_permission_as_user(file, user):
> 9 def probe_permission(q, file, user):
> 10 uid = pwd.getpwnam(user).pw_uid
> 11 gid = pwd.getpwnam(user).pw_gid
> 12 gids = [g.gr_gid for g in grp.getgrall() if user in g.gr_mem]
> 13 os.setgid(gid)
> 14 os.setgroups(gids)
> 15 os.setuid(uid)
> 16 try:
> 17 with open(file):
> 18 q.put(True)
> 19 except Exception:
> 20 q.put(False)
> 21
> 22 queue = Queue()
> 23 p = Process(target=probe_permission, args=(queue, file, user))
> 24 p.start()
> 25 p.join()
> 26 return queue.get()
> 27
> 28 if __name__ == '__main__':
> 29 print probe_file_permission_as_user(sys.argv[1], 'qemu')
>>> """
>>> 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.
>>> """
>>>
>>>
>>>>> 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
>>>> 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.
>>> 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?
>> 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/.
>>>>>> 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='http://libvirt.org/schemas/domain/qemu/1.0'"
>>>>>>
>>>>>> @@ -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('/'):
>>>> Regards,
>>>>
>>>> - Christy
>>>>
>> Regards,
>>
>> - Christy
>>
>
>
> --
> Thanks and best regards!
>
> Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com>
> IBM Linux Technology Center
4171
Age (days ago)
4171
Last active (days ago)
1 comments
2 participants
participants (2)
-
Royce Lv -
Sheldon