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 4hdrPv2Wjlz36Vw for ; Mon, 07 Sep 2026 15:21:23 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519) (Client CN "mail01.haj.ipfire.org", Issuer "YR2" (not verified)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4hdrPq5s8mz2xVG for ; Mon, 07 Sep 2026 15:21:19 +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 4hdrPg2M4tz1lm; Mon, 07 Sep 2026 15:21:11 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1788794471; 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=3AxpnjjCI4g6cIk+Ae4a6HhhKJPHLzSg2/e90FaBBrY=; b=R5yKSgY8HzsAGX52MV1m3+EZzQqEkuVVYBV5fiv1QKhTqJh3RavCGqspgkP8l243TMM0pF JiQ10jTknI99EpBg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1788794471; 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=3AxpnjjCI4g6cIk+Ae4a6HhhKJPHLzSg2/e90FaBBrY=; b=ZzKlZd7g6w+3xKuyRj/1mJwxJVYecTUlCMsMPMFvHRdlY9KimtllEQZ+ew0H/c6CVFjm1N t7K7bHiy4BypsHCuUNXFSZtbHAB4/LvMwpCMmRDpu2TFC++IJeqL6oCAkmg9jld6xMDrlR r6LrDkIeXw00AmwzccTWc6bYSB8QW8CHVHE/m9lhX2uxz/kOa3HvAxR4GrGDaWz0zddFDF cK01n8cIArIyiqa/lUpU+BB73VpdclrZaorcaUDfC7WbN+0/MThACsRbSOE+lK37PVKIMF 4732Qjy0KRe352LDXyNlyaJIBD4Ki4DeyYFwJa0Zal8bqSmcQ5aogSpKjTxhrg== 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: <84bb6171a5fd7cf63738b40d07aa50e7089edf1b.camel@disroot.org> Date: Mon, 7 Sep 2026 16:21:10 +0100 Cc: development@lists.ipfire.org Content-Transfer-Encoding: quoted-printable Message-Id: <284B83BB-1242-4C29-909E-840D9CFBE538@ipfire.org> References: <20260730195148.3278295-1-robin.roevens@disroot.org> <5c15819a5f758f2700cf725402c4d563699bd5bf.camel@disroot.org> <540F44D8-A2C1-4FC0-AFBF-4FDE92BE96DB@ipfire.org> <84bb6171a5fd7cf63738b40d07aa50e7089edf1b.camel@disroot.org> To: Robin Roevens Hello, > On 29 Aug 2026, at 16:41, Robin Roevens = wrote: >=20 > Hi Michael >=20 > Michael Tremer schreef op vr 28-08-2026 om 21:21 [+0200]: >> Hello Robin, >>=20 >>> 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 :-) >>=20 >> Haha, I hope you had a relaxing time and didn=E2=80=99t think too = much about >> this. >>=20 >>> 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 :-) >>=20 >> Probably only less code because we can let SQLite do all the work :) >>=20 >>>>=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. >>=20 >> Yes, you are right. >>=20 >> But don=E2=80=99t we need some changes so that this cannot completely = block >> the IO loop? >>=20 >> 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. >=20 > 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. >=20 > 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. >=20 > 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 Yes, the database could in theory block, but if this happens the = application cannot do much anyways. I would prefer a fully asynchronous = application, but that is still a massive pain in the back in Python. = Right now, the extra complexity isn=E2=80=99t worth it for us I think. >>=20 >> 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 > 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...? >=20 >>=20 >>>>=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 :-) >>=20 >> 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 >>>>=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 ;-)) >>=20 >> 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. >>=20 >> 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. >>=20 >>>> 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. >>=20 >> I like this option, because it gives us a lot of advantages. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >=20 > Ok, I've done some more thinking/analysing about this solution then, > combining it with the consumer idea below: >=20 > 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. This sounds good, although I would prefer alert_deliveries because all = my tables are always plural :) > - 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 ) This sounds good to me. The only question that remains is whether we = want to build this as a general thing so that we can have multiple = consumers and for each of them the batch code is already there and = simply hands over the data to a callback function? That would prevent us = from building the retry-logic multiple times. > 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 ? That is a very good compromise. We kind of want to keep a small log if = things have not been successful. That could even be a separate table we = just write to and reference the alerts maybe. > To determine which events to send to Zabbix, currently they are added > with a pending =3D 1 in the alerts table if zabbix =3D 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=3Dzabbix. I considered timestamps, > but that can start behaving strange and unexpected if time is for some > reason changed during runtime.=20 I wouldn=E2=80=99t use the ID as it might wrap around. The easiest way = is to just join them together: SELECT a.* FROM alerts a LEFT JOIN alert_deliveries d ON d.alert_id =3D a.id WHERE a.timestamp >=3D unixepoch() - X AND d.alert_id IS NULL ORDER BY a.created_at LIMIT Y This would give you all alerts that have not been submitted, yet. For the retries we can just join the two tables together and if you want = to pull all the data in one go, you can use UNION ALL. This should always be very fast since the alerts table has an index on = timestamp and the join is also an indexed-join. >>=20 >>> * 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. >>=20 >> 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. >>=20 >> A solution could be a timestamp because that would at least solve the >> problem with the counter not wrapping around. >=20 > 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. This is true. The whole application would still be consistent if the = system time remains wrong. But there is no way to know. > 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 =3D~ 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. If you assume a 64 bit counter, then chances are very small, but still = not zero. The question is only whether we can rely on this. I know a = couple of people who can tell a story about this happening thinking that = their integers were big enough, but then they were not. PostgreSQL still uses 32 bits for an integer by default. I kind of want = to be somewhat database-agnostic that we don=E2=80=99t have to rebuild = our code if we decide to change anything. SQLite seems to be using 8 = bytes, but I am only testing this on x86_64. > Arbitrary time adjustments are a far greater risk. Yes, but the join wouldn=E2=80=99t really care about this, except that = the cut-off window would exclude some data, but there is no way to get = around that. > 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. The join eliminates that problem. For as long as these rows become = visible inside the window (and we have chosen hours), they will be = submitted to the consumers at some point. So I believe this is the best = solution. >>=20 >> 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. >>=20 >>> 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 >>=20 >> 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. >>=20 >>> 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. >>=20 >> I know that some people add a lag or something with a timestamp, but >> I consider this way too hacky. >>=20 >>> 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 >>=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. >>=20 >> So there should be some way to at least give up at some point. >>=20 >>> 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. >>=20 >> 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. >=20 > 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). Maybe we should raise exceptions ourselves then so that the calling code = can handle these exceptions? There could be a SubmissionFailedException which takes a list of alerts = that could not be transmitted. Since we don=E2=80=99t know this for = Zabbix we include all (or use None to signal all) and the code will then = try again for each of them individually. That could then be adapted for = other consumers easily. >=20 >>=20 >>> 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.. >>=20 >> 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. >>=20 >>> 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. >>=20 >> Hopefully not. >>=20 >>> 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? >>=20 >> 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. >>=20 >> 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 > Ok, then I will go for method 2 as explained above ?=20 I lost count, but I would prefer the JOIN unless you have any = objections. Best, -Michael >=20 > Regards > Robin >=20 >>=20 >>>=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. >>=20 >> Agreed. >>=20 >>>>=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 >>=20 >> They are right. We should try really hard so that Zabbix sees the >> full picture. >>=20 >>>>=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=9Cpin= g - >>>> 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. >>=20 >> Cool. Feel free to ask questions on the way and we will get this all >> done in no time! >>=20 >> All the best, >> -Michael >>=20 >>> 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. >>=20 >>=20 >>=20 >=20 > --=20 > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn.