Support blacklisting fingerprinters #1949
Conversation
|
Hey thanks for reporting this. If you are comfortable with Go a PR is welcome. You can mainly follow the code path of the white list to implement this. |
|
@dadgar I gave it a shot. Added driver blacklisting at the same time since it was very similar, but I could pull that out if there is some different thinking about those. (It seems the github cli transformed this from an issue into a pull request? Not sure what happened...) |
|
I don't really understand the test failures, they seem unrelated to my changes? Or is master broken? |
| @@ -720,6 +720,9 @@ func (c *Client) reservePorts() { | |||
| func (c *Client) fingerprint() error { | |||
| whitelist := c.config.ReadStringListToMap("fingerprint.whitelist") | |||
| whitelistEnabled := len(whitelist) > 0 | |||
| blacklist := c.config.ReadStringListToMap("fingerprint.blacklist") | |||
| blacklistEnabled := len(blacklist) > 0 | |||
There was a problem hiding this comment.
White list needs this since you need to know both if the whitelist is active and if the fingerprinter is whitelisted (if not active nothing will be whitelisted). For blacklist you just need to know if it is contained in the map. so remove this variable and change :737 to if _, ok := blacklist[name]; ok {
There was a problem hiding this comment.
Same below and update the comments on those lines
| @@ -133,6 +133,19 @@ see the [drivers documentation](/docs/drivers/index.html). | |||
| } | |||
| ``` | |||
|
|
|||
| - `"driver.blacklist"` `(string: "")` - Specifies a comma-separated list of | |||
| blacklisted drivers . If specified, drivers in the blacklist will be | |||
| disabled. If the blacklist is empty, all drivers are fingerprinted and enabled | |||
There was a problem hiding this comment.
I would remove the last sentences in both of these since it is pretty clear what the behavior will be and the fingerprinters/drivers still need to take into account the whitelist
|
Awesome job! We will be cutting the RC2 soon, so depending when the comments get addressed this might make it |
|
@dadgar I've implemented the changes now, good points! |
|
Awesome! Thank you so much! |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Currently there is a client option
fingerprint.whitelist. It would be useful to extend it with a blacklisting option as well.Use case: we would like to disable
env_awssince it picks up things from our Openstack platform which are nonsensical. Currently we need to figure out the list of available fingerprinters on the host, and then whitelist all of them except the aws one.