Webhook headers added numberOfAlerts times

Describe the bug

I am using the Webhook output and have added custom headers (WEBHOOK_CUSTOMHEADERS). This works fine, however a new header is added each time Falcosidekick sends a request to my webhook (in my case netcat for testing purposes). After some time this looks like this:

POST / HTTP/1.1
Host: my-webhook.default.svc.cluster.local:8080
User-Agent: Falcosidekick
Content-Length: 1425
Authorization: Bearer ...
Authorization: Bearer ...
Authorization: Bearer ...
Authorization: Bearer ...
Authorization: Bearer ...
[repeated pages and pages]
Content-Type: application/json; charset=utf-8
Accept-Encoding: gzip

[json payload]

This will eventually exhaust available memory and/or go beyond the maximum header size, and generate additional network cost.

The problem is in https://github.com/falcosecurity/falcosidekick/blob/master/outputs/webhook.go#L35 where a new header is added to the list every time an alert is sent. From a brief check I suspect other outputs may have the same issue.

I'd propose a fix in https://github.com/falcosecurity/falcosidekick/blob/master/outputs/webhook.go#L35: this method should check whether the header (name and value) are already set and ignore it in case it is. This can be done by just checking every element in the list (which should be very short anyway). I can do that 😃 , I need to fork it anyway to go on.

How to reproduce it

Just configure the webhook and view the result with netcat.

Expected behaviour

no duplicate headers

Screenshots

Environment

Falco version: n/a
System info: n/a
Cloud provider or hardware configuration: n/a
OS: n/a
Kernel: n/a
Installation method: n/a

Additional context

Good catch. Which version of falcosidekick are you using? Just want to be sure this bug is in 2.28 and not introduced by our latest changes in the main branch.

I know the implementation of the Client is old and not efficient anymore. I think it's a race condition issue, because the list of headers is supposed to be cleared here

falcosidekick/outputs/client.go

Line 316 in 5c20d54

c.HeaderList = []Header{}

I'm not a big fan of checking the existence of each header. Right now, the list of headers is just a slice of this struct:

type Header struct {
	Key   string
	Value string
}

We could consider to use a map[string]string instead, the value will not be appended anymore, just replace in place. wdyt?

I found where's the real issue is. The list of headers is cleared only after successful requests. With netcat as server, the answer is not correct and the error is not nil, with another server that answers correctly, the bug doesn't happen. For example with https://httpbin.org/put as target, I wasn't able to replicate, when I was with nc.

Basically, here's the issue:

	resp, err := client.Do(req)
	if err != nil {
		log.Printf("[ERROR] : %v - %v\n", c.OutputType, err.Error())
		go c.CountMetric("outputs", 1, []string{"output:" + strings.ToLower(c.OutputType), "status:connectionrefused"})
		return err
	}
	defer resp.Body.Close()

	// Clear out headers - they will be set for the next request.
	c.HeaderList = []Header{}

the code should be:

	resp, err := client.Do(req)
	if err != nil {
		log.Printf("[ERROR] : %v - %v\n", c.OutputType, err.Error())
		go c.CountMetric("outputs", 1, []string{"output:" + strings.ToLower(c.OutputType), "status:connectionrefused"})
		// Clear out headers - they will be set for the next request.
		c.HeaderList = []Header{}
		return err
	}
	defer resp.Body.Close()

	// Clear out headers - they will be set for the next request.
	c.HeaderList = []Header{}

https://github.com/falcosecurity/falcosidekick/blob/5c20d54f22a863dfce9b2587addcc5f3a98a4cc3/outputs/client.go#L307,L316

@marwinski do you want to create the fix PR or do you prefer I do it?

fixed by #801