Re: [PATCH] capabilities: do not audit log BPRM_FCAPS on set*id

["Serge E. Hallyn" <serge@hallyn.com>]

Quoting Richard Guy Briggs (rgb@redhat.com):
> On 2017-03-07 12:10, Serge E. Hallyn wrote:
> > 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
> 
> I was talking about the case you brought up, but they could be the same case.
> 
> I was thinking of a case where the caps actually change, but are
> overridden by the blanket full permissions of setuid.
> 
> > # 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)'
> 
> I don't see this setuid sleep behave differently than the original one.

Oh, my /tmp is nosuid so actually I have to do it in $HOME.  There I get:

CapPrm: 0000003fffffffff
CapEff: 0000003fffffffff

for simple setuid-root, and

CapPrm: 0000000000200000
CapEff: 0000000000200000

for setuid-root plus file-caps.

> Was this intended to trigger that audit rule?  I don't see it doing that.

I was suggesting that it might be worth auditing, yes.

> > > 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?
> 
> I wonder if it should be logged.

Yeah I could see it being worth logging, but would be nice for audit
folks to decide.

-serge