Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matched Strings can be used as signatures #398

Open
mgaspar-godaddy opened this issue Oct 28, 2022 · 4 comments
Open

Matched Strings can be used as signatures #398

mgaspar-godaddy opened this issue Oct 28, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@mgaspar-godaddy
Copy link
Contributor

🐛 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

@mgaspar-godaddy mgaspar-godaddy added the bug Something isn't working label Oct 28, 2022
@dnasevic-godaddy
Copy link

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
Copy link
Contributor Author

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
Copy link

dnasevic-godaddy commented Mar 23, 2023

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants