apparmor: fix module parameters can be changed after policy is locked

commit 58acf9d911c8831156634a44d0b022d683e1e50c upstream.

the policy_lock parameter is a one way switch that prevents policy
from being further modified. Unfortunately some of the module parameters
can effectively modify policy by turning off enforcement.

split policy_admin_capable into a view check and a full admin check,
and update the admin check to test the policy_lock parameter.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index c28b0f20ab53ed69da7c1e6b8c60a8099370a4f8..52275f040a5f685e4c20b4d829d47c4c8223644f 100644 (file)
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -403,6 +403,8 @@ static inline int AUDIT_MODE(struct aa_profile *profile)
        return profile->audit;
 }
 
+bool policy_view_capable(void);
+bool policy_admin_capable(void);
 bool aa_may_manage_policy(int op);
 
 #endif /* __AA_POLICY_H */

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index fb99e18123b41b4f049fd98078e88aafccb7729b..00a92de97c8270f136bc954a6a5abfd6e4340631 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -762,51 +762,49 @@ __setup("apparmor=", apparmor_enabled_setup);
 /* set global flag turning off the ability to load policy */
 static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
-       if (aa_g_lock_policy)
-               return -EACCES;
        return param_set_bool(val, kp);
 }
 
 static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_view_capable())
                return -EPERM;
        return param_get_bool(buffer, kp);
 }
 
 static int param_set_aabool(const char *val, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
        return param_set_bool(val, kp);
 }
 
 static int param_get_aabool(char *buffer, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_view_capable())
                return -EPERM;
        return param_get_bool(buffer, kp);
 }
 
 static int param_set_aauint(const char *val, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
        return param_set_uint(val, kp);
 }
 
 static int param_get_aauint(char *buffer, const struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_view_capable())
                return -EPERM;
        return param_get_uint(buffer, kp);
 }
 
 static int param_get_audit(char *buffer, struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_view_capable())
                return -EPERM;
 
        if (!apparmor_enabled)
@@ -818,7 +816,7 @@ static int param_get_audit(char *buffer, struct kernel_param *kp)
 static int param_set_audit(const char *val, struct kernel_param *kp)
 {
        int i;
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
 
        if (!apparmor_enabled)
@@ -839,7 +837,7 @@ static int param_set_audit(const char *val, struct kernel_param *kp)
 
 static int param_get_mode(char *buffer, struct kernel_param *kp)
 {
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
 
        if (!apparmor_enabled)
@@ -851,7 +849,7 @@ static int param_get_mode(char *buffer, struct kernel_param *kp)
 static int param_set_mode(const char *val, struct kernel_param *kp)
 {
        int i;
-       if (!capable(CAP_MAC_ADMIN))
+       if (!policy_admin_capable())
                return -EPERM;
 
        if (!apparmor_enabled)

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 780712553651d04e9888bad6830441445167e1f3..179e68d7dc5f968e9d21e16cff26327ca0b7997a 100644 (file)
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -918,6 +918,22 @@ static int audit_policy(int op, gfp_t gfp, const char *name, const char *info,
                        &sa, NULL);
 }
 
+bool policy_view_capable(void)
+{
+       struct user_namespace *user_ns = current_user_ns();
+       bool response = false;
+
+       if (ns_capable(user_ns, CAP_MAC_ADMIN))
+               response = true;
+
+       return response;
+}
+
+bool policy_admin_capable(void)
+{
+       return policy_view_capable() && !aa_g_lock_policy;
+}
+
 /**
  * aa_may_manage_policy - can the current task manage policy
  * @op: the policy manipulation operation being done
@@ -932,7 +948,7 @@ bool aa_may_manage_policy(int op)
                return 0;
        }
 
-       if (!capable(CAP_MAC_ADMIN)) {
+       if (!policy_admin_capable()) {
                audit_policy(op, GFP_KERNEL, NULL, "not policy admin", -EACCES);
                return 0;
        }