From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- 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):

+ 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):

+ 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 +

+ +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 + +

+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

On 2014年01月07日 03:15, Aline Manera wrote: > On 01/06/2014 01:55 PM, shaohef(a)linux.vnet.ibm.com wrote: >> From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; >> >> 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(a)linux.vnet.ibm.com&gt; >> Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; >> --- >> 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(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >

On 01/07/2014 03:15 AM, Aline Manera wrote: > On 01/06/2014 01:55 PM, shaohef(a)linux.vnet.ibm.com wrote: >> From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; >> >> 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(a)linux.vnet.ibm.com&gt; >> Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; >> --- >> 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> sure, "" is good. will change it in next version,. I have check in the follow code: + if ret != 0 and group: + out, error, ret = run_command(set_group) In my option, if we can set user ACL successful, do we still need to set group ACL?

> >> + 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 > will change in the next version. >> + >> +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. > >> +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 > > >