3039 smarter brute force credentials generator #3048

mssalvatore · 2023-03-03T18:12:56Z

What does this PR do?

Fixes #3039

Add function for smarter brute force credentials generation

PR Checklist

Have you added an explanation of what your changes do and why you'd like to include them?
Is the TravisCI build passing?
~~Was the CHANGELOG.md updated to reflect the changes?~~
~~Was the documentation framework updated to reflect the changes?~~
Have you checked that you haven't introduced any duplicate code?

Testing Checklist

Added relevant unit tests?
Have you successfully tested your changes locally? Elaborate:

Tested by running unit tests
~~If applicable, add screenshots or log transcripts of the feature working~~

codecov · 2023-03-03T18:19:30Z

Codecov Report

Patch coverage has no change and project coverage change: +0.06 🎉

Comparison is base (3449dc9) 67.17% compared to head (2ad1ed2) 67.23%.

❗ Current head 2ad1ed2 differs from pull request most recent head 68507ed. Consider uploading reports for the commit 68507ed to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3048      +/-   ##
===========================================
+ Coverage    67.17%   67.23%   +0.06%     
===========================================
  Files          438      439       +1     
  Lines        12602    12627      +25     
===========================================
+ Hits          8465     8490      +25     
  Misses        4137     4137

Impacted Files	Coverage Δ
.../monkey/infection_monkey/exploit/tools/__init__.py	`100.00% <0.00%> (ø)`
...exploit/tools/brute_force_credentials_generator.py	`100.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

monkey/infection_monkey/exploit/tools/brute_force_credentials_generator.py

...ey/tests/unit_tests/infection_monkey/exploit/tools/test_brute_force_credentials_generator.py

VakarisZ · 2023-03-06T10:16:57Z

...ey/tests/unit_tests/infection_monkey/exploit/tools/test_brute_force_credentials_generator.py

+        Credentials(identity=IDENTITIES[2], secret=SECRETS[3]),
+    }
+
+    generate_and_compare_credentials(input_credentials, expected_credentials)


I think this could also assert the correct order as the output should be deterministic. IMO less UT code to maintain is worth it, but "test one thing only" has benefits as well, so I leave it up to you.

The interface doesn't make any promises about the order, except that complete credentials from the input are first. But, again, it does not make any promise about the order of those credentials.

VakarisZ · 2023-03-06T10:31:13Z

monkey/infection_monkey/exploit/tools/brute_force_credentials_generator.py

+            identity_type = type(credentials.identity)
+            identities.setdefault(identity_type, set()).add(credentials.identity)


What's the point of putting identities/secrets into dicts by types? This is an indirect way of grouping the output by type? Why do we want it?

It's fairly direct. There's a comment below. Basically, orderly output is better than disorderly, IMHO. It might be useful for debugging. If we need to dump out the result, at least there's some order to it.

So, yeah, it's analogous to an SQL GROUP BY clause. Order isn't guaranteed by the interface (other than ordering complete/known-good credentials first"

mssalvatore · 2023-03-06T12:26:33Z

monkey/infection_monkey/exploit/tools/brute_force_credentials_generator.py

+    identity_filters: Iterable[Callable[[Identity], bool]] = (),
+    secret_filters: Iterable[Callable[[Secret], bool]] = (),


What's everyone's thoughts on this? Would it be better to just allow the user to provide a single filter for each of these, instead of an iterable?

Options:

Allow the user to provide a single filter for each. This is probably the most common use case. If a user wants to filter on multiple criteria they can build a single compound filter and pass it in.

Leave it as is.

@VakarisZ
@shreyamalviya

I say leave it in, assuming there's no downside to doing that.

The interface is arguably more cumbersome if you just need a simple filter, but arguably nicer for those cases with more complex filtering criteria.

It's already implemented and it's not something whose logic will change often.

With our end goal being an easy-to-use plugin system, I like the idea of leaving this in.

After some discussion on webex, we decided to go with a single filter, as this will support the vast majority of cases.

…s docstring

The vast majority of uses for this function will require filtering certain types of secrets. Only one filter is required to this. Accepting an iterable of filters for identities and secrets unnecessarily complicates the code and interface.

shreyamalviya approved these changes Mar 6, 2023

View reviewed changes

monkey/infection_monkey/exploit/tools/brute_force_credentials_generator.py Outdated Show resolved Hide resolved

VakarisZ reviewed Mar 6, 2023

View reviewed changes

monkey/infection_monkey/exploit/tools/brute_force_credentials_generator.py Show resolved Hide resolved

VakarisZ reviewed Mar 6, 2023

View reviewed changes

...ey/tests/unit_tests/infection_monkey/exploit/tools/test_brute_force_credentials_generator.py Show resolved Hide resolved

VakarisZ reviewed Mar 6, 2023

View reviewed changes

...ey/tests/unit_tests/infection_monkey/exploit/tools/test_brute_force_credentials_generator.py Show resolved Hide resolved

VakarisZ reviewed Mar 6, 2023

View reviewed changes

mssalvatore commented Mar 6, 2023

View reviewed changes

VakarisZ approved these changes Mar 6, 2023

View reviewed changes

mssalvatore force-pushed the 3039-smarter-brute-force-credentials-generator branch from de428f6 to 1c568ae Compare March 6, 2023 14:00

mssalvatore added 4 commits March 6, 2023 09:21

Common: Export Identity and Secret types from common.Credentials

8c319f8

Agent: Add generate_brute_force_credentials()

8c77e62

Agent: Add secret_filters param to generate_brute_force_credentials()

3845938

Agent: Add identity_filters param to generate_brute_force_credentials()

c751290

mssalvatore force-pushed the 3039-smarter-brute-force-credentials-generator branch from 1c568ae to 2ad1ed2 Compare March 6, 2023 14:21

mssalvatore and others added 4 commits March 6, 2023 10:05

Project: Add generate_brute_force_credentials() vulture allowlist

cea7c34

Agent: Add missing param field in generate_brute_force_credentials()'…

07212a2

…s docstring

UT: Add test for generate_brute_force_credentials()

2aca006

mssalvatore force-pushed the 3039-smarter-brute-force-credentials-generator branch from 2ad1ed2 to 68507ed Compare March 6, 2023 15:05

mssalvatore merged commit 7f89663 into develop Mar 6, 2023

mssalvatore deleted the 3039-smarter-brute-force-credentials-generator branch March 6, 2023 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

3039 smarter brute force credentials generator #3048

3039 smarter brute force credentials generator #3048

Uh oh!

mssalvatore commented Mar 3, 2023 •

edited

Loading

Uh oh!

codecov bot commented Mar 3, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VakarisZ Mar 6, 2023

Uh oh!

mssalvatore Mar 6, 2023

Uh oh!

VakarisZ Mar 6, 2023 •

edited

Loading

Uh oh!

mssalvatore Mar 6, 2023

Uh oh!

mssalvatore Mar 6, 2023

Uh oh!

shreyamalviya Mar 6, 2023

Uh oh!

mssalvatore Mar 6, 2023

Uh oh!

shreyamalviya Mar 6, 2023

Uh oh!

mssalvatore Mar 6, 2023

Uh oh!

Uh oh!

		identity_type = type(credentials.identity)
		identities.setdefault(identity_type, set()).add(credentials.identity)

		identity_filters: Iterable[Callable[[Identity], bool]] = (),
		secret_filters: Iterable[Callable[[Secret], bool]] = (),

3039 smarter brute force credentials generator #3048

3039 smarter brute force credentials generator #3048

Uh oh!

Conversation

mssalvatore commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

PR Checklist

Testing Checklist

Uh oh!

codecov bot commented Mar 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VakarisZ Mar 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssalvatore commented Mar 3, 2023 •

edited

Loading

codecov bot commented Mar 3, 2023 •

edited

Loading

VakarisZ Mar 6, 2023 •

edited

Loading