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

Aline Manera alinefm at linux.vnet.ibm.com
Mon Jan 6 19:15:31 UTC 2014


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.

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




More information about the Kimchi-devel mailing list