3167 mock credential collector #3247

ilija-lazoroski · 2023-04-24T22:08:09Z

What does this PR do?

Fixes #3167

Things left to do:

Check the last TEMP commit. Figure out how to pass TargetHost.
Check the UT. Probably some of them are failing.

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?
Do all unit tests pass?
Do all end-to-end tests pass?
Any other testing performed?

Tested by {Running the Monkey locally with relevant config/running Island/...}
If applicable, add screenshots or log transcripts of the feature working

mssalvatore · 2023-04-25T00:31:59Z

monkey/common/agent_plugins/agent_plugin_type.py

@@ -2,7 +2,7 @@


 class AgentPluginType(Enum):
-    CREDENTIAL_COLLECTOR = "CredentialCollector"
+    CREDENTIAL_COLLECTOR = "Credential_Collector"


Credential Collector

Also, should this be plural?

No, it shouldn't be plural. According to how plugins are parsed right now, this needs to be the same as what the plugin tar name contains (in lowercase) AND what the plugin manifest has in the plugin_type field (which we've discussed is not good design and needs to be changed).

For example, for Mimikatz, the tar name needs to be mimikatz-credentialcollector.tar and the manifest needs to contain plugin_type: CredentialCollector.

I suggest we leave this as CredentialCollector. Adding a space would require the tar name to have a space (mimikatz-credential collector.tar), and adding an underscore would require the manifest to have plugin_type: Credential_Collector. Unless we decide to change plugin parsing before we get to migrating the credential collectors.

Everything on that is clear, but the issue was that the enum name and value were different and the plugin was not being loaded due to this.

So to get a plugin to work, we need 4 things to be written the same way, the enum name, the enum value, the plugin type in the manifest, and the last part of the tar name.

We have to go with Credential_Collector as the value in that case.

Ok, that's fine. But should we rename the enum name and value to be plural? The plugin collects credentials, not a single credential.

@cakekoa you should rebase on develop and drop this commit. It is already done there.

monkey/infection_monkey/puppet/puppet.py

mssalvatore · 2023-04-25T00:33:54Z

I think there are some issues with the interface here. We should probably complete the tasks to define the interface before doing any more work on this.

monkey/common/agent_plugins/agent_plugin_type.py

monkey/tests/data_for_tests/credential_collector_plugin/src/.plugin.py.swo

codecov · 2023-04-27T08:39:38Z

Codecov Report

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

Comparison is base (ea2da23) 73.35% compared to head (2466ffb) 73.36%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3247      +/-   ##
===========================================
+ Coverage    73.35%   73.36%   +0.01%     
===========================================
  Files          481      481              
  Lines        13875    13866       -9     
===========================================
- Hits         10178    10173       -5     
+ Misses        3697     3693       -4

see 8 files with indirect coverage changes

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

ilija-lazoroski · 2023-04-27T08:43:48Z

Travis is fixed in the #3260

…l plugins It will replace PluginSelectorTemplate and take its name when done.

Needed so we can disable options if the plugins are not selected

Exploiters will be also used as part of selectedPlugins in a later commit

…ription

…i-credentials-collectors Issue #3167 PR #3260

…al-collector Issue #3167 PR #3258

mssalvatore reviewed Apr 25, 2023

View reviewed changes

monkey/infection_monkey/puppet/puppet.py Outdated Show resolved Hide resolved

shreyamalviya reviewed Apr 25, 2023

View reviewed changes

monkey/common/agent_plugins/agent_plugin_type.py Outdated Show resolved Hide resolved

monkey/tests/data_for_tests/credential_collector_plugin/src/.plugin.py.swo Outdated Show resolved Hide resolved

ilija-lazoroski force-pushed the 3167-mock-credential-collector branch 2 times, most recently from fca73ae to 5a47852 Compare April 25, 2023 07:59

cakekoa force-pushed the 3167-mock-credential-collector branch 3 times, most recently from b7bbaa5 to bfb79d3 Compare April 25, 2023 21:02

ilija-lazoroski force-pushed the 3167-mock-credential-collector branch 2 times, most recently from 8e83fb8 to fe230be Compare April 27, 2023 08:33

ilija-lazoroski marked this pull request as ready for review April 27, 2023 11:51

ilija-lazoroski and others added 16 commits April 27, 2023 07:55

UT: Add Mock Credential Collector Plugin

3a23d74

Agent: Pass interrupt to run_credential_collector

d72d626

Agent: Construct credentials collector plugins

c298530

Common: Add CredentialsCollectorConfiguration to AgentConfiguration

2e28ea2

Island: Fix CREDENTIALS_COLLECTOR path in the schema compiler

