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@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn 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@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' " +
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@redhat.com>
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Nov 11, 2019 at 02:38:15PM +0000, 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@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
>+
The perl version actually counted the braces - hopefully we're not
quirky enough to have multiple of them on one line.
>+ return errs
>+
>+
>+status = 0
>+for filename in sys.argv[2:]:
>+ if process_file(filename):
>+ status = 1
>+
>+sys.exit(status)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.