SMB: Add SMBOptions to SMBPlugin #3089

ilija-lazoroski · 2023-03-10T16:23:59Z

Issue: #2952

What does this PR do?

Fixes part of #2952

Add any further explanations here.

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 unit tests
If applicable, add screenshots or log transcripts of the feature working

Issue: #2952 PR: #3089

mssalvatore · 2023-03-10T16:55:07Z

@ybasford Option descriptions could use your expert opinion.

mssalvatore

Vulture fails

mssalvatore · 2023-03-10T16:56:20Z

monkey/agent_plugins/exploiters/smb/src/smb_options.py

+    agent_binary_upload_timeout: float = Field(
+        default=30.0,
+        gt=0.0,
+        le=100.0,


These maximums seem somewhat arbitrary. Are they based on anything?

Should we get rid of the maximums altogether?

Only if we add a note to the description that this could cause the agent to hang on manual shutdown if it's set too high. Realistically, we shutdown after I think 120 seconds if things are hung.

Then we should also change things in Hadoop

codecov · 2023-03-10T17:48:42Z

Codecov Report

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

Comparison is base (c254126) 67.58% compared to head (04b6cb0) 67.63%.

❗ Current head 04b6cb0 differs from pull request most recent head 899673c. Consider uploading reports for the commit 899673c to get more accurate results

Additional details and impacted files

@@                      Coverage Diff                      @@
##           2952-smb-exploiter-plugin    #3089      +/-   ##
=============================================================
+ Coverage                      67.58%   67.63%   +0.05%     
=============================================================
  Files                            444      446       +2     
  Lines                          12730    12755      +25     
=============================================================
+ Hits                            8603     8627      +24     
- Misses                          4127     4128       +1

see 11 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

shreyamalviya

We should consider adding the option of choosing both or one of port 445 and port 139 instead of hard-coding it to be both. Probably checkboxes.

shreyamalviya · 2023-03-13T07:29:35Z

monkey/agent_plugins/exploiters/smb/src/smb_options.py

+    use_kerberos: bool = Field(
+        default=False, description="Use Kerberos authentication for RPC transport."
+    )


We probably don't need this. We don't have any exploiter logic for when Kerberos auth is set to true.

Yep, we don't but RPC transport has that option which we were hard-coding to False.

Exactly, we don't specifically handle cases where Kerberos is used. Do we need anything else for it to work or is our existing logic enough?

Probablyyyy. We need to check that.

shreyamalviya · 2023-03-13T08:09:59Z

monkey/agent_plugins/exploiters/smb/src/smb_options.py

+    rpc_connect_timeout: float = Field(
+        default=15.0,
+        gt=0.0,
+        le=100.0,
+        description="The maximum time (in seconds) to wait for a response on an RPC connection.",
+    )
+    smb_connect_timeout: float = Field(
+        default=15.0,
+        gt=0.0,
+        le=100.0,
+        description="The maximum time (in seconds) to wait for a response on an SMB connection.",
+    )


I'm also wondering if this should just be one option (smb_connect_timeout) which is used in both places. The user doesn't know about the internal workings of the exploiter i.e. where/when/why RPC is being used.

That probably make sense.

mssalvatore · 2023-03-13T11:53:43Z

We should consider adding the option of choosing both or one of port 445 and port 139 instead of hard-coding it to be both. Probably checkboxes.

@shreyamalviya We discussed it on Friday. The issue is that 445 and 139 aren't really the right configuration options. The right set of configuration options are checkboxes regarding whether or not we should use the protocol over NetBIOS (see https://www.upguard.com/blog/smb-port). After some discussion, we decided this requires the user to have more knowledge of SMB and its history than they probably want. We decided that, since it's so easy to modify and release new versions of the plugin, we could add this option at a later date if a user requests it. For now, we decided to just hard code both.

mssalvatore · 2023-03-13T11:56:15Z

monkey/agent_plugins/exploiters/smb/src/smb_options.py

+    rpc_connect_timeout: float = Field(
+        default=15.0,
+        gt=0.0,
+        le=100.0,
+        description="The maximum time (in seconds) to wait for a response on an RPC connection.",
+    )
+    smb_connect_timeout: float = Field(
+        default=15.0,
+        gt=0.0,
+        le=100.0,
+        description="The maximum time (in seconds) to wait for a response on an SMB connection.",
+    )


Should these just be the same option?

#3089 (comment) Yep.

rpc protocol is part of smb so we can use the same timeout

Issue: #2952 PR: #3089

ybasford · 2023-03-15T08:33:39Z

monkey/agent_plugins/exploiters/smb/src/smb_options.py

+        default=30.0,
+        gt=0.0,
+        le=100.0,
+        description="The timeout (in seconds) for uploading the Agent binary"


Change description text: "Timeout (in seconds) for uploading the Agent binary."

Issue: #2952 PR: #3089

ilija-lazoroski requested review from cakekoa and mssalvatore March 10, 2023 16:23

SMB: Add SMBOptions to SMBPlugin

d8af95c

Issue: #2952 PR: #3089

ilija-lazoroski force-pushed the 2952-smb-options branch from cd92d3b to d8af95c Compare March 10, 2023 16:24

mssalvatore requested changes Mar 10, 2023

View reviewed changes

cakekoa mentioned this pull request Mar 10, 2023

2952 config schema #3092

Merged

8 tasks

cakekoa and others added 2 commits March 13, 2023 11:32

SMB: Add vulture entries

06cc30d

Project: Fix Vulture allowlist entries

4bd34ee

shreyamalviya force-pushed the 2952-smb-options branch 2 times, most recently from 4bfe266 to 04b6cb0 Compare March 13, 2023 07:10

SMB: Reword descriptions of configuration options

278845a

shreyamalviya force-pushed the 2952-smb-options branch from 04b6cb0 to 278845a Compare March 13, 2023 07:11

shreyamalviya requested changes Mar 13, 2023

View reviewed changes

ilija-lazoroski requested a review from mssalvatore March 13, 2023 08:31

shreyamalviya changed the base branch from develop to 2952-smb-exploiter-plugin March 13, 2023 11:29

mssalvatore reviewed Mar 13, 2023

View reviewed changes

SMB: Remove SMBOption.rpc_connect_timeout

899673c

rpc protocol is part of smb so we can use the same timeout

shreyamalviya approved these changes Mar 13, 2023

View reviewed changes

shreyamalviya merged commit 327a563 into 2952-smb-exploiter-plugin Mar 13, 2023

shreyamalviya deleted the 2952-smb-options branch March 13, 2023 12:28

cakekoa pushed a commit that referenced this pull request Mar 13, 2023

SMB: Add SMBOptions to SMBPlugin

f7bdaba

Issue: #2952 PR: #3089

ilija-lazoroski added a commit that referenced this pull request Mar 13, 2023

SMB: Add SMBOptions to SMBPlugin

b2cc5fa

Issue: #2952 PR: #3089

ilija-lazoroski added a commit that referenced this pull request Mar 14, 2023

SMB: Add SMBOptions to SMBPlugin

678c600

Issue: #2952 PR: #3089

ybasford reviewed Mar 15, 2023

View reviewed changes

cakekoa pushed a commit that referenced this pull request Mar 20, 2023

SMB: Add SMBOptions to SMBPlugin

5426499

Issue: #2952 PR: #3089

mssalvatore pushed a commit that referenced this pull request Mar 23, 2023

SMB: Add SMBOptions to SMBPlugin

9ab4338

Issue: #2952 PR: #3089

mssalvatore pushed a commit that referenced this pull request Mar 23, 2023

SMB: Add SMBOptions to SMBPlugin

c5b2af1

Issue: #2952 PR: #3089

mssalvatore pushed a commit that referenced this pull request Mar 24, 2023

SMB: Add SMBOptions to SMBPlugin

197cf55

Issue: #2952 PR: #3089

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SMB: Add SMBOptions to SMBPlugin #3089

SMB: Add SMBOptions to SMBPlugin #3089

ilija-lazoroski commented Mar 10, 2023

mssalvatore commented Mar 10, 2023

mssalvatore left a comment

mssalvatore Mar 10, 2023

cakekoa Mar 10, 2023

mssalvatore Mar 13, 2023 •

edited

Loading

ilija-lazoroski Mar 13, 2023

codecov bot commented Mar 10, 2023 •

edited

Loading

shreyamalviya left a comment •

edited

Loading

shreyamalviya Mar 13, 2023

ilija-lazoroski Mar 13, 2023

shreyamalviya Mar 13, 2023

ilija-lazoroski Mar 13, 2023

shreyamalviya Mar 13, 2023

ilija-lazoroski Mar 13, 2023

mssalvatore commented Mar 13, 2023

mssalvatore Mar 13, 2023

ilija-lazoroski Mar 13, 2023

ybasford Mar 15, 2023

SMB: Add SMBOptions to SMBPlugin #3089

SMB: Add SMBOptions to SMBPlugin #3089

Conversation

ilija-lazoroski commented Mar 10, 2023

What does this PR do?

PR Checklist

Testing Checklist

mssalvatore commented Mar 10, 2023

mssalvatore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssalvatore Mar 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 10, 2023 • edited Loading

Codecov Report

shreyamalviya left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssalvatore commented Mar 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mssalvatore Mar 13, 2023 •

edited

Loading

codecov bot commented Mar 10, 2023 •

edited

Loading

shreyamalviya left a comment •

edited

Loading