Use of empty path_cond in AppArmor kernel code can lead to unexpected permission check results

Bug #1620791 reported by Tyler Hicks
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Triaged
High
Unassigned

Bug Description

IMPORTANT: Bug #1620635 should be fixed at the same time as this bug. Fixing
this bug will likely result in broken users of libapaprmor's
aa_query_file_path() so those applications will need an available alternative.

The AppArmor kernel code defines the path_cond struct as:

 struct path_cond {
  kuid_t uid;
  umode_t mode;
 };

There are several instances where a path_cond instance is initialized as:

 struct path_cond cond = { };

This results in a path_cond.uid of 0. The cond variable is then sometimes
passed to functions which conditionalize on the value of path_cond.uid.

An example of this can be seen in callers of aa_compute_fperms(). If the passed
in path_cond is simply zero-initialized and the fsuid of the current process is
0 (root), then the permissions are mapped differently than if the fsuid of the
current process was non-zero (non-root).

That's a problem in profile_query_cb() because it results in any "owner"
conditionals being ignored when a root-owned process performs a file
permissions query via libapparmor's aa_query_file_path().

Instead of initializing a path_cond struct to all zeroes, path_cond.uid should
be initialized to INVALID_UID so that it is impossible for the current task's
fsuid to match path_cond.uid.

Here are all of the instances I could find where a path_cond struct is
initialized to all zeroes:

$ git grep -p "path_cond.*{ }" security/apparmor
security/apparmor/af_unix.c=static inline int unix_fs_perm(const char *op, u32 mask, struct aa_label *label,
security/apparmor/af_unix.c: struct path_cond cond = { };
security/apparmor/apparmorfs.c=static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
security/apparmor/apparmorfs.c: struct path_cond cond = { };
security/apparmor/domain.c=static int label_compound_match(struct aa_profile *profile,
security/apparmor/domain.c: struct path_cond cond = { };
security/apparmor/domain.c=static int label_components_match(struct aa_profile *profile,
security/apparmor/domain.c: struct path_cond cond = { };
security/apparmor/lsm.c=static int common_perm_rm(const char *op, struct path *dir,
security/apparmor/lsm.c: struct path_cond cond = { };

Tags: aa-kernel
Tyler Hicks (tyhicks)
tags: added: aa-kernel
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.