Matched Strings can be used as signatures

[mgaspar-godaddy]

🐛 Bug Report

In a tartufo.toml file a matched_string value can be used as a signature and tartufo will accept and exclude that value when scanning.

This appears to be a side effect from here:

tartufo/tartufo/scanner.py

Lines 424 to 425 in c750412

	blob
	in self.excluded_signatures # Signatures themselves pop up as entropy matches

Those lines of code appear to be intended to ignore signatures from entropy checks, but the resulting outcome is if a matched_string is excluded as a signature (instead of using the signature value) tartufo will still exclude it.

To Reproduce

Take a repo with an item flagged by tartufo. Copy the matched_string value and add it to the tartufo.toml as a signature. For example, if I am getting a match on the string -----BEGIN RSA PRIVATE KEY----- I can do the following to exclude it:

exclude-signatures = [
  { signature = '-----BEGIN RSA PRIVATE KEY-----', reason = 'exclude signature' }
]

Expected Behavior

This should not result in excluding the matched_string. I would expect tartufo to throw an error indicating one of the signatures to exclude is not a valid signature.

Code Example

See the code example under To Reproduce.

Environment

tartufo, version 3.2.1
Python 3.9.13

[dnasevic-godaddy]

I actually think that is feature, not a bug.

By using a signature that Tartufo produces, we hide the real information that is excluded. I prefer using the exact offending string so that it's explicitly visible what is excluded. It probably needs a better named category than exclude-signatures though.

[mgaspar-godaddy]

I would say it's a bug because it is not intended for a matched string to be used in this way. The section is specifically meant to exclude signatures and not strings.

Further, as shown in the example, this can be dangerous if a wide ranging matched string is used that would exclude across multiple files, where signature exclusions are meant to be file specific (the file name I believe is used to generate the signature).

Although it is convenient to see the string being excluded inside the config, that does not seem to be the intended or expected behavior.

[dnasevic-godaddy]

I can agree with you if we look from a perspective of what the tool does now i.e. its current design.

The design could have been simplified to have only a single category instead of having 3 different categories (exclude-path-patterns, exclude-entropy-patterns, exclude-signatures).

With a single category we would have path-pattern, reason and optional pattern or string. That way no need for escaping by signatures which are bad by design because they hide the real secrets that are excluded, should instead prefer putting the verbatim string in the config to see what is excluded. If it's long, there's the use of a regex, but again it could be covered by a single config category.

[mgaspar-godaddy]

Yeah, as of how the tool is meant to work today it feels like a bug, and I filed this after the maintainers asked me to. I don't disagree with what you're suggesting, but I think that would be a feature enhancement possibly for a future version.