diff mbox

[07/11] UBUNTU: SAUCE: apparmor: Fix no_new_privs blocking change_onexec when using stacked namespaces

Message ID 20170331132536.18217-8-john.johansen@canonical.com
State New
Headers show

Commit Message

John Johansen March 31, 2017, 1:25 p.m. UTC
Push the no_new_privs logic into the per profile transition fns, so
that the no_new_privs check can be done at the ns level instead of the
aggregate stack level.

BugLink: http://bugs.launchpad.net/bugs/1648143
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Acked-by: Brad Figg <brad.figg@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
---
 security/apparmor/domain.c | 92 +++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 33 deletions(-)
diff mbox

Patch

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 27ffaf5..cfb0c28 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -559,6 +559,23 @@  static struct aa_label *profile_transition(struct aa_profile *profile,
 	if (!new)
 		goto audit;
 
+	/* Policy has specified a domain transitions. if no_new_privs and
+	 * confined and not transitioning to the current domain fail.
+	 *
+	 * NOTE: Domain transitions from unconfined and to stritly stacked
+	 * subsets are allowed even when no_new_privs is set because this
+	 * aways results in a further reduction of permissions.
+	 */
+	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
+	    !profile_unconfined(profile) &&
+	    !aa_label_is_subset(new, &profile->label)) {
+		error = -EPERM;
+		info = "no new privs";
+		aa_put_label(new);
+		new = NULL;
+		goto audit;
+	}
+
 	if (!(perms.xindex & AA_X_UNSAFE)) {
 		if (DEBUG_ON) {
 			dbg_printk("apparmor: scrubbing environment variables "
@@ -572,8 +589,11 @@  static struct aa_label *profile_transition(struct aa_profile *profile,
 audit:
 	aa_audit_file(profile, &perms, OP_EXEC, MAY_EXEC, name, target, new,
 		      cond->uid, info, error);
-	if (!new)
+	if (error) {
+		if (new)
+			aa_put_label(new);
 		return ERR_PTR(error);
+	}
 
 	return new;
 }
@@ -631,6 +651,21 @@  static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec,
 	if (error)
 		goto audit;
 
+	/* Policy has specified a domain transitions. if no_new_privs and
+	 * confined and not transitioning to the current domain fail.
+	 *
+	 * NOTE: Domain transitions from unconfined and to stritly stacked
+	 * subsets are allowed even when no_new_privs is set because this
+	 * aways results in a further reduction of permissions.
+	 */
+	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
+	    !profile_unconfined(profile) &&
+	    !aa_label_is_subset(onexec, &profile->label)) {
+		error = -EPERM;
+		info = "no new privs";
+		goto audit;
+	}
+
 	if (!(perms.xindex & AA_X_UNSAFE)) {
 		if (DEBUG_ON) {
 			dbg_printk("appaarmor: scrubbing environment "
@@ -750,19 +785,7 @@  int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		goto done;
 	}
 
-	/* Policy has specified a domain transitions. if no_new_privs and
-	 * confined and not transitioning to the current domain fail.
-	 *
-	 * NOTE: Domain transitions from unconfined and to stritly stacked
-	 * subsets are allowed even when no_new_privs is set because this
-	 * aways results in a further reduction of permissions.
-	 */
-	if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
-	    !unconfined(label) && !aa_label_is_subset(new, label)) {
-		error = -EPERM;
-		info = "no new privs";
-		goto audit;
-	}
+	/* TODO: Add ns level no_new_privs subset test */
 
 	if (bprm->unsafe & LSM_UNSAFE_SHARE) {
 		/* FIXME: currently don't mediate shared state */
@@ -1097,12 +1120,30 @@  static int change_profile_perms_wrapper(const char *op, const char *name,
 					struct aa_label *target, bool stack,
 					u32 request, struct aa_perms *perms)
 {
-	int error = change_profile_perms(profile, target,
-					 stack, request,
-					 profile->file.start, perms);
+	const char *info = NULL;
+	int error = 0;
+
+	/*
+	 * Fail explicitly requested domain transitions when no_new_privs
+	 * and not unconfined OR the transition results in a stack on
+	 * the current label.
+	 * Stacking domain transitions and transitions from unconfined are
+	 * allowed even when no_new_privs is set because this aways results
+	 * in a reduction of permissions.
+	 */
+	if (task_no_new_privs(current) && !stack &&
+	    !profile_unconfined(profile) &&
+	    !aa_label_is_subset(target, &profile->label)) {
+		info = "no new privs";
+		error = -EPERM;
+	}
+
+	if (!error)
+		error = change_profile_perms(profile, target, stack, request,
+					     profile->file.start, perms);
 	if (error)
 		error = aa_audit_file(profile, perms, op, request, name,
-				      NULL, target, GLOBAL_ROOT_UID, NULL,
+				      NULL, target, GLOBAL_ROOT_UID, info,
 				      error);
 
 	return error;
@@ -1182,21 +1223,6 @@  int aa_change_profile(const char *fqname, bool onexec,
 		goto check;
 	}
 
-	/*
-	 * Fail explicitly requested domain transitions when no_new_privs
-	 * and not unconfined OR the transition results in a stack on
-	 * the current label.
-	 * Stacking domain transitions and transitions from unconfined are
-	 * allowed even when no_new_privs is set because this aways results
-	 * in a reduction of permissions.
-	 */
-	if (task_no_new_privs(current) && !stack && !unconfined(label) &&
-	    !aa_label_is_subset(target, label)) {
-		info = "no new privs";
-		error = -EPERM;
-		goto audit;
-	}
-
 	/* self directed transitions only apply to current policy ns */
 	/* TODO: currently requiring perms for stacking and straight change
 	 *       stacking doesn't strictly need this. Determine how much