* Re: Patch to auparse to handle out of order messages 1 of 3
From: Steve Grubb @ 2016-01-06 15:43 UTC (permalink / raw)
To: linux-audit, burn
In-Reply-To: <1452076157.26850.87.camel@swtf.swtf.dyndns.org>
On Wednesday, January 06, 2016 09:29:17 PM Burn Alting wrote:
> The following three patches address this problem.
>
> #1 - convert the existing code to change auparse's auparse_state_t (aka
> struct opaque) event_list_t element 'le' to be a pointer, so the 'lol'
> code can more seamlessly fit in.
Applied thanks!
-Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 2 of 3
From: Steve Grubb @ 2016-01-06 15:45 UTC (permalink / raw)
To: linux-audit, burn
In-Reply-To: <1452076194.26850.88.camel@swtf.swtf.dyndns.org>
On Wednesday, January 06, 2016 09:29:54 PM Burn Alting wrote:
> #2 - the 'lol' patch itself. Integrate the ausearch/aureport 'lol' code
> into auparse() and adjust auparse() to deal with maintain an incore list
> of incomplete events.
Quick question...there is this:
#define LOL_EVENTS 1
Couldn't we just assume that this is desired and get rid of all the #ifdefs?
(You don't need to re-send the patch.) Ideally I would like everyone running
the same code for support purposes.
-Steve
^ permalink raw reply
* Re: Use case not covered by the audit library?
From: Steve Grubb @ 2016-01-06 16:28 UTC (permalink / raw)
To: Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <B41870ED03633F4092CDF476119204DF561D06FE@G9W0758.americas.hpqcorp.net>
On Tuesday, January 05, 2016 09:59:25 PM Gulland, Scott A wrote:
> > -----Original Message-----
> > From: Steve Grubb [mailto:sgrubb@redhat.com]
> > Sent: Thursday, December 17, 2015 6:51 PM
> >
> > > > My problem is I don't know what the proper set of "keys" are and the
> > > > values they should contain. If my assumptions are correct, is there
> > > > any documentation on on the key-value pairs and how to format the
> > > > contents of the message parameter? Based on what I've seen in the
> > > > audit log file, I would add "acct=<user>" to the contents of the
> > > > message to reflect the particular authenticated user who issued the
> > > > REST
> >
> > API call.
> >
> > > Well, Steve has published these as a starting point. I'm sure he'll
> > > chime in when he sees your message.
> > >
> > > http://people.redhat.com/sgrubb/audit/audit-events.txt
> > > http://people.redhat.com/sgrubb/audit/audit-parse.txt
> >
> > Thanks for pointing these out, Richard.
> >
> > The basic guidance for AUDIT_USYS_CONFIG is to record old and new values.
> > Typically old values are prefixed with 'old-' and new values are the name
> > of the field with no prefix.
> >
> > Any field that the user could influence the value has to be handled in
> > such a way as to not allow them to trick the parser if they are
> > malicious. For the most part, we hex encode those fields and then write
> > some code to label the fields as encoded so that interpretation can be
> > done later.
> >
> > Since your field names may not be official names in the audit system, you
> > may have to filter illegal characters the user sent during event
> > construction and fill in spaces with an underscore or dash.
>
> Thanks for the feedback and information. It has been very helpful. I've
> done some testing using a "val" and "old-val" field names with data values
> encoded by audit_encode_nv_string(...). However, when I try to display the
> event with "ausearch --interpret ..." neither field's data is decoded back
> into asci text.
It has to be a field name that auparse expects to be encoded.
> So I plan on using the "op", "data" and "euid" fields.
euid would be a kernel originating field name. User space could lie about it.
The kernel is the only thing that knows the truth.
> Only the data field needs to encoded and ausearch does decode this field
> correctly. My message text would look like:
>
> "op=<op text> data=<encoded data> euid=<uid>"
>
> When I was using ausearch I expected to be able to find events by uid using
> either the "-ua" or "-ue" option that would match the euid field's value,
but no matching events were found. Is this expected behavior?
What is the record type? ausearch is optimized to expect certain record types
to have fields in a specific order.
> The "-I" option did correctly convert the euid into the user name.
Interpreting and searching are different areas of the code base and are
independent. Interpreting is done after searching. No need to interpret fields
that will never be output.
-Steve
^ permalink raw reply
* RE: Use case not covered by the audit library?
From: Gulland, Scott A @ 2016-01-06 18:03 UTC (permalink / raw)
To: Steve Grubb, Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <2812121.l6i04u8maX@x2>
> -----Original Message-----
> From: Steve Grubb [mailto:sgrubb@redhat.com]
> Sent: Wednesday, January 06, 2016 8:29 AM
>
> On Tuesday, January 05, 2016 09:59:25 PM Gulland, Scott A wrote:
> > > -----Original Message-----
> > > From: Steve Grubb [mailto:sgrubb@redhat.com]
> > > Sent: Thursday, December 17, 2015 6:51 PM
> > >
> > > The basic guidance for AUDIT_USYS_CONFIG is to record old and new
> values.
> > > Typically old values are prefixed with 'old-' and new values are the
> > > name of the field with no prefix.
> > >
> > > Any field that the user could influence the value has to be handled
> > > in such a way as to not allow them to trick the parser if they are
> > > malicious. For the most part, we hex encode those fields and then
> > > write some code to label the fields as encoded so that
> > > interpretation can be done later.
> > >
> > > Since your field names may not be official names in the audit
> > > system, you may have to filter illegal characters the user sent
> > > during event construction and fill in spaces with an underscore or dash.
> >
> > Thanks for the feedback and information. It has been very helpful.
> > I've done some testing using a "val" and "old-val" field names with
> > data values encoded by audit_encode_nv_string(...). However, when I
> > try to display the event with "ausearch --interpret ..." neither
> > field's data is decoded back into asci text.
>
> It has to be a field name that auparse expects to be encoded.
>
>
> > So I plan on using the "op", "data" and "euid" fields.
>
> euid would be a kernel originating field name. User space could lie about it.
> The kernel is the only thing that knows the truth.
Unfortunately, that is not true for HTTP servers which run as root but
authenticates the true user issuing the REST request. The authentication is
done through PAM. The HTTP server then carries out the action on behalf
of that user. The kernel thinks it's a root user, but the HTTP server knows
otherwise. It sounds like there is no way for a trusted user app to inject
the correct uid into the audit event. Would you recommend I use the
"user" field instead of "euid" to indicate who is issuing the request?
> > Only the data field needs to encoded and ausearch does decode this
> > field correctly. My message text would look like:
> >
> > "op=<op text> data=<encoded data> euid=<uid>"
> >
> > When I was using ausearch I expected to be able to find events by uid
> > using either the "-ua" or "-ue" option that would match the euid
> > field's value,
> but no matching events were found. Is this expected behavior?
>
> What is the record type? ausearch is optimized to expect certain record types
> to have fields in a specific order.
I am using the AUDIT_USYS_CONFIG event type as I would like to use
"aureport -c" to get a summary of the configuration changes to the switch.
As an alternative, I could use the AUDIT_TRUSTED_APP event type.
Scott G.
>
> > The "-I" option did correctly convert the euid into the user name.
>
> Interpreting and searching are different areas of the code base and are
> independent. Interpreting is done after searching. No need to interpret
> fields
> that will never be output.
^ permalink raw reply
* Re: Use case not covered by the audit library?
From: Steve Grubb @ 2016-01-06 20:05 UTC (permalink / raw)
To: Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <B41870ED03633F4092CDF476119204DF561D498A@G4W3225.americas.hpqcorp.net>
On Wednesday, January 06, 2016 06:03:58 PM Gulland, Scott A wrote:
> > -----Original Message-----
> > From: Steve Grubb [mailto:sgrubb@redhat.com]
> > It has to be a field name that auparse expects to be encoded.
> >
> > > So I plan on using the "op", "data" and "euid" fields.
> >
> > euid would be a kernel originating field name. User space could lie about
> > it. The kernel is the only thing that knows the truth.
>
> Unfortunately, that is not true for HTTP servers which run as root but
> authenticates the true user issuing the REST request. The authentication
> is done through PAM. The HTTP server then carries out the action on behalf
> of that user. The kernel thinks it's a root user, but the HTTP server
> knows otherwise. It sounds like there is no way for a trusted user app to
> inject the correct uid into the audit event. Would you recommend I use
> the "user" field instead of "euid" to indicate who is issuing the request?
> > > Only the data field needs to encoded and ausearch does decode this
> > >
> > > field correctly. My message text would look like:
> > > "op=<op text> data=<encoded data> euid=<uid>"
> > >
> > > When I was using ausearch I expected to be able to find events by uid
> > > using either the "-ua" or "-ue" option that would match the euid
> > > field's value,
> >
> > but no matching events were found. Is this expected behavior?
> >
> > What is the record type? ausearch is optimized to expect certain record
> > types to have fields in a specific order.
>
> I am using the AUDIT_USYS_CONFIG event type as I would like to use
> "aureport -c" to get a summary of the configuration changes to the switch.
> As an alternative, I could use the AUDIT_TRUSTED_APP event type.
The USYS_CONFIG event is like this:
type=USYS_CONFIG msg=audit(1389095562.552:540): pid=2249 uid=0 auid=4325 ses=1
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='op=change-system-
time exe="/usr/sbin/hwclock" hostname=? addr=? terminal=pts/0 res=success'
The kernel supplies all the pieces up to the msg= portion. After that is what
you build. The only real field the event writer does is the op= field, The rest
are supplied by libaudit. Ausearch does not parse the op= field.
What I would suggest in a case like this is to create a small utility that
generates the exact report that you want. The auparse library makes that super
easy. I can dig up the skeleton code for something like this if you want.
-Steve
> > > The "-I" option did correctly convert the euid into the user name.
> >
> > Interpreting and searching are different areas of the code base and are
> > independent. Interpreting is done after searching. No need to interpret
> > fields
> > that will never be output.
^ permalink raw reply
* RE: Use case not covered by the audit library?
From: Gulland, Scott A @ 2016-01-06 20:27 UTC (permalink / raw)
To: Steve Grubb, Gulland, Scott A; +Cc: linux-audit@redhat.com
In-Reply-To: <8445926.khRdTJspS3@x2>
> -----Original Message-----
> From: Steve Grubb [mailto:sgrubb@redhat.com]
> Sent: Wednesday, January 06, 2016 12:06 PM
>
> > > euid would be a kernel originating field name. User space could lie
> > > about it. The kernel is the only thing that knows the truth.
> >
> > Unfortunately, that is not true for HTTP servers which run as root but
> > authenticates the true user issuing the REST request. The authentication
> > is done through PAM. The HTTP server then carries out the action on
> behalf
> > of that user. The kernel thinks it's a root user, but the HTTP server
> > knows otherwise. It sounds like there is no way for a trusted user app to
> > inject the correct uid into the audit event. Would you recommend I use
> > the "user" field instead of "euid" to indicate who is issuing the request?
> > > > Only the data field needs to encoded and ausearch does decode this
> > > >
> > > > field correctly. My message text would look like:
> > > > "op=<op text> data=<encoded data> euid=<uid>"
> > > >
> > > > When I was using ausearch I expected to be able to find events by
> > > > uid using either the "-ua" or "-ue" option that would match the
> > > > euid field's value,
> > >
> > > but no matching events were found. Is this expected behavior?
> > >
> > > What is the record type? ausearch is optimized to expect certain
> > > record types to have fields in a specific order.
> >
> > I am using the AUDIT_USYS_CONFIG event type as I would like to use
> > "aureport -c" to get a summary of the configuration changes to the switch.
> > As an alternative, I could use the AUDIT_TRUSTED_APP event type.
>
> The USYS_CONFIG event is like this:
>
> type=USYS_CONFIG msg=audit(1389095562.552:540): pid=2249 uid=0
> auid=4325 ses=1
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> msg='op=change-system- time exe="/usr/sbin/hwclock" hostname=?
> addr=? terminal=pts/0 res=success'
>
>
> The kernel supplies all the pieces up to the msg= portion. After that is what
> you build. The only real field the event writer does is the op= field, The rest
> are supplied by libaudit. Ausearch does not parse the op= field.
>
> What I would suggest in a case like this is to create a small utility that
> generates the exact report that you want. The auparse library makes that
> super easy. I can dig up the skeleton code for something like this if you want.
Thanks Steve! I'd appreciate the skeleton code. At some point we'll probably
want to create a custom report capability. It sounds like ausearch really only
handles the fields written by the kernel.
Scott G.
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 2 of 3
From: Burn Alting @ 2016-01-06 21:24 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <3311155.vSfdxMiWeB@x2>
Noted .. old habits ... I've always ifdef'd new code effecting a core
element of a capability and remove it a release or two later.
On Wed, 2016-01-06 at 10:45 -0500, Steve Grubb wrote:
> On Wednesday, January 06, 2016 09:29:54 PM Burn Alting wrote:
> > #2 - the 'lol' patch itself. Integrate the ausearch/aureport 'lol' code
> > into auparse() and adjust auparse() to deal with maintain an incore list
> > of incomplete events.
>
> Quick question...there is this:
>
> #define LOL_EVENTS 1
>
> Couldn't we just assume that this is desired and get rid of all the #ifdefs?
> (You don't need to re-send the patch.) Ideally I would like everyone running
> the same code for support purposes.
>
> -Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 2 of 3
From: Steve Grubb @ 2016-01-06 23:40 UTC (permalink / raw)
To: burn; +Cc: linux-audit
In-Reply-To: <1452115455.26850.93.camel@swtf.swtf.dyndns.org>
On Thursday, January 07, 2016 08:24:15 AM Burn Alting wrote:
> Noted .. old habits ... I've always ifdef'd new code effecting a core
> element of a capability and remove it a release or two later.
OK. This patch has been reformatted and then applied.
Thanks,
-Steve
> On Wed, 2016-01-06 at 10:45 -0500, Steve Grubb wrote:
> > On Wednesday, January 06, 2016 09:29:54 PM Burn Alting wrote:
> > > #2 - the 'lol' patch itself. Integrate the ausearch/aureport 'lol' code
> > > into auparse() and adjust auparse() to deal with maintain an incore list
> > > of incomplete events.
> >
> > Quick question...there is this:
> >
> > #define LOL_EVENTS 1
> >
> > Couldn't we just assume that this is desired and get rid of all the
> > #ifdefs? (You don't need to re-send the patch.) Ideally I would like
> > everyone running the same code for support purposes.
> >
> > -Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Steve Grubb @ 2016-01-07 22:31 UTC (permalink / raw)
To: linux-audit, burn
In-Reply-To: <1452076236.26850.89.camel@swtf.swtf.dyndns.org>
On Wednesday, January 06, 2016 09:30:36 PM Burn Alting wrote:
> #3 - modify the standard auparse() test code.
And this patch is applied. Thanks, Burn, for all the patches! This will make
analytical programs much more accurate since interlaced records won't split an
event up any more.
If anyone wants to try out the new audit code from svn please send any
feedback asap. (Same with other bug reports.) I am aiming for a release in the
next 2 days. I just have to finish working on Richard's audit by process name
patch and then its time to release a new package.
-Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Burn Alting @ 2016-01-07 23:05 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit
In-Reply-To: <5860216.amsKJMebuE@x2>
Steve,
Can I suggest you modify src/ausearch-lol.c:check_events() to add in the
AUDIT_PROCTITLE check (will reduce memory overhead as events will be
flushed faster).
Also can we ask Richard put a comment into the appropriate location in
the kernel code to indicate the link between ausearch/aurport/auparse
depending on AUDIT_PROCTITLE being the last record of an event if
present.
Regards
On Thu, 2016-01-07 at 17:31 -0500, Steve Grubb wrote:
> On Wednesday, January 06, 2016 09:30:36 PM Burn Alting wrote:
> > #3 - modify the standard auparse() test code.
>
> And this patch is applied. Thanks, Burn, for all the patches! This will make
> analytical programs much more accurate since interlaced records won't split an
> event up any more.
>
> If anyone wants to try out the new audit code from svn please send any
> feedback asap. (Same with other bug reports.) I am aiming for a release in the
> next 2 days. I just have to finish working on Richard's audit by process name
> patch and then its time to release a new package.
>
> -Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Steve Grubb @ 2016-01-07 23:44 UTC (permalink / raw)
To: burn; +Cc: linux-audit
In-Reply-To: <1452207913.27159.4.camel@swtf.swtf.dyndns.org>
On Friday, January 08, 2016 10:05:13 AM Burn Alting wrote:
> Steve,
>
> Can I suggest you modify src/ausearch-lol.c:check_events() to add in the
> AUDIT_PROCTITLE check (will reduce memory overhead as events will be
> flushed faster).
OK. Good suggestion. The SVN repo has been updated.
> Also can we ask Richard put a comment into the appropriate location in
> the kernel code to indicate the link between ausearch/aurport/auparse
> depending on AUDIT_PROCTITLE being the last record of an event if
> present.
I'll let them answer.
That said one of the things I want to add in the next development cycle is the
ability to get rid of proctitle records if the admin wants to. They waste a
lot of space. But if they are missing then we have the same performance as we
did before I added this patch.
-Steve
> On Thu, 2016-01-07 at 17:31 -0500, Steve Grubb wrote:
> > On Wednesday, January 06, 2016 09:30:36 PM Burn Alting wrote:
> > > #3 - modify the standard auparse() test code.
> >
> > And this patch is applied. Thanks, Burn, for all the patches! This will
> > make analytical programs much more accurate since interlaced records
> > won't split an event up any more.
> >
> > If anyone wants to try out the new audit code from svn please send any
> > feedback asap. (Same with other bug reports.) I am aiming for a release in
> > the next 2 days. I just have to finish working on Richard's audit by
> > process name patch and then its time to release a new package.
> >
> > -Steve
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Paul Moore @ 2016-01-08 3:06 UTC (permalink / raw)
To: Steve Grubb, burn; +Cc: linux-audit
In-Reply-To: <9882122.aVvjtOsnp8@x2>
On January 7, 2016 6:47:02 PM Steve Grubb <sgrubb@redhat.com> wrote:
> On Friday, January 08, 2016 10:05:13 AM Burn Alting wrote:
>> Steve,
>>
>> Can I suggest you modify src/ausearch-lol.c:check_events() to add in the
>> AUDIT_PROCTITLE check (will reduce memory overhead as events will be
>> flushed faster).
>
> OK. Good suggestion. The SVN repo has been updated.
>
>
>> Also can we ask Richard put a comment into the appropriate location in
>> the kernel code to indicate the link between ausearch/aurport/auparse
>> depending on AUDIT_PROCTITLE being the last record of an event if
>> present.
>
> I'll let them answer.
Good thing I happened to read this message, I had stopped reading this
thread...
I really dislike comment only patches and I really, really dislike the
fixed format fields/records/etc. that permeates so much of audit these
days. I'll reserve final judgement for if/when any patches are posted, but
just to be clear, I'm not very excited about stuff like this.
> That said one of the things I want to add in the next development cycle is the
> ability to get rid of proctitle records if the admin wants to. They waste a
> lot of space. But if they are missing then we have the same performance as we
> did before I added this patch.
I wouldn't have a problem with that.
>> On Thu, 2016-01-07 at 17:31 -0500, Steve Grubb wrote:
>> > On Wednesday, January 06, 2016 09:30:36 PM Burn Alting wrote:
>> > > #3 - modify the standard auparse() test code.
>> >
>> > And this patch is applied. Thanks, Burn, for all the patches! This will
>> > make analytical programs much more accurate since interlaced records
>> > won't split an event up any more.
>> >
>> > If anyone wants to try out the new audit code from svn please send any
>> > feedback asap. (Same with other bug reports.) I am aiming for a release in
>> > the next 2 days. I just have to finish working on Richard's audit by
>> > process name patch and then its time to release a new package.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Burn Alting @ 2016-01-08 7:27 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-audit
In-Reply-To: <1521f32d0d0.2852.85c95baa4474aabc7814e68940a78392@paul-moore.com>
On Thu, 2016-01-07 at 22:06 -0500, Paul Moore wrote:
> On January 7, 2016 6:47:02 PM Steve Grubb <sgrubb@redhat.com> wrote:
>
> > On Friday, January 08, 2016 10:05:13 AM Burn Alting wrote:
> >> Steve,
> >>
> >> Can I suggest you modify src/ausearch-lol.c:check_events() to add in the
> >> AUDIT_PROCTITLE check (will reduce memory overhead as events will be
> >> flushed faster).
> >
> > OK. Good suggestion. The SVN repo has been updated.
> >
> >
> >> Also can we ask Richard put a comment into the appropriate location in
> >> the kernel code to indicate the link between ausearch/aurport/auparse
> >> depending on AUDIT_PROCTITLE being the last record of an event if
> >> present.
> >
> > I'll let them answer.
>
> Good thing I happened to read this message, I had stopped reading this
> thread...
>
> I really dislike comment only patches and I really, really dislike the
> fixed format fields/records/etc. that permeates so much of audit these
> days. I'll reserve final judgement for if/when any patches are posted, but
> just to be clear, I'm not very excited about stuff like this.
This is just a request to the kernel audit team, to note that the user
level audit capability is making use of the AUDIT_PROCTITLE record to be
an end of event marker. If you believe this is an unacceptable risk for
downstream processing, then we can take this out and hence withdraw the
request. The alternative is to maintain status quo, and/or optionally
emit the AUDIT_EOE record into the stored audit and be done with it, and
accept the storage cost.
I wholeheartedly agree about the challenges we have with respect to the
current format of the audit events emitted by the kernel. I spend a lot
of effort converting the unstructured, sometimes inconsistently
displayed events into more structured data.
I believe the way forward is to define a more correct, efficient AND
extensible output form for the kernel. On the user space side, we assist
the existing consumers of our audit data (our customers so to speak) by
providing a legacy audispd plugin to take the 'refined' data and format
it into the current 'mash-up'. To assist this interim measure/plugin,
and state that is IS interim, when converting the kernel code to emit a
record, ensure our new method can record the old/legacy format in some
way. The legacy formatting should be able to be compiled out.
Like Paul, I don't like being 'forced' to keep bugs in place within a
data source because those downstream don't want to sustain their data
consumption capability.
>
> > That said one of the things I want to add in the next development cycle is the
> > ability to get rid of proctitle records if the admin wants to. They waste a
> > lot of space. But if they are missing then we have the same performance as we
> > did before I added this patch.
>
> I wouldn't have a problem with that.
>
> >> On Thu, 2016-01-07 at 17:31 -0500, Steve Grubb wrote:
> >> > On Wednesday, January 06, 2016 09:30:36 PM Burn Alting wrote:
> >> > > #3 - modify the standard auparse() test code.
> >> >
> >> > And this patch is applied. Thanks, Burn, for all the patches! This will
> >> > make analytical programs much more accurate since interlaced records
> >> > won't split an event up any more.
> >> >
> >> > If anyone wants to try out the new audit code from svn please send any
> >> > feedback asap. (Same with other bug reports.) I am aiming for a release in
> >> > the next 2 days. I just have to finish working on Richard's audit by
> >> > process name patch and then its time to release a new package.
>
>
> --
> paul moore
> www.paul-moore.com
>
>
^ permalink raw reply
* Re: Patch to auparse to handle out of order messages 3 of 3
From: Paul Moore @ 2016-01-08 23:22 UTC (permalink / raw)
To: burn; +Cc: linux-audit
In-Reply-To: <1452238033.3119.44.camel@swtf.swtf.dyndns.org>
On Fri, Jan 8, 2016 at 2:27 AM, Burn Alting <burn@swtf.dyndns.org> wrote:
> On Thu, 2016-01-07 at 22:06 -0500, Paul Moore wrote:
>> On January 7, 2016 6:47:02 PM Steve Grubb <sgrubb@redhat.com> wrote:
>>
>> > On Friday, January 08, 2016 10:05:13 AM Burn Alting wrote:
>> >> Steve,
>> >>
>> >> Can I suggest you modify src/ausearch-lol.c:check_events() to add in the
>> >> AUDIT_PROCTITLE check (will reduce memory overhead as events will be
>> >> flushed faster).
>> >
>> > OK. Good suggestion. The SVN repo has been updated.
>> >
>> >
>> >> Also can we ask Richard put a comment into the appropriate location in
>> >> the kernel code to indicate the link between ausearch/aurport/auparse
>> >> depending on AUDIT_PROCTITLE being the last record of an event if
>> >> present.
>> >
>> > I'll let them answer.
>>
>> Good thing I happened to read this message, I had stopped reading this
>> thread...
>>
>> I really dislike comment only patches and I really, really dislike the
>> fixed format fields/records/etc. that permeates so much of audit these
>> days. I'll reserve final judgement for if/when any patches are posted, but
>> just to be clear, I'm not very excited about stuff like this.
>
> This is just a request to the kernel audit team, to note that the user
> level audit capability is making use of the AUDIT_PROCTITLE record to be
> an end of event marker. If you believe this is an unacceptable risk for
> downstream processing, then we can take this out and hence withdraw the
> request. The alternative is to maintain status quo, and/or optionally
> emit the AUDIT_EOE record into the stored audit and be done with it, and
> accept the storage cost.
I've already ranted plenty about field ordering so I won't bore you
all again, but you can apply the same rants to record ordering for an
event. I'm begrudgingly going to accept the field ordering in the
current fixed-string format, but that will go away in the future. I'm
much, much less inclined to accept the record ordering, especially
given the idea that there is talk of optionally removing the
AUDIT_PROCTITLE record.
> I wholeheartedly agree about the challenges we have with respect to the
> current format of the audit events emitted by the kernel. I spend a lot
> of effort converting the unstructured, sometimes inconsistently
> displayed events into more structured data.
>
> I believe the way forward is to define a more correct, efficient AND
> extensible output form for the kernel. On the user space side, we assist
> the existing consumers of our audit data (our customers so to speak) by
> providing a legacy audispd plugin to take the 'refined' data and format
> it into the current 'mash-up'. To assist this interim measure/plugin,
> and state that is IS interim, when converting the kernel code to emit a
> record, ensure our new method can record the old/legacy format in some
> way. The legacy formatting should be able to be compiled out.
Due to other projects, it is taking me much longer than anticipated,
but I have no given up on my effort to arrive at a new
kernel/userspace format. I started some work towards removing
audit_log_format() before the holidays, which is the important first
step.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Patch to add support for more syslog facilities
From: Aleksander Adamowski @ 2016-01-09 0:56 UTC (permalink / raw)
To: Linux-audit@redhat.com
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
The set of syslog facilities that can be configured for the builting syslog
plugin is pretty limited (LOG_LOCAL0 - LOG_LOCAL9).
This patch adds a bunch of other facilities that might make sense for some
people (like us). Facilities that wouldn¹t make any sense (like LOG_NEWS or
LOG_LPR) are still left out.
Best Regards,
--
Olo
[-- Attachment #2: audispd_syslog_facilities.patch --]
[-- Type: application/octet-stream, Size: 834 bytes --]
Index: audisp/audispd-builtins.c
===================================================================
--- audisp/audispd-builtins.c (revision 1150)
+++ audisp/audispd-builtins.c (working copy)
@@ -302,6 +302,16 @@
facility = LOG_LOCAL6;
else if (strcasecmp(conf->args[i], "LOG_LOCAL7") == 0)
facility = LOG_LOCAL7;
+ else if (strcasecmp(conf->args[i], "LOG_AUTH") == 0)
+ facility = LOG_AUTH;
+ else if (strcasecmp(conf->args[i], "LOG_AUTHPRIV") == 0)
+ facility = LOG_AUTHPRIV;
+ else if (strcasecmp(conf->args[i], "LOG_DAEMON") == 0)
+ facility = LOG_DAEMON;
+ else if (strcasecmp(conf->args[i], "LOG_SYSLOG") == 0)
+ facility = LOG_SYSLOG;
+ else if (strcasecmp(conf->args[i], "LOG_USER") == 0)
+ facility = LOG_USER;
else {
syslog(LOG_ERR,
"Unknown log priority/facility %s",
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Excluding selected CRYPTO_KEY_USER events
From: Richard Young @ 2016-01-09 16:26 UTC (permalink / raw)
To: linux-audit
[-- Attachment #1.1: Type: text/plain, Size: 388 bytes --]
I know I could exclude all msgtype CRYPTO_KEY_USER audit events, but would
like to exclude just specific ones.
I would like to exclude ones for a specific UID, hostname, or IP.
There are many example of how to exclude specific files, directory events,
or syscall events.
Can somebody suggest a way to suppress specific CRYPTO_KEY_USER events by
UID, hostname, or IP?
[-- Attachment #1.2: Type: text/html, Size: 475 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: Excluding selected CRYPTO_KEY_USER events
From: Steve Grubb @ 2016-01-09 19:35 UTC (permalink / raw)
To: linux-audit; +Cc: Richard Young
In-Reply-To: <201601091626.u09GQVCl006045@d01av01.pok.ibm.com>
On Saturday, January 09, 2016 10:26:06 AM Richard Young wrote:
> I know I could exclude all msgtype CRYPTO_KEY_USER audit events, but would
> like to exclude just specific ones.
> I would like to exclude ones for a specific UID, hostname, or IP.
>
> There are many example of how to exclude specific files, directory events,
> or syscall events.
>
> Can somebody suggest a way to suppress specific CRYPTO_KEY_USER events by
> UID, hostname, or IP?
I opened a bz to ask for this capability a little over a month ago:
https://bugzilla.redhat.com/show_bug.cgi?id=1287745
Unfortunately, I don't think you can do anything until that lands.
This particular event comes from user space. So, the kernel cannot filter on IP
address. And specifically, the kernel can never really filter on IP address
because its typically not an argument to any but 2 or 3 syscalls.
There is a chance that you might be able to use the USER filter if the selinux
type is unique to whatever you wanted to remove.
-a never,user -F subj_type=httpd_t
-Steve
^ permalink raw reply
* [RESEND][PATCH 00/15] Rework tty audit
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1447207560-16410-1-git-send-email-peter@hurleysoftware.com>
Hi Greg,
Here's a resend of the original tty audit series from Nov 10;
no objections from the audit maintainers.
Original message follows:
This patch series overhauls tty audit support. The goal was to simplify
and speed up tty auditing, which was a significant performance hit even
when disabled.
The main features of this series are:
* Remove reference counting; the purpose of reference counting the per-
process tty_audit_buf was to prevent premature deletion if the
buffer was in-use when tty auditing was exited for the process.
However, since the process is single-threaded at tty_audit_exit(),
the buffer cannot be in-use by another thread. Patch 11/15.
* Remove functionally dead code, such as tty_put_user(). Patch 2/15.
* Atomically modify tty audit enable/disable flags to support lockless
read. Patch 9/15.
Regards,
Peter Hurley (15):
tty: audit: Early-out pty master reads earlier
tty: audit: Never audit packet mode
tty: audit: Remove icanon mode from call chain
tty: audit: Defer audit buffer association
tty: audit: Take siglock directly
tty: audit: Ignore current association for audit push
tty: audit: Combine push functions
tty: audit: Track tty association with dev_t
tty: audit: Handle tty audit enable atomically
tty: audit: Remove false memory optimization
tty: audit: Remove tty_audit_buf reference counting
tty: audit: Simplify first-use allocation
tty: audit: Check audit enable first
tty: audit: Always push audit buffer before TIOCSTI
tty: audit: Poison tty_audit_buf while process exits
drivers/tty/n_tty.c | 25 ++----
drivers/tty/tty_audit.c | 231 ++++++++++++++----------------------------------
include/linux/audit.h | 4 +
include/linux/sched.h | 1 -
include/linux/tty.h | 12 +--
kernel/audit.c | 27 +++---
6 files changed, 97 insertions(+), 203 deletions(-)
--
2.7.0
^ permalink raw reply
* [RESEND][PATCH 01/15] tty: audit: Early-out pty master reads earlier
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
Reads from pty masters are not logged; early-out before taking
locks.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/tty_audit.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 3d245cd..ead924e 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -276,16 +276,16 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
if (unlikely(size == 0))
return;
+ if (tty->driver->type == TTY_DRIVER_TYPE_PTY
+ && tty->driver->subtype == PTY_TYPE_MASTER)
+ return;
+
spin_lock_irqsave(¤t->sighand->siglock, flags);
audit_log_tty_passwd = current->signal->audit_tty_log_passwd;
spin_unlock_irqrestore(¤t->sighand->siglock, flags);
if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
return;
- if (tty->driver->type == TTY_DRIVER_TYPE_PTY
- && tty->driver->subtype == PTY_TYPE_MASTER)
- return;
-
buf = tty_audit_buf_get(tty, icanon);
if (!buf)
return;
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 02/15] tty: audit: Never audit packet mode
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
tty audit never logs pty master reads, but packet mode only works for
pty masters, so tty_audit_add_data() was never logging packet mode
anyway.
Don't audit packet mode data. As those are the lone call sites, remove
tty_put_user().
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 90eca26..eeae6bc 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -153,15 +153,6 @@ static inline unsigned char *echo_buf_addr(struct n_tty_data *ldata, size_t i)
return &ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)];
}
-static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
- unsigned char __user *ptr)
-{
- struct n_tty_data *ldata = tty->disc_data;
-
- tty_audit_add_data(tty, &x, 1, ldata->icanon);
- return put_user(x, ptr);
-}
-
static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
size_t tail, size_t n)
{
@@ -2203,11 +2194,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
cs = tty->link->ctrl_status;
tty->link->ctrl_status = 0;
spin_unlock_irq(&tty->link->ctrl_lock);
- if (tty_put_user(tty, cs, b++)) {
+ if (put_user(cs, b)) {
retval = -EFAULT;
- b--;
break;
}
+ b++;
nr--;
break;
}
@@ -2253,11 +2244,11 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
/* Deal with packet mode. */
if (packet && b == buf) {
- if (tty_put_user(tty, TIOCPKT_DATA, b++)) {
+ if (put_user(TIOCPKT_DATA, b)) {
retval = -EFAULT;
- b--;
break;
}
+ b++;
nr--;
}
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 03/15] tty: audit: Remove icanon mode from call chain
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
The tty termios bits cannot change while n_tty_read() is in the
i/o loop; the termios_rwsem ensures mutual exclusion with termios
changes in n_tty_set_termios(). Check L_ICANON() directly and
eliminate icanon parameter.
NB: tty_audit_add_data() => tty_audit_buf_get() => tty_audit_buf_alloc()
is a single path; ie., tty_audit_buf_get() and tty_audit_buf_alloc()
have no other callers.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 6 +++---
drivers/tty/tty_audit.c | 22 +++++++++-------------
include/linux/tty.h | 4 ++--
3 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index eeae6bc..5d060fc 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -162,7 +162,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
int uncopied;
if (n > size) {
- tty_audit_add_data(tty, from, size, ldata->icanon);
+ tty_audit_add_data(tty, from, size);
uncopied = copy_to_user(to, from, size);
if (uncopied)
return uncopied;
@@ -171,7 +171,7 @@ static int tty_copy_to_user(struct tty_struct *tty, void __user *to,
from = ldata->read_buf;
}
- tty_audit_add_data(tty, from, n, ldata->icanon);
+ tty_audit_add_data(tty, from, n);
return copy_to_user(to, from, n);
}
@@ -1984,7 +1984,7 @@ static int copy_from_read_buf(struct tty_struct *tty,
retval = copy_to_user(*b, from, n);
n -= retval;
is_eof = n == 1 && *from == EOF_CHAR(tty);
- tty_audit_add_data(tty, from, n, ldata->icanon);
+ tty_audit_add_data(tty, from, n);
smp_store_release(&ldata->read_tail, ldata->read_tail + n);
/* Turn single EOF into zero-length read */
if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index ead924e..d2a004a 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,8 +22,7 @@ struct tty_audit_buf {
unsigned char *data; /* Allocated size N_TTY_BUF_SIZE */
};
-static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
- unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
{
struct tty_audit_buf *buf;
@@ -35,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(int major, int minor,
goto err_buf;
atomic_set(&buf->count, 1);
mutex_init(&buf->mutex);
- buf->major = major;
- buf->minor = minor;
- buf->icanon = icanon;
+ buf->major = tty->driver->major;
+ buf->minor = tty->driver->minor_start + tty->index;
+ buf->icanon = !!L_ICANON(tty);
buf->valid = 0;
return buf;
@@ -216,8 +215,7 @@ int tty_audit_push_current(void)
* if TTY auditing is disabled or out of memory. Otherwise, return a new
* reference to the buffer.
*/
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
- unsigned icanon)
+static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
{
struct tty_audit_buf *buf, *buf2;
unsigned long flags;
@@ -234,9 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
}
spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- buf2 = tty_audit_buf_alloc(tty->driver->major,
- tty->driver->minor_start + tty->index,
- icanon);
+ buf2 = tty_audit_buf_alloc(tty);
if (buf2 == NULL) {
audit_log_lost("out of memory in TTY auditing");
return NULL;
@@ -265,13 +261,13 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty,
*
* Audit @data of @size from @tty, if necessary.
*/
-void tty_audit_add_data(struct tty_struct *tty, const void *data,
- size_t size, unsigned icanon)
+void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
{
struct tty_audit_buf *buf;
int major, minor;
int audit_log_tty_passwd;
unsigned long flags;
+ unsigned int icanon = !!L_ICANON(tty);
if (unlikely(size == 0))
return;
@@ -286,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data,
if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
return;
- buf = tty_audit_buf_get(tty, icanon);
+ buf = tty_audit_buf_get(tty);
if (!buf)
return;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 56d1133..c1d1f08 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -604,7 +604,7 @@ extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
/* tty_audit.c */
#ifdef CONFIG_AUDIT
extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
- size_t size, unsigned icanon);
+ size_t size);
extern void tty_audit_exit(void);
extern void tty_audit_fork(struct signal_struct *sig);
extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
@@ -612,7 +612,7 @@ extern void tty_audit_push(struct tty_struct *tty);
extern int tty_audit_push_current(void);
#else
static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
- size_t size, unsigned icanon)
+ size_t size)
{
}
static inline void tty_audit_tiocsti(struct tty_struct *tty, char ch)
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 04/15] tty: audit: Defer audit buffer association
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
The tty audit buffer used to audit/record tty input is allocated on
the process's first call to tty_audit_add_data(), and not freed until
the process exits. On each call to tty_audit_add_data(), the current
tty is compared (by major:minor) with the last tty associated with
the audit buffer, and if the tty has changed the existing data is
logged to the audit log. The audit buffer is then re-associated with
the new tty.
Currently, the audit buffer is immediately associated with the tty;
however, the association must be re-checked when the buffer is locked
prior to copying the tty input. This extra step is always necessary,
since a concurrent read of a different tty by another thread of the
process may have used the buffer in between allocation and buffer
lock.
Rather than associate the audit buffer with the tty at allocation,
leave the buffer initially un-associated (null dev_t); simply let the
re-association check also perform the initial association.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/tty_audit.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index d2a004a..9effa81 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -22,7 +22,7 @@ struct tty_audit_buf {
unsigned char *data; /* Allocated size N_TTY_BUF_SIZE */
};
-static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_alloc(void)
{
struct tty_audit_buf *buf;
@@ -34,9 +34,9 @@ static struct tty_audit_buf *tty_audit_buf_alloc(struct tty_struct *tty)
goto err_buf;
atomic_set(&buf->count, 1);
mutex_init(&buf->mutex);
- buf->major = tty->driver->major;
- buf->minor = tty->driver->minor_start + tty->index;
- buf->icanon = !!L_ICANON(tty);
+ buf->major = 0;
+ buf->minor = 0;
+ buf->icanon = 0;
buf->valid = 0;
return buf;
@@ -211,11 +211,11 @@ int tty_audit_push_current(void)
/**
* tty_audit_buf_get - Get an audit buffer.
*
- * Get an audit buffer for @tty, allocate it if necessary. Return %NULL
+ * Get an audit buffer, allocate it if necessary. Return %NULL
* if TTY auditing is disabled or out of memory. Otherwise, return a new
* reference to the buffer.
*/
-static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
+static struct tty_audit_buf *tty_audit_buf_get(void)
{
struct tty_audit_buf *buf, *buf2;
unsigned long flags;
@@ -232,7 +232,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty)
}
spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- buf2 = tty_audit_buf_alloc(tty);
+ buf2 = tty_audit_buf_alloc();
if (buf2 == NULL) {
audit_log_lost("out of memory in TTY auditing");
return NULL;
@@ -282,7 +282,7 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
if (!audit_log_tty_passwd && icanon && !L_ECHO(tty))
return;
- buf = tty_audit_buf_get(tty);
+ buf = tty_audit_buf_get();
if (!buf)
return;
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 05/15] tty: audit: Take siglock directly
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
lock_task_sighand() is for situations where the struct task_struct*
may disappear while trying to deref the sighand; this never applies
to 'current'.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/tty_audit.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 9effa81..5f65653 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -180,22 +180,19 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
int tty_audit_push_current(void)
{
struct tty_audit_buf *buf = ERR_PTR(-EPERM);
- struct task_struct *tsk = current;
unsigned long flags;
- if (!lock_task_sighand(tsk, &flags))
- return -ESRCH;
-
- if (tsk->signal->audit_tty) {
- buf = tsk->signal->tty_audit_buf;
+ spin_lock_irqsave(¤t->sighand->siglock, flags);
+ if (current->signal->audit_tty) {
+ buf = current->signal->tty_audit_buf;
if (buf)
atomic_inc(&buf->count);
}
- unlock_task_sighand(tsk, &flags);
+ spin_unlock_irqrestore(¤t->sighand->siglock, flags);
/*
* Return 0 when signal->audit_tty set
- * but tsk->signal->tty_audit_buf == NULL.
+ * but current->signal->tty_audit_buf == NULL.
*/
if (!buf || IS_ERR(buf))
return PTR_ERR(buf);
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 06/15] tty: audit: Ignore current association for audit push
From: Peter Hurley @ 2016-01-10 4:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
In canonical read mode, each line read and logged is pushed separately
with tty_audit_push(). For all single-threaded processes and multi-threaded
processes reading from only one tty, this patch has no effect; the last line
read will still be the entry pushed to the audit log because the tty
association cannot have changed between tty_audit_add_data() and
tty_audit_push().
For multi-threaded processes reading from different ttys concurrently,
the audit log will have mixed log entries anyway. Consider two ttys
audited concurrently:
CPU0 CPU1
---------- ------------
tty_audit_add_data(ttyA)
tty_audit_add_data(ttyB)
tty_audit_push()
tty_audit_add_data(ttyB)
tty_audit_push()
This patch will now cause the ttyB output to be split into separate
audit log entries.
However, this possibility is equally likely without this patch:
CPU0 CPU1
---------- ------------
tty_audit_add_data(ttyB)
tty_audit_add_data(ttyA)
tty_audit_push()
tty_audit_add_data(ttyB)
tty_audit_push()
Mixed canonical and non-canonical reads have similar races.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/n_tty.c | 2 +-
drivers/tty/tty_audit.c | 11 +++--------
include/linux/tty.h | 2 +-
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 5d060fc..6bab08a 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2078,7 +2078,7 @@ static int canon_copy_from_read_buf(struct tty_struct *tty,
ldata->line_start = ldata->read_tail;
else
ldata->push = 0;
- tty_audit_push(tty);
+ tty_audit_push();
}
return 0;
}
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5f65653..5ae4839 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -313,9 +313,9 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
/**
* tty_audit_push - Push buffered data out
*
- * Make sure no audit data is pending for @tty on the current process.
+ * Make sure no audit data is pending on the current process.
*/
-void tty_audit_push(struct tty_struct *tty)
+void tty_audit_push(void)
{
struct tty_audit_buf *buf;
unsigned long flags;
@@ -331,13 +331,8 @@ void tty_audit_push(struct tty_struct *tty)
spin_unlock_irqrestore(¤t->sighand->siglock, flags);
if (buf) {
- int major, minor;
-
- major = tty->driver->major;
- minor = tty->driver->minor_start + tty->index;
mutex_lock(&buf->mutex);
- if (buf->major == major && buf->minor == minor)
- tty_audit_buf_push(buf);
+ tty_audit_buf_push(buf);
mutex_unlock(&buf->mutex);
tty_audit_buf_put(buf);
}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index c1d1f08..21e3722 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -608,7 +608,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
extern void tty_audit_exit(void);
extern void tty_audit_fork(struct signal_struct *sig);
extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(struct tty_struct *tty);
+extern void tty_audit_push(void);
extern int tty_audit_push_current(void);
#else
static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
--
2.7.0
^ permalink raw reply related
* [RESEND][PATCH 07/15] tty: audit: Combine push functions
From: Peter Hurley @ 2016-01-10 4:59 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-audit, linux-kernel, Peter Hurley
In-Reply-To: <1452401948-30790-1-git-send-email-peter@hurleysoftware.com>
tty_audit_push() and tty_audit_push_current() perform identical
tasks; eliminate the tty_audit_push() implementation and the
tty_audit_push_current() name.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/tty_audit.c | 35 +++--------------------------------
include/linux/tty.h | 8 ++------
kernel/audit.c | 2 +-
3 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 5ae4839..6b82c3c 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -172,12 +172,11 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch)
}
/**
- * tty_audit_push_current - Flush current's pending audit data
+ * tty_audit_push - Flush current's pending audit data
*
- * Try to lock sighand and get a reference to the tty audit buffer if available.
- * Flush the buffer or return an appropriate error code.
+ * Returns 0 if success, -EPERM if tty audit is disabled
*/
-int tty_audit_push_current(void)
+int tty_audit_push(void)
{
struct tty_audit_buf *buf = ERR_PTR(-EPERM);
unsigned long flags;
@@ -309,31 +308,3 @@ void tty_audit_add_data(struct tty_struct *tty, const void *data, size_t size)
mutex_unlock(&buf->mutex);
tty_audit_buf_put(buf);
}
-
-/**
- * tty_audit_push - Push buffered data out
- *
- * Make sure no audit data is pending on the current process.
- */
-void tty_audit_push(void)
-{
- struct tty_audit_buf *buf;
- unsigned long flags;
-
- spin_lock_irqsave(¤t->sighand->siglock, flags);
- if (likely(!current->signal->audit_tty)) {
- spin_unlock_irqrestore(¤t->sighand->siglock, flags);
- return;
- }
- buf = current->signal->tty_audit_buf;
- if (buf)
- atomic_inc(&buf->count);
- spin_unlock_irqrestore(¤t->sighand->siglock, flags);
-
- if (buf) {
- mutex_lock(&buf->mutex);
- tty_audit_buf_push(buf);
- mutex_unlock(&buf->mutex);
- tty_audit_buf_put(buf);
- }
-}
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 21e3722..b17f759 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -608,8 +608,7 @@ extern void tty_audit_add_data(struct tty_struct *tty, const void *data,
extern void tty_audit_exit(void);
extern void tty_audit_fork(struct signal_struct *sig);
extern void tty_audit_tiocsti(struct tty_struct *tty, char ch);
-extern void tty_audit_push(void);
-extern int tty_audit_push_current(void);
+extern int tty_audit_push(void);
#else
static inline void tty_audit_add_data(struct tty_struct *tty, const void *data,
size_t size)
@@ -624,10 +623,7 @@ static inline void tty_audit_exit(void)
static inline void tty_audit_fork(struct signal_struct *sig)
{
}
-static inline void tty_audit_push(struct tty_struct *tty)
-{
-}
-static inline int tty_audit_push_current(void)
+static inline int tty_audit_push(void)
{
return 0;
}
diff --git a/kernel/audit.c b/kernel/audit.c
index 5ffcbd3..270dfb9 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -921,7 +921,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
if (err == 1) { /* match or error */
err = 0;
if (msg_type == AUDIT_USER_TTY) {
- err = tty_audit_push_current();
+ err = tty_audit_push();
if (err)
break;
}
--
2.7.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox