Ransomware bitflip encryption #1264

VakarisZ · 2021-06-23T05:44:18Z

What does this PR do?

Fixes #1238.

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?

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.

VakarisZ · 2021-06-23T05:46:28Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+        LOG.info(f"Windows dir configured for encryption is " f"{config['windows_dir']}")
+        LOG.info(f"Linux dir configured for encryption is " f"{config['linux_dir']}")


These should be moved/remove in this PR, because it's where they actually get used.

I think logging this information is still valuable.

VakarisZ · 2021-06-23T05:54:10Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+        self._target_dir = Path(config["windows_dir"] if is_windows_os() else config["linux_dir"])
+        self._valid_file_extensions_for_encryption = VALID_FILE_EXTENSIONS_FOR_ENCRYPTION.copy()
+        self._valid_file_extensions_for_encryption.discard(EXTENSION)


Should this be moved to separate class which represents an action?

VakarisZ · 2021-06-23T05:58:34Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+        self._valid_file_extensions_for_encryption = VALID_FILE_EXTENSIONS_FOR_ENCRYPTION.copy()
+        self._valid_file_extensions_for_encryption.discard(EXTENSION)


Why not just remove EXTENSION from the VALID_FILE_EXTENSIONS_FOR_ENCRYPTION list?

What is this doing, actually? VALID_FILE_EXTENSIONS_FOR_ENCRYPTION doesn't have m0nk3y in it. Why are you trying to discard it?

It's a little bit preemptive/defensive. When we implement #1242, we'll want to ensure that the extension isn't in the set of extensions we'll encrypt. I wrote the code this way to avoid introducing bugs later on.

VakarisZ · 2021-06-23T07:22:01Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+
+    def _encrypt_file(self, filepath):
+        with open(filepath, "rb+") as f:
+            data = f.read(CHUNK_SIZE)


Why do we read in chunks?

So that if we're encrypting a 30GB file we don't run out of RAM.

VakarisZ · 2021-06-23T07:24:52Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+                f.seek(-num_bytes_read, 1)
+                f.write(encrypted_data)


Could this be moved into a method with an explanatory name? Like overwrite_bytes_in_file or something?

If you think it's still necessary. It now lives inside _encrypt_single_file_in_place(). The "in_place" portion of the name better indicates what's going on. IMHO, this function is short and focused enough that with the new name, I think it's clear what's going in. But I can still change it if you feel strongly about it.

VakarisZ · 2021-06-23T07:30:10Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+        ]
+
+        all_files = get_all_regular_files_in_directory(self._target_dir)
+        return filter_files(all_files, file_filters)

    def _encrypt_files(self, file_list):


The fact that this method and the methods that get called by this method doesn't use self indicates that this shouldn't be in a different class or at least static methods.

Addressed as part of a refactor.

VakarisZ · 2021-06-23T08:31:13Z

monkey/tests/unit_tests/infection_monkey/ransomware/test_ransomware_payload.py

+
+from infection_monkey.ransomware.ransomware_payload import RansomewarePayload
+
+EXTENSION = ".m0nk3y"


Can't we just import the extension from monkey/infection_monkey/ransomware/ransomware_payload.py?

shreyamalviya

Where are we checking if ransomware is configured or not? If those fields in the config are empty, Path('') will default to the root directory (at least on Linux, it will). All files in the root directory will be encrypted.
Tests fail: TypeError: copytree() got an unexpected keyword argument 'dirs_exist_ok'.

shreyamalviya · 2021-06-23T10:20:32Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+        self._valid_file_extensions_for_encryption = VALID_FILE_EXTENSIONS_FOR_ENCRYPTION.copy()
+        self._valid_file_extensions_for_encryption.discard(EXTENSION)


What is this doing, actually? VALID_FILE_EXTENSIONS_FOR_ENCRYPTION doesn't have m0nk3y in it. Why are you trying to discard it?

shreyamalviya · 2021-06-23T10:25:36Z

