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

Check for empty result in Modify shell files #1510

Merged
merged 1 commit into from
Oct 5, 2021
Merged

Conversation

ilija-lazoroski
Copy link
Contributor

@ilija-lazoroski ilija-lazoroski commented Oct 4, 2021

What does this PR do?

Fixes #1507 and #1055 .

In docker container /etc/passwd doesn't have any regular users. The modify shell script searches for those kind of users and if it doesn't find it return empty list which then we send telemetry without any result making the post breach action to fail.
Here we check if we have empty result from the running of command and send failed result with explanation.

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?

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

image

@ilija-lazoroski ilija-lazoroski changed the title Agent: Check for empty result in Modify shell files Check for empty result in Modify shell files Oct 4, 2021
@codecov
Copy link

codecov bot commented Oct 4, 2021

Codecov Report

Merging #1510 (1883ecd) into develop (0a4973a) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 1883ecd differs from pull request most recent head dc1735e. Consider uploading reports for the commit dc1735e to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1510      +/-   ##
===========================================
- Coverage    43.00%   42.98%   -0.02%     
===========================================
  Files          477      477              
  Lines        14171    14176       +5     
===========================================
  Hits          6094     6094              
- Misses        8077     8082       +5     
Impacted Files Coverage Δ
.../post_breach/actions/modify_shell_startup_files.py 0.00% <0.00%> (ø)
monkey/infection_monkey/system_info/__init__.py 41.26% <0.00%> (-1.36%) ⬇️
...ction_monkey/system_info/windows_info_collector.py 0.00% <0.00%> (ø)

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 0a4973a...dc1735e. Read the comment docs.

Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

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

Approved, provided you tested this and actually get this output in the report

Copy link
Contributor

@shreyamalviya shreyamalviya left a comment

Choose a reason for hiding this comment

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

Minor rewording

@ilija-lazoroski ilija-lazoroski force-pushed the 1507/fix-docker branch 2 times, most recently from 019219b to 6f51b4c Compare October 5, 2021 09:08
CHANGELOG.md Outdated
@@ -46,7 +46,7 @@ Changelog](https://keepachangelog.com/en/1.0.0/).
- Some of the gathered credentials no longer appear in database plaintext. #1454
- Encryptor breaking with UTF-8 characters. (Passwords in different languages can be submitted in
the config successfully now.) #1490

- Malfunctioning Modify shell startup files PBA on Docker. #1507
Copy link
Collaborator

@mssalvatore mssalvatore Oct 5, 2021

Choose a reason for hiding this comment

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

Changelog entries should be meaningful to the user of the changed component. "Malfunction" is a little bit broad, and the issue isn't specifically docker related.

Suggested change
- Malfunctioning Modify shell startup files PBA on Docker. #1507
- Unhandled error when "modify shell startup files PBA" is unable to find regular users. #1507

@mssalvatore mssalvatore merged commit e80662f into develop Oct 5, 2021
@mssalvatore mssalvatore deleted the 1507/fix-docker branch October 5, 2021 14:40
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.

Docker post breach telemetry bug
4 participants