[userspace PATCH v2 0/2] Add support for loginuid_set

[Richard Guy Briggs]

On 2016-10-11 18:15, Paul Moore wrote:
> On Tue, Oct 11, 2016 at 5:31 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> > On Tuesday, October 11, 2016 4:54:26 PM EDT Paul Moore wrote:
> >> On Tue, Oct 11, 2016 at 4:50 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> >> > On Tuesday, October 11, 2016 4:42:58 PM EDT Paul Moore wrote:
> >> >> On Tue, Oct 11, 2016 at 3:22 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> >> >> > On Tuesday, October 11, 2016 2:27:54 PM EDT Richard Guy Briggs wrote:
> >> >> >> On 2016-10-11 12:40, Steve Grubb wrote:
> >> >> >> > On Monday, October 10, 2016 5:10:39 PM EDT Paul Moore wrote:
> >> >> >> > > On Mon, Oct 10, 2016 at 1:24 PM, Steve Grubb <sgrubb at redhat.com>
> >> >
> >> > wrote:
> >> >> >> > > > On Thursday, August 18, 2016 2:18:55 PM EDT Richard Guy Briggs
> >> >
> >> > wrote:
> >> >> >> > > >> loginuid_set support should have been added to userspace when
> >> >> >> > > >> it
> >> >> >> > > >> was
> >> >> >> > > >> added to the kernel around v3.10.  Add it before we do similar
> >> >> >> > > >> for
> >> >> >> > > >> sessionID and sessionID_set.
> >> >> >> > > >
> >> >> >> > > > If this were accepted, how would this change writing rules? IOW,
> >> >> >> > > > can
> >> >> >> > > > you
> >> >> >> > > > give an example rule so we can see what this looks like?
> >> >> >> > >
> >> >> >> > > We have a RFE feature page which documents some rule examples:
> >> >> >> > >
> >> >> >> > > *
> >> >> >> > > https://github.com/linux-audit/audit-kernel/wiki/RFE-Session-ID-Us
> >> >> >> > > er-> >> > > Fil ter
> >> >> >> >
> >> >> >> > OK, thanks. This is helpful. So, what is the difference between
> >> >> >> > these
> >> >> >> > rules?
> >> >> >> >
> >> >> >> > -a always,exit -F path=/tmp/sessionid_test -F loginuid=-1
> >> >> >> >
> >> >> >> > -a always,exit -F path=/tmp/sessionid_set_test -F loginuid_set=0
> >> >> >>
> >> >> >> The only difference is one flag in the kernel to indicate how it was
> >> >> >> invoked to be able to report when queried exactly the same way it was
> >> >> >> invoked, but there is no difference in the actual behaviour of the
> >> >> >> filter.  This was added because of your report that "f24=0" was
> >> >> >> reported
> >> >> >> instead of loginuid_set=0 for backwards compatibility.
> >> >> >
> >> >> > OK. Generally its bad to have 2 ways to do the same thing. People use
> >> >> > SCAP
> >> >> > content to check system configurations. If there's two ways to do the
> >> >> > same
> >> >> > thing, then someone can accidentally choose the wrong way and fail
> >> >> > their
> >> >> > scan. We run into this in the past where we allowed -a exit,always and
> >> >> > -a
> >> >> > always,exit. All the rules had to be reworked to be consistent.
> >> >> > Therefore, I would recommend not using the loginuid_set option. We
> >> >> > still
> >> >> > get questions about -w /path/file -p wa  vs -a always,exit -F
> >> >> > path=/path/file -F perm=wa. But that one is so deeply embedded that it
> >> >> > should not be fixed.
> >> >> >
> >> >> >> Going forward, the implementation of the sessionid_set field (which
> >> >> >> works similarly) will not allow an unset value of sessionid since
> >> >> >> these
> >> >> >> are a new addition that didn't need to accomodate backward
> >> >> >> compatibility.
> >> >> >
> >> >> > As long as we can trigger on sessionid=-1, then we are fine.
> >> >>
> >> >> Wait a minute ... what happened to the loginuid_set patches?  Didn't
> >> >> those get merged to userspace?
> >> >
> >> > I'm reviewing this patch set for merging now that we are past all the 2.6
> >> > bug fixing.
> >>
> >> Ah, nevermind ... I confused loginuid and sessionid, sorry about the
> >> confusion.
> >>
> >> Anyway, I thought the desire for having a dedicated "is the loginuid
> >> value set?" filter came from userspace?  If not, where did this
> >> requirement come from?
> >
> > I don't know where it came from. We have always used -1 for unset loginuid and
> > session id.
> 
> Looking back through the git logs, it looks like it originally came
> out of the user namespace work by Eric Biederman.

That's exactly where it came from.  Eric submitted the patch 780a7654 to
fix the regression caused by e1760bd (userns: Convert the audit loginuid
to be a kuid) and its set of 9 patches that were part of a 41-patch set.
I notice Paul was Cc:-ed on that set...  I had to work around the work
around when Steve reported the "f24=..." values.

I can accept that Steve doesn't want to add more ways of doing the same
thing, so I don't have an easy answer in terms of AUDIT_LOGINUID_SET
being exposed in the UAPI.

Since sessionid is a new field for filter specification (but not
reporting and searching), I blocked sessionid==-1 in the api for setting
filters.  This unfortunately makes it a different way to specify it than
loginuid when it is not set.


> paul moore

- 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