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
