From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.haj.ipfire.org (localhost [IPv6:::1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4hWpCq6C2xz36Wb for ; Fri, 28 Aug 2026 19:21:43 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [IPv6:2001:678:b28::25]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (secp384r1 raw public key) server-digest SHA384 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mail01.haj.ipfire.org", Issuer "YR2" (not verified)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4hWpCm26G7z2xWF for ; Fri, 28 Aug 2026 19:21:40 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4hWpCb0PpNz3R0; Fri, 28 Aug 2026 19:21:30 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1787944891; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6rbDvWX4aXa60ZgJKbmcmXdSF6T5izofuM7E21DIVw0=; b=0MOX/uE9DHv3/wsR29A12rISz3MKNNxxTF38VRbfDH4ug2bWrx59kf3hfATA/Wlko/2GVp f25zYRFFhMV8JLDA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1787944891; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6rbDvWX4aXa60ZgJKbmcmXdSF6T5izofuM7E21DIVw0=; b=ZihKcAJkJg0zIhUxlh908ICaSxg+taOBT8z6SMSPWgak7dcSR+6DPtyy3P3UknzuyDQFKY MtHFeOf/b8N2fEXSeOdpTIh1SbNAPSx6ehImd1Z45dgaJf+XhqjdBlAoGpNH4vn+4fRNgO DLPocmNyeP8I4q1Hv03KXJzs3OI2nPH082PvOESkntF/pmghCZvY+uo45JibuMpQr7io9x RAImwtIes1i4NIXmqRv7p3/0eaSNYXf+kXc7GEpD8df129ac0VT03smBFFzz2OKNTHz/V/ 4m2GKSvJDn1G0BJARY4gjen8dZPVb+2u8wc6IWV4cHFrvNYGXTO784b9vSBW0Q== Content-Type: text/plain; charset=utf-8 Precedence: list List-Id: List-Subscribe: , List-Unsubscribe: , List-Post: List-Help: Sender: Mail-Followup-To: Mime-Version: 1.0 Subject: Re: [PATCH 0/5] Add Zabbix functionality to suricata-reporter From: Michael Tremer In-Reply-To: <5c15819a5f758f2700cf725402c4d563699bd5bf.camel@disroot.org> Date: Fri, 28 Aug 2026 21:21:30 +0200 Cc: development@lists.ipfire.org Content-Transfer-Encoding: quoted-printable Message-Id: <540F44D8-A2C1-4FC0-AFBF-4FDE92BE96DB@ipfire.org> References: <20260730195148.3278295-1-robin.roevens@disroot.org> <5c15819a5f758f2700cf725402c4d563699bd5bf.camel@disroot.org> To: Robin Roevens Hello Robin, > On 28 Aug 2026, at 00:51, Robin Roevens = wrote: >=20 > Hi Michael >=20 > 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=E2=80=99t think too much = about this. > Michael Tremer schreef op vr 31-07-2026 om 11:24 [+0100]: >> Hello Robin, >>=20 >> Thank you very much for sending these patches. >>=20 >> 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 :) >>=20 >>> On 30 Jul 2026, at 20:15, Robin Roevens >>> wrote: >>>=20 >>> Hi all, >>>=20 >>> 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. >>>=20 >>> 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. >>=20 >> 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. >>=20 >>> 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. >>=20 >> Okay, this makes sense. But I believe that there is already a small >> race in the implementation: >>=20 >> 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. >>=20 >> 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=E2=80=99t = a lot >> of time then and we will have to make the system a little bit more >> resilient against this. >=20 > Is this really the case? For what I understood of the Python async > methods is that by using await in: >=20 > async def _periodic_zabbix_sender_flush(self): > ... > if await self.flush_pending_to_zabbix(): > ... > await asyncio.sleep(1) >=20 > 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=20 > -> send pending events, taking 20s > -> wait 20s until flush_pending_to_zabbix finishes > -> wait an additional 1s > -> select pending rows again =20 > 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=E2=80=99t 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()=E2=80=9D 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. >>=20 >>> 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. >>=20 >> Good thinking here. >>=20 >>> 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. >>=20 >> So, this is a very crucial thing we probably need to discuss :) >>=20 >> What is the rationale behind this? Obviously there are some easy >> answers: >>=20 >> 1) We don=E2=80=99t want to loose any history if the network or = Zabbix is >> down >>=20 >> 2) We can even restart the reporter without losing any alerts >=20 > 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=E2=80=99t work. >>=20 >> But then I am already running out of ideas why this could be a good >> idea. The cons that I can see are: >>=20 >> * 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. >=20 > True >=20 >>=20 >> * You will only ever go back by about 1h by default. Could we just >> not keep things in RAM for that long? >=20 > 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.=20 > 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=E2=80=99t 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? >=20 > 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. >=20 > I may have a solution for adding more alert consumers a bit further in > this mail.=20 >=20 >>=20 >> 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. >>=20 >=20 > 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: >=20 > * 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. >=20 > * 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 =E2=80=9Ctry again after=E2=80=9D timestamp = and a counter of how many attempts we have done. That way, this will be = persistent and we won=E2=80=99t 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: >=20 > zabbix_state > ------------ > last_sent_id INTEGER NOT NULL >=20 > it could even be used for possible additional future consumers or = maybe > even for tracking succesfully sent alerts by syslog or email? >=20 > consumer_state > ------------ > consumer TEXT PRIMARY KEY -> in this case consumer =3D "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 <=3D 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.=20 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=3D3.. 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=3D3 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. >=20 >>> 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. >>=20 >> I like the mechanism, but whenever I am building something like this, >> I am never sure what would be a reasonable window. >=20 > 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. >>=20 >> 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. >>=20 >> With email and syslog we do more of a =E2=80=9Cfire and forget=E2=80=9D= approach. If >> we send the syslog message and syslog wasn=E2=80=99t ready to receive = it, we >> wouldn=E2=80=99t know and we would not try again... >=20 > 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.=20 They are right. We should try really hard so that Zabbix sees the full = picture. >>=20 >>> 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. >>>=20 >>> 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=20 >>> being registered in only a few milliseconds. >>> I did not notice any problems with suricata-reporter in any of >>> these >>> cases. >>=20 >> This is good testing. Usually, if I need to create a lot of events, I >> enable the =E2=80=9CPING=E2=80=9D rule in =E2=80=9Cicmp_info=E2=80=9D = and just send a lot of ping >> packets to the firewall. You could try a flood ping with =E2=80=9Cping = -f=E2=80=9D. >>=20 >> I will send some more comments about the code in the other emails. >=20 > 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 >>=20 >> Best, >> -Michael >>=20 >>>=20 >>> Regards >>>=20 >>> Robin >>>=20 >>> --=20 >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>>=20 >>>=20 >>=20 >=20 > --=20 > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn.