certtools / intelmq

IntelMQ is a solution for IT security teams for collecting and processing security feeds using a message queuing protocol.
https://docs.intelmq.org/latest/
GNU Affero General Public License v3.0
975 stars 296 forks source link

ENH: Caching, advanced mapping and separating events for MISP Feed output bot #2509

Open kamil-certat opened 4 months ago

kamil-certat commented 4 months ago

MISP Feed output bot got new features:

The bot is fully backward-compatible. By default, the previous behaviour is kept.

In addition, code related to Python 3.6 was removed and the message library was fixed not to modify the original dict instance.

This PR replaces on PR #2505.

kamil-certat commented 4 months ago

Yeah, finally green :tada:

sebix commented 4 months ago

Are you using it in production?

kamil-certat commented 4 months ago

We started using it on staging, found some pain points, and now I'll test it & hopefully promote to prod in a few days - so, not yet, but soon ;)

sebix commented 4 months ago

There's not much I can check here without reading the MISP code and docs and setting up an instance.

Maybe @Rafiot, can you have a glimpse?

aaronkaplan commented 4 months ago

My main request is:

1) give the possibility (or document it) how to add default tags to events (such as "FEED:Source=intelmq") to any event which gets generated via this feed. Adding a default tag helps in sorting out automatically added events and attributes in case something goes wrong. 2) make sure the users understand in the documentation how feeds work. Linking to the MISP documentation and what each parameter when including the MISP feed actually means. For example, did you know that if you set "caching:enabled" in a MISP feed, then that means the feed events and attributes don't get added to the events (and attributes) table in MISP. It's totally non-intuitive. Why am I saying this? I had to figure it out the hard way myself. 3) basically we need a tutorial on docs.intelmq.org on how to run this properly! It's a very cool feature and addition but you can quickly shoot yourself in the foot if you don't know how it works and what every parameter in MISP means.

kamil-certat commented 4 months ago

Thanks for the review!

re 1: It's a good idea, I'll look into it. I was concentrated on what we actually need, and I do think there are more things we can improve in the bot. re 2: It's a good point, I will point to the MISP documentation. I also had to explore what we really can do, the PyMISP documentation wasn't really helpful :D re 3: yeah, I agree it needs a tutorial (and I've planned it), but it's a separated thing, I won't include it here. My plan is to write a blog post once we establish the final integration internally, and then we could include it in the documentation as well. I expect to publish it around September, hopefully. There is a queue of what I'd like to write about, and a limited time ;)

kamil-certat commented 4 months ago

@aaronkaplan

Could you have a look again? I have implemented tagging as well as rewritten the documentation, added config examples, implemented validation in the check method and refactored code, including renaming methods to be more clear what they do or adding doc strings where I found it useful. I have also added descriptions to complex test cases explaining what should happen.

I do admit that the configuration of the bot is complex. I did my best to marriage flexibility and readability, but I think in the future we may eventually redesign this configuration, based on feedback. I'm also open for any suggestions.

sebix commented 2 months ago

@aaronkaplan you requested some changes to this PR and @kamil-certat implemented various changes based on your feedback. Can you please re-check?