Most exec rules name no target: "Px" resolves by attachment, add support for fallback in this case too
/usr/bin/foo Px fallback=(restricted-helper), Use the spare AA_X_TYPE_MASK value to mean attach first, and walk the entry only if that finds nothing. verify_perms() has to bound and count the new type too, or the table it indexes is freed or shrunk. A conflicting attachment denies rather than walking the list. Advertise domain/exec_fallback here, with the feature complete, so the parser can emit the new syntax instead of degrading it to the old behavior. Signed-off-by: Maxime Bélair <[email protected]> --- security/apparmor/apparmorfs.c | 1 + security/apparmor/domain.c | 54 ++++++++++++++++++++++++++----- security/apparmor/include/file.h | 4 +++ security/apparmor/policy_unpack.c | 7 ++-- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 5b42140e12e8..045ef4570ce7 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2598,6 +2598,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), AA_SFS_FILE_BOOLEAN("computed_longest_left", 1), + AA_SFS_FILE_BOOLEAN("exec_fallback", 1), AA_SFS_DIR("attach_conditions", aa_sfs_entry_attach), AA_SFS_FILE_BOOLEAN("disconnected.path", 1), AA_SFS_FILE_BOOLEAN("kill.signal", 1), diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 28bc82bebcd5..db610d358c16 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -535,6 +535,19 @@ static struct list_head *x_attach_list(struct aa_profile *profile, u32 xindex) : &profile->ns->base.profiles; } +/** + * struct x_attach - the attachment a '&' element stacks on top of + * @label: what attached, or NULL if nothing did + * @searched: whether the search has been done + * + * The search walks every profile in the namespace and does not look at the + * element, so do it once per exec rather than once per element. + */ +struct x_attach { + struct aa_label *label; + bool searched; +}; + /** * x_resolve_elem - resolve a single exec transition table element * @profile: current profile (NOT NULL) @@ -543,6 +556,7 @@ static struct list_head *x_attach_list(struct aa_profile *profile, u32 xindex) * @xindex: the transition index @elem was taken from * @elem: transition table element to resolve (NOT NULL) * @stack: returns: unmerged stack of a '&' element that found no base + * @attach: the attachment a '&' element stacks on, searched for on demand * @info: info message if there was an error (NOT NULL) * * A '&' element supplies the stack; its base comes from an attachment @@ -559,11 +573,12 @@ static struct aa_label *x_resolve_elem(struct aa_profile *profile, const char *name, u32 xindex, const char *elem, struct aa_label **stack, + struct x_attach *attach, const char **info) { bool is_stack = *elem == '&'; const char *lookup = is_stack ? elem + 1 : elem; - struct aa_label *target, *base, *new; + struct aa_label *target, *new; if (xindex & AA_X_CHILD) { /* TODO: switich to parse to get stack of child */ @@ -582,9 +597,14 @@ static struct aa_label *x_resolve_elem(struct aa_profile *profile, /* released by caller */ return target; - base = find_attach(path, profile->ns, x_attach_list(profile, xindex), - name, info); - if (!base) { + if (!attach->searched) { + /* released by the caller of x_to_label() */ + attach->label = find_attach(path, profile->ns, + x_attach_list(profile, xindex), + name, info); + attach->searched = true; + } + if (!attach->label) { /* keep the first: the search fails alike for every element */ if (*stack) aa_put_label(target); @@ -592,8 +612,7 @@ static struct aa_label *x_resolve_elem(struct aa_profile *profile, *stack = target; return ERR_PTR(-ENOENT); } - new = aa_label_merge(base, target, GFP_KERNEL); - aa_put_label(base); + new = aa_label_merge(attach->label, target, GFP_KERNEL); aa_put_label(target); /* released by caller */ @@ -622,6 +641,7 @@ static struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex, const char *name, const char **lookupname, struct aa_label **stack, + struct x_attach *attach, const char **info) { struct aa_ruleset *rules = profile->label.rules[0]; @@ -647,7 +667,7 @@ static struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex, /* a stepped-over target did not decide this exec; no-op first pass */ *info = saved_info; label = x_resolve_elem(profile, path, name, xindex, next, - &pending, info); + &pending, attach, info); /* resolved, or failed for a reason that is not absence */ if (!IS_ERR(label) || PTR_ERR(label) != -ENOENT) { aa_put_label(pending); @@ -687,6 +707,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile, { struct aa_label *new = NULL; struct aa_label *stack = NULL; + struct x_attach attach = { }; u32 xtype = xindex & AA_X_TYPE_MASK; /* Used for info checks during fallback handling */ const char *old_info = NULL; @@ -701,7 +722,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile, /* TODO: fix when perm mapping done at unload */ /* released by caller */ new = x_table_lookup(profile, xindex, path, name, lookupname, - &stack, info); + &stack, &attach, info); break; case AA_X_NAME: /* released by caller */ @@ -709,6 +730,22 @@ static struct aa_label *x_to_label(struct aa_profile *profile, x_attach_list(profile, xindex), name, info); *lookupname = name; break; + case AA_X_NAME_TABLE: + /* implicit transition with fallbacks: attach, then the entry */ + /* released by caller */ + new = find_attach(path, profile->ns, + x_attach_list(profile, xindex), name, info); + /* a conflict is a policy error, not a missing target */ + if (!new && *info != CONFLICTING_ATTACH_STR) { + /* the search just failed; '&' elements share it */ + attach.searched = true; + /* released by caller */ + new = x_table_lookup(profile, xindex, path, name, + lookupname, &stack, &attach, info); + } + /* no named target, so report the executable as AA_X_NAME does */ + *lookupname = name; + break; } if (IS_ERR(new)) { @@ -762,6 +799,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile, } aa_put_label(stack); + aa_put_label(attach.label); /* released by caller */ return new; } diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h index 1614c07fc53e..9e5a071d1623 100644 --- a/security/apparmor/include/file.h +++ b/security/apparmor/include/file.h @@ -57,6 +57,10 @@ struct aa_file_ctx { #define AA_X_NONE AA_INDEX_NONE #define AA_X_NAME 0x04000000 /* use executable name px */ #define AA_X_TABLE 0x08000000 /* use a specified name ->n# */ +/* the fourth type value: attach as AA_X_NAME, and if that finds nothing + * walk the entry as AA_X_TABLE. Both table-reaching types carry AA_X_TABLE. + */ +#define AA_X_NAME_TABLE (AA_X_NAME | AA_X_TABLE) #define AA_X_UNSAFE 0x10000000 #define AA_X_CHILD 0x20000000 diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 06b458a926d2..1cbf3e2b413c 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -1555,8 +1555,11 @@ static bool verify_perms(struct aa_policydb *pdb) for (i = 0; i < pdb->size; i++) { if (!verify_perm(&pdb->perms[i])) return false; - /* verify indexes into str table */ - if ((pdb->perms[i].xindex & AA_X_TYPE_MASK) == AA_X_TABLE) { + /* verify indexes into str table. AA_X_NAME_TABLE reaches it + * too, so it must be bounded and counted in xmax or the + * table is freed or shrunk out from under it below. + */ + if (pdb->perms[i].xindex & AA_X_TABLE) { xidx = pdb->perms[i].xindex & AA_X_INDEX_MASK; if (xidx >= pdb->trans.size) return false; -- 2.51.0
