Wire the remaining verbs to their chokepoints, each checked against the
target namespace before the operation takes effect: load and replace in
aa_replace_profiles() once the target ns is resolved, remove in
aa_remove_profiles() and ns_rmdir_op(). All run before the resource-quota
admission so a permission deny wins, and share the create OP_POLICYNS
record.

Signed-off-by: Maxime Bélair <[email protected]>
---
 security/apparmor/apparmorfs.c | 14 ++++++++++++--
 security/apparmor/policy.c     | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index fc9bf0fe8ac6..cfc6d6f55a6a 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2158,12 +2158,14 @@ static int ns_rmdir_op(struct inode *dir, struct dentry 
*dentry)
        struct aa_label *label;
        int error;
 
+       /* held to the remove-verb check below, once the target ns is known */
        label = begin_current_label_crit_section();
        error = aa_may_manage_policy(current_cred(), label, NULL, NULL,
                                     AA_MAY_LOAD_POLICY);
-       end_current_label_crit_section(label);
-       if (error)
+       if (error) {
+               end_current_label_crit_section(label);
                return error;
+       }
 
        parent = get_ns_common_ref(dir->i_private);
        /* rmdir calls the generic securityfs functions to remove files
@@ -2182,11 +2184,19 @@ static int ns_rmdir_op(struct inode *dir, struct dentry 
*dentry)
        }
        AA_BUG(ns_dir(ns) != dentry);
 
+       /* mediate the policyns remove verb on the target ns */
+       error = aa_policyns_perm(label, ns, AA_POLICYNS_REMOVE, OP_POLICYNS);
+       if (error) {
+               aa_put_ns(ns);
+               goto out;
+       }
+
        __aa_remove_ns(ns);
        aa_put_ns(ns);
 
 out:
        mutex_unlock(&parent->lock);
+       end_current_label_crit_section(label);
        inode_lock_nested(dir, I_MUTEX_PARENT);
        inode_lock(dentry->d_inode);
        aa_put_ns(parent);
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 9f6901020b10..67e63a31641a 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -1326,6 +1326,20 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, 
struct aa_label *label,
        } else
                ns = aa_get_ns(policy_ns ? policy_ns : labels_ns(label));
 
+       /*
+        * Mediate the policyns load/replace verb on the target ns. A create
+        * that this load triggered was already mediated in __aa_create_ns.
+        */
+       error = aa_policyns_perm(label, ns,
+                                (mask & AA_MAY_REPLACE_POLICY) ?
+                                        AA_POLICYNS_REPLACE : AA_POLICYNS_LOAD,
+                                OP_POLICYNS);
+       if (error) {
+               info = "policyns permission denied";
+               ent = NULL;
+               goto fail;
+       }
+
        mutex_lock_nested(&ns->lock, ns->level);
        /* Tentative copies of the ns caps */
        pend_limits = ns->acct.limits;
@@ -1628,6 +1642,14 @@ ssize_t aa_remove_profiles(struct aa_ns *policy_ns, 
struct aa_label *subj,
                /* released below */
                ns = aa_get_ns(policy_ns ? policy_ns : labels_ns(subj));
 
+       /* mediate the policyns remove verb on the target ns */
+       error = aa_policyns_perm(subj, ns, AA_POLICYNS_REMOVE, OP_POLICYNS);
+       if (error) {
+               info = "policyns permission denied";
+               aa_put_ns(ns);
+               goto fail;
+       }
+
        if (!name) {
                /* remove namespace - can only happen if fqname[0] == ':' */
                mutex_lock_nested(&ns->parent->lock, ns->parent->level);
-- 
2.51.0


Reply via email to