blackbox_exporter icon indicating copy to clipboard operation
blackbox_exporter copied to clipboard

Add a way to export labels with content matched by the probe

Open lapo-luchini opened this issue 1 year ago • 7 comments

I wanted to create an exporter to check if all my hosts had an updated sshd (due to recent CVEs) then I resorted to using blackbock exporter by matching the "fixed" version and the timeout:

  ssh_freebsd_secure:
    prober: tcp
    timeout: 5s
    tcp:
      query_response:
      - expect: "^SSH-2.0-OpenSSH_[0-9.]+ FreeBSD-20240806$"

but then again I'd prefer to capture the value as a label and do my checks by coloring the value in Grafana table later on or in alertmanager rules so I did this change instead.

I tried to do it in a generic way so that it can be used also for very different cases when matching part of the protocol chat is useful.

This produces scraping values like these:

# HELP probe_content Explicit content matched
# TYPE probe_content gauge
probe_content{ssh_comments="FreeBSD-20240806",ssh_version="OpenSSH_9.7"} 1

BTW: I know no Go, except a advent-of-code example, these are the first Go lines that I wrote.

Feel free to close if uninteresting for the project (though I hope it isn't, to avoid the need to always compile my branch) or suggest what I should change in the PR to have it accepted. Thanks!

lapo-luchini avatar Aug 21 '24 14:08 lapo-luchini

I wonder if calling prometheus.NewGaugeVec inside the loop is fine, normally in a language that I know I would do it only once during configuration parsing, but I'm not sure how to do that in Go. Doesn't seems to produce problems anyways.

lapo-luchini avatar Aug 21 '24 15:08 lapo-luchini

Hey, this opens up blackbox_exporter to export data out from the check results, and that feel like something that we should not be doing because it can be used to exfiltrate data using blackbox_exporter.

I share the same concern that @SuperQ mentioned here

This use-case feel like more like something that the should be solved by the software itself by exporting the version label in build_info metric. or with https://github.com/prometheus/node_exporter processes or Textfile Collector of the node_exporter. I also recommend look into something like osquery for this use-case.

electron0zero avatar Aug 21 '24 23:08 electron0zero

Those are valid concerns. On the other hand, an attacker would need local access to modify the yaml file to exfiltrate anything but what was "explicitly matched" by the template, an attacker with only network access to blackbox_exporter couldn't do much (but it could use remote targets to query server outside the expected domain, that's true).

A node_exporter textfile collector would be another solution, right. (but it would require a new software to be installed on each and every host, not only on one centralized host)

I didn't know of osquery, looks interesting, thanks!

I'll close this PR for now, maybe I'll write a "security-oriented" more all-around exporter from scratch.

lapo-luchini avatar Aug 22 '24 07:08 lapo-luchini

I'm not too concerned about the data exposure here. Like @lapo-luchini says, the exposure values would be up to the admin's regexp.

The concern #1098 and similar are the ability to modify the behavior of the probe from the URL params.

Since this proposal is entirely contained in the config file, it's safe enough. I think this is actually a reasonably good idea.

SuperQ avatar Aug 22 '24 07:08 SuperQ

exposure values would be up to the admin's regexp.

ah I see, I think then it's a non- issue.

(but it would require a new software to be installed on each and every host, not only on one centralized host)

that's true, I was under the assumption that node_exporter is already running to monitor these nodes. which might not be the case for everyone so my assumption doesn't hold true. :)

let me find some time to give it a through review :)

electron0zero avatar Aug 22 '24 09:08 electron0zero

that's true, I was under the assumption that node_exporter is already running to monitor these nodes. which might not be the case for everyone so my assumption doesn't hold true. :)

It is my case too, but it does not export the running ssh version, so it would need "something else" to add that to a textfile (to the bare minimum, a shell script in cron).

Anyways… that's just my use-case, returning to the feature proposal of this PR (which tries to be useful to use-cases different than mine too) I'll wait for a review and make adjustments if needed. Thanks!

lapo-luchini avatar Aug 22 '24 10:08 lapo-luchini

I renamed the metric as requested, and also added some documentation.

lapo-luchini avatar Aug 22 '24 13:08 lapo-luchini

Will do tests tomorrow.

lapo-luchini avatar Aug 30 '24 19:08 lapo-luchini

OK, should be good for review!

lapo-luchini avatar Aug 31 '24 10:08 lapo-luchini