REL-4502 - Python SDK v16.6.5 - Handling broken encryption in records…

… and attachments (#621)

* KSM-529 Handle broken records, files and records. (#620)

Prior, if a record, file or folder had bad encryption the entire
application would break.

This change will cause bad items to be skipped. Their UID will be
logged as an error along with the exception information.

The mock classes were changed to handle creating bad items. And
unit tests created to handle bad encryption on all the items.

---------

Co-authored-by: John Walstra <[email protected]>

sdk/python/core/README.md

@@ -4,6 +4,9 @@ For more information see our official documentation page https://docs.keeper.io/
 
 # Change Log
 
+## 16.6.5
+* KSM-529 - Hande broken encryption in records and files
+
 ## 16.6.4
 * KSM-488 - Remove unused package dependencies
 

sdk/python/core/keeper_secrets_manager_core/core.py

@@ -755,21 +755,35 @@ def fetch_and_decrypt_secrets(self, query_options: QueryOptions):
         self.logger.debug("Individual record count: {}".format(len(records_resp or [])))
         self.logger.debug("Folder count: {}".format(len(folders_resp or [])))
 
+        sm_response = SecretsManagerResponse()
+
         if records_resp:
             for r in records_resp:
-                record = Record(r, secret_key)
-                records.append(record)
+                try:
+                    record = Record(r, secret_key)
+                    records.append(record)
+                except Exception as err:
+                    msg = f"{err.__class__.__name__}, {str(err)}"
+                    sm_response.bad_records.append({
+                        "r": r,
+                        "err": msg
+                    })
 
         if folders_resp:
             for f in folders_resp:
-                folder = Folder(f, secret_key)
-                records.extend(folder.records)
-                shared_folders.append(folder)
+                try:
+                    folder = Folder(f, secret_key)
+                    records.extend(folder.records)
+                    shared_folders.append(folder)
+                except Exception as err:
+                    msg = f"{err.__class__.__name__}, {str(err)}"
+                    sm_response.bad_folders.append({
+                        "f": f,
+                        "err": msg
+                    })
 
         self.logger.debug("Total record count: {}".format(len(records)))
 
-        sm_response = SecretsManagerResponse()
-
         if 'appData' in decrypted_response_dict:
             app_data_json = CryptoUtils.decrypt_aes(
                 url_safe_str_to_bytes(decrypted_response_dict['appData']),
@@ -823,6 +837,17 @@ def get_secrets_with_options(self, query_options=None, full_response=False):
             for warning in records_resp.warnings:
                 self.logger.warning(warning)
 
+        if records_resp.had_bad_records:
+            for error in records_resp.bad_records:
+                uid = error.get('r').get("recordUid")
+                err = error.get('err')
+                self.logger.error(f"Record {uid} skipped due to error: {err}")
+
+        if records_resp.had_bad_folders:
+            for error in records_resp.bad_folders:
+                uid = error.get('f').get("folderUid")
+                err = error.get('err')
+                self.logger.error(f"Folder {uid} skipped due to error: {err}")
         if full_response:
             return records_resp
         else:

sdk/python/core/keeper_secrets_manager_core/dto/dtos.py

@@ -66,9 +66,12 @@ def __init__(self, record_dict, secret_key, folder_uid = ''):
         if record_dict.get('files'):
             for f in record_dict.get('files'):
 
-                file = KeeperFile(f, self.record_key_bytes)
-
-                self.files.append(file)
+                try:
+                    file = KeeperFile(f, self.record_key_bytes)
+                    self.files.append(file)
+                except Exception as err:
+                    msg = f"{err.__class__.__name__}, {str(err)}"
+                    raise Exception(f"attached file caused exception: {msg}")
 
         # password (if `login` type)
         if self.type == 'login':
@@ -573,13 +576,23 @@ def __init__(self):
         self.expiresOn = None
         self.warnings = None
         self.justBound = False
+        self.bad_records = []
+        self.bad_folders = []
 
     def expires_on_str(self, date_format='%Y-%m-%d %H:%M:%S'):
         """
         Retrieve string formatted expiration date
         """
         return datetime.fromtimestamp(self.expiresOn/1000).strftime(date_format)
 
+    @property
+    def had_bad_records(self):
+        return len(self.bad_records) > 0
+
+    @property
+    def had_bad_folders(self):
+        return len(self.bad_folders) > 0
+
 
 class SecretsManagerAddFileResponse:
 

sdk/python/core/keeper_secrets_manager_core/mock.py

@@ -18,10 +18,10 @@
 from .keeper_globals import keeper_public_keys
 import string
 import random
-
 from keeper_secrets_manager_core.crypto import CryptoUtils
 from keeper_secrets_manager_core.configkeys import ConfigKeys
 from keeper_secrets_manager_core.dto.payload import KSMHttpResponse
+from cryptography.hazmat.primitives.ciphers.aead import AESGCM
 
 
 class ResponseQueue:
@@ -182,12 +182,12 @@ def instance(self, transmission_key):
 
         return KSMHttpResponse(res.status_code, res.content, res)
 
-    def add_record(self, title=None, record_type=None, uid=None, record=None, keeper_record=None):
+    def add_record(self, title=None, record_type=None, uid=None, record=None, keeper_record=None, **kwargs):
 
         if keeper_record is not None:
             record = Record.convert_keeper_record(keeper_record)
         elif record is None:
-            record = Record(title=title, record_type=record_type, uid=uid)
+            record = Record(title=title, record_type=record_type, uid=uid, **kwargs)
 
         if isinstance(object, Record.__class__) is False:
             raise ValueError("Record being added to the response is not a "
@@ -196,10 +196,10 @@ def add_record(self, title=None, record_type=None, uid=None, record=None, keeper
         self.records[record.uid] = record
         return record
 
-    def add_folder(self, uid=None, folder=None):
+    def add_folder(self, uid=None, folder=None, **kwargs):
 
         if folder is None:
-            folder = Folder(uid=uid)
+            folder = Folder(uid=uid, **kwargs)
 
         if isinstance(object, Folder.__class__) is False:
             raise ValueError("Folder being added to the response is not a "
@@ -211,16 +211,18 @@ def add_folder(self, uid=None, folder=None):
 
 class Folder:
 
-    def __init__(self, uid=None):
+    def __init__(self, uid=None, **kwargs):
         if uid is None:
             uid = uuid.uuid4().hex[:22]
         self.uid = uid
         self.records = {}
 
-    def add_record(self, title=None, record_type=None, uid=None, record=None):
+        self.has_bad_encryption = kwargs.get("has_bad_encryption")
+
+    def add_record(self, title=None, record_type=None, uid=None, record=None, is_bad_record=False, **kwargs):
 
         if record is None:
-            record = Record(record_type=record_type, uid=uid, title=title)
+            record = Record(record_type=record_type, uid=uid, title=title, is_bad_record=is_bad_record, **kwargs)
 
         if isinstance(object, Record.__class__) is False:
             raise ValueError("Record being added to the response is not a "
@@ -231,16 +233,20 @@ def add_record(self, title=None, record_type=None, uid=None, record=None):
 
     def dump(self, secret, flags=None):
 
+        folder_key = secret
+        if self.has_bad_encryption is True:
+            secret = AESGCM.generate_key(128)
+
         return {
             "folderUid": self.uid,
-            "folderKey": base64.b64encode(CryptoUtils.encrypt_aes(secret, secret)).decode(),
+            "folderKey": base64.b64encode(CryptoUtils.encrypt_aes(folder_key, secret)).decode(),
             "records": [self.records[uid].dump(secret=secret, flags=flags) for uid in self.records]
         }
 
 
 class File:
 
-    def __init__(self, name, title=None, content_type=None, url=None, content=None, last_modified=None):
+    def __init__(self, name, title=None, content_type=None, url=None, content=None, last_modified=None, **kwargs):
         self.uid = uuid.uuid4().hex[:22]
         self.secret_used = None
 
@@ -262,6 +268,8 @@ def __init__(self, name, title=None, content_type=None, url=None, content=None,
             last_modified = int(time.time())
         self.last_modified = last_modified
 
+        self.has_bad_encryption = kwargs.get("has_bad_encryption")
+
     def downloadable_content(self):
 
         # The dump method will generate the content that the secrets manager would return. The
@@ -279,6 +287,10 @@ def downloadable_content(self):
 
     def dump(self, secret, flags=None):
 
+        file_key = secret
+        if self.has_bad_encryption is True:
+            secret = AESGCM.generate_key(128)
+
         # No special flags for download. Do this to make PEP8 happy for unused vars.
         if flags is not None:
             pass
@@ -294,7 +306,7 @@ def dump(self, secret, flags=None):
         data = json.dumps(d)
         file_data = {
             "fileUid": self.uid,
-            "fileKey": base64.b64encode(CryptoUtils.encrypt_aes(secret, secret)).decode(),
+            "fileKey": base64.b64encode(CryptoUtils.encrypt_aes(file_key, secret)).decode(),
             "data": base64.b64encode(CryptoUtils.encrypt_aes(data.encode(), secret)).decode(),
             "url": self.url,
             "thumbnailUrl": None
@@ -306,7 +318,7 @@ class Record:
 
     no_label = "__NONE__"
 
-    def __init__(self, record_type=None, uid=None, title=None):
+    def __init__(self, record_type=None, uid=None, title=None, **kwargs):
 
         if uid is None:
             uid = uuid.uuid4().hex[:22]
@@ -323,6 +335,8 @@ def __init__(self, record_type=None, uid=None, title=None):
         self._fields = []
         self._custom_fields = []
 
+        self.has_bad_encryption = kwargs.get("has_bad_encryption")
+
     @staticmethod
     def convert_keeper_record(keeper_record):
 
@@ -375,21 +389,26 @@ def custom_field(self, label, value, field_type="text", required=None, privacy_s
             self._field(field_type, value, label, required, privacy_screen)
         )
 
-    def add_file(self, name, title=None, content_type=None, url=None, content=None, last_modified=None):
+    def add_file(self, name, title=None, content_type=None, url=None, content=None, last_modified=None, **kwargs):
 
         file = File(
             name=name,
             title=title,
             content_type=content_type,
             url=url,
             content=content,
-            last_modified=last_modified
+            last_modified=last_modified,
+            **kwargs
         )
         self.files[file.uid] = file
         return file
 
     def dump(self, secret, flags=None):
 
+        record_key = secret
+        if self.has_bad_encryption is True:
+            secret = AESGCM.generate_key(128)
+
         fields = list(self._fields) if isinstance(self._fields, list) else self._fields
 
         # If no files, the JSON has null
@@ -420,7 +439,7 @@ def dump(self, secret, flags=None):
 
         data = {
             "recordUid": self.uid,
-            "recordKey": base64.b64encode(CryptoUtils.encrypt_aes(secret, secret)).decode(),
+            "recordKey": base64.b64encode(CryptoUtils.encrypt_aes(record_key, secret)).decode(),
             "data": base64.b64encode(CryptoUtils.encrypt_aes(json.dumps(record_data).encode(), secret)).decode(),
             "isEditable": self.is_editable,
             "files": files

sdk/python/core/setup.py

@@ -18,7 +18,7 @@
 
 setup(
     name="keeper-secrets-manager-core",
-    version="16.6.4",
+    version="16.6.5",
     description="Keeper Secrets Manager for Python 3",
     long_description=long_description,
     long_description_content_type="text/markdown",