apparmor: fix: permissions test to view and manage policy

Drop may_open_profiles and unify with policy_view_capable()

Adjust policy_view_capable() so that it is slightly less restricted.
  user_namespaces can now manage policy iff
  - the task has cap_mac_admin in the namespace
  - the user_namespace->level == apparmor policy_namespace->level.
  This ensures a usernamespace can never be used to manage the
  system namespace, and can only be used to manage the namespace at its
  view level.
  If for some reason a user namespace is setup without an apparmor
  policy namespace it will not be able to manage or view policy.

  However this also means an extra level of apparmor policy namespaces
  can not be setup and used with user namespaces at this time.
  ie. this blocks user confinement stacking, and user defined policy
  use cases from being used with user namespaces atm.

Add the ability to output a debug message in relation to
  capable(cap_mac_admin) &&
  policy_locked
as it is possible for these to cause failures that are not audited and
thus hard to trace down.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 38d1650e1f23c3b703955507cf65fbccbf5ce3fa..feb501f078c96d1129e071f953b0a809573678ab 100644 (file)
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -992,7 +992,7 @@ static const struct seq_operations aa_fs_profiles_op = {
 
 static int profiles_open(struct inode *inode, struct file *file)
 {
-       if (!aa_may_open_profiles())
+       if (!policy_view_capable())
                return -EACCES;
 
        return seq_open(file, &aa_fs_profiles_op);

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 562c8a7cd6e90521fec609bfb541003c269e0769..e9eb8ea90ca6301037f456193091655c0ef6ef40 100644 (file)
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -621,12 +621,14 @@ bool policy_view_capable(void)
 {
        struct user_namespace *user_ns = current_user_ns();
        struct aa_ns *ns = aa_get_current_ns();
+       bool root_in_user_ns = uid_eq(current_euid(), make_kuid(user_ns, 0)) ||
+                              in_egroup_p(make_kgid(user_ns, 0));
        bool response = false;
 
-       if (ns_capable(user_ns, CAP_MAC_ADMIN) &&
+       if (root_in_user_ns &&
            (user_ns == &init_user_ns ||
             (unprivileged_userns_apparmor_policy != 0 &&
-             user_ns->level == 1 && ns != root_ns)))
+             user_ns->level == ns->level)))
                response = true;
        aa_put_ns(ns);
 
@@ -634,26 +636,14 @@ bool policy_view_capable(void)
 }
 
 bool policy_admin_capable(void)
-{
-       return policy_view_capable() && !aa_g_lock_policy;
-}
-
-bool aa_may_open_profiles(void)
 {
        struct user_namespace *user_ns = current_user_ns();
-       struct aa_ns *ns = aa_get_current_ns();
-       bool root_in_user_ns = uid_eq(current_euid(), make_kuid(user_ns, 0)) ||
-                              in_egroup_p(make_kgid(user_ns, 0));
-       bool response = false;
+       bool capable = ns_capable(user_ns, CAP_MAC_ADMIN);
 
-       if (root_in_user_ns &&
-           (user_ns == &init_user_ns ||
-            (unprivileged_userns_apparmor_policy != 0 &&
-             user_ns->level == 1 && ns != root_ns)))
-               response = true;
-       aa_put_ns(ns);
+       AA_DEBUG("cap_mac_admin? %d\n", capable);
+       AA_DEBUG("policy locked? %d\n", aa_g_lock_policy);
 
-       return response;
+       return policy_view_capable() && capable && !aa_g_lock_policy;
 }
 
 /**