Re: [PATCH 0/5] Add Zabbix functionality to suricata-reporter

[Michael Tremer <michael.tremer@ipfire.org>]

Hello Robin,

> On 28 Aug 2026, at 00:51, Robin Roevens <robin.roevens@disroot.org> wrote:
> 
> Hi Michael
> 
> Vacation period here is officially over.. So I have no more excuses and
> I'm ready to dive into this again :-)

Haha, I hope you had a relaxing time and didn’t think too much about this.

> Michael Tremer schreef op vr 31-07-2026 om 11:24 [+0100]:
>> Hello Robin,
>> 
>> Thank you very much for sending these patches.
>> 
>> Before we dig into the code, I have a couple of questions about the
>> design...
> Ok, I will try to answer them first, as discussing this may result in
> significant design changes :-)

Probably only less code because we can let SQLite do all the work :)

>> 
>>> On 30 Jul 2026, at 20:15, Robin Roevens <robin.roevens@disroot.org>
>>> wrote:
>>> 
>>> Hi all,
>>> 
>>> As discussed here earlier, I've worked on implementing sending
>>> Suricata alerts straight to Zabbix from within suricata-reporter
>>> instead
>>> of trying to parse the suricata logging separately using the Zabbix
>>> agent.
>>> 
>>> For this I use the zabbix-utils python library, which I submited
>>> here
>>> also as a separate pak (but meanwhile already requires an update,
>>> which
>>> I will post soon). This set of patches makes suricata-reporter able
>>> to
>>> directly communicate to a Zabbix server without having the
>>> zabbix_agentd
>>> pak installed, sending suricata alerts in real-time.
>> 
>> Yes, this is a good choice and I like that suricate-reporter will try
>> to load support for Zabbix and if the module is not available, it
>> simply disables support for Zabbix. That allows us to have a smaller
>> configuration file if things like this are auto-detected.
>> 
>>> As Zabbix supports sending items in bulk, I have opted to create an
>>> async background task that will send all events from last 1 second
>>> in
>>> bulk so that even in the case that there are hundreds of incoming
>>> alerts, Zabbix server is only contacted once per second.
>> 
>> Okay, this makes sense. But I believe that there is already a small
>> race in the implementation:
>> 
>> If the client side (in this case suricata-reporter) does not finish
>> the call of flush_pending_to_zabbix() within that second, it will be
>> called again which will result in the same rows being selected again,
>> transmitted again, and assuming that there are just thousands of
>> alarms it will take over a second again, the function will be called
>> again, and so on. So the application will stall very quickly.
>> 
>> Although we should not see thousands of alerts per second under
>> normal conditions, there could be other reasons why this is taking
>> some time. For example, the Zabbix host could be in a different
>> location and round-trips around half the planet are taking some time;
>> it could be busy writing other things to its database or the database
>> has just decided to do a little cleanup job. One second isn’t a lot
>> of time then and we will have to make the system a little bit more
>> resilient against this.
> 
> Is this really the case? For what I understood of the Python async
> methods is that by using await in:
> 
> async def _periodic_zabbix_sender_flush(self):
>   ...
>   if await self.flush_pending_to_zabbix():
>    ...
>   await asyncio.sleep(1)
> 
> The task will wait for the flush/sending to complete before waiting 1
> sec so the next itteration should not be able to begin until
> flush_pending_to_zabbix effectively returns.
> Hence if the sending would take 20s the flow would be:
> flush starts 
>  -> send pending events, taking 20s
>  -> wait 20s until flush_pending_to_zabbix finishes
>  -> wait an additional 1s
>  -> select pending rows again  
> So there should be no overlaps with previous calls and the same rows
> are not concurrently selected and transmitted by this task.

Yes, you are right.

But don’t we need some changes so that this cannot completely block the IO loop?

Right now, the entire process would pause at the "await self.flush_pending_to_zabbix()” stage which means that we are not able to collect any events from the Suricata socket.

I suppose we can leave this code as is for now and address the other things first as it does the job. But I think we might be able to come up with a solution here that gives us some stronger guarantees.

>> 
>>> When for some reason sending to Zabbix server fails, it will be
>>> retried
>>> 3 times and then the background task will be suspended until a new
>>> suricata event comes in. That will wake the task again and retry to
>>> send all
>>> pending events. In environments with many events, that may actually
>>> not
>>> have that much of an effect. But in the average environment, this
>>> will
>>> give the Zabbix Server some breathing space as it failing to
>>> receive our
>>> events, may indicate a Zabbix server overload.
>> 
>> Good thinking here.
>> 
>>> For this I have to keep track which events are sent and which are
>>> pending. So I added a column in the database that keeps track of
>>> that.
>> 
>> So, this is a very crucial thing we probably need to discuss :)
>> 
>> What is the rationale behind this? Obviously there are some easy
>> answers:
>> 
>> 1) We don’t want to loose any history if the network or Zabbix is
>> down
>> 
>> 2) We can even restart the reporter without losing any alerts
> 
> I would add that it can even be killed, crash, powerfail, kernelfail or
> any other disaster may happen. When it restarts (and still has its
> database) the events won't be lost :-)

Well, if there is no power, there is no code that we can run. So no matter how smart we are, it won’t work.

>> 
>> But then I am already running out of ideas why this could be a good
>> idea. The cons that I can see are:
>> 
>> * A lot of additional I/O on the database. Although we would be
>> updating rows very briefly after they have been written to the
>> database, it will create a copy of the row and change the append-only
>> architecture of the database. It will have a lot more cleaning up to
>> do to evict all updated rows.
> 
> True
> 
>> 
>> * You will only ever go back by about 1h by default. Could we just
>> not keep things in RAM for that long?
> 
> I would rather not only keep it in memory. On systems with only one
> alert every x time, that won't be a problem, but on systems with many
> alerts per second, we risk losing many alerts by any failure. 
> Maybe postponing DB writes a few alert-batch sends is possible, but
> with memory-only the risk of lost alerts is too high for me.
> I considered only updating the DB on shutdown, but an unexpected
> shutdown/crash would then potentially cause large replay bursts
> depending on how long reporter has been running, which could be days,
> months, years (hopefully not, as they should upgrade their IPFire
> regularly ;-))

I agree. Memory can be helpful and be used for caching, but we want to have guarantees that we have done our best to submit any alerts to Zabbix and anything else.

I don’t think that we should try too hard to save any IO operations, because with everyone on SSD storage, these are becoming all extremely cheap.

>> I am not saying that I hate the idea, but I am not sure whether it is
>> worth paying the price. The good side is that if people are not using
>> Zabbix, there is no overhead except the space for the extra column.
>> But if we would add another monitoring solution, we would potentially
>> have to add another field, and another, and another?
> 
> That is indeed one of the goals: no extra overhead when Zabbix is not
> used. But I do think if people bother to set up a proper monitoring
> and/or logging system, they generally would like the data flow as
> robust as possible. Such systems can also be configured to react on
> incoming data, possible starting whole workflows, making it even more
> important that there is no data missing.
> 
> I may have a solution for adding more alert consumers a bit further in
> this mail. 
> 
>> 
>> So a possible other solution that I can come up with would be:
>> Creating a separate table with all pending events that have to be
>> transmitted. And every once in a while we truncate it should it
>> become too long. We could even keep a list of IDs in memory only if
>> we want to go down that route.
>> 
> 
> Considering your valid remarks, I have been rethinking possible other
> methods, trying to keep the alerts-table append-only and SQLite work
> minimal and came up with 3 alternatives to the current method:
> 
> * A separate table for keeping the queue of event id's to be sent
> -> Pending queries should be cheaper and the table would remain bounded
> if delivery keeps up.
> However, it would require deletes for every event that is sent, still
> causing extra SQLite writes and cleanup work, especially with high
> event counts. I don't see how to implement once-in-a-while truncating
> on this kind of table if we are not updating every event once it is
> sent to actually mark it as sent. So I don't think this would be less
> work for SQLite?
> Also a crash between inserting an alert in the alert table and adding
> the alert to the queue table would cause missing deliveries or it
> should be done in one transaction, possible causing longer lock times.
> So I don't think that is a good solution.
> 
> * A separate table for keeping successfully sent event id's
> -> This makes the separate table also append-only and would allow for
> once-in-a-while truncating the table as you suggest.
> It would require a query on the alerts table using NOT EXISTS which
> will probably be a little more costly for SQLite than current method or
> the pending table method, although an indexed primary key should make
> this inexpensive enough?
> Worst case after a crash between sending a batch of events succesfully
> and recording it in this table, would be that those events would be
> sent again, causing duplicate entries in Zabbix. But in my opinion it
> is better to have an alert reported twice than missing it.

I like this option, because it gives us a lot of advantages.

We could basically decide what the window is we are interested in and truncate the table from that point. If we submit any new events to Zabbix, we create a row and mark it as successful. That way, we can easily check what alerts inside our window have been successfully transmitted.

If something does not transmit, we can still add the row and keep some state here. We could add a “try again after” timestamp and a counter of how many attempts we have done. That way, this will be persistent and we won’t just fire and forget too much.

The table itself would be really small and with a regular truncate, SQLite will be able to re-use the same pages over and over again.

The solution above is very similar but would basically create rows even though Zabbix is not in use. Or we add some complex logic, but we basically make the INSERT of a new event more complicated because multiple tables are being touched. This solution keeps the second table independent.

> * Using a high-watermark in a separate "state" table
> -> This would record only the last successfully sent alert ID and
> update it when a new alert or batch of alerts is successfully sent.
> So even with a high alert rate, only a single update is required after
> send a batch of alerts succesfully. So in worst case there is still
> only a single update once per second.
> The query on the alerts table would also be cheap only using a ">"
> equation on the primary key.

In theory this is an option, but in reality things might become complicated. I never consider an ID strictly incrementing. Integers could wrap around and we could have different transactions committed at different times which results in rows with lower IDs becoming visible to other processes later.

A solution could be a timestamp because that would at least solve the problem with the counter not wrapping around.

With SQLite, we are not very likely to have many concurrent transactions, but if this grows bigger, we might run a PostgreSQL database or something similar, or even make some other design changes, so I would rather be careful now and now make my own life harder in the future.

> It could look something like:
> 
> zabbix_state
> ------------
> last_sent_id INTEGER NOT NULL
> 
> it could even be used for possible additional future consumers or maybe
> even for tracking succesfully sent alerts by syslog or email?
> 
> consumer_state
> ------------
> consumer       TEXT PRIMARY KEY  -> in this case consumer = "zabbix"
> last_sent_id   INTEGER NOT NULL

I like the consumer idea, because the table with the successfully transmitted events could have this row and we already have a solution that allows us to extend this all to other monitoring solutions.

> Caveats are that I must make sure alerts are always sent in order of
> their ID and when an alert failed to be sent, it would block newer
> alerts to be sent.

I know that some people add a lag or something with a timestamp, but I consider this way too hacky.

> And as I send alerts in bulk, which in turn is chopped into chunks by
> zabbix_utils itself, it is possible that out of 3 chunks the middle
> chunk failed and in that case there are events with higher ID's sent
> and lower ID's that failed, rendering this method unusable. So I will
> then need to split the batches into chunks <= zabbix_utils chunk-size
> and send them separately myself. Possibly generating more DB writes
> within a second. (However current default chunk-size is 250, so it
> takes > 250 alerts within a second to cause an extra DB update)
> But then I still risk that if, for some reason a single alert
> consequently fails to be sent (I don't think this should ever happen,
> but you never know..Maybe a bug in Zabbix failing to parse the event
> due to some unexpected character or something like that?), would still
> block any subsequent alert to be sent. 

I have been working on similar software that sends data to AWS SQS and ElasticSearch and this has indeed been a problem. A bunch of messages that simply could not be parsed and the software was looping for forever.

So there should be some way to at least give up at some point.

> And by batch-sending, there is unfortunately no way of knowing which
> alert(s) in the batch failed, and which where successfully accepted.
> Zabbix server only returns how much have failed and how much have
> succeeded. Currently I retry sending the whole chunk for an hour (by
> default), but I don't block newer events.

Hmm, this is slightly bad design of the API because we could simply drop that alerts and send the rest again. Other solutions could simply be to attempt submitting everything individually if the batch was not successful as a whole. But I am not sure whether we are able to get a clear exception raised to judge that.

> In this method a failed batch would be resent indefinitely so some sane
> threshold should also be implemented here. Possibly something like
> retries=3.. however this would risk dropping alerts too soon when
> Zabbix or the network is effectively having troubles itself.. Not sure
> yet how to handle this, maybe retries=3 if there are any succesfully
> sent alerts in the batch and up to 1 hour if all events in the batch
> fail. Or something like that..

That would be indeed bad. We cannot send indefinitely. But we could store a counter for each attempt and then set a limit of 5 or maybe even 10 times if we want to try very hard.

> Or I could try implementing to recursively split a failed batch until
> such a possible single failing alert is separated, and then drop that
> alert, but I think that feels quite overkill as this situation should
> actually never happen.

Hopefully not.

