[PATCH V5] check and set search permission for a directory
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V4 -> V5 check the group permission. V3 -> V4 rebase add src/kimchi/utils.py to PEP8 list ShaoHe Feng (1): add a method to fix search permissions Makefile.am | 1 + src/kimchi/utils.py | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 1 deletion(-) -- 1.8.4.2
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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): + 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 -- 1.8.4.2
On 01/06/2014 11:55 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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): + 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 + + so many duplicate code in check_path_permission and fix_path_permission maybe need a decorator. +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
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 01/06/2014 01:55 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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.
+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 01/06/2014 01:55 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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
On 2014年01月07日 03:15, Aline Manera wrote: 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 01/07/2014 12:27 AM, Royce Lv wrote:
On 01/06/2014 01:55 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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
On 2014年01月07日 03:15, Aline Manera wrote: permission after mounted, if it is not valid, we deny pool creation. So under nfs use case, check is still needed.
Got it. Thanks
+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@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@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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> 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
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 01/07/2014 05:51 AM, Sheldon wrote:
On 01/07/2014 03:15 AM, Aline Manera wrote:
On 01/06/2014 01:55 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@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> 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?
I don't think so. Only fixing the user ACL should be enough
+ 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
participants (4)
-
Aline Manera -
Royce Lv -
shaohef＠linux.vnet.ibm.com -
Sheldon