Suricata

I see how it can be useful for some use cases. I don't think we will have the resources to implement this any time soon though. Can you assist us with this?

Sure, I can test any patches on our test system if needed.

Victor,

That's fine, it's not a life threatening situation :P This is the only feature missing in suricata to be able to support your IDS, and I guess we can use Barnyard2 during that wait time.

Thanks

Thanks!

I'll test this patch this week and I'll keep you posted.

Hi Mike,

I will also check out this patch tomorrow. Is your patch compatible with Snort 2.9 unsock output ?

Hi Mike,

I will also check out this patch tomorrow. Is your patch compatible with Snort 2.9 unsock output ?

I have reviewed Vivek's patch. Looking very good overall. Here are some comments:

1. please use strlcpy and strlcat in every place strncpy, strcpy, etc are used.
2. indent looks pretty messed up in the redmine patch viewer, please use 4 spaces indent everywhere.
3. "remote" in UnixSocketAlert looks like it can be initialized in the thread init function and then stored in the thread local memory. Saves some cycles per alert.
4. If sendto fails, it does "return TM_ECODE_FAILED;". This will make Suricata close the thread and try to respawn it. In what conditions can sendto fail?
5. minor style comment: overall in Suricata we do most checks explicit, so instead of if (p->payload_len) we do if (p->payload_len > 0), instead of if (p->icmp4h) we do if (p->icmpv4h != NULL).

Vivek will do first review on Mike's patch, I'll review the end result :)

Thanks Vivek and Mike, appreciate it!

Vivek Rajagopalan wrote:

Hi Mike,

I will also check out this patch tomorrow. Is your patch compatible with Snort 2.9 unsock output ?

(Vivek, thanks and sorry I didn't see your response.)

Re: stream v. dgram, as long as the typical alert can fit in an AF_LOCAL dgram, dgram is fine by me. (Indeed, I think that paves the way to remove output mutexen, since whole datagram sends should never be interleaved.)

Re: unsock/alert_unixsock compatibility, no, my patch is not compatible. FWIW and IIUC, I find having a different output format just because you have a different output device (file, socket, network socket, POSIX message queue, whatever) a little weird and certainly un-UNIXy. For compatibility, if needed, I'd suggest a new output format module with configurable output device.

I tested out the patch and it works fine. I also could not find any issues with Suricata coding style based on Victor's feedback to my patch.

Test
A tiny ruby program

require 'socket'

raise "Usage : usock sock_path" unless ARGV.size == 1

socket  = UNIXServer.new(ARGV.shift).accept

# Just read messages from sock and print out 
while(line = socket.gets) do
   puts line
end

I sent alert_fastlog to the unix socket on which the above script was running and it printed alerts as expected.

Comment to Mike

It may help to ignore SIGPIPE - especially since we are fdopen-ing the stream unix socket. At the moment, Suricata will trip on SIGPIPE and crash if the process listening on the unix socket closes socket. To test this run the above script and press Ctrl+C while alerts are coming in. This is only an issue with STREAM. Alternately you can move to DGRAM but then we need to write a generic replacement for fwrite as well.

Mike Pomraning wrote:

For compatibility, if needed, I'd suggest a new output format module with configurable output device.

I tend to agree here. It would also lead to a lot less code duplication.

The fast log and syslog outputs are pretty much the same, think we can merge those and add the unix socket output while at it?

It may help to ignore SIGPIPE [...] This is only an issue with STREAM.
Alternately you can move to DGRAM but then we need to write a generic replacement for fwrite as well.

Yes, let me think about that but I think SIG_IGNoring PIPE on stream sockets may be best in the near term.

On my Linux box, a DGRAM masquerading as a FILE* isn't suitable: fprintf(fdopen(dgram_sock), ...) cheerfully ignores ENOTCONN if the reader should die -- a situation less desirable IMHO than having suricata itself die. (I assume you meant fprintf, not fwrite.)

Digression: Long term, yes, IMHO an appropriate approach for suricata's file-like output is "polymorphic:" opaque output handles provide function pointers that effectively implement out->printf (if needed), out->write and (when a formatting module gets serious about performance) out->writev as appropriate to the output channel. Then under the hood we can pass MSG_NOSIGNAL if needed, detect/recover from errors, etc., without being constrained by stdio.h, and the caller won't have a clue whether the output is a file, a socket, shared memory, whatever.

Mike's patches have been applied to the git master, thanks a lot Mike!

This adds unixsock output to all the line based outputs, but not to unified2.