Modify Upgrade - add fields comparison

[kalyparker]

Hello,

Today, I worked a little bit on the modify bot for suit my needs :) Result I had a special functionality for comparing 2 fields with regex:

{
        "rulename": "FeodoIP online",
        "if": {
             "feed.name": "Feodo IPs",
             "cmp": {"time.lastseen": "([0-9]{4}-[0-9]{2}-[0-9]{2})",
                     "time.observation": "([0-9]{4}-[0-9]{2}-[0-9]{2})"}
        },
        "then": {
             "status": "online"
 }

Here on the original feed, we have:

Firstseen,DstIP,DstPort,LastOnline,Malware 2019-07-23 00:40:42,77.244.219.49,447,2019-07-23,TrickBot

For all the field where LastOnline is lesser than time.observation I put the status "online"

Are you interested by this functionality?

If yes, could you help me to push this update:

• On which branch I'm supposed to push it?
• Regarding the issue #1370, @wagner-certat you asked me to document it. Where?
  • in examples, should I add new conf file or update an existing one?
  • in tests, I probably need to add something, but what (still newbie :) )
• something else?

And finally I spotted a mistake in the actual bot, which execute the "then" when things do not match. missing "return None" ;)

else:
    self.logger.warn("Type of rule (%r) and data (%r) do not "
                               "match in %s, %s!",
                                type(rule), type(event[name]), identifier, name)
    return None

[ghost]

Nice idea!

Are you interested by this functionality?

Definitely :)

But I don't like that you are mixing a level with the field names and commands. Maybe use a different operator compare_fields on the same level as if?

Does it make sense to make this compare_fields section a list of comparisons (dicts)? Or do you think it does not make sense to have multiple comparisons in one rule combined?

Just an idea:

{
        "rulename": "FeodoIP online",
        "if": {
             "feed.name": "Feodo IPs"
        },
        "compare_fields": [
             {"time.lastseen": "([0-9]{4}-[0-9]{2}-[0-9]{2})",
              "time.observation": "([0-9]{4}-[0-9]{2}-[0-9]{2})"}
        ],
        "then": {
             "status": "online"
 }

On which branch I'm supposed to push it?

As it is a new feature, please develop - the default one.

Regarding the issue #1370, @wagner-certat you asked me to document it. Where?

Already did that (see my next comment).

in examples, should I add new conf file or update an existing one?

I'd have added it to the existing one.

in tests, I probably need to add something, but what (still newbie :) )

That's quite easy in this case. The modify bot test already loads the example file, so no changes needed here. So then you only need to add an example event (input and expected output) to the predefined structure. All in intelmq/tests/bots/experts/modify/test_expert.py.

And finally I spotted a mistake in the actual bot, which execute the "then" when things do not match. missing "return None" ;)

Thanks, I will have a look

[kalyparker]

Ok,

For the last part, I'll push tonight the mistake in the branch maintenance.

For the evolution:

But I don't like that you are mixing a level with the field names and commands. Maybe use a different operator compare_fields on the same level as if?

I did it on the same level because condition in if should be true and also the comparison. Let me look at it.

Does it make sense to make this compare_fields section a list of comparisons (dicts)? Or do you think it does not make sense to have multiple comparisons in one rule combined?

Good question! I did not change the logic, so all fields should be in the harmonization file. Having more than 2 comparisons is probably rare. Anyway, if I find a way to do it, I'll change it. For the regex, I compare only the first group because I did not found a simple way to configure it otherwise.

I'll try to do that this week.

[ghost]

I did it on the same level because condition in if should be true and also the comparison.

Then you have field names ("feed.name" in the example) and operators ("cmp") in the same structure. I don't like that.