monkey/infection_monkey/ransomware/ransomware_payload.py

-        self.config = config
+        LOG.info(f"Windows dir configured for encryption is " f"{config['windows_dir']}")
+        LOG.info(f"Linux dir configured for encryption is " f"{config['linux_dir']}")

-    def run_payload(self):
-        LOG.info(
-            f"Windows dir configured for encryption is " f"{self.config['windows_dir_ransom']}"
-        )
-        LOG.info(f"Linux dir configured for encryption is " f"{self.config['linux_dir_ransom']}")
+        self._target_dir = Path(config["windows_dir"] if is_windows_os() else config["linux_dir"])
+        self._valid_file_extensions_for_encryption = VALID_FILE_EXTENSIONS_FOR_ENCRYPTION.copy()
+        self._valid_file_extensions_for_encryption.discard(EXTENSION)


If we're not having an if statement in monkey.py, we're running all this code in any case, ransomware configured or not, which doesn't really make sense.

I had mistakenly assumed that if the path was empty, no files would be found. In fact, it appears that Path("") evaluates to Path("."). Good catch.

shreyamalviya · 2021-06-23T10:27:49Z

monkey/infection_monkey/ransomware/ransomware_payload.py

+    def run_payload(self):
        file_list = self._find_files()
        self._encrypt_files(file_list)


This function should return the data about the run. List of tuples with each filename and its result.

This function should send the appropriate telemetry. For the moment, we're going to avoid having payloads return anything.

vulture_allowlist.py

monkey/infection_monkey/ransomware/ransomware_payload.py

The dirs_exist_ok parameter of shutil.copytree() was introduced in Python 3.8. Since the agent uses python3.7 in order to be more compatible with older systems, we can't use this parameter.

The larger chunk size improves efficiency by reducing the number of reads.

The .gitignore file prevents dlls from being added to git. Since this isn't a real dll, but is only used for testing, we can add it anyway.

codecov · 2021-06-23T14:04:32Z

Codecov Report

Merging #1264 (ef209a6) into develop (2b1ba99) will increase coverage by 0.50%.
The diff coverage is 96.59%.

@@             Coverage Diff             @@
##           develop    #1264      +/-   ##
===========================================
+ Coverage    29.47%   29.97%   +0.50%     
===========================================
  Files          434      441       +7     
  Lines        13184    13283      +99     
===========================================
+ Hits          3886     3982      +96     
- Misses        9298     9301       +3

Impacted Files	Coverage Δ
.../infection_monkey/ransomware/ransomware_payload.py	`92.30% <89.65%> (ø)`
...y/infection_monkey/ransomware/bitflip_encryptor.py	`100.00% <100.00%> (ø)`
...nkey/infection_monkey/ransomware/file_selectors.py	`100.00% <100.00%> (ø)`
monkey/infection_monkey/utils/bit_manipulators.py	`100.00% <100.00%> (ø)`
...unit_tests/infection_monkey/ransomware/conftest.py	`100.00% <100.00%> (ø)`
...ction_monkey/ransomware/ransomware_target_files.py	`100.00% <100.00%> (ø)`
monkey/tests/utils.py	`78.57% <100.00%> (+28.57%)`	⬆️
...fection_monkey/ransomware/valid_file_extensions.py	`100.00% <0.00%> (ø)`
... and 4 more

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 2b1ba99...ef209a6. Read the comment docs.

shreyamalviya

Just a few questions

monkey/tests/unit_tests/infection_monkey/ransomware/test_bitflip_encryptor.py

monkey/tests/unit_tests/infection_monkey/ransomware/test_ransomware_payload.py

VakarisZ commented Jun 23, 2021

View reviewed changes

shreyamalviya requested changes Jun 23, 2021

View reviewed changes

mssalvatore added 5 commits June 23, 2021 06:57

agent: Add utility functions for flipping bits

3edaffa

agent: Add in-place, bitflip encryption to RansomwarePayload

1ff348d

agent: Rename files encrypted by ransomware with .m0nk3y extension

2dd75d7

agent: Test that ransomware payload does not encrypt recursively

cef3bd6

agent: Rename RansomewarePayload.target_dir -> _target_dir

447138c

mssalvatore force-pushed the ransomware-bitflip-encryption branch from 4dda012 to 447138c Compare June 23, 2021 10:57

mssalvatore added 12 commits June 23, 2021 07:10

agent: Don't redefine EXTENSION in ransomware tests

297adcf

Remove get_files_to_encrypt from Vulture's allow list

6307606

agent: Remove dirs_exist_ok from shutil.copytree() call

7149e11

The dirs_exist_ok parameter of shutil.copytree() was introduced in Python 3.8. Since the agent uses python3.7 in order to be more compatible with older systems, we can't use this parameter.

Agent: Don't run ransomware payload if no directory was specified

2c97d04

agent: Extract bitflip encryption into its own class

ab40518

tests: Move hash_file() into tests/utils.py

45ba743

tests: Move ransomware_target() fixture to ransomware/conftest.py

d99811f

tests: Extract ransomware target files to ransomware_target_files.py

707b406

agent: Add unit test for RansomwareBitflipEncryptor

f1a365d

agent: Return results from RansomwareBitflipEncryptor.encrypt_files()

ae0dfec

agent: Use larger chunk size in RansomwarePayload

9165737

The larger chunk size improves efficiency by reducing the number of reads.

tests: Add missing test_lib.dll

2ea5dc6

The .gitignore file prevents dlls from being added to git. Since this isn't a real dll, but is only used for testing, we can add it anyway.

mssalvatore added 3 commits June 23, 2021 10:50

agent: Narrow the responsibilities of RansomwareBitflipEncryptor

97cf198

agent: Add file_selectors.select_production_safe_target_files()

1929ea7

agent: Rename RansomwareBitflipEncryptor -> BitflipEncryptor

70480c7

mssalvatore marked this pull request as ready for review June 23, 2021 23:16

shreyamalviya reviewed Jun 24, 2021

View reviewed changes

monkey/tests/unit_tests/infection_monkey/ransomware/test_bitflip_encryptor.py Show resolved Hide resolved

monkey/tests/unit_tests/infection_monkey/ransomware/test_ransomware_payload.py Outdated Show resolved Hide resolved

mssalvatore added 2 commits June 24, 2021 07:00

agent: Rename test_file_with_included_extension_encrypted

f1e5923

agent: Remove second file from test_file_encrypted_in_place()

ef209a6

mssalvatore merged commit 6744ee7 into develop Jun 24, 2021

mssalvatore deleted the ransomware-bitflip-encryption branch July 29, 2021 14:14

		LOG.info(f"Windows dir configured for encryption is " f"{config['windows_dir']}")
		LOG.info(f"Linux dir configured for encryption is " f"{config['linux_dir']}")

		self._valid_file_extensions_for_encryption = VALID_FILE_EXTENSIONS_FOR_ENCRYPTION.copy()
		self._valid_file_extensions_for_encryption.discard(EXTENSION)


		from infection_monkey.ransomware.ransomware_payload import RansomewarePayload

		EXTENSION = ".m0nk3y"

Ransomware bitflip encryption #1264

Ransomware bitflip encryption #1264

Uh oh!

Conversation

VakarisZ commented Jun 23, 2021 • 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

Explain Changes

Uh oh!

VakarisZ Jun 23, 2021 • 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!

VakarisZ Jun 23, 2021 • 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!

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!

shreyamalviya left a comment

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!

Uh oh!

codecov bot commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shreyamalviya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VakarisZ commented Jun 23, 2021 •

edited

Loading

VakarisZ Jun 23, 2021 •

edited

Loading

VakarisZ Jun 23, 2021 •

edited

Loading

codecov bot commented Jun 23, 2021 •

edited

Loading