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.

There will be a number of users of features_bitmap within the same function (exclude filter extension, sessionID filter), so refactor audit_rule_fieldpair_data() to put audit_get_features earlier in the function. Richard Guy Briggs (2): get feature list only once Add user filter option loginuid_set from uapi macro AUDIT_LOGINUID_SET trunk/lib/errormsg.h | 2 ++ trunk/lib/fieldtab.h | 2 ++ trunk/lib/libaudit.c | 17 ++++++++++++++++- trunk/lib/libaudit.h | 6 ++++++ 4 files changed, 26 insertions(+), 1 deletions(-)

On Mon, Oct 10, 2016 at 1:24 PM, Steve Grubb <sgrubb(a)redhat.com&gt; 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-User-Filter

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(a)redhat.com&gt; 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-User-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.

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.

On Tue, Oct 11, 2016 at 3:22 PM, Steve Grubb <sgrubb(a)redhat.com&gt; 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(a)redhat.com&gt;

>> > > > On Thursday, August 18, 2016 2:18:55 PM EDT Richard Guy Briggs

>> > > >> 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-User-> >> > > 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?

On Tue, Oct 11, 2016 at 4:50 PM, Steve Grubb <sgrubb(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; > > 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?

On Tue, Oct 11, 2016 at 5:31 PM, Steve Grubb <sgrubb(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; 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(a)redhat.com&gt; >> > >> > 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.

paul moore

On Monday, October 17, 2016 11:40:17 AM EDT Richard Guy Briggs wrote: > 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. Can we unblock that?

-Steve

On 2016-10-11 18:15, Paul Moore wrote: > 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.

On Monday, October 17, 2016 5:19:59 PM EDT Paul Moore wrote: > We haven't merged any of the session ID code into the kernel so > changes are still possible. The logic for supporting loginuid_set > (UID namespace issues) don't really apply to session IDs so I think we > can drop the sessionid_set part of the API and just use the -1 > sentinel. OK, that's good to hear. I'll fix up and merge the sessionid patch - no need to re-send the user space piece.

-Steve