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

From: sgrubb@redhat.com (Steve Grubb)
To: linux-security-module@vger.kernel.org
Subject: [PATCH] capabilities: do not audit log BPRM_FCAPS on set*id
Date: Wed, 12 Apr 2017 14:49:02 -0400	[thread overview]
Message-ID: <1833607.dNomLM3Ooh@x2> (raw)
In-Reply-To: <20170412064321.GN18559@madcap2.tricolour.ca>

On Wednesday, April 12, 2017 2:43:21 AM EDT Richard Guy Briggs wrote:
> On 2017-04-11 15:36, Paul Moore wrote:
> > On Wed, Mar 29, 2017 at 6:29 AM, Richard Guy Briggs <rgb@redhat.com> 
wrote:
> > > On 2017-03-09 09:34, Steve Grubb wrote:
> > >> On Tuesday, March 7, 2017 4:10:49 PM EST Richard Guy Briggs wrote:
> > >> > > > > > 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.
> > >> 
> > >> If there actually is a change in capability bits besides the implied
> > >> change of capabilities based on the change of the uid alone, then it
> > >> should be logged.> > 
> > > Are you speaking of a change in pP' only from pI, or also pI', pE' and
> > > pA'?
> > > 
> > > Something like ( pP' xor pI ) not empty?
> > > 
> > > The previous patch I'd sent was reasonably easy to understand, but I'm
> > > having trouble adding this new twist to the logic expression in question
> > > due to the inverted combination of pre-existing items.  I'm having
> > > trouble visualizing a 5 or more-dimensional Karnaugh map...
> > > 
> > > While I am at it, I notice pA is missing from the audit record.  The
> > > record contains fields "old_pp", "old_pi", "old_pe", "new_pp", "new_pi",
> > > "new_pe" so in keeping with the previous record normalizations, I'd like
> > > to change the "new_*" variants to simply drop the "new_" prefix.
> > > 
> > >         https://github.com/linux-audit/audit-kernel/issues/40
> > 
> > Yes, there is the separate ambient capabilities record patch, but
> > where do we stand with this patch?  From what I gather there is still
> > some uncertainty here?
> 
> Yes, I put this on my back burner thinking about how best to re-approach
> this, hoping others would offer some insight or advice how to attack
> this, otherwise I'm going to end up with a horrendous conditional
> expression, I fear.
> 
> Steve, I was hoping to get a clarification from you about which
> capability bits had changed.

I am not sure what you are asking of me.

-Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html