Re: [PATCH] [BZ905179] audit: omit check for uid and gid validity in audit rules and data

[Eric W. Biederman]

Steve Grubb <sgrubb@xxxxxxxxxx> writes:

> On Tuesday, April 09, 2013 02:39:32 AM Eric W. Biederman wrote:
>> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> writes:
>> > On Wed, 20 Mar 2013 15:18:17 -0400 Richard Guy Briggs <rgb@xxxxxxxxxx> 
> wrote:
>> >> audit rule additions containing "-F auid!=4294967295" were failing with
>> >> EINVAL.
>> >> 
>> >> UID_INVALID (and GID_INVALID) is actually a valid uid (gid) for setting
>> >> and
>> >> testing against audit rules.  Remove the check for invalid uid and gid
>> >> when
>> >> parsing rules and data for logging.
>> 
>> In general testing against invalid uid appears completely bogus, and
>> should always return true.  As it is and essentially always has been
>> incorrect to explicitly set any kernel uid to that value.
>
> This is the unset value that daemons would have. 

As their uid, or gid, or euid, or fsuid.  Not in the least.

> When a real person logs in, 
> pam_loginuid writes the loginuid that was authenticated to. So, any time the 
> value is -1, we are dealing with a daemon or system process. When it comes to 
> auditing, people usually make an exception so that daemon and normal system 
> activity is not recorded. So, you would make a rule something like

> -a always,exit -S open -F exit=-EPERM -F auid>=500 -F auid!=-1

My point is that -1 is a special case that applies only to loginuid, and
that when testing for -1 is not testing for a specific loginuid value
but instead it is testing to see if loginuid has been set.  Semantically
the last is very different.

>> The only case where this appears to make the least little bit of sense
>> is if the goal of the test is to test to see if an audit logloginuid
>> has been set at all.  In which case depending on a test against
>> 4294967295 is bogus because you are depending on an intimate internal
>> kernel implementation detail.
>
> Its been this way and documented since at least 9 years ago. The audit system 
> has been broken for all intents and purposes since the 3.7 kernel was 
> released.

I certainly haven't seen the documentation.  And no one has much cared
about the audit subsystem this "breakage" of the audit
subsystem. Despite things failing with a clear error code.  So there are
two choices.  We mark the audit subsystem as broken moving it to staging
and then delete it because no one cares enough to look after it and
maintain it.  Or we have a constructive conversation about what to do
with it.

I have proposed a patch that will preserve the existing behavior while
adding maintainable semantics.  Will someone who cares please test my
proposed fix?

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/