From patchwork Wed Feb 1 09:06:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Johansen X-Patchwork-Id: 722409 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3vCy2Q6Ym8z9snk; Wed, 1 Feb 2017 20:07:42 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1cYqt8-000190-Mg; Wed, 01 Feb 2017 09:07:38 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1cYqsh-0000iA-BZ for kernel-team@lists.ubuntu.com; Wed, 01 Feb 2017 09:07:11 +0000 Received: from static-50-53-52-155.bvtn.or.frontiernet.net ([50.53.52.155] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1cYqsg-0003gs-N1 for kernel-team@lists.ubuntu.com; Wed, 01 Feb 2017 09:07:11 +0000 From: John Johansen To: kernel-team@lists.ubuntu.com Subject: [PATCH 11/14] UBUNTU: SAUCE: apparmor: Fix no_new_privs blocking change_onexec when using stacked namespaces Date: Wed, 1 Feb 2017 01:06:03 -0800 Message-Id: <20170201090606.22422-12-john.johansen@canonical.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20170201090606.22422-1-john.johansen@canonical.com> References: <20170201090606.22422-1-john.johansen@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com 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 --- security/apparmor/domain.c | 92 +++++++++++++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 33 deletions(-) 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