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

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.

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.

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. 

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.

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.

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.

Regards

Robin
  

Small correction to my explanation, the statement I made about sending
older events from the time when zabbix functionality is not yet
enabled, is not true, as when zabbix functionality is disabled, events
recorded in the database are not marked as pending, so they won't be
sent to zabbix when that functionality is enabled on a later time.

It does however give the user the implicit functionality of augmenting
the max_age in case zabbix server was unreachable for longer than
current max_age to recover events it missed due to previous max_age
setting.

Regards
Robin

Robin Roevens schreef op do 30-07-2026 om 21:15 [+0200]:
> 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.
> 
> 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.
> 
> 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. 
> 
> 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.
> 
> 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.
> 
> 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.
> 
> Regards
> 
> Robin
  

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...

> 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.

> 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

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.

* You will only ever go back by about 1h by default. Could we just not keep things in RAM for that long?

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?

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.

> 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.

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...

> 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.

Best,
-Michael

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

Hi Michael

Vacation period here is officially over.. So I have no more excuses and
I'm ready to dive into this again :-)

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 :-)

> 
> > 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.

> 
> > 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 :-)


> 
> 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 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.

* 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.

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

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.
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. 
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.
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..
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.

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 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.

> 
> 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. 

> 
> > 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.


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

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.
  

Hi Michael

Michael Tremer schreef op vr 28-08-2026 om 21:21 [+0200]:
> 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.

Not necessarily, I think.. For as far as I have understood it, await
itself is "cooperative" and should allow the loop to continue doing
it's work as long as the awaited function doesn't do long-running
synchronous calls.. In current implementation, I think only the DB
query to select the queries is synchronous as we use the async zabbix
sender. So as long as the DB query itself is fast enough, it should not
block the main loop, even if the zabbix sending takes too long as that
should happen asynchronous.

To mitigate possible slow DB queries, I'm thinking about limiting them
to a max of zabbix_utils chunk size (currently 250), resulting in
possibly faster queries (in case there are tons of events to return)
and a better control over the zabbix sender functionality and faster
recording of the succesfully sent events in a separate table.
This should keep the synchronous work inside the function to a minimum.

Alternatively we could write a _trigger_zabbix_flush that would launch
a separate task for the work to be done in the background.. But then we
will need to mitigate possible race conditions as you described earlier
as that would then effectively be a possibility

> 
> 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.

For as far as I understand all the async python, I think the risks
should already be quite small currently as long as the DB is
responsive. If the DB is not responsive, the main loop would also have
troubles inserting new events into the DB, so as long as I try to keep
the DB queries in check...?

> 
> > > 
> > > > 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.

Ok, I've done some more thinking/analysing about this solution then,
combining it with the consumer idea below:

We can create an alerts_delivery table with colums for alert_id,
consumer, sent_at-timestamp, failed-flag, retry_after-timestamp and
retry_attempts-count.

- When an alerts batch is sent successfully to Zabbix, the individual
alerts are recorded in the table as successful.
- When an alerts batch to Zabbix has failed as a whole (with an
exception in python indicating network or communication error) they
won't be recorded, so that they are retried as if they where never
tried before on the next second. Pausing the process after 3 retries
until a new event comes in (exactly like the current logic).
- When an alerts batch gets sent to Zabbix, but Zabbix replies about
failed alerts, the alerts will be recorded in the table as failed and
retried individually at retry_after-timestamp until retry_attempts-
count reached a configurable max_retry_count. Retry_after-timestamp can
be increased with an exponential backoff after each retry (maybe up to
a specific max backoff-time to prevent )

Periodically the table gets cleaned up removing all successful and
permanently failed entries older than a preset max_alert_age, based on
the sent_at-timestamp, but still keeping retryable entries, for in case
the user has set a very high max_retries setting ?

To determine which events to send to Zabbix, currently they are added
with a pending = 1 in the alerts table if zabbix = enabled, which made
it very easy, but we will no longer have with this method..
I was thinking, during zabbix_sender_init, I can fetch the currently
latest alert-id and keep that in memory as start_alert_id, so I can
query alerts with alert_id > start_alert_id and not exists in
alters_delivery table with consumer=zabbix. I considered timestamps,
but that can start behaving strange and unexpected if time is for some
reason changed during runtime. 


> 
> > * 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.

Except for when the system time was wrong during start of reporter and
it gets adjusted during runtime.. then time could be running backwards
or even jump if it is set manually.

The chances of the id wrapping around are not very high I think. The
integer primary key that alert.id currently is should be a signed 64bit
integer with the largest value being 2^63 - 1 =~ 9.22 * 10^18. So even
when 1 million rows are inserted every second, it would still take
about 9.22 * 10^12 seconds or roughly 292.000.000 years. By the time
that wraps around, I assume me and you won't be around anymore, and the
system that may be running suricata-reporter that long will probably
also not be very relevant anymore by that time..
So I don't think we have to be afraid of the ID wrapping around.
Arbitrary time adjustments are a far greater risk.

I concur about the risk of of getting lower IDs becoming visible to
other processes later, when reporter becomes much more complex than it
currently is. So the high watermark idea then indeed will need some
inventive hacking to work around that. Keeping the modified successful
alerts table is then probably the best out of my 3 proposals..
There I will need to use the last ID in DB + 1 as the first ID to
include in next zabbix-send-task during startup/zabbix init and I
assume there won't be much risk of lower ID's coming in at that point
in the code ever. Depending on a timestamp there would be much more
risky, in my opinion.


> 
> 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.

When sending to Zabbix itself was successful, it always replies with
how many values are successfully processed and how many have failed.
There will be no python exceptions, but it can be read from the reply.
So as proposed above, I would then mark all alerts in the batch as
failed and start sending them individually. This will result in
duplicates in Zabbix, but in the end we will know which alerts failed
to process (if those then still fail).

> 
> > 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.

Ok, then I will go for method 2 as explained above ? 

Regards
Robin

> 
> > 
> > > > 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.
> 
> 
>