graylog-plugin-alert-wizard icon indicating copy to clipboard operation
graylog-plugin-alert-wizard copied to clipboard

Wrong result when adding another condition in addition to a list condition

Open frantz45 opened this issue 2 years ago • 5 comments

  1. Create a list (for example the list "users" with 3 users : toto, tata, titi)
  2. Create a COUNT rule with 2 conditions linked by an AND
  • 1st condition : field "user" is in list "users"
  • 2st condition: field "source" match exactly "source123"
  1. Send a log with user=toto and source=sourceABC. It will be placed in the Stream because the pipeline function found the user in the list. So the rule will trigger but it is wrong because "source" is not equal to "source123"
  2. Send a log with user=xxx and source=source123. It will be placed in the Stream beauce the only Stream rule is field "source" match exactly "source123". So the rule will trigger but it is wrong because "user" is not present in the list

So with this current behavior, it's an OR between conditions and not an AND.

Some ideas:

  • Deny the ability to add other conditions when one condition is based on a list (but the list feature will loose interest and won't be used in my opinion)
  • Use 2 Streams: one for classical conditions and one dedicated for the list condition. Connect the pipeline to the first Stream, and if the pipeline function find the value in the list then put the log in the 2nd Stream. Event Definition of the rule must only be linked to the 2nd Stream.
  • Don't use Stream rules for classical conditions but translate them in the pipeline function (when has field("source") and to_string($message.source)=="source123")
  • Use an extractor to check the lookup instead of a pipeline
  • Use the Event Definition query
    • Warning: lookup limited to 1024 entries
    • https://www.graylog.org/post/risk-based-alerts/
  • Create a new plugin of type "Stream Rule Matcher"
    • This last solution seems to be the best, but a PoC is required to check feasability
    • Be careful to performance
    • Maybe we can use the same code as the Extractor based on a lookup

The case with multiple list conditions should also be handled. It seems to be well handled currently but I prefer to have it in mind when we'll treat this issue.

frantz45 avatar Jul 07 '23 14:07 frantz45

I changed the size to large, because we need to select the solution we want to implement and POCs may be needed. The warning: lookup limited to 1024 entries is for the Event Definition query solution or for the extractor solution?

c8y3 avatar Jan 03 '24 07:01 c8y3

Allow 3 days to evaluate the feasibility of the "Stream Rule Matcher" plugin. Keep in the mind the performance impact. Test whether putting a costly stream condition last (when they are all connected by AND) improves performances in comparison of having it first (maybe the AND evaluation of graylog is lazy). If this is not possible, adopt the second solution in the item list: two streams connected by a pipeline (only when the condition connector is AND, for OR keep the current implementation)

c8y3 avatar Jan 16 '24 15:01 c8y3

Most probably, this issue implies a breaking in the storage representation of rules. It will require an import/export of all rules.

c8y3 avatar Jan 16 '24 16:01 c8y3

Requests

  • to get all streams

    GET /streams
    
  • to get a given stream

    GET /streams/65bc9effed531b17498c5425 => {
      "id": "65bc9effed531b17498c5425",
      "creator_user_id": "admin",
      "outputs": [],
      "matching_type": "AND",
      "description": "Generated by the alert wizard",
      "created_at": "2024-02-02T07:51:27.384Z",
      "disabled": false,
      "rules": [
      	{
      		"field": "fgdfggf",
      		"stream_id": "65bc9effed531b17498c5425",
      		"description": "Generated by the alert wizard",
      		"id": "65bc9f10ed531b17498c5490",
      		"type": 2,
      		"inverted": false,
      		"value": "ghg"
      	}
      ],
      "title": "AAAA",
      "content_pack": "",
      "remove_matches_from_default_stream": false,
      "index_set_id": "65bc9e84ed531b17498c51d3",
      "is_default": false,
      "is_editable": true
    }
    
  • to get the available stream rule types

    GET /streams/null/rules/types => [
      {
      	"id": 1,
      	"name": "EXACT",
      	"short_desc": "match exactly",
      	"long_desc": "match exactly"
      },
      {
      	"id": 2,
      	"name": "REGEX",
      	"short_desc": "match regular expression",
      	"long_desc": "match regular expression"
      },
      {
      	"id": 3,
      	"name": "GREATER",
      	"short_desc": "greater than",
      	"long_desc": "be greater than"
      },
      {
      	"id": 4,
      	"name": "SMALLER",
      	"short_desc": "smaller than",
      	"long_desc": "be smaller than"
      },
      {
      	"id": 5,
      	"name": "PRESENCE",
      	"short_desc": "field presence",
      	"long_desc": "be present"
      },
      {
      	"id": 6,
      	"name": "CONTAINS",
      	"short_desc": "contain",
      	"long_desc": "contain"
      },
      {
      	"id": 7,
      	"name": "ALWAYS_MATCH",
      	"short_desc": "always match",
      	"long_desc": "always match"
      },
      {
      	"id": 8,
      	"name": "MATCH_INPUT",
      	"short_desc": "match input",
      	"long_desc": "match input"
      }
    ]
    
  • to create a stream rule

    POST /streams/65bc9effed531b17498c5425/rules {
      "description": "",
      "field": "a",
      "inverted": false,
      "type": 1,
      "value": "a"
    }
    

    Creation of a stream rule with type 9 succeeds, but there are stack traces which happen when starting the testing if the rule matches or starting the stream

Interface

Server

Conclusion

The stream rule types are hard-coded in org.graylog2.plugin.streams.StreamRuleType. But also in org.graylog2.streams.StreamRuleMatcherFactory.build(), which is called in the org.graylog2.streams.StreamRouterEngine. A switch according to the rule type is also performed in the same class: https://github.com/Graylog2/graylog2-server/blob/5.1.3/graylog2-server/src/main/java/org/graylog2/streams/StreamRouterEngine.java#L111. There may be other reasons, but this shows that adding a new kind of matcher would already require several patches to Graylog. For the time being, matchers are not designed to be an extension point for plugin. We could submit a feature request to ask for that. As it stands, I am going for the next solution: two consecutive pipelines.

c8y3 avatar Feb 02 '24 10:02 c8y3

Introduction

This is a (hopefully) complete description of the encoding we want to have for the various kind of alert rules

Encoding of the conditions

The conditions are divided in two groups: classic conditions and list conditions. Classic conditions are translated into a stream with rules s. List conditions are translated into a pipeline p which takes messages from a stream (it may be the default stream) to another stream. The conditions are combined differently according to the matching type:

  • all: p takes message from s, if they match the list conditions it routes them to another stream s'
  • at least one: p routes messages which match the list conditions in s

The output stream, from which event definitions will get their messages depends on the matching type:

  • all: s is the output stream
  • at least one: s' is the output stream

We call this whole construction c.

Encoding of the alert rules

The alert rules are encoded differently according to their types:

  • count/group/distinct/statistics: the conditions are translated to c, the rule is translated to an aggregation event definition which searches the output stream of c
  • then/and: the first conditions are translated to c1, the second to c2, then the rule is translated to a correlation event definition which is plugged on the outputs of both c1 and c2
  • or: the first conditions are translated to c1, the second to c2, then the rule is translated into two aggregation event definitions each plugged to c1 or c2. Note that both event definitions notify the same notification.

Concluding remarks

  • since the database format is going to change anyway (to accommodate for the additional streams), we might as well try to structure the data as cleanly as possible
  • the update of rules must also be correctly implemented

c8y3 avatar Feb 02 '24 13:02 c8y3

I confirm it's fixed in Wizard v5.2.0

frantz45 avatar Jun 20 '24 14:06 frantz45