Island: Use SHA256 to hash OTPs

The encryption algorithm is not deterministic, making searching
impossible. Salted SHA256 is considered to be secure enough for one-time
passwords with a 2-minute TTL.

Issue #3078
PR #3204

Author: mssalvatore

monkey/monkey_island/cc/services/authentication_service/mongo_otp_repository.py

@@ -1,3 +1,5 @@
+import hashlib
+import secrets
 from functools import lru_cache
 from typing import Any, Mapping
 
@@ -12,7 +14,6 @@
     StorageError,
     UnknownRecordError,
 )
-from monkey_island.cc.server_utils.encryption import ILockableEncryptor
 
 from .i_otp_repository import IOTPRepository
 
@@ -21,21 +22,36 @@ class MongoOTPRepository(IOTPRepository):
     def __init__(
         self,
         mongo_client: MongoClient,
-        encryptor: ILockableEncryptor,
     ):
-        self._encryptor = encryptor
+        # SECURITY: A new salt is generated for each instance of this repository. This effectively
+        # makes all preexisting OTPS invalid on Island startup.
+        self._salt = secrets.token_bytes(16)
+
         self._otp_collection = mongo_client.monkey_island.otp
         self._otp_collection.create_index("otp", unique=True)
 
     def insert_otp(self, otp: OTP, expiration: float):
         try:
-            encrypted_otp = self._encryptor.encrypt(otp.get_secret_value().encode())
             self._otp_collection.insert_one(
-                {"otp": encrypted_otp, "expiration_time": expiration, "used": False}
+                {"otp": self._hash_otp(otp), "expiration_time": expiration, "used": False}
             )
         except Exception as err:
             raise StorageError(f"Error inserting OTP: {err}")
 
+    def _hash_otp(self, otp: OTP) -> bytes:
+        # SECURITY: A single round of salted SHA256 is usually not considered sufficient for
+        # protecting passwords. However, OTPs have a very short life span (2 minutes at the time of
+        # this writing). Additionally, they can only be used once. Finally, they are 32 bytes long.
+        # At the present time, we consider this to be sufficient protection. I'm unaware of any
+        # technology in existence that can brute force SHA256 for (roughly) 48-byte inputs in under
+        # 2 minutes.
+        #
+        # Note that if any of these conditions change (timeouts become very long or OTPs become very
+        # short), this should be revisited. For now, we prefer the significantly faster performance
+        # of a single round of salted SHA256 over a more secure but slower algorithm.
+        otp_bytes = otp.get_secret_value().encode()
+        return hashlib.sha256(self._salt + otp_bytes).digest()
+
     def set_used(self, otp: OTP):
         try:
             otp_id = self._get_otp_object_id(otp)
@@ -71,11 +87,10 @@ def _get_otp_document(self, otp: OTP) -> Mapping[str, Any]:
 
     @lru_cache
     def _get_otp_object_id(self, otp: OTP) -> ObjectId:
-        otp_str = otp.get_secret_value()
-
         try:
-            encrypted_otp = self._encryptor.encrypt(otp_str.encode())
-            otp_dict = self._otp_collection.find_one({"otp": encrypted_otp}, [MONGO_OBJECT_ID_KEY])
+            otp_dict = self._otp_collection.find_one(
+                {"otp": self._hash_otp(otp)}, [MONGO_OBJECT_ID_KEY]
+            )
         except Exception as err:
             raise RetrievalError(f"Error retrieving OTP: {err}")
 

monkey/tests/unit_tests/monkey_island/cc/services/authentication_service/test_mongo_otp_repository.py

@@ -11,7 +11,6 @@
     StorageError,
     UnknownRecordError,
 )
-from monkey_island.cc.server_utils.encryption import ILockableEncryptor
 from monkey_island.cc.services.authentication_service.i_otp_repository import IOTPRepository
 from monkey_island.cc.services.authentication_service.mongo_otp_repository import MongoOTPRepository
 
@@ -35,10 +34,8 @@ def mongo_client() -> mongomock.MongoClient:
 
 
 @pytest.fixture
-def otp_repository(
-    mongo_client: mongomock.MongoClient, repository_encryptor: ILockableEncryptor
-) -> IOTPRepository:
-    return MongoOTPRepository(mongo_client, repository_encryptor)
+def otp_repository(mongo_client: mongomock.MongoClient) -> IOTPRepository:
+    return MongoOTPRepository(mongo_client)
 
 
 @pytest.fixture
@@ -59,7 +56,7 @@ def error_raising_mongo_client() -> mongomock.MongoClient:
 def error_raising_otp_repository(
     error_raising_mongo_client, repository_encryptor
 ) -> IOTPRepository:
-    return MongoOTPRepository(error_raising_mongo_client, repository_encryptor)
+    return MongoOTPRepository(error_raising_mongo_client)
 
 
 def test_insert_otp(otp_repository: IOTPRepository):