From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH] capabilities: do not audit log BPRM_FCAPS on set*id Date: Tue, 7 Mar 2017 12:10:49 -0600 Message-ID: <20170307181049.GA31834@mail.hallyn.com> References: <515427654218b7ce22441f635115e93cf74d6302.1488491988.git.rgb@redhat.com> <20170303020757.GA9920@mail.hallyn.com> <20170303025014.GG18258@madcap2.tricolour.ca> <20170307172227.GM3818@madcap2.tricolour.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20170307172227.GM3818@madcap2.tricolour.ca> Sender: owner-linux-security-module@vger.kernel.org To: Richard Guy Briggs Cc: "Serge E. Hallyn" , linux-security-module@vger.kernel.org, linux-audit@redhat.com, Andy Lutomirski , "Serge E. Hallyn" , Kees Cook , James Morris , Eric Paris , Paul Moore , Steve Grubb List-Id: linux-audit@redhat.com Quoting Richard Guy Briggs (rgb@redhat.com): > On 2017-03-02 21:50, Richard Guy Briggs wrote: > > On 2017-03-02 20:07, Serge E. Hallyn wrote: > > > On Thu, Mar 02, 2017 at 08:10:29PM -0500, Richard Guy Briggs wrote: > > > > The audit subsystem is adding a BPRM_FCAPS record when auditing setuid > > > > application execution (SYSCALL execve). This is not expected as it was > > > > supposed to be limited to when the file system actually had capabilities > > > > in an extended attribute. It lists all capabilities making the event > > > > really ugly to parse what is happening. The PATH record correctly > > > > records the setuid bit and owner. Suppress the BPRM_FCAPS record on > > > > set*id. > > > > > > > > See: https://github.com/linux-audit/audit-kernel/issues/16 > > > > > > Hey Richard, > > > > Hi Serge, > > > > > one possibly audit-worth case which (if I read correctly) this will > > > skip is where a setuid-root binary has filecaps which *limit* its privs. > > > Does that matter? > > > > I hadn't thought of that case, but I did consider in the setuid case > > comparing before and after without setuid forcing the drop of all > > capabilities via "ambient". Mind you, this bug has been around before > > Luto's patch that adds the ambient capabilities set. > > Can you suggest a scenario where that might happen? Sorry, do you mean the case I brought up, or the one you mentioned? I don't quite understnad the one you brought up. For mine it's pretty simple to reproduce, just # as root cp `which sleep` /tmp/sleep chown root: /tmp/sleep chmod u+s /tmp/sleep setcap cap_sys_admin+pe /tmp/sleep # as non-root /tmp/sleep 200 & cat /proc/$!/status | egrep -e '(^[UG]id|^Cap)' > Can you come up with an idea for a test case? At first I figured I > could simply go from root and su to an unprivileged user, but that Ok - that sounds like you're talking about the case you brought up then. Certainly setuid to nonroot should clear ambient, but what's the problem? Is that broken, or are you wondering whether that should be logged? > doesn't trigger it and then naively thought I could strace both > directions to find out the difference and su or sudo to root really > doesn't like being straced. > > > Paul? > > > > > > Signed-off-by: Richard Guy Briggs > > > > --- > > > > security/commoncap.c | 5 +++-- > > > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/security/commoncap.c b/security/commoncap.c > > > > index 14540bd..8f6bedf 100644 > > > > --- a/security/commoncap.c > > > > +++ b/security/commoncap.c > > > > @@ -594,16 +594,17 @@ skip: > > > > /* > > > > * Audit candidate if current->cap_effective is set > > > > * > > > > - * We do not bother to audit if 3 things are true: > > > > + * We do not bother to audit if 4 things are true: > > > > * 1) cap_effective has all caps > > > > * 2) we are root > > > > * 3) root is supposed to have all caps (SECURE_NOROOT) > > > > + * 4) we are running a set*id binary > > > > * Since this is just a normal root execing a process. > > > > * > > > > * Number 1 above might fail if you don't have a full bset, but I think > > > > * that is interesting information to audit. > > > > */ > > > > - if (!cap_issubset(new->cap_effective, new->cap_ambient)) { > > > > + if (!is_setid && !cap_issubset(new->cap_effective, new->cap_ambient)) { > > > > if (!cap_issubset(CAP_FULL_SET, new->cap_effective) || > > > > !uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) || > > > > issecure(SECURE_NOROOT)) { > > > > -- > > > > 1.7.1 > > > > - RGB > > - RGB > > -- > Richard Guy Briggs > Kernel Security Engineering, Base Operating Systems, Red Hat > Remote, Ottawa, Canada > Voice: +1.647.777.2635, Internal: (81) 32635