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

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