Add a flag to disable ssl verify

[rgmz]

Description:

This creates an experimental flag to disable SSL verification for detectors. This is useful in corporate environments where MITMing SSL traffic is common and certificates are replaced with self-signed ones.

This implementation is an incomplete POC.

• There may not be total coverage for http clients
• Detectors that rely on third-party libraries likely need to be configured separately.

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[zricethezav]

Slapping a draft label on this since it's an incomplete POC

[rgmz]

It could use a review to determine whether it's worth putting any more effort into.

[ahrav]

It could use a review to determine whether it's worth putting any more effort into.

I like the idea, but I'm a bit nervous about the global variable. Is there a way to use the flag to construct the transport instead? I'm concerned that it's too easy for anyone to reach into the transport and change the TLSConfig. That being said, if others are okay with this approach, I won't object. 😅

[rgmz]

I like the idea, but I'm a bit nervous about the global variable.

It's a bit of a smell. I couldn't think of an easier way to propagate such a value to multiple packages before the dependent code initializes.

[rgmz]

Unfortunately, it seems this isn't possible. The hundreds of detectors with logic like defaultClient = common.SaneHttpClient() means that SaneHttpClient() gets invoked well before init in main.go.

As far as I'm aware, there's no equivalent to something like Kotlin's lazy { } initializer. Would it be feasible to initialize the client on each FromData call instead?

[rgmz]

Unfortunately, it seems this isn't possible. The hundreds of detectors with logic like defaultClient = common.SaneHttpClient() means that SaneHttpClient() gets invoked well before init in main.go.

As far as I'm aware, there's no equivalent to something like Kotlin's lazy { } initializer. Would it be feasible to initialize the client on each FromData call instead?

For this to work, the top-level var would need to be replaced with a lazy initialization.

trufflehog/pkg/detectors/scrapingbee/scrapingbee.go

Lines 61 to 63 in 316316e

	if s.client == nil {
	s.client = common.SaneHttpClient()
	}

[rgmz]

For this to work, the top-level var would need to be replaced with a lazy initialization.

I've managed to partially solve this, at least for detectors that were using detectors.DetectorHttpClientWithNoLocalAddresses and detectors.DetectorHttpClientWithLocalAddresses.

[rosecodym]

In theory, if you could toggle ssl-verify while the scan was running, changes to it wouldn't affect detectors that had already been initialized, right?

If so - and I really hate to be the bearer of bad news on this - that means the new feature flag operates differently from the other defined ones in a way that actually matters not to you (or any other OSS user) but to TruffleHog Enterprise, which also uses this code. TruffleHog Enterprise runs as a persistent daemon, so we need the ability to dynamically toggle feature flags of running processes and have the changes be reflected. Having some flags require scanner restarts and other flags not require them is an accident waiting to happen.

I know this kind of sends you back to the drawing board, but I really want to avoid some future person having a really irritating surprise.

(Or I'm misreading this and imagined a problem that doesn't exist!)

[rgmz]

In theory, if you could toggle ssl-verify while the scan was running, changes to it wouldn't affect detectors that had already been initialized, right?

That is correct for HTTP-based detectors. Non-HTTP detectors like MongoDB would work dynamically.

Detectors that use common.SaneClient() or detectors.DetectorHttpClientWithNoLocalAddresses create a singleton at start-up, either globally or per-detector. 'Fixing' this would require changing 800+ detectors to dynamically create HTTP clients, and that may have other implications.

Since this pattern is literally from the first commit of v3, I think a change of that scale is probably outside of the scope of this. :/