On Thu, Mar 19, 2015 at 03:52:14AM -0700, John Johansen wrote:
> This adds several new equality tests and turned up a couple of more
> bugs
> https://launchpad.net/bugs/1433829
> https://launchpad.net/bugs/1434018
> 
> In addition to adding the new tests, it changes the output of the
> equality tests, so that if the $verbose variable is not set successful
> tests only output a period, with failed tests outputing the full
> info.  If verbose is set the full test info is output as before.
> 
> Signed-off-by: John Johansen <[email protected]>

I haven't looked at the added tests specifically, but could you merge
in the following changes? This will do the following:

  - make the verbose output of equality.sh honor whether or not
    the environment variable VERBOSE is set

  - thereby making the output verbose when 'make check V=1' or 'make
    check VERBOSE=1' is given from within the parser/ directory. This
    will make distribution packagers happy when diagnosing build
    failures caused by test failures.

  - if verbose output is not emitted and the tests were successful, emit
    a newline before printing PASS.

Thanks.
---
 parser/tst/Makefile    |    2 ++
 parser/tst/equality.sh |    3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Index: b/parser/tst/equality.sh
===================================================================
--- a/parser/tst/equality.sh
+++ b/parser/tst/equality.sh
@@ -27,7 +27,7 @@ _SCRIPTDIR=$(dirname "${BASH_SOURCE[0]}"
 APPARMOR_PARSER="${APPARMOR_PARSER:-${_SCRIPTDIR}/../apparmor_parser}"
 fails=0
 errors=0
-verbose=
+verbose="${VERBOSE:-}"
 
 hash_binary_policy()
 {
@@ -430,5 +430,6 @@ then
        exit $(($fails + $errors))
 fi
 
+[ -z "${verbose}" ] && printf "\n"
 printf "PASS\n"
 exit 0
Index: b/parser/tst/Makefile
===================================================================
--- a/parser/tst/Makefile
+++ b/parser/tst/Makefile
@@ -9,6 +9,8 @@ PROVE_ARG=-f
 ifeq ($(VERBOSE),1)
   PROVE_ARG+=-v
   PYTEST_ARG = -v
+else
+  undefine VERBOSE
 endif
 
 all: tests


> ---
>  parser/tst/equality.sh | 142 
> +++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 133 insertions(+), 9 deletions(-)
> 
> diff --git a/parser/tst/equality.sh b/parser/tst/equality.sh
> index 2258758..a66e739 100755
> --- a/parser/tst/equality.sh
> +++ b/parser/tst/equality.sh
> @@ -25,6 +25,7 @@ set -o pipefail
>  APPARMOR_PARSER="${APPARMOR_PARSER:-../apparmor_parser}"
>  fails=0
>  errors=0
> +verbose=
>  
>  hash_binary_policy()
>  {
> @@ -59,10 +60,11 @@ verify_binary()
>               return $((ret + 1))
>       fi
>  
> -     printf "Binary %s %s" "$t" "$desc"
> +     if [ -n "$verbose" ] ; then printf "Binary %s %s" "$t" "$desc" ; fi
>       good_hash=$(hash_binary_policy "$good_profile")
>       if [ $? -ne 0 ]
>       then
> +             if [ -z "$verbose" ] ; then printf "Binary %s %s" "$t" "$desc" 
> ; fi
>               printf "\nERROR: Error hashing the following \"known-good\" 
> profile:\n%s\n\n" \
>                      "$good_profile" 1>&2
>               ((errors++))
> @@ -74,12 +76,14 @@ verify_binary()
>               hash=$(hash_binary_policy "$profile")
>               if [ $? -ne 0 ]
>               then
> +                     if [ -z "$verbose" ] ; then printf "Binary %s %s" "$t" 
> "$desc" ; fi
>                       printf "\nERROR: Error hashing the following 
> profile:\n%s\n\n" \
>                              "$profile" 1>&2
>                       ((errors++))
>                       ((ret++))
>               elif [ "$t" == "equality" ] && [ "$hash" != "$good_hash" ]
>               then
> +                     if [ -z "$verbose" ] ; then printf "Binary %s %s" "$t" 
> "$desc" ; fi
>                       printf "\nFAIL: Hash values do not match\n" 2>&1
>                       printf "known-good (%s) != profile-under-test (%s) for 
> the following profile:\n%s\n\n" \
>                               "$good_hash" "$hash" "$profile" 1>&2
> @@ -87,6 +91,7 @@ verify_binary()
>                       ((ret++))
>               elif [ "$t" == "inequality" ] && [ "$hash" == "$good_hash" ]
>               then
> +                     if [ -z "$verbose" ] ; then printf "Binary %s %s" "$t" 
> "$desc" ; fi
>                       printf "\nFAIL: Hash values match\n" 2>&1
>                       printf "known-good (%s) == profile-under-test (%s) for 
> the following profile:\n%s\n\n" \
>                               "$good_hash" "$hash" "$profile" 1>&2
> @@ -97,9 +102,13 @@ verify_binary()
>  
>       if [ $ret -eq 0 ]
>       then
> -             printf " ok\n"
> -     fi
> +             if [ -z "$verbose" ] ; then
> +                     printf "."
> +             else
> +                     printf " ok\n"
>  
> +             fi
> +     fi
>       return $ret
>  }
>  
> @@ -113,6 +122,8 @@ verify_binary_inequality()
>       verify_binary "inequality" "$@"
>  }
>  
> +printf "Equality Tests:\n"
> +
>  verify_binary_equality "dbus send" \
>       "/t { dbus send, }" \
>       "/t { dbus write, }" \
> @@ -253,6 +264,7 @@ verify_binary_equality "dbus minimization found in dbus 
> abstractions" \
>             dbus send bus=session, }"
>  
>  # Rules compatible with audit, deny, and audit deny
> +# note: change_profile does not support audit/allow/deny atm
>  for rule in "capability" "capability mac_admin" \
>       "network" "network tcp" "network inet6 tcp"\
>       "mount" "mount /a" "mount /a -> /b" "mount options in (ro) /a -> b" \
> @@ -268,7 +280,10 @@ for rule in "capability" "capability mac_admin" \
>       "unix" "unix (create, listen, accept)" "unix addr=@*" "unix addr=none" \
>        "unix peer=(label=foo)" \
>       "/f r" "/f w" "/f rwmlk" "/** r" "/**/ w" \
> -     "file /f r" "file /f w" "file /f rwmlk"
> +     "file /f r" "file /f w" "file /f rwmlk" \
> +     "link /a -> /b" "link subset /a -> /b" \
> +     "l /a -> /b" "l subset /a -> /b" \
> +     "file l /a -> /b" "l subset /a -> /b"
>  do
>       verify_binary_equality "allow modifier for \"${rule}\"" \
>               "/t { ${rule}, }" \
> @@ -283,21 +298,130 @@ do
>  done
>  
>  # Rules that need special treatment for the deny modifier
> -for rule in "/f ux" "/f Ux" "/f px" "/f Px" "/f ix" \
> -     "file /f ux" "file /f UX" "file /f px" "file /f Px" "file /f ix"
> +for rule in "/f ux" "/f Ux" "/f px" "/f Px" "/f cx" "/f Cx" "/f ix" \
> +            "/f pux" "/f Pux" "/f pix" "/f Pix" \
> +            "/f cux" "/f Cux" "/f cix" "/f Cix" \
> +            "/* ux" "/* Ux" "/* px" "/* Px" "/* cx" "/* Cx" "/* ix" \
> +            "/* pux" "/* Pux" "/* pix" "/* Pix" \
> +            "/* cux" "/* Cux" "/* cix" "/* Cix" \
> +         "/f px -> b " "/f Px -> b" "/f cx -> b" "/f Cx -> b" \
> +            "/f pux -> b" "/f Pux -> b" "/f pix -> b" "/f Pix -> b" \
> +            "/f cux -> b" "/f Cux -> b" "/f cix -> b" "/f Cix -> b" \
> +            "/* px -> b" "/* Px -> b" "/* cx -> b" "/* Cx -> b" \
> +            "/* pux -> b" "/* Pux -> b" "/* pix -> b" "/* Pix -> b" \
> +            "/* cux -> b" "/* Cux -> b" "/* cix -> b" "/* Cix -> b" \
> +         "file /f ux" "file /f Ux" "file /f px" "file /f Px" \
> +            "file /f cx" "file /f Cx" "file /f ix" \
> +            "file /f pux" "file /f Pux" "file /f pix" "file /f Pix" \
> +            "/f cux" "/f Cux" "/f cix" "/f Cix" \
> +            "file /* ux" "file /* Ux" "file /* px" "file /* Px" \
> +            "file /* cx" "file /* Cx" "file /* ix" \
> +            "file /* pux" "file /* Pux" "file /* pix" "file /* Pix" \
> +            "file /* cux" "file /* Cux" "file /* cix" "file /* Cix" \
> +         "file /f px -> b " "file /f Px -> b" "file /f cx -> b" "file /f Cx 
> -> b" \
> +            "file /f pux -> b" "file /f Pux -> b" "file /f pix -> b" "file 
> /f Pix -> b" \
> +            "file /f cux -> b" "file /f Cux -> b" "file /f cix -> b" "file 
> /f Cix -> b" \
> +            "file /* px -> b" "file /* Px -> b" "file /* cx -> b" "file /* 
> Cx -> b" \
> +            "file /* pux -> b" "file /* Pux -> b" "file /* pix -> b" "file 
> /* Pix -> b" \
> +            "file /* cux -> b" "file /* Cux -> b" "file /* cix -> b" "file 
> /* Cix -> b"
> +
>  do
>       verify_binary_equality "allow modifier for \"${rule}\"" \
>               "/t { ${rule}, }" \
> -             "/t { allow ${rule}, }" \
> +             "/t { allow ${rule}, }"
> +
> +     # skip rules that don't end with x perm
> +     if [ -n "${rule##*x}" ] ; then continue ; fi
>  
>       verify_binary_inequality "deny, audit deny modifier for \"${rule}\"" \
>               "/t { ${rule}, }" \
>               "/t { audit ${rule}, }" \
>               "/t { audit allow ${rule}, }" \
> -             "/t { deny /f x, }" \
> -             "/t { audit deny /f x, }"
> +             "/t { deny ${rule% *} x, }" \
> +             "/t { audit deny ${rule% *} x, }"
> +done
> +
> +#Test equality of leading and trailing file permissions
> +for audit in "" "audit" ; do
> +     for allow in "" "allow" "deny" ; do
> +             for owner in "" "owner" ; do
> +                     for f in "" "file" ; do
> +                             prefix="$audit $allow $owner $f"
> +                             for perm in "r" "w" "a" "l" "k" "m" "rw" "ra" \
> +                                         "rl" "rk" "rm" "wl" "wk" "wm" \
> +                                         "rwl" "rwk" "rwm" "ral" "rak" \
> +                                         "ram" "rlk" "rlm" "rkm" "wlk" \
> +                                         "wlm" "wkm" "alk" "alm" "akm" \
> +                                         "lkm" "rwlk" "rwlm" "rwkm" \
> +                                         "ralk" "ralm" "wlkm" "alkm" \
> +                                         "rwlkm" "ralkm" ; do
> +                                     verify_binary_equality "leading and 
> trailing perms" \
> +                                             "/t { ${prefix} /f ${perm}, }" \
> +                                             "/t { ${prefix} ${perm} /f, }"
> +                             done
> +                             if [ "$allow" == "deny" ] ; then continue ; fi
> +                             for perm in "ux" "Ux" "px" "Px" "cx" "Cx" \
> +                                         "ix" "pux" "Pux" "pix" "Pix" \
> +                                         "cux" "Cux" "cix" "Cix"
> +                             do
> +                                     verify_binary_equality "leading and 
> trailing perms" \
> +                                             "/t { ${prefix} /f ${perm}, }" \
> +                                             "/t { ${prefix} ${perm} /f, }"
> +                             done
> +                             for perm in "px" "Px" "cx" "Cx" \
> +                                         "pux" "Pux" "pix" "Pix" \
> +                                         "cux" "Cux" "cix" "Cix"
> +                             do
> +                                     verify_binary_equality "leading and 
> trailing perms" \
> +                                             "/t { ${prefix} /f ${perm} -> 
> b, }" \
> +                                             "/t { ${prefix} ${perm} /f -> 
> b, }"
> +                             done
> +                     done
> +             done
> +     done
>  done
>  
> +#Test rule overlap for x most specific match
> +for perm1 in "ux" "Ux" "px" "Px" "cx" "Cx" "ix" "pux" "Pux" \
> +          "pix" "Pix" "cux" "Cux" "cix" "Cix" "px -> b" \
> +          "Px -> b" "cx -> b" "Cx -> b" "pux -> b" "Pux ->b" \
> +          "pix -> b" "Pix -> b" "cux -> b" "Cux -> b" \
> +          "cix -> b" "Cix -> b"
> +do
> +     for perm2 in "ux" "Ux" "px" "Px" "cx" "Cx" "ix" "pux" "Pux" \
> +                  "pix" "Pix" "cux" "Cux" "cix" "Cix" "px -> b" \
> +                  "Px -> b" "cx -> b" "Cx -> b" "pux -> b" "Pux ->b" \
> +                  "pix -> b" "Pix -> b" "cux -> b" "Cux -> b" \
> +                  "cix -> b" "Cix -> b"
> +     do
> +             if [ "$perm1" ==  "$perm2" ] ; then
> +                     verify_binary_equality "Exec - most specific match: 
> same as glob" \
> +                             "/t { /* px, /f px, }" \
> +                             "/t { /* px, }"
> +             else
> +                     verify_binary_inequality "Exec - most specific match: 
> different from glob" \
> +                             "/t { /* px, /f cx, }" \
> +                             "/t { /* px, }"
> +             fi
> +     done
> +done
> +
> +#Test deny carves out permission
> +verify_binary_inequality "Deny removes r perm" \
> +                    "/t { /foo/[abc] r, audit deny /foo/b r, }" \
> +                    "/t { /foo/[abc] r, }"
> +
> +verify_binary_equality "Deny removes r perm" \
> +                    "/t { /foo/[abc] r, audit deny /foo/b r, }" \
> +                    "/t { /foo/[ac] r, }"
> +
> +#this one may not be true in the future depending on if the compiled profile
> +#is explicitly including deny permissions for dynamic composition
> +verify_binary_equality "Deny of ungranted perm" \
> +                    "/t { /foo/[abc] r, audit deny /foo/b w, }" \
> +                    "/t { /foo/[abc] r, }"
> +
> +
>  if [ $fails -ne 0 -o $errors -ne 0 ]
>  then
>       printf "ERRORS: %d\nFAILS: %d\n" $errors $fails 2>&1
> -- 
> 2.1.4
> 
> 
> -- 
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

-- 
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to