[apparmor] [PATCH 2/2] tests: Mount without updating mtab in mount.sh

[Tyler Hicks]

The mount.sh script mixes calls to the regression test 'mount' binary
and /sbin/mount. This can result in stale mtab entries being left around
after a test run because /sbin/mount adds an mtab entry but the test
'mount' binary, which is also used for unmounting, does not remove mtab
entries.

To solve this problem, the -n option is passed to /sbin/mount so that it
doesn't add an mtab entry when mounting.

Signed-off-by: Tyler Hicks tyhi...@canonical.com
---
 tests/regression/apparmor/mount.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/regression/apparmor/mount.sh 
b/tests/regression/apparmor/mount.sh
index b4951a8..86bfecb 100755
--- a/tests/regression/apparmor/mount.sh
+++ b/tests/regression/apparmor/mount.sh
@@ -33,8 +33,8 @@ loop_device=unset
 fstype=ext2
 
 setup_mnt() {
-   /bin/mount -t${fstype} ${loop_device} ${mount_point}
-#  /bin/mount -t${fstype} ${loop_device} ${mount_bad}
+   /bin/mount -n -t${fstype} ${loop_device} ${mount_point}
+#  /bin/mount -n -t${fstype} ${loop_device} ${mount_bad}
 }
 remove_mnt() {
mountpoint -q ${mount_point}
-- 
1.9.1


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

[apparmor] [patch] parser language tests: force using a features file

[Steve Beattie]

With the recent addition of features like ptrace and signals that
give warnings and then ignore the subset of rules when the features
directory indicates that the kernel does not support mediating such
features, at least one of the language tests fails in a chroot
environment where the apparmor securityfs tree is not mounted
inside it.

To compensate, a features file containing the current supported features
is included, and the simple.pl test driver is modified to pass it as an
argument to the parser, so that it will act as if the environment
supports all our current features.

A simple python script is included that was used to generate the
features file based on the current feature set. I'm not sure how to
keep it up to date in an automated fashion as we add more supported
features, however. (make check can't just fail on the features
directory being different; we want builds and tests to run successfully
on older releases where the kernel may not support mediating all the
features we include.)

Signed-off-by: Steve Beattie st...@nxnw.org
---
 parser/tst/features_files/features.all |   49 +
 parser/tst/mk_features_file.py |   37 
 parser/tst/simple.pl   |2 -
 3 files changed, 87 insertions(+), 1 deletion(-)

Index: b/parser/tst/features_files/features.all
===
--- /dev/null
+++ b/parser/tst/features_files/features.all
@@ -0,0 +1,49 @@
+dbus {mask {acquire send receive
+}
+}
+signal {mask {hup int quit ill trap abrt bus fpe kill usr1 segv usr2 pipe alrm 
term stkflt chld cont stop stp ttin ttou urg xcpu xfsz vtalrm prof winch io pwr 
sys emt lost
+}
+}
+ptrace {mask {read trace
+}
+}
+caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid 
setuid setpcap linux_immutable net_bind_service net_broadcast net_admin net_raw 
ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace sys_pacct 
sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config mknod lease 
audit_write audit_control setfcap mac_override mac_admin syslog wake_alarm 
block_suspend
+}
+}
+rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks 
sigpending msgqueue nice rtprio rttime
+}
+}
+capability {0xff
+}
+namespaces {pivot_root {yes
+}
+profile {yes
+}
+}
+mount {mask {mount umount
+}
+}
+network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge 
atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc rds 
sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet 
ieee802154 caif alg nfc vsock max
+}
+}
+file {mask {create read write exec append mmap_exec link lock
+}
+}
+domain {change_profile {yes
+}
+change_onexec {yes
+}
+change_hatv {yes
+}
+change_hat {yes
+}
+}
+policy {set_load {yes
+}
+versions {v6 {yes
+}
+v5 {yes
+}
+}
+}
+
Index: b/parser/tst/simple.pl
===
--- a/parser/tst/simple.pl
+++ b/parser/tst/simple.pl
@@ -81,7 +81,7 @@ sub test_profile {
 # child
 open(STDOUT, /dev/null) or die Failed to redirect STDOUT;
 open(STDERR, /dev/null) or die Failed to redirect STDERR;
-exec($config{'parser'}, -S, -I, $config{'includedir'}) or die 
Bail out! couldn't open parser;
+exec($config{'parser'}, -M, features_files/features.all, -S, -I, 
$config{'includedir'}) or die Bail out! couldn't open parser;
 # noreturn
   }
 
