Re: [userspace PATCH v2 2/3] Add sessionid_set option from kernel uapi macro AUDIT_SESSIONID_SET

[Richard Guy Briggs <rgb@redhat.com>]

On 2016-10-13 08:32, Paul Moore wrote:
> On Wed, Oct 12, 2016 at 2:02 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Thursday, August 18, 2016 2:47:33 PM EDT Richard Guy Briggs wrote:
> >> Add sessionid_set field option from kernel uapi macro SESSIONID_SET to
> >> enable specifying that sessionID is set or not in user filters.
> >
> > Is there any compelling reason to support two differents fields that essentially
> > decide how to audit sessions? I think its a bit clunky to expect that people
> > write rules
> >
> > -a always,exit -S open -F path=/path/file  -F sessionid>0
> >
> > but if you want to record daemons, then its not as simple as using -1 which is
> > what is in the logs and the intuitive answer. Instead you have to use a new
> > field.
> >
> > -a always,exit -S open -F path=/path/file  -F sessionid_set=0
> >
> > But then you can also do the first rule as:
> >
> > -a always,exit -S open -F path=/path/file  -F sessionid_set=1
> >
> > So, we have 2 ways of doing almost the same thing. I don't really like this.
> 
> I originally thought the loginuid_set filter functionality was added
> to satisfy a request made by you for the audit userspace; I suggested
> to Richard that we do the same for the session ID since there were
> some definite similarities.  However, having looked back at the
> original motivation for adding the loginuid_set functionality, I don't
> we will face the same problem with the session ID.  If Richard can't
> think of any compelling reason to keep a dedicated sessionid_set
> filter, I think we can drop it.
> 
> Further, while I understand why Eric B. needed to make the internal
> audit kernel changes to the loginuid code (and other audit UID/GID
> bits for that matter), I'm less convinced on the need to change the
> kernel/userspace filter API to add the loginuid_set capability.
> However, that was before my time, and it's done now, we can't yank it
> out at this point.

Well, Steve argues that -1 is more intuitive than "unset", which I
disagree unless you have always done that or you are a programmer.  I'd
argue most of the users aren't programmers.  And certainly that huge
number of 4 billion and something isn't something I'm going to remember
off the top of my head (and I'm a programmer) and the users certainly
aren't going to understand the significance of when making rules.  Eric
isn't the first to reasonably propose not to use special values in-band
due to hoops that need to be jumped to work around complications that
can create (including needing to check for rolling the counter).

As far as Eric's patch set is concerned, he needed to make that change
only as a check that we weren't carelessly throwing around UIDs and GIDs
in the kernel without proper translation to the right user namespace,
but he didn't need to break the existing API to do that.  That was a
bug.  Now that cat was out of the bag, it was harder to stuff it back
in.  I agreed with the fix at the time to add AUDIT_LOGINUID_SET due to
my aversion to in-band signalling, but the better solution would have
been to unbreak userspace and re-allow -1 and not complicate things
further with the internal representation of another field to represent
unset.

Given sessionID is a new filter field, I'd prefer the dedicated field to
indicate it is unset from the get-go to avoid the more complicated mess
we are now in with loginuid_set in deprecating loginuid==-1, but that
isn't the one obvious solution.

I suspect there are no users of AUDIT_LOGINUID_SET, so we could likely
rip it out and nobody would notice, and in that case, it would be
obvious to make sessionID behave the same way, in-banding -1 to indicate
unset.

All of which to say, I guess deprecating AUDIT_LOGINUID_SET is the best
way to go and not add loginuid_set in userspace tools and not implement
sessionid_set at all.
> paul moore

- RGB

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