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

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(a)redhat.com&gt; --- 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' " + + "for function '%s'" % + (filename, lineno, func, maybefunc), + file=sys.stderr) + errs = True + aclfilters[maybefunc] = True + elif stub: + # Handles case where we replaced an API with a new + # one which adds new parameters, and we're left with + # a simple stub calling the new API. + callfunc = stub.group(1) + if callfunc in acls: + acls[maybefunc] = True + + if callfunc in aclfilters: + aclfilters[maybefunc] = True + + # Pass the vir*DriverPtr tables and make sure that + # every func listed there, has an impl which calls + # an ACL function + if intable: + assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line) + if line.find("}") != -1: + intable = False + table = None + elif assign is not None: + api = assign.group(1) + impl = assign.group(2) + + if (impl != "NULL" and + api not in ["no", "name"] and + table != "virStateDriver"): + if (impl not in acls and + api not in whitelist and + impl not in implwhitelist): + print("%s:%d Missing ACL check in " + + "function '%s' for '%s'" % + (filename, lineno, impl, api), + file=sys.stderr) + errs = True + + if api in filteredmap and impl not in aclfilters: + print("%s:%d Missing ACL filter in " + + "function '%s' for '%s'" % + (filename, lineno, impl, api), + file=sys.stderr) + errs = True + else: + m = re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''', + line) + if m is not None: + name = m.group(1) + if name not in ["virNWFilterCallbackDriver", + "virNWFilterTechDriver", + "virDomainConfNWFilterDriver"]: + intable = True + table = name + + if line.find("{") != -1: + brace = brace + 1 + if line.find("}") != -1: + brace = brace - 1 + + return errs + + +status = 0 +for filename in sys.argv[2:]: + if process_file(filename): + status = 1 + +sys.exit(status) diff --git a/src/Makefile.am b/src/Makefile.am index 62f1d55402..bb63e2486c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -357,7 +357,7 @@ check-driverimpls: $(filter-out /%,$(DRIVER_SOURCE_FILES)))) check-aclrules: - $(AM_V_GEN)$(PERL) $(srcdir)/check-aclrules.pl \ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/check-aclrules.py \ $(REMOTE_PROTOCOL) \ $(addprefix $(srcdir)/,$(filter-out /%,$(STATEFUL_DRIVER_SOURCE_FILES))) @@ -366,8 +366,6 @@ check-aclperms: $(srcdir)/access/viraccessperm.h \ $(srcdir)/access/viraccessperm.c -EXTRA_DIST += check-aclrules.pl - check-local: check-protocol check-symfile check-symsorting \ check-drivername check-driverimpls check-aclrules \ check-aclperms check-admin diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl deleted file mode 100755 index 0d4cac17ca..0000000000 --- a/src/check-aclrules.pl +++ /dev/null @@ -1,252 +0,0 @@ -#!/usr/bin/env perl -# -# Copyright (C) 2013-2014 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. -# -use strict; -use warnings; - -my $status = 0; - -my $brace = 0; -my $maybefunc; -my $intable = 0; -my $table; - -my %acls; -my %aclfilters; - -my %whitelist = ( - "connectClose" => 1, - "connectIsEncrypted" => 1, - "connectIsSecure" => 1, - "connectIsAlive" => 1, - "networkOpen" => 1, - "networkClose" => 1, - "nwfilterOpen" => 1, - "nwfilterClose" => 1, - "secretOpen" => 1, - "secretClose" => 1, - "storageOpen" => 1, - "storageClose" => 1, - "interfaceOpen" => 1, - "interfaceClose" => 1, - "connectURIProbe" => 1, - "localOnly" => 1, - "domainQemuAttach" => 1, - ); - -# XXX this vzDomainMigrateConfirm3Params looks -# bogus - determine why it doesn't have a valid -# ACL check. -my %implwhitelist = ( - "vzDomainMigrateConfirm3Params" => 1, - ); - -my $lastfile; - -sub fixup_name { - my $name = shift; - - $name =~ s/Nwfilter/NWFilter/; - $name =~ s/Xml$/XML/; - $name =~ s/Uri$/URI/; - $name =~ s/Uuid$/UUID/; - $name =~ s/Id$/ID/; - $name =~ s/Mac$/MAC/; - $name =~ s/Cpu$/CPU/; - $name =~ s/Os$/OS/; - $name =~ s/Nmi$/NMI/; - $name =~ s/Pm/PM/; - $name =~ s/Fstrim$/FSTrim/; - $name =~ s/Scsi/SCSI/; - $name =~ s/Wwn$/WWN/; - - return $name; -} - -sub name_to_ProcName { - my $name = shift; - - my @elems; - if ($name =~ /_/ || (lc $name) eq "open" || (lc $name) eq "close") { - @elems = split /_/, $name; - @elems = map lc, @elems; - @elems = map ucfirst, @elems; - } else { - @elems = $name; - } - @elems = map { fixup_name($_) } @elems; - my $procname = join "", @elems; - - $procname =~ s/^([A-Z])/lc $1/e; - - return $procname; -} - - -my $proto = shift @ARGV; - -open PROTO, "<$proto" or die "cannot read $proto"; - -my %filtered; -my $incomment = 0; -my $filtered = 0; -while (<PROTO>) { - if (m,/\*\*,) { - $incomment = 1; - $filtered = 0; - } elsif ($incomment) { - if (m,\*\s\@aclfilter,) { - $filtered = 1; - } elsif ($filtered && - m,REMOTE_PROC_(.*)\s+=\s*\d+,) { - my $api = name_to_ProcName($1); - # Event filtering is handled in daemon/remote.c instead of drivers - if (! m,_EVENT_REGISTER,) { - $filtered{$api} = 1; - } - $incomment = 0; - } - } -} - -close PROTO; - -while (<>) { - if (!defined $lastfile || - $lastfile ne $ARGV) { - %acls = (); - $brace = 0; - $maybefunc = undef; - $lastfile = $ARGV; - } - 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 - if (m,\b(\w+)\(,) { - $maybefunc = $1; - } - } elsif ($brace > 0) { - if (m,(\w+)EnsureACL,) { - # Record the fact that maybefunc contains an - # ACL call, and make sure it is the right call! - my $func = $1; - $func =~ s/^vir//; - if (!defined $maybefunc) { - print "$ARGV:$. Unexpected check '$func' outside function\n"; - $status = 1; - } else { - unless ($maybefunc =~ /$func$/i) { - print "$ARGV:$. Mismatch check 'vir${func}EnsureACL' for function '$maybefunc'\n"; - $status = 1; - } - } - $acls{$maybefunc} = 1; - } elsif (m,(\w+)CheckACL,) { - # Record the fact that maybefunc contains an - # ACL filter call, and make sure it is the right call! - my $func = $1; - $func =~ s/^vir//; - if (!defined $maybefunc) { - print "$ARGV:$. Unexpected check '$func' outside function\n"; - $status = 1; - } else { - unless ($maybefunc =~ /$func$/i) { - print "$ARGV:$. Mismatch check 'vir${func}CheckACL' for function '$maybefunc'\n"; - $status = 1; - } - } - $aclfilters{$maybefunc} = 1; - } elsif (m,\b(\w+)\(,) { - # Handles case where we replaced an API with a new - # one which adds new parameters, and we're left with - # a simple stub calling the new API. - my $callfunc = $1; - if (exists $acls{$callfunc}) { - $acls{$maybefunc} = 1; - } - if (exists $aclfilters{$callfunc}) { - $aclfilters{$maybefunc} = 1; - } - } - } - - # Pass the vir*DriverPtr tables and make sure that - # every func listed there, has an impl which calls - # an ACL function - if ($intable) { - if (/\}/) { - $intable = 0; - $table = undef; - } elsif (/\.(\w+)\s*=\s*(\w+),?/) { - my $api = $1; - my $impl = $2; - - next if $impl eq "NULL"; - - if ($api ne "no" && - $api ne "name" && - $table ne "virStateDriver" && - !exists $acls{$impl} && - !exists $whitelist{$api} && - !exists $implwhitelist{$impl}) { - print "$ARGV:$. Missing ACL check in function '$impl' for '$api'\n"; - $status = 1; - } - - if (exists $filtered{$api} && - !exists $aclfilters{$impl}) { - print "$ARGV:$. Missing ACL filter in function '$impl' for '$api'\n"; - $status = 1; - } - } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { - if ($1 ne "virNWFilterCallbackDriver" && - $1 ne "virNWFilterTechDriver" && - $1 ne "virDomainConfNWFilterDriver") { - $intable = 1; - $table = $1; - } - } - - - my $count; - $count = s/{//g; - $brace += $count; - $count = s/}//g; - $brace -= $count; -} continue { - close ARGV if eof; -} - -exit $status; -- 2.21.0