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

[Richard Guy Briggs]

On 2017-03-07 12:10, Serge E. Hallyn wrote:
> Quoting Richard Guy Briggs (rgb at 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.

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

> > 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.

> > 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.

(I solved this testing problem by setting strace setuid temporarily.
Not sure what I learned from that...  Output was larger than I expected.)

> > > Paul?
> > > 
> > > > > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> > > > > ---
> > > > >  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 <rgb at redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635