[libvirt] [PATCH 0/6] sVirt AppArmor security driver

[Jamie Strandboge]

On Fri, 04 Sep 2009, Daniel Veillard wrote:

> On Fri, Sep 04, 2009 at 12:23:33PM -0500, Jamie Strandboge wrote:
> > This patch series implements the AppArmor security driver for sVirt.
> > This implementation was developed for the Ubuntu AppArmorLibvirtProfile
> > specification[1], but is general enough for any AppArmor deployment
> > (such as Ubuntu, *SUSE and Mandriva).
> > 
> > This patch has seen quite a bit of real world testing in Ubuntu 9.10
> > (our development release) in our 0.7.0-1ubuntu3 package. I did make a
> > few small changes after going through HACKING, but mostly I got the
> > tests going and added documentation.
> 
>   Could you triple check that the make syntax-check run clean with
> your patches applied, because a very quick glimpse shows up a number
> of malloc() which we don't allow for example. It may be because you
> didn't enter the files in the SCM but the checks should be done anyway
> on the new files please.
> 

I noticed that syntax-check wouldn't scan the new files unless I did a
git commit first. I did not run 'syntax-check' after adding the
documention examples, and just found that examples/apparmor/libvirt-qemu
had a trailing blank line (I was a bit surprised syntax-check checked
these files (attached is an updated docs patch)).

I can demonstrate that the files are being checked by removing the
'#include <config.h>' line in security_apparmor.c and virt-aa-helper.c
and syntax-check failing:
$ make syntax-check
..
require_config_h
src/virt-aa-helper.c
maint.mk: the above files do not include <config.h>
make: *** [sc_require_config_h] Error 1
$ make syntax-check
..
require_config_h
src/security_apparmor.c
maint.mk: the above files do not include <config.h>
make: *** [sc_require_config_h] Error 1

I also knew the files were checked because, for example, I used
str[n]cmp() in a couple places and prohibit_strcmp_and_strncmp caught
them. I changed those to STR* before submission.

I don't see that 'syntax-check' enforces not using *alloc, but I did
read in HACKING that *alloc are deprecated, though I had already written
the code before reading it. I do see other examples of *alloc in the
codebase, and I do properly check the return code of all calls to *alloc.
If you insist on those calls being replaced, I can do that, but I didn't
before submitting because I knew the current calls were correct and
didn't want to introduce bugs into the code.


Here is what I do after a 'git pull':

$ cd libvirt
$ git checkout
<apply patches> (using the attached patch_5_docs.patch)
$ chmod 755 ./tests/virt-aa-helper-test
$ git add .
$ git commit
...
[master 4b08401] add apparmor security driver
 18 files changed, 1966 insertions(+), 3 deletions(-)
 create mode 100644 examples/apparmor/TEMPLATE
 create mode 100644 examples/apparmor/libvirt-qemu
 create mode 100644 examples/apparmor/usr.bin.virt-aa-helper
 create mode 100644 examples/apparmor/usr.sbin.libvirtd
 create mode 100644 src/security_apparmor.c
 create mode 100644 src/security_apparmor.h
 create mode 100644 src/virt-aa-helper.c
 create mode 100644 tests/secaatest.c
 create mode 100755 tests/virt-aa-helper-test
$ ./autogen.sh
$ ./configure --enable-compile-warnings=error
$ make check
..
  CC     security_apparmor.lo
..
  CC     virt_aa_helper-virt-aa-helper.o
  CCLD   virt-aa-helper
..
  CC     secaatest.o
  CCLD   secaatest
..
PASS: virt-aa-helper-test
..
PASS: secaatest
..
(no errors, and I see my two added tests passed)
$ make syntax-check
void_if_before_free
cast_of_argument_to_free
cast_of_x_alloc_return_value
prohibit_strcmp
error_message_warn_fatal
error_message_period
prohibit_have_config_h
require_config_h
require_config_h_first
prohibit_HAVE_MBRTOWC
prohibit_assert_without_use
prohibit_getopt_without_use
prohibit_long_options_without_use
prohibit_inttostr_without_use
prohibit_error_without_use
prohibit_safe_read_without_use
prohibit_argmatch_without_use
prohibit_root_dev_ino_without_use
prohibit_c_ctype_without_use
prohibit_signal_without_use
changelog
the_the
trailing_blank
unmarked_diagnostics
prohibit_cvs_keyword
proper_name_utf8_requires_ICONV
redundant_const
const_long_option
makefile_TAB_only_indentation
m4_quote_check
po_check
copyright_check
avoid_write
prohibit_strcmp_and_strncmp
prohibit_asprintf
prohibit_VIR_ERR_NO_MEMORY
prohibit_nonreentrant
prohibit_ctype_h
TAB_in_indentation
avoid_ctype_macros
prohibit_virBufferAdd_with_string_literal
prohibit_gethostby
libvirt_unmarked_diagnostics
prohibit_trailing_blank_lines


Thanks!

Jamie

-- 
Jamie Strandboge             | http://www.canonical.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch_5_docs.patch
Type: text/x-diff
Size: 8797 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090904/7130e462/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090904/7130e462/attachment-0001.sig>