[apparmor] [PATCH 04/11] Fix the x intersection consistency test

[Steve Beattie]

On Wed, Mar 07, 2012 at 11:52:32AM -0800, John Johansen wrote:
> On 03/07/2012 10:27 AM, Steve Beattie wrote:
> > On Wed, Mar 07, 2012 at 06:17:23AM -0800, John Johansen wrote:
> >> The in x intersection consistency test for minimization was failing because
> >> it was screening off the AA_MAY_EXEC permission before passing the exec
> >> information to the consistency test fn.  This resulted in the consistency
> >> test fn not testing the consistency because it treated the permission set
> >> as not having x permissions.
> >>
> >> Signed-off-by: John Johansen <john.johansen at canonical.com>
> > 
> > It *looks* sensible, but applying this patch plus the first 2 (but
> > not the third, which doesn't apply for some reason) results in the
> > parser testsuite having 11 new failures; without this patch, there's
> > no failures. I have yet to look into why.
> > 
> Yes it will, because this particular bug is part of the reason the next one
> wasn't caught sooner.  Fixing it turns up a lot of errors happening

Hrm, okay.

Acked-By: Steve Beattie <sbeattie at ubuntu.com>

Sorry for the delay, but I got confused by the added testcase in
this patch, in that it doesn't actually demonstrate the issue this
patch is supposed to address. (There is no change in behavior of the
parser for the testcase with or without the parser change in this patch
applied. It's a fine testcase to have, assuming it's not overlapping
with another testcase we have.) Do you have such a testcase?

> In fact you won't get the complete test suite passing until the very last
> patch.
> 
> >> ---
> >>  parser/immunix.h                             |    3 +++
> >>  parser/libapparmor_re/hfa.h                  |    8 ++++----
> >>  parser/tst/simple_tests/xtrans/x-conflict.sd |   11 +++++++++++
> >>  3 files changed, 18 insertions(+), 4 deletions(-)
> >>  create mode 100644 parser/tst/simple_tests/xtrans/x-conflict.sd
> >>
> >> diff --git a/parser/immunix.h b/parser/immunix.h
> >> index 72446fc..8dc157a 100644
> >> --- a/parser/immunix.h
> >> +++ b/parser/immunix.h
> >> @@ -96,6 +96,9 @@
> >>  
> >>  #define ALL_AA_EXEC_TYPE		(AA_USER_EXEC_TYPE | AA_OTHER_EXEC_TYPE)
> >>  
> >> +#define ALL_USER_EXEC			(AA_USER_EXEC | AA_USER_EXEC_TYPE)
> >> +#define ALL_OTHER_EXEC			(AA_OTHER_EXEC | AA_OTHER_EXEC_TYPE)
> >> +
> >>  #define AA_LINK_BITS			((AA_MAY_LINK << AA_USER_SHIFT) | \
> >>  					 (AA_MAY_LINK << AA_OTHER_SHIFT))
> >>  
> >> diff --git a/parser/libapparmor_re/hfa.h b/parser/libapparmor_re/hfa.h
> >> index c77905d..a35becb 100644
> >> --- a/parser/libapparmor_re/hfa.h
> >> +++ b/parser/libapparmor_re/hfa.h
> >> @@ -50,8 +50,8 @@ public:
> >>  	{
> >>  		deny |= rhs.deny;
> >>  
> >> -		if (!is_merged_x_consistent(allow & AA_USER_EXEC_TYPE,
> >> -					    rhs.allow & AA_USER_EXEC_TYPE)) {
> >> +		if (!is_merged_x_consistent(allow & ALL_USER_EXEC,
> >> +					    rhs.allow & ALL_USER_EXEC)) {
> >>  			if ((exact & AA_USER_EXEC_TYPE) &&
> >>  			    !(rhs.exact & AA_USER_EXEC_TYPE)) {
> >>  				/* do nothing */
> >> @@ -64,8 +64,8 @@ public:
> >>  		} else
> >>  			allow |= rhs.allow & AA_USER_EXEC_TYPE;
> >>  
> >> -		if (!is_merged_x_consistent(allow & AA_OTHER_EXEC_TYPE,
> >> -					    rhs.allow & AA_OTHER_EXEC_TYPE)) {
> >> +		if (!is_merged_x_consistent(allow & ALL_OTHER_EXEC,
> >> +					    rhs.allow & ALL_OTHER_EXEC)) {
> >>  			if ((exact & AA_OTHER_EXEC_TYPE) &&
> >>  			    !(rhs.exact & AA_OTHER_EXEC_TYPE)) {
> >>  				/* do nothing */
> >> diff --git a/parser/tst/simple_tests/xtrans/x-conflict.sd b/parser/tst/simple_tests/xtrans/x-conflict.sd
> >> new file mode 100644
> >> index 0000000..92215f9
> >> --- /dev/null
> >> +++ b/parser/tst/simple_tests/xtrans/x-conflict.sd
> >> @@ -0,0 +1,11 @@
> >> +#
> >> +#=DESCRIPTION test for conflict resolution in minimization phase of dfa gen
> >> +#=EXRESULT FAIL
> >> +#=TODO
> >> +#
> >> +/usr/bin/foo {
> >> +  /b* px,
> >> +  /* Pixr,
> >> +  /a* Cx -> foo,
> >> +}
> >> +
> >> -- 
> >> 1.7.9
> >>
> >>
> >> -- 
> >> AppArmor mailing list
> >> AppArmor at lists.ubuntu.com
> >> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
> > 
> > 
> > 
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20120308/a2500f0d/attachment.pgp>