Index: b/parser/tst/mk_features_file.py
===
--- /dev/null
+++ b/parser/tst/mk_features_file.py
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+# --
+#
+#   Copyright (C) 2014 Canonical Ltd.
+#   Author: Steve Beattie st...@nxnw.org
+#
+#   This program is free software; you can redistribute it and/or
+#   modify it under the terms of version 2 of the GNU General Public
+#   License published by the Free Software Foundation.
+#
+# --
+
+from testlib import read_features_dir
+from argparse import ArgumentParser
+import os
+from sys import stderr, exit
+
+DEFAULT_FEATURES_DIR='/sys/kernel/security/apparmor/features'
+
+def main():
+p = ArgumentParser()
+
+p.add_argument('fdir', action=store, nargs='?', metavar=features_dir,
+   default=DEFAULT_FEATURES_DIR, help=path to features 
directory)
+config = p.parse_args()
+
+if not os.path.exists(config.fdir):
+print('Unable to find apparmor features directory %s' % config.fdir, 
file=stderr)
+return 1
+
+features = read_features_dir(config.fdir)
+print(features)
+
+return 0
+
+if __name__ == __main__:
+exit(main())

-- 
Steve Beattie
sbeat...@ubuntu.com
http://NxNW.org/~steve/


signature.asc
Description: Digital signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or 

Re: [apparmor] [PATCH 2/2] tests: Mount without updating mtab in mount.sh

[Steve Beattie]

On Thu, Apr 24, 2014 at 01:06:06AM -0500, Tyler Hicks wrote:
 The mount.sh script mixes calls to the regression test 'mount' binary
 and /sbin/mount. This can result in stale mtab entries being left around
 after a test run because /sbin/mount adds an mtab entry but the test
 'mount' binary, which is also used for unmounting, does not remove mtab
 entries.
 
 To solve this problem, the -n option is passed to /sbin/mount so that it
 doesn't add an mtab entry when mounting.
 
 Signed-off-by: Tyler Hicks tyhi...@canonical.com

Acked-by: Steve Beattie st...@nxnw.org

Thanks!


-- 
Steve Beattie
sbeat...@ubuntu.com
http://NxNW.org/~steve/


signature.asc
Description: Digital signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Re: [apparmor] [PATCH 1/2] tests: Fix mount.sh test error

[Seth Arnold]

On Thu, Apr 24, 2014 at 01:06:05AM -0500, Tyler Hicks wrote:
 The end of the mount.sh regression test script contained cleanup
 commands to unmount and detach the loop device used for testing.
 However, the second losetup command fails and, with the recent
 regression test suite fix to not ignore failed shell commands, an error
 is triggered at the end of the test run.
 
 Additionally, these cleanup commands are not ran when the test fails
 during the test run and an immediate exit is requested upon failure
 (with the -r flag).
 
 This patch fixes and moves the cleanup logic into a function that is
 assigned to do_onexit so that the cleanup is always performed at exit
 and the test can run successfully.
 
 Signed-off-by: Tyler Hicks tyhi...@canonical.com

Acked-by: Seth Arnold seth.arn...@canonical.com

