SysmonForLinux icon indicating copy to clipboard operation
SysmonForLinux copied to clipboard

Consider a unix domain socket for log forwarding

Open scudette opened this issue 4 years ago • 11 comments

Writing events to syslog is inefficient as they hit the disk and then some other forwarding agent needs to read the back out from disk, parse the xml and forward the events.

On Windows we can use ETW to bypass the logging system anyway but on linux this is not an option as there is no system level mechanism to intercept syslog in a more efficient way.

Having logs fill up the disk when you are going to forward them anyway is kind of pointless too.

It would be nice to have an option where sysmon could open a unix domain socket (or maybe even a tcp socket) where other programs can just read events directly from there and not have the events being written to disk.

scudette avatar Dec 04 '21 02:12 scudette

That's a great idea. What happens if nothing consumes the logs from the unix domain socket? Or isn't listening on the TCP socket? Should the unix domain socket be created if it doesn't already exist? I guess we should check the perms on it first too.

kesheldr avatar Dec 07 '21 11:12 kesheldr

Currently the events are dropped on the floor if nothing is reading it - this saves cpu cycles too if no event forwarder is interested sysmon doesn't even bother to encode the event.

Currently the socket is created with the default umask owned by root:root

Please see #50 for an implementation. This implementation only allows a single connection at the moment - when a second connection happens it kicks the old one off. It might be possible to arrange for multiple connections if we see the need.

The forwarding agent needs to implement a reconnection scheme because when sysmon crashes (e.g. due to #9 this happens all the time) the socket disappears and if the service file is configured to restart (currently it is not but this is a separate issue) the socket will come back later.

scudette avatar Dec 07 '21 11:12 scudette

Hello I've just had a look at your implementation. I'm not sure we should include the frozen JSON generator and would prefer that we rely on a supported library instead - this means that we don't get tied to an outdated version with potential bugs etc.

I appreciate the issue over parsing new options and can see your approach as a fair compromise - will discuss internally whether we'd prefer to up-issue the manifest instead. Either way, good idea.

As for the crashes (#9) - I'm interested in any more information you have. The crash in #9 looked like it was related to not discovering offsets and continued when it should have stopped, which I couldn't recreate. If that was the case, then it shouldn't be crashing all the time. If, however, it is crashing all the time, please let me know anything that can help recreate and/or debug this.

kesheldr avatar Dec 07 '21 12:12 kesheldr

Considering json serialization is dead simple and to reduce dependencies it makes sense to include it in (the license seems compatible). The current way of processing command line options is very rigid (since it is tied to the config schema) and so my hacky workaround avoids having to update the schema.

As for #9 the issue is that here

https://github.com/Sysinternals/SysinternalsEBPF/blob/43aed3ec3bb09330d475dd4479829e5a64e5f363/telemetryLoader.c#L1437

there were no events delivered for a while and so this code calls ebpfStart() but erroneously sets the last parameter (restart) to false. That parameter is intended to avoid re-calculating the offsets again because the offsets are already known (since it is just reinstalling the ebpf code). It is not possible to re-calculate the offsets because the code here

https://github.com/Sysinternals/SysinternalsEBPF/blob/43aed3ec3bb09330d475dd4479829e5a64e5f363/discoverOffsets.c#L1790

Calls setsid() which can not be called on process leader already (so that fails, failing discoverOffets()) (The calling code is process leader because it was forked already and previously discovered the offsets).

The ebpfStart() function is supposed to skip discovering the offset if the restart parameter is true https://github.com/Sysinternals/SysinternalsEBPF/blob/43aed3ec3bb09330d475dd4479829e5a64e5f363/telemetryLoader.c#L1289

but somehow this does not work - even if the restart parameter is set to true - not sure why yet - I have to look at it further.

In any case the service file that is created for sysmon (used with sysmon -i ) does not specify the restart commands:

https://github.com/Sysinternals/SysmonForLinux/blob/4577b377f1d15b2cd457b8f8493f79189e863a3c/sysmon.service#L19

It should be changed to something like:

[Service]
Type=forking
User=root
WorkingDirectory=/opt/sysmon
ExecStart=/opt/sysmon/sysmon -i /opt/sysmon/config.xml -service -socket /var/run/sysmon.sock -json
Restart=always
RestartSec=10

so when sysmon crashes it just never restarts - not ideal in any case.

The other issue I found is that since the service file is included in the deb package, when the package is reinstalled, the service file is replaced with the default so even after fixing the service file, it just gets wiped after a reinstall - probably there needs to be some tweaking of the deb control file.

scudette avatar Dec 07 '21 13:12 scudette

This is very useful - changing restart to true causes ebpfStart() to not rediscover the offsets, but the return value was being incorrectly checked; it was treating it as a bool instead of success == 0. Fixed both and that seems to fix the issue with #9. I'll close that one.

I'll look further into your proposed change and hope to incorporate it.

Many thanks for your help on this.

kesheldr avatar Dec 07 '21 14:12 kesheldr

Thanks! I realised that SysinternalsEBPF already links to libjson-glib which seems a bit more tedious to use but probably can be used to build the json objects instead. Let me know if this is more preferable and I will rework the PR to do this.

scudette avatar Dec 07 '21 15:12 scudette

Yes that would be much more preferable. Maybe break it out into two PRs - one for JSON and one for the socket. I'll check internally that we have no issues and can rework the command line switch for the socket into SysmonCommon.

kesheldr avatar Dec 07 '21 15:12 kesheldr

hi, this one and https://github.com/Sysinternals/SysmonForLinux/pull/50 would be really nice! any news on this?

pH-T avatar Jan 22 '22 18:01 pH-T

Hi - Between the two asks here, log forwarder and log format, which is more important? I.e., Sysmon can continue to log in XML format (over say a socket) and the consumer can transform to whatever format they want.

MarioHewardt avatar Mar 06 '23 23:03 MarioHewardt

Hi Mario,

I think as a first step, it would be great to have the socket option available. I still think JSON can be valuable, as it can be difficult or more expensive for certain platforms to parse or transform XML, but I think this option would be the first choice.

Thanks! Wes

weslambert avatar Mar 07 '23 12:03 weslambert

Hi @scudette - would it be possible to break the PR into two parts? One for the socket work and the other for JSON? I think we want to have the socket work first and foremost and we can explore more about the formats.

MarioHewardt avatar Mar 09 '23 01:03 MarioHewardt