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

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

 



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' " +
+                                  "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@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