> This last method introduces some challenges, but I think it has a very
> high potential of being extremely cheap, both on database operations
> and size as only ever one single row is maintained, and makes it also
> cheap to add even more alert consumers, therefore may be worth
> investigating deeper?

I think the extra table is cheap enough. Each row will hold the ID (8 bytes), when we tried last or after when we want to try again (8 bytes), as well as a counter (8 bytes). In total that would be 24 bytes, so a megabyte of data on disk would hold around 45,000 entries - not considering any overhead. That would be quite a lot of records in the window of one hour.

Cheaper is possible, but then we will have to find solutions for other problems. This one is simple and extensible to other solutions, too.

> 
>>> I have also added an alert_max_age config parameter that allows the
>>> user
>>> to set how long suricata-reporter should retry to send events to
>>> Zabbix.
>>> Events older than that set age, will no longer be sent to Zabbix.
>>> This also give the user the implicit option to send older events
>>> when
>>> only just enabling the zabbix sending functionality, since the DB
>>> column
>>> exists and no event was ever sent to Zabbix, all events will be
>>> 'pending". At first run with zabbix functionality enabled, all
>>> events up
>>> to alert_max_age that are in the database will be sent to zabbix
>>> immediatly.
>> 
>> I like the mechanism, but whenever I am building something like this,
>> I am never sure what would be a reasonable window.
> 
> Me neither, I think this highly depends on the user's infrastructure
> and/or needs. If network outages or zabbix server outages are expected
> to be longer than 1 hour in some environments, then 1 hour is probably
> not a good threshold.. Therefore I would definitely make it user
> configurable. But a default of one hour, feels sane to me.

Agreed.

>> 
>> Locally, suricate-reporter is keeping the events for pretty much
>> forever. So we could even go back three days or something. Or we
>> could give up really quickly. After maybe a minute. I never know what
>> is right, but for the implementation, the length of the window plays
>> a role - see above.
>> 
>> With email and syslog we do more of a “fire and forget” approach. If
>> we send the syslog message and syslog wasn’t ready to receive it, we
>> wouldn’t know and we would not try again...
> 
> If you have no way of knowing, there are no other options, I think. But
> in the case of Zabbix, we do know. And knowing how much fuss the SoC
> team at my work makes when some security related logs are missing, I
> think many really do like it to be as reliable as possible when
> exporting the alerts to a monitoring or other collecting system. 

They are right. We should try really hard so that Zabbix sees the full picture.

>> 
>>> All events sent to Zabbix contain the timestamp of retrieval by
>>> suricata-reporter, so Zabbix will register and order them as
>>> received on that
>>> timestamp independently of the actual time Zabbix itself received
>>> the
>>> event.
>>> 
>>> This is my first adventure in Python async programming, so I hope I
>>> did
>>> not make any flagrant mistakes. But the code has been running here
>>> for
>>> weeks now without any problem. I have not actually tested large
>>> bursts
>>> of events, as I could not simulate that.. But I did make Zabbix
>>> server
>>> slow, unavailable and finally replaced it with netcat (to accept
>>> the connection, but
>>> not react on it) and I had the connection with the server off for a
>>> few
>>> hours to then re-establish the connection to see hundereds of
>>> pending events 
>>> being registered in only a few milliseconds.
>>> I did not notice any problems with suricata-reporter in any of
>>> these
>>> cases.
>> 
>> This is good testing. Usually, if I need to create a lot of events, I
>> enable the “PING” rule in “icmp_info” and just send a lot of ping
>> packets to the firewall. You could try a flood ping with “ping -f”.
>> 
>> I will send some more comments about the code in the other emails.
> 
> For now I will await your (or maybe other list members'?) reaction to
> above db approach proposals before diving into your code comments. But
> I will make sure to review and properly implement/consider or answer
> them when I start changing the code based on the outcome of current
> discussion.

Cool. Feel free to ask questions on the way and we will get this all done in no time!

All the best,
-Michael

> Regards
> Robin
>> 
>> Best,
>> -Michael
>> 
>>> 
>>> Regards
>>> 
>>> Robin
>>> 
>>> -- 
>>> Dit bericht is gescanned op virussen en andere gevaarlijke
>>> inhoud door MailScanner en lijkt schoon te zijn.
>>> 
>>> 
>> 
> 
> -- 
> Dit bericht is gescanned op virussen en andere gevaarlijke
> inhoud door MailScanner en lijkt schoon te zijn.