Merge pull request #1508 from guardicore/encryptor-with-utf8-chars

Change KeyBasedEncryptor's padding

CHANGELOG.md

@@ -44,6 +44,9 @@ Changelog](https://keepachangelog.com/en/1.0.0/).
 - PBA table collapse in security report on data change. #1423
 - Unsigned Windows agent binaries in Linux packages are now signed. #1444
 - 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
+
 
 ### Security
 - Generate a random password when creating a new user for CommunicateAsNewUser

monkey/monkey_island/cc/server_utils/encryption/encryptors/key_based_encryptor.py

@@ -5,6 +5,7 @@ import logging
 # is maintained.
 from Crypto import Random  # noqa: DUO133  # nosec: B413
 from Crypto.Cipher import AES  # noqa: DUO133  # nosec: B413
+from Crypto.Util import Padding  # noqa: DUO133
 
 from monkey_island.cc.server_utils.encryption import IEncryptor
 
@@ -29,19 +30,12 @@ class KeyBasedEncryptor(IEncryptor):
     def encrypt(self, plaintext: str) -> str:
         cipher_iv = Random.new().read(AES.block_size)
         cipher = AES.new(self._key, AES.MODE_CBC, cipher_iv)
-        return base64.b64encode(cipher_iv + cipher.encrypt(self._pad(plaintext).encode())).decode()
+        padded_plaintext = Padding.pad(plaintext.encode(), self._BLOCK_SIZE)
+        return base64.b64encode(cipher_iv + cipher.encrypt(padded_plaintext)).decode()
 
     def decrypt(self, ciphertext: str):
         enc_message = base64.b64decode(ciphertext)
         cipher_iv = enc_message[0 : AES.block_size]
         cipher = AES.new(self._key, AES.MODE_CBC, cipher_iv)
-        return self._unpad(cipher.decrypt(enc_message[AES.block_size :]).decode())
+        padded_plaintext = cipher.decrypt(enc_message[AES.block_size :])
-
+        return Padding.unpad(padded_plaintext, self._BLOCK_SIZE).decode()
-    # TODO: Review and evaluate the security of the padding function
-    def _pad(self, message):
-        return message + (self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)) * chr(
-            self._BLOCK_SIZE - (len(message) % self._BLOCK_SIZE)
-        )
-
-    def _unpad(self, message: str):
-        return message[0 : -ord(message[len(message) - 1])]

monkey/tests/unit_tests/monkey_island/cc/server_utils/encryption/test_key_based_encryptor.py

@@ -0,0 +1,32 @@
+import pytest
+
+from monkey_island.cc.server_utils.encryption import KeyBasedEncryptor
+
+PLAINTEXT = "password"
+PLAINTEXT_MULTIPLE_BLOCK_SIZE = "banana" * KeyBasedEncryptor._BLOCK_SIZE
+PLAINTEXT_UTF8_1 = "slaptažodis"  # "password" in Lithuanian
+PLAINTEXT_UTF8_2 = "弟"  # Japanese
+PLAINTEXT_UTF8_3 = "ж"  # Ukranian
+
+KEY = b"\x84\xd4qA\xb5\xd4Y\x9bH.\x14\xab\xd8\xc7+g\x12\xfa\x80'%\xfd#\xf8c\x94\xb9\x96_\xf4\xc51"
+
+kb_encryptor = KeyBasedEncryptor(KEY)
+
+
+def test_encrypt_decrypt_string_with_key():
+    encrypted = kb_encryptor.encrypt(PLAINTEXT)
+    decrypted = kb_encryptor.decrypt(encrypted)
+    assert decrypted == PLAINTEXT
+
+
+@pytest.mark.parametrize("plaintext", [PLAINTEXT_UTF8_1, PLAINTEXT_UTF8_2, PLAINTEXT_UTF8_3])
+def test_encrypt_decrypt_string_utf8_with_key(plaintext):
+    encrypted = kb_encryptor.encrypt(plaintext)
+    decrypted = kb_encryptor.decrypt(encrypted)
+    assert decrypted == plaintext
+
+
+def test_encrypt_decrypt_string_multiple_block_size_with_key():
+    encrypted = kb_encryptor.encrypt(PLAINTEXT_MULTIPLE_BLOCK_SIZE)
+    decrypted = kb_encryptor.decrypt(encrypted)
+    assert decrypted == PLAINTEXT_MULTIPLE_BLOCK_SIZE