[libvirt] [PATCH v5 12/23] src: rewrite ACL rule checker in Python
Daniel P. Berrangé
berrange at redhat.com
Mon Nov 11 14:38:15 UTC 2019
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 at redhat.com>
---
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
More information about the libvir-list
mailing list