Re: [PATCH V3 0/3] Add support for session ID user filtering

[Paul Moore]

On Fri, Oct 21, 2016 at 2:46 AM, Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> On 2016-10-20 15:27, Paul Moore wrote:
>> On Thursday, August 18, 2016 01:43:12 PM Richard Guy Briggs wrote:
>> > https://github.com/linux-audit/audit-kernel/wiki/RFE-Session-ID-User-Filter
>> > RFE Session ID User Filter
>> >
>> > https://github.com/linux-audit/audit-kernel/issues/4
>> > RFE: add a session ID filter to the kernel's user filter
>> >
>> > See also the set of userspace suport patches:
>> >     Add support for sessionid user filters, sessionid_set and loginuid_set
>> >     https://www.redhat.com/archives/linux-audit/2016-August/msg00005.html
>> >     (userspace update expected to be posted 2016-08-18)
>> > and the test case:
>> >     https://github.com/rgbriggs/audit-testsuite/tree/ghak4-test-for-sessionID-u
>> > ser-filter
>> >
>> > This third patch is expected to have a merge conflict with:
>> >     "audit: add exclude filter extension to feature bitmap"
>> > posted on 2016-08-18.
>> >
>> > Richard Guy Briggs (3):
>> >   audit: add support for session ID user filter
>> >   audit: add AUDIT_SESSIONID_SET support
>> >   audit: add sessionid filter extension to feature bitmap
>> >
>> >  include/linux/audit.h      |   10 ++++++++++
>> >  include/uapi/linux/audit.h |    6 +++++-
>> >  kernel/auditfilter.c       |    5 +++++
>> >  kernel/auditsc.c           |    6 ++++++
>> >  4 files changed, 26 insertions(+), 1 deletions(-)
>>
>> In light of our current decision to drop the session ID "set" filter, I'm
>> taking another look at these patches and Richard's comment to simply drop
>> patch 2/3 and apply 1/3 and 3/3.
>>
>> Richard, as I mentioned earlier, perhaps not clearly enough, I think we should
>> put a check in audit_set_loginuid() to skip the (int)-1 value from appearing
>> in session_id during normal operation.  In other words, roll/reset the value
>> in session_id one value early so we don't run into problems with the (int)-1
>> unset sentinel value.
>
> I noted your comment earlier and I agree skipping the sentinel is
> required, but if we are rolling this counter, we have bigger issues
> unless there is a way to determine if a sessionID value is still in use
> by at least one task.

The session ID reuse problem is independent of the rollover problem;
the session ID value is going to roll at some point, regardless of if
we skip one value or not.  I guess if there is any comfort in the
value rolling, it is only bumped on a new interactive user session and
given that systemd is now taking to killing all user processes on
logout (no more nohup'ing things) it is unlikely that this will be an
issue on a modern Linux system (it would appear that most everyone is
moving to systemd, for better or worse).  For those truly paranoid
admins, they don't have to filter on it - problem solved - for
everyone else, it can be a useful tool.

-- 
paul moore
www.paul-moore.com