Thanks

 ---
  tests/regression/apparmor/mount.sh | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)
 
 diff --git a/tests/regression/apparmor/mount.sh 
 b/tests/regression/apparmor/mount.sh
 index fa3e931..b4951a8 100755
 --- a/tests/regression/apparmor/mount.sh
 +++ b/tests/regression/apparmor/mount.sh
 @@ -47,6 +47,15 @@ remove_mnt() {
   fi
  }
  
 +mount_cleanup() {
 + remove_mnt  /dev/null
 + if [ $loop_device != unset ]
 + then
 + /sbin/losetup -d ${loop_device}  /dev/null
 + fi
 +}
 +do_onexit=mount_cleanup
 +
  dd if=/dev/zero of=${mount_file} bs=1024 count=512 2 /dev/null
  /sbin/mkfs -t${fstype} -F ${mount_file}  /dev/null 2 /dev/null
  /bin/mkdir ${mount_point}
 @@ -163,8 +172,3 @@ else
  fi
  
  #need tests for move mount, remount, bind mount, chroot
 -
 -# cleanup, umount file
 -/bin/umount ${loop_device}  /dev/null 2 /dev/null  || /sbin/losetup -d 
 ${loop_device}  /dev/null 2 /dev/null
 -
 -/sbin/losetup -d ${loop_device}  /dev/null 2 /dev/null
 -- 
 1.9.1
 
 
 -- 
 AppArmor mailing list
 AppArmor@lists.ubuntu.com
 Modify settings or unsubscribe at: 
 https://lists.ubuntu.com/mailman/listinfo/apparmor
 


signature.asc
Description: Digital signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Re: [apparmor] [PATCH 2/2] tests: Mount without updating mtab in mount.sh

[Seth Arnold]

On Thu, Apr 24, 2014 at 01:06:06AM -0500, Tyler Hicks wrote:
 The mount.sh script mixes calls to the regression test 'mount' binary
 and /sbin/mount. This can result in stale mtab entries being left around
 after a test run because /sbin/mount adds an mtab entry but the test
 'mount' binary, which is also used for unmounting, does not remove mtab
 entries.
 
 To solve this problem, the -n option is passed to /sbin/mount so that it
 doesn't add an mtab entry when mounting.
 
 Signed-off-by: Tyler Hicks tyhi...@canonical.com

Acked-by: Seth Arnold seth.arn...@canonical.com