f1a8e9c

UI: Implement PluginSelectorTemplateGeneral which will be used for al…

5338040

…l plugins It will replace PluginSelectorTemplate and take its name when done.

UI: Add PluginSelectorTemplateGeneral for Credentials Collector

7b717f1

Agent: Add interrupt to IPuppet.run_credentials_collector

19d7149

UI: Implement FormConfig which will keep state of the plugin UiSchema

6dd1749

Needed so we can disable options if the plugins are not selected

UT: Rename Mock {Credential,s} Collector plugin

2638961

UI: Generalize plugin form in ConfigurePage

eff04f7

Exploiters will be also used as part of selectedPlugins in a later commit

UI: Move credentials collectors to plugins in SafeOptionValidator

a46ae86

UI: Use PluginSelectorTemplateGeneral for Exploiters plugins

8fbd20b

UI: Remove exploiter specific PluginSelectorTemplate

2067e44

UI: Rename PluginSelectorTemplateGeneral to PluginSelectorTemplate

8b71843

UI: Fix plugin schema path TODO in PluginSelectorTemplate

99da7c1

ilija-lazoroski and others added 19 commits April 27, 2023 07:55

Common: Remove CredentialsCollectorsConfiguration and use dictionary

7dea52b

Island: Fix Credentials Collectors path in schema

0fe8104

UT: Fix Schema for Credentials Collectors

82b9747

UI: Fix credentials collectors plugin path

1669204

Agent: Rename ICredentialsCollector.{collect_credentials, run}

01af8c1

UI: Fix top level set of UiSchema

8f967bb

Agent: Run credentials collectors as plugins in AutomatedMaster

9c5a391

BB: Adjust BB test configurations to new credentials collectors config

8879aea

UT: Rename ICredentialsCollector.{collect_credentials, run}

1bb830a

UI: Remove unused constants

fcc5270

Common: Fix credentials collectors description

c0230f4

UI: Refactor FormConfig

c7b8855

UI: Put credentials collectors consts in PluginSelectorTemplate

d9afd68

BB: Fix default credentials_collectors value in noop test configuration

afe18ff

BB: Simplify add_credentials_collectors() logic in utils

976a598

Common: Fix grammar in AgentConfiguration.credentials_collectors desc…

3a5b362

…ription

Merge branch '3167-run-credentials-collectors-as-plugins' into 3167-u…

d0f038c

…i-credentials-collectors Issue #3167 PR #3260

UT: Return sequence of Credentials in Mock credentials collector

8a39b66

Merge branch '3167-ui-credentials-collectors' into 3167-mock-credenti…

2466ffb

…al-collector Issue #3167 PR #3258

mssalvatore force-pushed the 3167-mock-credential-collector branch from 5763477 to 2466ffb Compare April 27, 2023 11:55

mssalvatore merged commit f23437b into develop Apr 27, 2023

mssalvatore deleted the 3167-mock-credential-collector branch April 27, 2023 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3167 mock credential collector #3247

3167 mock credential collector #3247

ilija-lazoroski commented Apr 24, 2023 •

edited

Loading

mssalvatore Apr 25, 2023

shreyamalviya Apr 25, 2023

ilija-lazoroski Apr 25, 2023

shreyamalviya Apr 25, 2023

shreyamalviya Apr 25, 2023

mssalvatore Apr 25, 2023 •

edited

Loading

ilija-lazoroski Apr 25, 2023

mssalvatore commented Apr 25, 2023

codecov bot commented Apr 27, 2023 •

edited

Loading

ilija-lazoroski commented Apr 27, 2023

3167 mock credential collector #3247

3167 mock credential collector #3247

Conversation

ilija-lazoroski commented Apr 24, 2023 • edited Loading

What does this PR do?

PR Checklist

Testing Checklist

mssalvatore Apr 25, 2023

Choose a reason for hiding this comment

shreyamalviya Apr 25, 2023

Choose a reason for hiding this comment

ilija-lazoroski Apr 25, 2023

Choose a reason for hiding this comment

shreyamalviya Apr 25, 2023

Choose a reason for hiding this comment

shreyamalviya Apr 25, 2023

Choose a reason for hiding this comment

mssalvatore Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

ilija-lazoroski Apr 25, 2023

Choose a reason for hiding this comment

mssalvatore commented Apr 25, 2023

codecov bot commented Apr 27, 2023 • edited Loading

Codecov Report

ilija-lazoroski commented Apr 27, 2023

ilija-lazoroski commented Apr 24, 2023 •

edited

Loading

mssalvatore Apr 25, 2023 •

edited

Loading

codecov bot commented Apr 27, 2023 •

edited

Loading