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

1996 island worm config decouple #2017

Merged
merged 14 commits into from
Jun 16, 2022
Merged

Conversation

VakarisZ
Copy link
Contributor

@VakarisZ VakarisZ commented Jun 14, 2022

What does this PR do?

Fixes #1996.

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 {Running the Monkey locally with relevant config/running Island/...}

  • If applicable, add screenshots or log transcripts of the feature working

Explain Changes

Are the commit messages enough? If not, elaborate.

@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #2017 (2fa7606) into develop (ac172dc) will decrease coverage by 0.00%.
The diff coverage is 73.07%.

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

@@             Coverage Diff             @@
##           develop    #2017      +/-   ##
===========================================
- Coverage    56.81%   56.80%   -0.01%     
===========================================
  Files          462      463       +1     
  Lines        12810    12822      +12     
===========================================
+ Hits          7278     7284       +6     
- Misses        5532     5538       +6     
Impacted Files Coverage Δ
monkey/common/config_value_paths.py 100.00% <ø> (ø)
monkey/monkey_island/cc/services/config.py 86.58% <ø> (+0.01%) ⬆️
...onkey_island/cc/services/config_schema/internal.py 100.00% <ø> (ø)
...s/unit_tests/monkey_island/cc/services/conftest.py 81.81% <ø> (-18.19%) ⬇️
...land/cc/services/attack/technique_reports/T1065.py 72.72% <58.33%> (-7.28%) ⬇️
monkey/monkey_island/cc/resources/ip_addresses.py 80.00% <80.00%> (ø)
monkey/monkey_island/cc/app.py 80.40% <100.00%> (+0.26%) ⬆️
...y/monkey_island/cc/services/utils/network_utils.py 41.30% <100.00%> (+2.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac172dc...b14c0dd. Read the comment docs.

"""
local_ips = local_ip_addresses()

return {"ip_addresses": local_ips}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return {"ip_addresses": local_ips}
return local_ips

Since this is a collection, we can just return a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do that without changes to monkey/monkey_island/cc/services/representations.py:normalize_object

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to fix that at some point, but now probably isn't the time. The :return: in the docstring is wrong, though.

@@ -14,6 +14,9 @@ class T1065(AttackTechnique):

@staticmethod
def get_report_data():
port = ConfigService.get_config_value(CURRENT_SERVER_PATH).split(":")[1]
monkey = Monkey.objects.first()
tunnel = monkey.get_tunnel_info()["tunnel"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work in general. Tunnel is another monkey:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fixing this now

Non standard ports attack technique should include ports agent used for tunneling
@VakarisZ VakarisZ force-pushed the 1996-island-worm-config-decouple branch from 3253502 to 5fbe01a Compare June 16, 2022 09:12
@VakarisZ VakarisZ marked this pull request as ready for review June 16, 2022 09:40
@VakarisZ VakarisZ force-pushed the 1996-island-worm-config-decouple branch from 34fa56d to 5817fbd Compare June 16, 2022 09:42
@VakarisZ VakarisZ requested a review from mssalvatore June 16, 2022 09:43
Copy link
Collaborator

@mssalvatore mssalvatore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, but fix the docstring for the ip_addresses endpoint.

@VakarisZ VakarisZ force-pushed the 1996-island-worm-config-decouple branch from 5817fbd to 0082cd2 Compare June 16, 2022 12:46
@mssalvatore mssalvatore merged commit fd36aca into develop Jun 16, 2022
@mssalvatore mssalvatore deleted the 1996-island-worm-config-decouple branch June 16, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove current_server from the configuration
3 participants