[libvirt] [PATCH 19/19] Add validation that all APIs contain ACL checks
Michal Privoznik
mprivozn at redhat.com
Fri Jun 21 09:17:09 UTC 2013
On 19.06.2013 19:01, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
>
> Add a script which parses the driver API code and validates
> that every API registered in a virNNNDriverPtr table contains
> an ACL check matching the API name.
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
> src/Makefile.am | 22 +++++++-
> src/check-aclrules.pl | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 164 insertions(+), 2 deletions(-)
> create mode 100644 src/check-aclrules.pl
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 647b1f2..9e22e42 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -478,16 +478,34 @@ DRIVER_SOURCE_FILES = \
> $(XENAPI_DRIVER_SOURCES) \
> $(NULL)
>
> +STATEFUL_DRIVER_SOURCE_FILES = \
> + $(INTERFACE_DRIVER_SOURCES) \
> + $(LIBXL_DRIVER_SOURCES) \
> + $(LXC_DRIVER_SOURCES) \
> + $(NETWORK_DRIVER_SOURCES) \
> + $(NODE_DEVICE_DRIVER_SOURCES) \
> + $(NWFILTER_DRIVER_SOURCES) \
> + $(QEMU_DRIVER_SOURCES) \
> + $(SECRET_DRIVER_SOURCES) \
> + $(STORAGE_DRIVER_SOURCES) \
> + $(UML_DRIVER_SOURCES) \
> + $(XEN_DRIVER_SOURCES) \
> + $(NULL)
> +
>
> check-driverimpls:
> $(AM_V_GEN)$(PERL) $(srcdir)/check-driverimpls.pl \
> $(filter /%,$(DRIVER_SOURCE_FILES)) \
> $(addprefix $(srcdir)/,$(filter-out /%,$(DRIVER_SOURCE_FILES)))
>
> -EXTRA_DIST += check-driverimpls.pl
> +check-aclrules:
> + $(AM_V_GEN)$(PERL) $(srcdir)/check-aclrules.pl \
> + $(STATEFUL_DRIVER_SOURCE_FILES)
> +
> +EXTRA_DIST += check-driverimpls.pl check-aclrules.pl
>
> check-local: check-protocol check-symfile check-symsorting \
> - check-drivername check-driverimpls
> + check-drivername check-driverimpls check-aclrules
> .PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct)
>
> # Mock driver, covering domains, storage, networks, etc
> diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl
> new file mode 100644
> index 0000000..62da2b7
> --- /dev/null
> +++ b/src/check-aclrules.pl
> @@ -0,0 +1,144 @@
> +#!/usr/bin/perl
> +#
> +# Copyright (C) 2013 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 %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,
> + );
> +
> +my $lastfile;
> +
> +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,\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;
> + }
> + }
> + }
> +
> + # 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;
> +
> + if ($api ne "no" &&
> + $api ne "name" &&
> + $table ne "virStateDriver" &&
> + !exists $acls{$impl} &&
> + !exists $whitelist{$api}) {
> + print "$ARGV:$. Missing ACL check 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;
> +}
> +
> +exit $status;
>
You need to update the ACL checks as I'm getting these errors:
libxl/libxl_driver.c:3771 Mismatch check 'virDomainLookupByIDEnsureACL' for function 'libxlDomainCreateXML'
libxl/libxl_driver.c:3802 Mismatch check 'virDomainLookupByUUIDEnsureACL' for function 'libxlDomainLookupByID'
libxl/libxl_driver.c:3831 Mismatch check 'virDomainLookupByNameEnsureACL' for function 'libxlDomainLookupByUUID'
libxl/libxl_driver.c:6775 Missing ACL check in function 'libxlDomainLookupByName' for 'domainLookupByName'
GEN check-augeas-lockd
xen/xen_driver.c:113189 Mismatch check 'virDomainGetSchedulerParametersEnsureACL' for function 'xenUnifiedDomainGetSchedulerParametersFlags'
xen/xen_driver.c:113800 Missing ACL check in function 'xenUnifiedDomainRestore' for 'domainRestore'
xen/xen_driver.c:113801 Missing ACL check in function 'xenUnifiedDomainRestoreFlags' for 'domainRestoreFlags'
xen/xen_driver.c:113831 Missing ACL check in function 'xenUnifiedDomainMigratePrepare' for 'domainMigratePrepare'
xen/xen_driver.c:113841 Missing ACL check in function 'xenUnifiedNodeDeviceDettach' for 'nodeDeviceDettach'
xen/xen_driver.c:113842 Missing ACL check in function 'xenUnifiedNodeDeviceDetachFlags' for 'nodeDeviceDetachFlags'
xen/xen_driver.c:113844 Missing ACL check in function 'xenUnifiedNodeDeviceReset' for 'nodeDeviceReset'
xen/xen_driver.c:113847 Missing ACL check in function 'xenUnifiedDomainIsActive' for 'domainIsActive'
xen/xen_driver.c:113848 Missing ACL check in function 'xenUnifiedDomainIsPersistent' for 'domainIsPersistent'
xen/xen_driver.c:113849 Missing ACL check in function 'xenUnifiedDomainIsUpdated' for 'domainIsUpdated'
xen/xen_driver.c:113852 Missing ACL check in function 'xenUnifiedDomainOpenConsole' for 'domainOpenConsole'
make[3]: *** [check-aclrules] Error 1
make[3]: Leaving directory `/home/zippy/work/libvirt/libvirt.git/src'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/home/zippy/work/libvirt/libvirt.git/src'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/home/zippy/work/libvirt/libvirt.git/src'
make: *** [check-recursive] Error 1
Michal
More information about the libvir-list
mailing list