[Kimchi-devel] [PATCH V5] add a method to fix search permissions

[Royce Lv]

On 2014年01月07日 03:15, Aline Manera wrote:
> On 01/06/2014 01:55 PM, shaohef at linux.vnet.ibm.com wrote:
>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>
>> First will use setfacl to set the path search permission for user or
>> group, if failed then set search permission for others on the path.
>>
>> Usage:
>> check_path_permission("/tmp/need/to/fix/path", username)
>> fix_path_permission("/tmp/need/to/fix/path", username)
>>
>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>> Makefile.am | 1 +
>> src/kimchi/utils.py | 125 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 125 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 07b2aa0..bf8e0a0 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -46,6 +46,7 @@ PEP8_WHITELIST = \
>> src/kimchi/cachebust.py \
>> src/kimchi/config.py.in \
>> src/kimchi/control/*.py \
>> + src/kimchi/utils.py \
>> src/kimchi/disks.py \
>> src/kimchi/featuretests.py \
>> src/kimchi/iscsi.py \
>> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
>> index b6d84fd..5de982f 100644
>> --- a/src/kimchi/utils.py
>> +++ b/src/kimchi/utils.py
>> @@ -18,11 +18,16 @@
>> #
>> # 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
>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>> 02110-1301 USA
>> #
>>
>> import cherrypy
>> +import grp
>> import os
>> +import pwd
>> +import re
>> +import stat
>> +import subprocess
>>
>>
>> from cherrypy.lib.reprconf import Parser
>> @@ -33,6 +38,7 @@ from kimchi import config
>>
>> kimchi_log = cherrypy.log.error_log
>>
>> +
>> def is_digit(value):
>> if isinstance(value, int):
>> return True
>> @@ -84,3 +90,120 @@ def import_class(class_path):
>>
>> def import_module(module_name):
>> return __import__(module_name, globals(), locals(), [''])
>> +
>> +
>> +def run_command(cmd):
>> + try:
>> + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
>> + stderr=subprocess.PIPE)
>> + out, error = proc.communicate()
>> +
>> + kimchi_log.debug("Run command '%s'", " ".join(cmd))
>> + if out or error:
>> + kimchi_log.debug("out:\n %s\nerror:\n %s", out, error)
>> + except OSError as e:
>> + kimchi_log.error("Failed to run command %s. %s", " ".join(cmd),
>> + e.message)
>> + return (None, None, None)
>> + if proc.returncode != 0:
>> + kimchi_log.debug("Cmd '%s' failed, error code: %s", cmd, error)
>> + return (out, error, proc.returncode)
>> +
>> +
>> +def name_uid(user):
>> + return pwd.getpwnam(user).pw_uid
>> +
>> +
>> +def get_group_ids(user):
>> + gids = [g.gr_gid for g in grp.getgrall() if user in g.gr_mem]
>> + gid = pwd.getpwnam(user).pw_gid
>> + gids.append(grp.getgrgid(gid).gr_gid)
>> + return gids
>> +
>> +
>> +def get_groups(user):
>> + gids = get_group_ids(user)
>> + return [grp.getgrgid(gid).gr_name for gid in gids]
>> +
>> +
>> +def run_setfacl_set_x(path, user="qemu", group=None):
>
> I think the correct default value for 'groups' is "" instead of None
> Or we will run: setfacl --modify group:None:x <path>
>
>> + set_user = ["setfacl", "--modify", "user:%s:x" % user, path]
>> + set_group = ["setfacl", "--modify", "group:%s:x" % group, path]
>> + out, error, ret = run_command(set_user)
>> + if ret != 0 and group:
>> + out, error, ret = run_command(set_group)
>> + return ret == 0
>> +
>> +
>> +def run_getfacl(path, user="qemu", groups=None):
>
> Same I commented above about the default value for 'groups' parameter
>
>> + cmd = ["getfacl", path]
>> + out, error, ret = run_command(cmd)
>> + if out and ret == 0:
>> + res = re.findall("user:%s:..x" % user, out)
>> + if res:
>> + return True
>> + res = re.findall("(?<=group:)(.*)(?=:..x)",
>> + out) if groups else []
>> + return list(set(res) & set(groups)) != []
>> + return False
>> +
>> +
>> +def set_x_on_path(path, who="other"):
>> + S_IXWHO = ((stat.S_IXUSR if who == "user" else None) or
>> + (stat.S_IXGRP if who == "group" else None) or
>> + stat.S_IXOTH)
>> + mode = os.stat(path).st_mode | S_IXWHO
>> + os.chmod(path, mode)
>> + ret = os.stat(path).st_mode == mode
>> + if not ret:
>> + kimchi_log.debug("faild to set +x attribute on %s", path)
>> + return ret
>> +
>> +
>> +def get_x_on_path(path, uid, gids=[]):
>> + try:
>> + st = os.stat(path)
>> + mode = st.st_mode
>> + return ((uid == st.st_uid and mode & stat.S_IXUSR == stat.S_IXUSR) or
>> + (st.st_gid in gids and mode & stat.S_IXGRP == stat.S_IXGRP) or
>> + mode & stat.S_IXOTH == stat.S_IXOTH)
>> + except OSError as e:
>> + kimchi_log.error("faild to get attribute on %s as user id: %s. %s",
>> + path, uid, e.message)
>> + return False
>> +
>> +
>
>> +def reverse_paths(path):
>> + paths = []
>> + while path:
>> + paths.append(path)
>> + if path == "/":
>> + break
>> + path = os.path.dirname(path)
>> +
>> + paths.reverse()
>> + return paths
>> +
>
> As I commented before we can simplify it by using just one loop:
>
> paths = path.split("/")
> paths = paths[1:]
>
> path = "/"
> for p in paths:
> path = os.path.join(path, p)
> # check_path_permission code
>
>> +
>> +def check_path_permission(path, user='qemu', groups=[]):
>> + groups = groups if groups else get_groups(user)
>> + gids = [grp.getgrnam(group).gr_gid for group in groups] if groups 
>> else []
>> + paths = reverse_paths(path)
>> + for path in paths:
>> + if not (get_x_on_path(path, name_uid(user), gids) or
>> + run_getfacl(path, user, groups)):
>> + return False
>> + return True
>> +
>> +
>
> We don't need this check_path_permission function
> We can run fix_path_permission directly and it solves the problem when 
> needed.
For local path we can fix permission directly, for nfs paths, the ACL is 
on nfs server side. My nfs validate function plan to check the 
permission after mounted, if it is not valid, we deny pool creation.
So under nfs use case, check is still needed.
>
>> +def fix_path_permission(path, user='qemu', groups=[]):
>> + groups = groups if groups else get_groups(user)
>> + gids = [grp.getgrnam(group).gr_gid for group in groups] if groups 
>> else []
>> + paths = reverse_paths(path)
>> + for path in paths:
>> + if not (get_x_on_path(path, name_uid(user), gids) or
>> + run_getfacl(path, user, groups) or
>> + run_setfacl_set_x(path, user) or
>> + set_x_on_path(path)):
>> + return False
>> + return True
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>