Re: [PATCH 19/19] Add validation that all APIs contain ACL checks

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

 



On 19.06.2013 19:01, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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

--
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]