Thanks

 ---
  tests/regression/apparmor/mount.sh | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/tests/regression/apparmor/mount.sh 
 b/tests/regression/apparmor/mount.sh
 index b4951a8..86bfecb 100755
 --- a/tests/regression/apparmor/mount.sh
 +++ b/tests/regression/apparmor/mount.sh
 @@ -33,8 +33,8 @@ loop_device=unset
  fstype=ext2
  
  setup_mnt() {
 - /bin/mount -t${fstype} ${loop_device} ${mount_point}
 -#/bin/mount -t${fstype} ${loop_device} ${mount_bad}
 + /bin/mount -n -t${fstype} ${loop_device} ${mount_point}
 +#/bin/mount -n -t${fstype} ${loop_device} ${mount_bad}
  }
  remove_mnt() {
   mountpoint -q ${mount_point}
 -- 
 1.9.1
 
 
 -- 
 AppArmor mailing list
 AppArmor@lists.ubuntu.com
 Modify settings or unsubscribe at: 
 https://lists.ubuntu.com/mailman/listinfo/apparmor
 


signature.asc
Description: Digital signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Re: [apparmor] [patch] parser language tests: force using a features file

[Seth Arnold]

On Thu, Apr 24, 2014 at 12:09:42AM -0700, Steve Beattie wrote:
 With the recent addition of features like ptrace and signals that
 give warnings and then ignore the subset of rules when the features
 directory indicates that the kernel does not support mediating such
 features, at least one of the language tests fails in a chroot
 environment where the apparmor securityfs tree is not mounted
 inside it.
 
 To compensate, a features file containing the current supported features
 is included, and the simple.pl test driver is modified to pass it as an
 argument to the parser, so that it will act as if the environment
 supports all our current features.
 
 A simple python script is included that was used to generate the
 features file based on the current feature set. I'm not sure how to
 keep it up to date in an automated fashion as we add more supported
 features, however. (make check can't just fail on the features
 directory being different; we want builds and tests to run successfully
 on older releases where the kernel may not support mediating all the
 features we include.)

This looks like a useful start but I wouldn't be surprised if there's more
pain to be had before we find a solution that makes sense for the
optionally supported mediations. The first thing that comes to mind is
maintaining two piles of expected outcomes for each feature, but I
really don't care for that much.

 Signed-off-by: Steve Beattie st...@nxnw.org

Anyway, this code as-is looks like a useful improvement while we can
consider future options.

Acked-by: Seth Arnold seth.arn...@canonical.com

Thanks


 ---
  parser/tst/features_files/features.all |   49 
 +
  parser/tst/mk_features_file.py |   37 
  parser/tst/simple.pl   |2 -
  3 files changed, 87 insertions(+), 1 deletion(-)
 
 Index: b/parser/tst/features_files/features.all
 ===
 --- /dev/null
 +++ b/parser/tst/features_files/features.all
 @@ -0,0 +1,49 @@
 +dbus {mask {acquire send receive
 +}
 +}
 +signal {mask {hup int quit ill trap abrt bus fpe kill usr1 segv usr2 pipe 
 alrm term stkflt chld cont stop stp ttin ttou urg xcpu xfsz vtalrm prof winch 
 io pwr sys emt lost
 +}
 +}
 +ptrace {mask {read trace
 +}
 +}
 +caps {mask {chown dac_override dac_read_search fowner fsetid kill setgid 
 setuid setpcap linux_immutable net_bind_service net_broadcast net_admin 
 net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot sys_ptrace 
 sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time sys_tty_config 
 mknod lease audit_write audit_control setfcap mac_override mac_admin syslog 
 wake_alarm block_suspend
 +}
 +}
 +rlimit {mask {cpu fsize data stack core rss nproc nofile memlock as locks 
 sigpending msgqueue nice rtprio rttime
 +}
 +}
 +capability {0xff
 +}
 +namespaces {pivot_root {yes
 +}
 +profile {yes
 +}
 +}
 +mount {mask {mount umount
 +}
 +}
 +network {af_mask {unspec unix local inet ax25 ipx appletalk netrom bridge 
 atmpvc x25 inet6 rose netbeui security key netlink packet ash econet atmsvc 
 rds sna irda pppox wanpipe llc ib can tipc bluetooth iucv rxrpc isdn phonet 
 ieee802154 caif alg nfc vsock max
 +}
 +}
 +file {mask {create read write exec append mmap_exec link lock
 +}
 +}
 +domain {change_profile {yes
 +}
 +change_onexec {yes
 +}
 +change_hatv {yes
 +}
 +change_hat {yes
 +}
 +}
 +policy {set_load {yes
 +}
 +versions {v6 {yes
 +}
 +v5 {yes
 +}
 +}
 +}
 +
 Index: b/parser/tst/simple.pl
 ===
 --- a/parser/tst/simple.pl
 +++ b/parser/tst/simple.pl
 @@ -81,7 +81,7 @@ sub test_profile {
  # child
  open(STDOUT, /dev/null) or die Failed to redirect STDOUT;
  open(STDERR, /dev/null) or die Failed to redirect STDERR;
 -exec($config{'parser'}, -S, -I, $config{'includedir'}) or die 
 Bail out! couldn't open parser;
 +exec($config{'parser'}, -M, features_files/features.all, -S, 
 -I, $config{'includedir'}) or die Bail out! couldn't open parser;
  # noreturn
}
  
 Index: b/parser/tst/mk_features_file.py
 ===
 --- /dev/null
 +++ b/parser/tst/mk_features_file.py
 @@ -0,0 +1,37 @@
 +#!/usr/bin/env python3
 +# --
 +#
 +#   Copyright (C) 2014 Canonical Ltd.
 +#   Author: Steve Beattie st...@nxnw.org
 +#
 +#   This program is free software; you can redistribute it and/or
 +#   modify it under the terms of version 2 of the GNU General Public
 +#   License published by the Free Software Foundation.
 +#
 +# --
 +
 +from testlib import read_features_dir
 +from argparse import ArgumentParser
 +import os
 +from sys import stderr, exit
 +
 +DEFAULT_FEATURES_DIR='/sys/kernel/security/apparmor/features'
 +
 +def main():
 +p = ArgumentParser()
 +

Re: [apparmor] [patch] parser language tests: force using a features file

[Steve Beattie]

On Thu, Apr 24, 2014 at 12:06:14PM -0700, Seth Arnold wrote:
 On Thu, Apr 24, 2014 at 12:09:42AM -0700, Steve Beattie wrote:
  A simple python script is included that was used to generate the
  features file based on the current feature set. I'm not sure how to
  keep it up to date in an automated fashion as we add more supported
  features, however. (make check can't just fail on the features
  directory being different; we want builds and tests to run successfully
  on older releases where the kernel may not support mediating all the
  features we include.)
 
 This looks like a useful start but I wouldn't be surprised if there's more
 pain to be had before we find a solution that makes sense for the
 optionally supported mediations.

I agree, more pain here is probably coming.

 The first thing that comes to mind is maintaining two piles of
 expected outcomes for each feature, but I really don't care for
 that much.

Interestingly, with the current set of committed language tests (I have
more forthcoming), only one ptrace test failed (ptrace/bad_10.sd). This
is because most of the parsing occurs before determining what mediation
features are supported by the kernel; only stuff like regex conversions
(and the error checks that go with them) occur after the features
check. The parser is (currently) almost entirely static with regards
to what language elements it will accept as input.

That said, less parser code is being exercised by the language tests
if when features are detected as not being supported, as some of the
post processing won't occur. But exercising the post processing code
completely is not the primary intent of the language tests; it's a
better target for the regression test suite.

  Signed-off-by: Steve Beattie st...@nxnw.org
 
 Anyway, this code as-is looks like a useful improvement while we can
 consider future options.
 
 Acked-by: Seth Arnold seth.arn...@canonical.com

Thanks for the review.

-- 
Steve Beattie
sbeat...@ubuntu.com
http://NxNW.org/~steve/


signature.asc
Description: Digital signature
-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Re: [apparmor] [patch] parser language tests: force using a features file

[John Johansen]

On 04/24/2014 12:27 PM, Steve Beattie wrote:
 On Thu, Apr 24, 2014 at 12:06:14PM -0700, Seth Arnold wrote:
 On Thu, Apr 24, 2014 at 12:09:42AM -0700, Steve Beattie wrote:
 A simple python script is included that was used to generate the
 features file based on the current feature set. I'm not sure how to
 keep it up to date in an automated fashion as we add more supported
 features, however. (make check can't just fail on the features
 directory being different; we want builds and tests to run successfully
 on older releases where the kernel may not support mediating all the
 features we include.)

 This looks like a useful start but I wouldn't be surprised if there's more
 pain to be had before we find a solution that makes sense for the
 optionally supported mediations.
 
 I agree, more pain here is probably coming.
 
oh a lot more pain by the time I am done with it :)

 The first thing that comes to mind is maintaining two piles of
 expected outcomes for each feature, but I really don't care for
 that much.
 
