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

fix(secter): set minimal number of characters for AsymmetricPrivateKey #7700

Open
DmitriyLewen opened this issue Oct 10, 2024 Discussed in #7695 · 2 comments
Open

fix(secter): set minimal number of characters for AsymmetricPrivateKey #7700

DmitriyLewen opened this issue Oct 10, 2024 Discussed in #7695 · 2 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. scan/secret Issues relating to secret scanning

Comments

@DmitriyLewen
Copy link
Contributor

Description

IIUC AsymmetricPrivateKeys have size linits depending on the key type.
Looks like minimal size is 128bit - https://www.cryptomathic.com/news-events/blog/classification-of-cryptographic-keys-functions-and-properties

So we can calculate minimal number of characters between --------BEGIN PRIVATE KEY----- and -----END PRIVATE KEY----- to avoid false positives as in #7695

Discussed in #7695

@DmitriyLewen DmitriyLewen added kind/bug Categorizes issue or PR as related to a bug. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. scan/secret Issues relating to secret scanning labels Oct 10, 2024
@OverOrion
Copy link

Hey @DmitriyLewen!

Thanks for creating an issue for it. I also thought of just checking for its' length, however it will still yield false positives for *.pyc files if there are arbitrary strings in between the BEGIN / END markers.

I think the proper fix (==no false positives) would be to parse the string and only report it as a private key if it actually is.

I skimmed through the codebase and it seems that rulesets rely on regexp validation only, so there is no way to insert a custom validator function (if there is then please point me to it).

Specifying a minimal length should help with the current state but won't be enough to eliminate false positives.

@DmitriyLewen
Copy link
Contributor Author

Hello @OverOrion
I have seen cases where examples use real secrets. So we will never be able to avoid all false positives.

We already have some allow rules to avoid examples, test secrets, etc. - https://github.com/aquasecurity/trivy/blob/main/pkg/fanal/secret/builtin-allow-rules.go

Using a secret length will also help to avoid false positives. At the moment I think this is all we can do.

For the remaining cases, Trivy has the ability to filter files and disable/create your own rules (for example, you can disable the default correctly and make your own taking into account your case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. scan/secret Issues relating to secret scanning
Projects
None yet
Development

No branches or pull requests

2 participants