[apparmor] query_label regression test failures
Steve Beattie
steve at nxnw.org
Mon Jul 13 08:40:40 UTC 2015
On Sat, Jul 11, 2015 at 12:04:37PM -0700, John Johansen wrote:
> So after further investigation there are a couple of problems.
>
> 1. The test is using the wrong defines: It is using the defines from the
> parser for the packed dfa permissions. This set of permissions is not
> meant to be exposed to the outside world
>
> 2. The kernel is using the wrong mapping function for the permissions
> in the file class. This results in partially exposing the packed
> permissions, but even then it doesn't fully line up with the packed
> permissions, and is not correct for several of the potential permissions.
>
>
> Attached is a patch that fixes the test, and moves the two tests that
> fail due to the kernel to xpass.
>
> === modified file 'tests/regression/apparmor/query_label.c'
> --- tests/regression/apparmor/query_label.c 2015-05-28 19:48:46 +0000
> +++ tests/regression/apparmor/query_label.c 2015-07-10 20:45:07 +0000
> @@ -35,28 +35,68 @@
> #define AA_MAY_APPEND (1 << 3)
> #endif
>
> +#ifndef AA_MAY_CREATE
> +#define AA_MAY_CREATE (1 << 4)
> +#endif
The reason I haven't submitted a patch that fixes the macro
redefinition compilation warnings in the link_subset test in the same
way as you do here is that, at least for that test, one of the
definitions that it copied from the parser, AA_MAY_MOUNT, is not
exported through the libapparmor headers. I'm not sure if that's a
change or not, because it had defined all its macros without including
external headers.
The problem this highlights is that if, in the future, the macro
definitions get changed *and* possibly some of them disappear a
la AA_MAY_MOUNT, some of the definitions will get pulled from the
libapparmor exported header *and some won't* and then it's possible
that multiple macro definitions might map to the same value, which
is almost certainly problematic.
And for Tyler's benefit, hiding the definitions behind a function call
doesn't really help if the value definitions can go away with
libapparmor changes and default values are used.
That said, I don't have a good solution for how to verify that all
the macro definitions are internally consistent and don't define
multiple macros to the same value. The only way I can come up with is
to not define the macros at all in the tests, and rely solely on the
libapparmor macro definitions, and then if some of those go away due to
interface changes, the tests and other consumers break at compile time.
> +#ifndef AA_MAY_DELETE
> +#define AA_MAY_DELETE (1 << 5)
> +#endif
> +
> +#ifndef AA_MAY_OPEN
> +#define AA_MAY_OPEN (1 << 6)
> +#endif
> +
> +#ifndef AA_MAY_RENAME
> +#define AA_MAY_RENAME (1 << 7)
> +#endif
> +
> +#ifndef AA_MAY_SETATTR
> +#define AA_MAY_SETATTR (1 << 8)
> +#endif
> +
> +#ifndef AA_MAY_GETATTR
> +#define AA_MAY_GETATTR (1 << 9)
> +#endif
> +
> +#ifndef AA_MAY_SETCRED
> +#define AA_MAY_SETCRED (1 << 10)
> +#endif
> +
> +#ifndef AA_MAY_GETCRED
> +#define AA_MAY_GETCRED (1 << 11)
> +#endif
> +
> +#ifndef AA_MAY_CHMOD
> +#define AA_MAY_CHMOD (1 << 12)
> +#endif
> +
> +#ifndef AA_MAY_CHOWN
> +#define AA_MAY_CHOWN (1 << 13)
> +#endif
> +
> +#ifndef AA_MAY_LCOK
> +#define AA_MAY_LOCK 0x8000
> +#endif
> +
> +#ifndef AA_EXEC_MMAP
> +#define AA_EXEC_MMAP 0x10000
> +#endif
> +
> #ifndef AA_MAY_LINK
> -#define AA_MAY_LINK (1 << 4)
> -#endif
> -
> -#ifndef AA_MAY_LOCK
> -#define AA_MAY_LOCK (1 << 5)
> -#endif
> -
> -#ifndef AA_EXEC_MMAP
> -#define AA_EXEC_MMAP (1 << 6)
> -#endif
> -
> -#ifndef AA_EXEC_PUX
> -#define AA_EXEC_PUX (1 << 7)
> -#endif
> -
> -#ifndef AA_EXEC_UNSAFE
> -#define AA_EXEC_UNSAFE (1 << 8)
> -#endif
> -
> -#ifndef AA_EXEC_INHERIT
> -#define AA_EXEC_INHERIT (1 << 9)
> +#define AA_MAY_LINK 0x40000
> +#endif
> +
> +#ifndef AA_LINK_SUBSET /* overlayed perm in pair */
> +#define AA_LINK_SUBSET AA_MAY_LOCK
> +#endif
> +
> +#ifndef AA_MAY_ONEXEC
> +#define AA_MAY_ONEXEC 0x20000000
> +#endif
> +
> +#ifndef AA_MAY_CHANGE_PROFILE
> +#define AA_MAY_CHANGE_PROFILE 0x40000000
> #endif
>
> static char *progname = NULL;
> @@ -148,18 +188,26 @@
> *mask |= AA_MAY_READ;
> else if (!strcmp(perm, "append"))
> *mask |= AA_MAY_APPEND;
> + else if (!strcmp(perm, "create"))
> + *mask |= AA_MAY_CREATE;
> + else if (!strcmp(perm, "delete"))
> + *mask |= AA_MAY_DELETE;
> + else if (!strcmp(perm, "setattr"))
> + *mask |= AA_MAY_SETATTR;
> + else if (!strcmp(perm, "getattr"))
> + *mask |= AA_MAY_GETATTR;
> + else if (!strcmp(perm, "chmod"))
> + *mask |= AA_MAY_CHMOD;
> + else if (!strcmp(perm, "chown"))
> + *mask |= AA_MAY_CHOWN;
> else if (!strcmp(perm, "link"))
> *mask |= AA_MAY_LINK;
> else if (!strcmp(perm, "lock"))
> *mask |= AA_MAY_LOCK;
> + else if (!strcmp(perm, "linksubset"))
> + *mask |= AA_LINK_SUBSET;
> else if (!strcmp(perm, "exec_mmap"))
> *mask |= AA_EXEC_MMAP;
> - else if (!strcmp(perm, "exec_pux"))
> - *mask |= AA_EXEC_PUX;
> - else if (!strcmp(perm, "exec_unsafe"))
> - *mask |= AA_EXEC_UNSAFE;
> - else if (!strcmp(perm, "exec_inherit"))
> - *mask |= AA_EXEC_INHERIT;
> else {
> fprintf(stderr, "FAIL: unknown perm: %s\n", perm);
> return 1;
> @@ -264,8 +312,8 @@
> (allowed == should_allow && audited == should_audit)) {
> printf("PASS\n");
> } else {
> - fprintf(stderr, "FAIL: the access should %sbe allowed and should %sbe audited\n",
> - allowed ? "" : "not ", audited ? "" : "not ");
> + fprintf(stderr, "FAIL: the access should %sbe allowed and should %sbe audited. mask 0x%x\n",
> + allowed ? "" : "not ", audited ? "" : "not ", mask);
> exit(1);
> }
>
>
> === modified file 'tests/regression/apparmor/query_label.sh'
> --- tests/regression/apparmor/query_label.sh 2015-05-28 19:48:53 +0000
> +++ tests/regression/apparmor/query_label.sh 2015-07-11 18:54:55 +0000
> @@ -212,9 +212,9 @@
>
> genqueryprofile "file,"
> expect allow
> -perms file exec,write,read,append,link,lock
> -querytest "QUERY file (all base perms #1)" pass /anything
> -querytest "QUERY file (all base perms #2)" pass /everything
> +perms file exec,write,read,append,create,delete,setattr,getattr,chmod,chown,link,linksubset,lock,exec_mmap
> +querytest "QUERY file (all base perms #1)" xpass /anything
> +querytest "QUERY file (all base perms #2)" xpass /everything
>
> genqueryprofile "/etc/passwd r,"
> expect allow
>
>
> --
> 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: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150713/524745d4/attachment.pgp>
More information about the AppArmor
mailing list