From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750919AbcFAUGJ (ORCPT ); Wed, 1 Jun 2016 16:06:09 -0400 Received: from smtp.nsa.gov ([8.44.101.9]:16319 "EHLO emsm-gh1-uea11.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750744AbcFAUGI (ORCPT ); Wed, 1 Jun 2016 16:06:08 -0400 X-IronPort-AV: E=Sophos;i="5.26,402,1459814400"; d="scan'208";a="16575201" IronPort-PHdr: =?us-ascii?q?9a23=3AH6xr2xDizcCU0RqiU4vdUyQJP3N1i/DPJgcQr6Af?= =?us-ascii?q?oPdwSP/8ocbcNUDSrc9gkEXOFd2CrakU2qyL7Ou5Aj1IyK3CmU5BWaQEbwUCh8?= =?us-ascii?q?QSkl5oK+++Imq/EsTXaTcnFt9JTl5v8iLzG0FUHMHjew+a+SXqvnYsExnyfTB4?= =?us-ascii?q?Ov7yUtaLyZ/nhqbvpNaKOFQArQH+SI0xBS3+lR/WuMgSjNkqAYcK4TyNnEF1ff?= =?us-ascii?q?9Lz3hjP1OZkkW0zM6x+Jl+73YY4Kp5pIZoGJ/3dKUgTLFeEC9ucyVsvJWq5iTZ?= =?us-ascii?q?SUOzwldUEiBMylsbSzTCuTrzRZrg+hDxtuNg1i2XJ4WiRrkvVCWK9KxrQQLmjC?= =?us-ascii?q?odcjUwtmrQj5o0xIZghVr1ox153p6Re4yeKeB/YrKYeNQWWG5MdtheWjYHAY6m?= =?us-ascii?q?aYYLSe0bMrAc56v0olYV5T65HxOtH6u7yDpPnGX3xoU82uEsEEfBxgN2TPwUt3?= =?us-ascii?q?GBl8n4LKcfV6iOyaDMyTjSJ6dN1SzV9JnDchdnp+qFG713b5yCmgEUCwrZgwDI?= =?us-ascii?q?+sTeNDSP27FI6jLD4g=3D=3D?= X-IPAS-Result: =?us-ascii?q?A2G0AwDGP09X/wHyM5BdGgEBAQGDH4EBUqVkAQECAQEGkh2?= =?us-ascii?q?EF4YRAoEzTAEBAQEBAQICYieCMIIVAQEBAQIBIwQnIAsQCw0KAQICJgICMRUJC?= =?us-ascii?q?AYBBwUGAgEBGQSIBgiwJ5EtAQEBAQEBAQEBAQEBAQEBAQEBAQEBHIEBhF6CPwi?= =?us-ascii?q?CToQSEQGDHYJZBZg3iHiFKIFph2+FREWFbokZYoQJIDKJAoE1AQEB?= Subject: Re: [PATCH] LSM: Reorder security_capset to do access checks properly To: Casey Schaufler , LSM , James Morris References: <0243a591-a6e1-8827-3f03-884c3ad331d0@schaufler-ca.com> Cc: LKLM , SE Linux From: Stephen Smalley Organization: National Security Agency Message-ID: <439266f9-6431-4c59-db2e-7eb48d49aaf8@tycho.nsa.gov> Date: Wed, 1 Jun 2016 16:06:53 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <0243a591-a6e1-8827-3f03-884c3ad331d0@schaufler-ca.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/01/2016 03:27 PM, Casey Schaufler wrote: > Subject: [PATCH] LSM: Reorder security_capset to do access checks properly > > The security module hooks that check whether a process should > be able to set a new capset are currently called after the new > values are set in cap_capset(). This change reverses the order. > The capability module no longer adds cap_capset to the module list. > Instead, it is invoked directly by the LSM infrastructure. > This isn't an approach that generalizes well. Is this change necessary? The fact that cap_capset() modifies new before the other hooks are called does no harm, because if any hook returns an error, then the caller returns that error and never commits the new cred. It is actually possibly beneficial for the other security hooks to be called after cap_capset() so that they can adjust the new values if desired (e.g. to reduce them) before they are finally committed. > > Signed-off-by: Casey Schaufler > --- > security/commoncap.c | 2 +- > security/security.c | 24 ++++++++++++++++++++++-- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/security/commoncap.c b/security/commoncap.c > index 48071ed..f5bce18 100644 > --- a/security/commoncap.c > +++ b/security/commoncap.c > @@ -1073,7 +1073,7 @@ struct security_hook_list capability_hooks[] = { > LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check), > LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme), > LSM_HOOK_INIT(capget, cap_capget), > - LSM_HOOK_INIT(capset, cap_capset), > + /* Carefull! Do not include cap_capset! */ > LSM_HOOK_INIT(bprm_set_creds, cap_bprm_set_creds), > LSM_HOOK_INIT(bprm_secureexec, cap_bprm_secureexec), > LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv), > diff --git a/security/security.c b/security/security.c > index 92cd1d8..1610be8 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -177,8 +177,28 @@ int security_capset(struct cred *new, const struct cred *old, > const kernel_cap_t *inheritable, > const kernel_cap_t *permitted) > { > - return call_int_hook(capset, 0, new, old, > - effective, inheritable, permitted); > + struct security_hook_list *hp; > + int rc; > + > + /* > + * Special case handling because the "new" capabilities > + * should not be set until it has been determined that > + * all modules approve of the change. Passing NULL pointers > + * to all modules except the capabilty module as it is > + * expected that only the capability modules needs the > + * result pointers. > + * > + * cap_capset() must not be in the capability module hook list! > + */ > + list_for_each_entry(hp, &security_hook_heads.capset, list) { > + rc = hp->hook.capset(new, old, NULL, NULL, NULL); > + if (rc != 0) > + return rc; > + } > + /* > + * Call cap_capset now to update the new capset. > + */ > + return cap_capset(new, old, effective, inheritable, permitted); > } > > int security_capable(const struct cred *cred, struct user_namespace *ns, > > _______________________________________________ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov. > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov. >