On 07/12/2015 08:36 PM, Tyler Hicks wrote: > On 2015-07-11 12:04:37, John Johansen wrote: >> On 06/25/2015 11:55 AM, Tyler Hicks wrote: >>> On 2015-06-25 01:21:39, Steve Beattie wrote: >>>> Hi, >>>> >>>> When running the apparmor regression tests on wily with the trunk of >>>> the userspace tools, I'm getting the following two failures in the >>>> query_label test: >>>> >>>> Error: query_label failed. Test 'QUERY file (all base perms #1)' was >>>> expected to 'pass'. Reason for failure 'FAIL: the access should not be >>>> allowed and should be audited' >>>> Error: query_label failed. Test 'QUERY file (all base perms #2)' was >>>> expected to 'pass'. Reason for failure 'FAIL: the access should not be >>>> allowed and should be audited' >>> >>> Note that the test passes when we run them against the wily apparmor >>> userspace (2.9.2-0ubuntu1). Seems to be something broken specifically in >>> trunk. >>> >> So after further investigation there are a couple of problems. > > Thanks for looking into this. I was lost and wasn't sure what the > solution was. :) > >> >> 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. > > Is the kernel only using the wrong mapping function for behind the query > interface or is it using the wrong mapping function in other areas too? > It is only using the wrong mapping on the interface
>> >> >> Attached is a patch that fixes the test, and moves the two tests that >> fail due to the kernel to xpass. > > Can the 'xpass' tests be moved back to 'pass' once the kernel fix is in > place? > yes, and we will make sure to add some extra info to the exported abi/features so this bug can be detected from user space and we can be confident the fix is in place. >> >> --- >> >> === 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 >> + >> +#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 > > There's a typo in the line above. _LCOK -> _LOCK > > With that fixed, > > Acked-by: Tyler Hicks <[email protected]> > > Thanks again! > > Tyler > >> +#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 [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