so this is going to be a necessity, and it will actually be more than 2 out
comes. Once the policy versioning hits it will be the combination of
  policy version for feature (yes this is really needed)
  kernel abi support
  parser support for feature

we can probably knock out the parser support for feature, as the tests
get updated with the parser but that still at least 4 combinations we need
to test and have expected out comes for

 Interestingly, with the current set of committed language tests (I have
 more forthcoming), only one ptrace test failed (ptrace/bad_10.sd). This
yes it is interesting, and a little disappointing our test coverage is
not thorough enough to have broken more

 is because most of the parsing occurs before determining what mediation
 features are supported by the kernel; only stuff like regex conversions
 (and the error checks that go with them) occur after the features
 check. The parser is (currently) almost entirely static with regards
 to what language elements it will accept as input.
right this will change at some point. The basic plan for the parser is
to make it a hybrid of recursive decent and lalr.  We are going to
start passing the block context down to the parsing as soon as I can
get those patches finished. This will give us context to handle some
things within the parse better.

Also its required to pick-up the user defined rule templates I keep
babbling about. Which are how I am planning to support new features
in policy on old parsers. Basically a way of adding rule patterns
for I know this rule exists and it looks like this so you can ignore it
if you don't understand it.

 
 That said, less parser code is being exercised by the language tests
 if when features are detected as not being supported, as some of the
 post processing won't occur. But exercising the post processing code
 completely is not the primary intent of the language tests; it's a
 better target for the regression test suite.
 
yep


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor