The file PDB implements "owner" checks by either encoding them in one of
two ways. The, historically, more common way was to use DFA accept{1,2}
tables - which are then picked up by the compatibility logic and work
correctly. The new way is to use the permissions table and index it with
a per-DFA-state index stored in the accept1 table. To indicate presence
of owner/non-owner distinction, the accept2 table contains, at each
index, the ACCEPT_FLAG_OWNER bit. This indicates that the permissions
table is twice as large as the number of states in the DFA, and that
each state corresponds with a pair of permissions, not just one. The
even index is used for owner permissions and the odd index is used for
non-owner permissions.AppArmor parser 4.1.2 can emit a file PDB without the accept2 table being defined in the binary blob. This causes the kernel to create a zero-filled accept2 table for compatibility. The rest of the logic remains intact and in the absence of the ACCEPT_FLAG_OWNER bits, the kernel grants "owner" permissions to non-owners. To remedy this problem, when loading the file PDB, if the accept2 table is missing but the permission table exists and has size compatible with the owner/non-owner encoding, set the ACCEPT_FLAG_OWNER bit at every index of the place-holder table. Signed-off-by: Zygmunt Krynicki <[email protected]> --- security/apparmor/policy_unpack.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 7523971e37d9c..bbd5ed805c00e 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -710,7 +710,7 @@ static ssize_t unpack_perms_table(struct aa_ext *e, struct aa_perms **perms) } static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy, - bool required_dfa, bool required_trans, + bool required_dfa, bool required_trans, bool uses_owner, const char **info) { struct aa_policydb *pdb; @@ -787,6 +787,14 @@ static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy, *info = "failed to alloc dfa flags table"; goto out; } + /* The accept1 table is clearly indexing perms table and is twice the + * number of states (as indicated by the base table), so the user/other + * split is in effect - the permissions table has two entries for each + * state. Synthesize the missing owner flag all entries. */ + if (uses_owner && pdb->size == 2 * pdb->dfa->tables[YYTD_ID_BASE]->td_lolen) { + for (u32 i=0; i<noents; i++) + pdb->dfa->tables[YYTD_ID_ACCEPT2]->td_data[i] |= ACCEPT_FLAG_OWNER; + } pdb->dfa->tables[YYTD_ID_ACCEPT2]->td_lolen = noents; pdb->dfa->tables[YYTD_ID_ACCEPT2]->td_flags = tdflags; } @@ -895,7 +903,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach"); /* xmatch is optional and may be NULL */ - error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info); + error = unpack_pdb(e, &profile->attach.xmatch, false, false, false, &info); if (error) { info = "bad xmatch"; goto fail; @@ -1028,7 +1036,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) if (aa_unpack_nameX(e, AA_STRUCT, "policydb")) { /* generic policy dfa - optional and may be NULL */ info = "failed to unpack policydb"; - error = unpack_pdb(e, &rules->policy, true, false, + error = unpack_pdb(e, &rules->policy, true, false, false, &info); if (error) goto fail; @@ -1053,7 +1061,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) rules->policy = aa_get_pdb(nullpdb); } /* get file rules */ - error = unpack_pdb(e, &rules->file, false, true, &info); + error = unpack_pdb(e, &rules->file, false, true, true, &info); if (error) { goto fail; } else if (rules->file->dfa) { -- 2.52.0
