Re: [PATCH v5 12/23] src: rewrite ACL rule checker in Python

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/11/19 9:38 AM, Daniel P. Berrangé wrote:
> As part of an goal to eliminate Perl from libvirt build tools,
> rewrite the check-aclrules.pl tool in Python.
> 
> This was a straight conversion, manually going line-by-line to
> change the syntax from Perl to Python. Thus the overall structure
> of the file and approach is the same.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  Makefile.am               |   1 +
>  scripts/check-aclrules.py | 263 ++++++++++++++++++++++++++++++++++++++
>  src/Makefile.am           |   4 +-
>  src/check-aclrules.pl     | 252 ------------------------------------
>  4 files changed, 265 insertions(+), 255 deletions(-)
>  create mode 100755 scripts/check-aclrules.py
>  delete mode 100755 src/check-aclrules.pl
> 
> diff --git a/Makefile.am b/Makefile.am
> index 407a664626..f28b07d814 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -47,6 +47,7 @@ EXTRA_DIST = \
>    AUTHORS.in \
>    scripts/augeas-gentest.py \
>    scripts/check-aclperms.py \
> +  scripts/check-aclrules.py \
>    scripts/check-drivername.py \
>    scripts/check-driverimpls.py \
>    scripts/check-spacing.py \
> diff --git a/scripts/check-aclrules.py b/scripts/check-aclrules.py
> new file mode 100755
> index 0000000000..3fab126a4a
> --- /dev/null
> +++ b/scripts/check-aclrules.py
> @@ -0,0 +1,263 @@
> +#!/usr/bin/env python
> +#
> +# Copyright (C) 2013-2019 Red Hat, Inc.
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library.  If not, see
> +# <http://www.gnu.org/licenses/>.
> +#
> +# This script validates that the driver implementation of any
> +# public APIs contain ACL checks.
> +#
> +# As the script reads each source file, it attempts to identify
> +# top level function names.
> +#
> +# When reading the body of the functions, it looks for anything
> +# that looks like an API called named  XXXEnsureACL. It will
> +# validate that the XXX prefix matches the name of the function
> +# it occurs in.
> +#
> +# When it later finds the virDriverPtr table, for each entry
> +# point listed, it will validate if there was a previously
> +# detected EnsureACL call recorded.
> +#
> +
> +from __future__ import print_function
> +
> +import re
> +import sys
> +
> +whitelist = {
> +    "connectClose": True,
> +    "connectIsEncrypted": True,
> +    "connectIsSecure": True,
> +    "connectIsAlive": True,
> +    "networkOpen": True,
> +    "networkClose": True,
> +    "nwfilterOpen": True,
> +    "nwfilterClose": True,
> +    "secretOpen": True,
> +    "secretClose": True,
> +    "storageOpen": True,
> +    "storageClose": True,
> +    "interfaceOpen": True,
> +    "interfaceClose": True,
> +    "connectURIProbe": True,
> +    "localOnly": True,
> +    "domainQemuAttach": True,
> +}
> +
> +# XXX this vzDomainMigrateConfirm3Params looks
> +# bogus - determine why it doesn't have a valid
> +# ACL check.
> +implwhitelist = {
> +    "vzDomainMigrateConfirm3Params": True,
> +}
> +
> +lastfile = None
> +
> +
> +def fixup_name(name):
> +    name.replace("Nwfilter", "NWFilter")
> +    name.replace("Pm", "PM")
> +    name.replace("Scsi", "SCSI")
> +    if name.endswith("Xml"):
> +        name = name[:-3] + "XML"
> +    elif name.endswith("Uri"):
> +        name = name[:-3] + "URI"
> +    elif name.endswith("Uuid"):
> +        name = name[:-4] + "UUID"
> +    elif name.endswith("Id"):
> +        name = name[:-2] + "ID"
> +    elif name.endswith("Mac"):
> +        name = name[:-3] + "MAC"
> +    elif name.endswith("Cpu"):
> +        name = name[:-3] + "MAC"
> +    elif name.endswith("Os"):
> +        name = name[:-2] + "OS"
> +    elif name.endswith("Nmi"):
> +        name = name[:-3] + "NMI"
> +    elif name.endswith("Fstrim"):
> +        name = name[:-6] + "FSTrim"
> +    elif name.endswith("Wwn"):
> +        name = name[:-3] + "WWN"
> +
> +    return name
> +
> +
> +def name_to_ProcName(name):
> +    elems = []
> +    if name.find("_") != -1 or name.lower() in ["open", "close"]:
> +        elems = [n.lower().capitalize() for n in name.split("_")]
> +    else:
> +        elems = [name]
> +
> +    elems = [fixup_name(n) for n in elems]
> +    procname = "".join(elems)
> +
> +    return procname[0:1].lower() + procname[1:]
> +
> +
> +proto = sys.argv[1]
> +
> +filteredmap = {}
> +with open(proto, "r") as fh:
> +    incomment = False
> +    filtered = False
> +
> +    for line in fh:
> +        if line.find("/**") != -1:
> +            incomment = True
> +            filtered = False
> +        elif incomment:
> +            if line.find("* @aclfilter") != -1:
> +                filtered = True
> +            elif filtered:
> +                m = re.search(r'''REMOTE_PROC_(.*)\s+=\s*\d+''', line)
> +                if m is not None:
> +                    api = name_to_ProcName(m.group(1))
> +                    # Event filtering is handled in daemon/remote.c
> +                    # instead of drivers
> +                    if line.find("_EVENT_REGISTER") == -1:
> +                        filteredmap[api] = True
> +                    incomment = False
> +
> +
> +def process_file(filename):
> +    brace = 0
> +    maybefunc = None
> +    intable = False
> +    table = None
> +
> +    acls = {}
> +    aclfilters = {}
> +    errs = False
> +    with open(filename, "r") as fh:
> +        lineno = 0
> +        for line in fh:
> +            lineno = lineno + 1
> +            if brace == 0:
> +                # Looks for anything which appears to be a function
> +                # body name. Doesn't matter if we pick up bogus stuff
> +                # here, as long as we don't miss valid stuff
> +                m = re.search(r'''\b(\w+)\(''', line)
> +                if m is not None:
> +                    maybefunc = m.group(1)
> +            elif brace > 0:
> +                ensureacl = re.search(r'''(\w+)EnsureACL''', line)
> +                checkacl = re.search(r'''(\w+)CheckACL''', line)
> +                stub = re.search(r'''\b(\w+)\(''', line)
> +                if ensureacl is not None:
> +                    # Record the fact that maybefunc contains an
> +                    # ACL call, and make sure it is the right call!
> +                    func = ensureacl.group(1)
> +                    if func.startswith("vir"):
> +                        func = func[3:]
> +
> +                    if maybefunc is None:
> +                        print("%s:%d Unexpected check '%s' outside function" %
> +                              (filename, lineno, func), file=sys.stderr)
> +                        errs = True
> +                    else:
> +                        if not maybefunc.lower().endswith(func.lower()):
> +                            print("%s:%d Mismatch check 'vir%sEnsureACL'" +
> +                                  "for function '%s'" %
> +                                  (filename, lineno, func, maybefunc),
> +                                  file=sys.stderr)
> +                            errs = True
> +                    acls[maybefunc] = True
> +                elif checkacl:
> +                    # Record the fact that maybefunc contains an
> +                    # ACL filter call, and make sure it is the right call!
> +                    func = checkacl.group(1)
> +                    if func.startswith("vir"):
> +                        func = func[3:]
> +
> +                    if maybefunc is None:
> +                        print("%s:%d Unexpected check '%s' outside function" %
> +                              (filename, lineno, func), file=sys.stderr)
> +                        errs = True
> +                    else:
> +                        if not maybefunc.lower().endswith(func.lower()):
> +                            print("%s:%d Mismatch check 'vir%sEnsureACL' " +

The string here should be vir%sCheckACL

After that, and the flake8 fixes, I could trigger the two 'mismatch
check' and 'Missing ACL' errors in qemu_driver.c, which are the
important ones

Tested-by: Cole Robinson <crobinso@xxxxxxxxxx>

- Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux