On 12.05.2016 09:09, Peter Krempa wrote: > On Tue, May 10, 2016 at 17:24:14 +0200, Michal Privoznik wrote: >> This script will check output generated by virtestmock against a >> white list. All non matching records found are printed out. So >> far, the white list is rather sparse at the moment. >> This test should be ran only after all other tests finished, and >> should cleanup the temporary file before their execution. Because >> I'm unable to reflect these requirements in Makefile.am >> correctly, I've introduced new target 'check-access' under which >> this test is available. >> >> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >> --- >> Makefile.am | 3 ++ >> tests/Makefile.am | 13 +++++ >> tests/check-file-access.pl | 106 ++++++++++++++++++++++++++++++++++++++++ >> tests/file_access_whitelist.txt | 23 +++++++++ >> 4 files changed, 145 insertions(+) >> create mode 100755 tests/check-file-access.pl >> create mode 100644 tests/file_access_whitelist.txt >> >> diff --git a/Makefile.am b/Makefile.am >> index c52a4cb..da07e6c 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -67,6 +67,9 @@ rpm: clean >> >> check-local: all tests >> >> +check-access: >> + @($(MAKE) $(AM_MAKEFLAGS) -C tests check-access) >> + >> cov: clean-cov >> $(MKDIR_P) $(top_builddir)/coverage >> $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \ >> diff --git a/tests/Makefile.am b/tests/Makefile.am >> index da68f2e..7cd641d 100644 >> --- a/tests/Makefile.am >> +++ b/tests/Makefile.am >> @@ -451,6 +451,19 @@ test_libraries += virusbmock.la \ >> $(NULL) >> endif WITH_LINUX >> >> +if WITH_LINUX >> +check-access: file-access-clean >> + $(MAKE) $(AM_MAKEFLAGS) check >> + $(PERL) check-file-access.pl > > I added '| sort -u' while trying this. There's a lot of multiplicated > lines. Okay. > >> + >> +file-access-clean: >> + > test_file_access.txt >> +endif WITH_LINUX >> + >> +EXTRA_DIST += \ >> + check-file-access.pl \ >> + file_access_whitelist.txt >> + >> if WITH_TESTS >> noinst_PROGRAMS = $(test_programs) $(test_helpers) >> noinst_LTLIBRARIES = $(test_libraries) >> diff --git a/tests/check-file-access.pl b/tests/check-file-access.pl >> new file mode 100755 >> index 0000000..6e59201 >> --- /dev/null >> +++ b/tests/check-file-access.pl >> @@ -0,0 +1,106 @@ >> +#!/usr/bin/perl -w >> +# >> +# Copyright (C) 2016 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 is supposed to check test_file_access.txt file and >> +# warn about file accesses outside our working tree. >> +# >> +# >> + >> +use strict; >> +use warnings; >> + >> +my $access_file = "test_file_access.txt"; >> +my $whitelist_file = "file_access_whitelist.txt"; >> + >> +my @files; >> +my @whitelist; >> + >> +open FILE, "<", $access_file or die "Unable to open $access_file: $!"; >> +while (<FILE>) { >> + chomp; >> + if (/^\s*#.*$/) { >> + # comment > > Will this file ever contain comments? I guess not. I can remove it. > >> + } elsif (/^(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { >> + my %rec; >> + ${rec}{path} = $1; >> + ${rec}{progname} = $2; >> + if (defined $4) { >> + ${rec}{testname} = $4; >> + } >> + push (@files, \%rec); >> + } else { >> + die "Malformed line $_"; >> + } >> +} >> +close FILE; >> + >> +open FILE, "<", $whitelist_file or die "Unable to open $whitelist_file: $!"; >> +while (<FILE>) { >> + chomp; >> + if (/^\s*#.*$/) { >> + # comment >> + } elsif (/^(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/) { >> + my %rec; >> + ${rec}{path} = $1; >> + if (defined $3) { >> + ${rec}{progname} = $3; >> + } >> + if (defined $5) { >> + ${rec}{testname} = $5; >> + } >> + push (@whitelist, \%rec); >> + } else { >> + die "Malformed line $_"; >> + } >> +} >> +close FILE; >> + >> +# Now we should check if %traces is included in $whitelist. For >> +# now checking just keys is sufficient >> +my $error = 0; >> +for my $file (@files) { >> + my $match = 0; >> + >> + for my $rule (@whitelist) { >> + if (not %${file}{path} =~ m/^$rule->{path}$/) { >> + next; >> + } >> + >> + if (defined %${rule}{progname} and >> + not %${file}{progname} =~ m/^$rule->{progname}$/) { >> + next; >> + } >> + >> + if (defined %${rule}{testname} and >> + defined %${file}{testname} and >> + not %${file}{testname} =~ m/^$rule->{testname}$/) { >> + next; >> + } >> + >> + $match = 1; >> + } >> + >> + if (not $match) { >> + $error = 1; >> + print "$file->{path}: $file->{progname}"; >> + print ": $file->{testname}" if defined %${file}{testname}; >> + print "\n"; >> + } >> +} >> + >> +$error; > > perl complains that the above line is useless. Also the script doesn't > return failure if it finds files out of the build path. > > >> diff --git a/tests/file_access_whitelist.txt b/tests/file_access_whitelist.txt >> new file mode 100644 >> index 0000000..850b285 >> --- /dev/null >> +++ b/tests/file_access_whitelist.txt >> @@ -0,0 +1,23 @@ >> +# This is a whitelist that allows accesses to files not in our >> +# build directory nor source directory. The records are in the >> +# following format: >> +# >> +# $path: $progname: $testname >> +# >> +# All these three are evaluated as perl RE. So to allow /dev/sda >> +# and /dev/sdb, you can just '/dev/sd[a-b]', or to allow >> +# /proc/$pid/status you can '/proc/\d+/status' and so on. >> +# Moreover, $progname and $testname can be empty, in which which >> +# case $path is allowed for all tests. >> + >> +/bin/cat: sysinfotest >> +/bin/dirname: sysinfotest: x86 sysinfo >> +/bin/sleep: commandtest >> +/bin/true: commandtest >> +/dev/null >> +/dev/urandom >> +/etc/hosts >> +/proc/\d+/status > > My test run showed that there is a looot of stuff to add unfortunately. > > Looks good to me, but my perl knowledge is rather poor. > Correct. There's still plenty of rules to add. That's why the perl is not returning any error. Not just yet. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list