security data update fixes + compliance reporting minor improvements …

…(removed references to "SOX compliance")

keepercommander/__init__.py

@@ -10,4 +10,4 @@
 # Contact: ops@keepersecurity.com
 #
 
-__version__ = '16.10.13'
+__version__ = '16.10.14'

keepercommander/breachwatch.py

@@ -19,6 +19,7 @@
 from .proto import breachwatch_pb2, client_pb2, APIRequest_pb2
 from .error import KeeperApiError
 from .params import KeeperParams
+from .vault import KeeperRecord
 
 
 class BreachWatch(object):
@@ -106,44 +107,49 @@ def scan_passwords(self, params, passwords):
         for password in results:
             yield password, results[password]
 
-    def scan_and_store_record_status(self, params, record_uid, force_update=False):
-        # type: (BreachWatch, KeeperParams, str, Optional[bool]) -> None
-        record = vault.KeeperRecord.load(params, record_uid)
-        if not record:
-            return
-        if not isinstance(record, (vault.PasswordRecord, vault.TypedRecord)):
-            return
-
-        record_password = BreachWatch.extract_password(record)
-        if record_password:
-            bw_record = params.breach_watch_records.get(record_uid) if params.breach_watch_records else None
-            euid = None
+    def scan_and_store_record_status(self, params, record, force_update=False):
+        # type: (KeeperParams, KeeperRecord, Optional[bool]) -> None
+        def get_euid():
+            result = None
             if bw_record:
-                data_obj = bw_record.get('data_unencrypted')
-                if data_obj and 'passwords' in data_obj:
-                    password = next((x for x in data_obj['passwords'] if x.get('value', '') == record_password), None)
-                    if password and not force_update:
-                        return
-                    euid = next((base64.b64decode(x['euid']) for x in data_obj['passwords'] if 'euid' in x), None)
-
-            hash_status = self.scan_password(params, record_password, euid)
-            if hash_status.breachDetected:
-                logging.info('High-Risk password detected')
-                if self.send_audit_events:
-                    params.queue_audit_event('bw_record_high_risk')
-
+                bw_pw_objs = bw_record.get('data_unencrypted', {}).get('passwords', [])
+                euids = [x.get('euid') for x in bw_pw_objs if x.get('euid')]
+                if euids:
+                    result = base64.b64decode(next(iter(euids)))
+            return result
+
+        def get_last_pw():
+            result = ''
+            if bw_record:
+                bw_pw_objs = bw_record.get('data_unencrypted', {}).get('passwords', [])
+                passwords = [x.get('value') for x in bw_pw_objs if x.get('value')]
+                if passwords:
+                    result = next(iter(passwords))
+            return result
+
+        def update_bw_data():
+            result = None
             bwrq = breachwatch_pb2.BreachWatchRecordRequest()
             bwrq.recordUid = utils.base64_url_decode(record_uid)
             bwrq.breachWatchInfoType = breachwatch_pb2.RECORD
             bwrq.updateUserWhoScanned = True
-            bw_password = client_pb2.BWPassword()
-            bw_password.value = record_password
-            bw_password.status = client_pb2.WEAK if hash_status.breachDetected else client_pb2.GOOD
-            bw_password.euid = hash_status.euid
             bw_data = client_pb2.BreachWatchData()
-            bw_data.passwords.append(bw_password)
-            data = bw_data.SerializeToString()
+
+            if record_password:
+                hash_status = self.scan_password(params, record_password, get_euid())
+
+                if hash_status.breachDetected:
+                    logging.info('High-Risk password detected')
+                    if self.send_audit_events:
+                        params.queue_audit_event('bw_record_high_risk')
+                bw_password = client_pb2.BWPassword()
+                bw_password.value = record_password
+                bw_password.status = client_pb2.WEAK if hash_status.breachDetected else client_pb2.GOOD
+                bw_password.euid = hash_status.euid
+                bw_data.passwords.append(bw_password)
+                result = bw_password.status
             try:
+                data = bw_data.SerializeToString()
                 record_key = params.record_cache[record_uid]['record_key_unencrypted']
                 bwrq.encryptedData = crypto.encrypt_aes_v2(data, record_key)
                 rq = breachwatch_pb2.BreachWatchUpdateRequest()
@@ -155,59 +161,86 @@ def scan_and_store_record_status(self, params, record_uid, force_update=False):
                     raise Exception(status.reason)
             except Exception as e:
                 logging.warning('BreachWatch: %s', str(e))
-            api.sync_down(params)
+            return result
+
+        def skip_update():
+            if record_password == get_last_pw():
+                return True
+            return False
+
+        record_uid = record.record_uid
+        bw_record = params.breach_watch_records.get(record_uid) if params.breach_watch_records else None
 
-            bw_res = bw_data.passwords[0].status if bw_data else None
-            BreachWatch.update_security_data(params, record_uid, record, bw_res)
+        record_password = BreachWatch.extract_password(record) or ''
+        if skip_update():
+            return None
+
+        bw_res = update_bw_data()
+        if not record_password:
+            euid = get_euid()
+            if euid:
+                params.breach_watch.delete_euids(params, [euid])
+        api.sync_down(params)
+        return bw_res
 
     @staticmethod
-    def update_security_data(params, rec_uid, record=None, bw_result=None, force_update=False):
-        # type: (KeeperParams, str, Optional[vault.KeeperRecord], Optional[int], Optional[bool]) -> None
+    def update_security_data(params, record, bw_result=None, force_update=False):
+        # type: (KeeperParams, KeeperRecord, Optional[int], Optional[bool]) -> None
         def calculate_security_data():  # type: () -> APIRequest_pb2.SecurityData
+            def prepare_security_data():
+                strength = utils.password_score(record_pw)
+                result = {'strength': strength}
+                if bw_result is not None:
+                    result['bw_result'] = bw_result
+                elif bw_enabled:
+                    logging.error(f'No BreachWatch status for record {record.record_uid}')
+
+                url = BreachWatch.extract_url(record)
+                parse_results = urlparse(url)
+                domain = parse_results.hostname or parse_results.path
+                if domain:
+                    # truncate domain string if needed to avoid reaching RSA encryption data size limitation
+                    result['domain'] = domain[:200]
+                return result
+
             sec_data = APIRequest_pb2.SecurityData()
-            passwd = BreachWatch.extract_password(password_record)
-            strength = utils.password_score(passwd)
-            rec_sd = {'strength': strength}
-            if isinstance(bw_result, int):
-                rec_sd['bw_result'] = bw_result
-            else:
-                logging.info('No BreachWatch data to update or incorrect status (must be int)')
-            url = BreachWatch.extract_url(password_record)
-            parse_results = urlparse(url)
-            domain = parse_results.hostname or parse_results.path
-            if domain:
-                # truncate domain string if needed to avoid reaching RSA encryption data size limitation
-                rec_sd['domain'] = domain[:200]
-            sec_data.uid = utils.base64_url_decode(rec_uid)
-            sec_data.data = crypto.encrypt_rsa(json.dumps(rec_sd).encode('utf-8'), params.enterprise_rsa_key)
+            sec_data.uid = utils.base64_url_decode(record.record_uid)
+            if record_pw:
+                rec_sd = prepare_security_data()
+                sec_data.data = crypto.encrypt_rsa(json.dumps(rec_sd).encode('utf-8'), params.enterprise_rsa_key)
+
             return sec_data
 
-        # Action not allowed for non-enterprise users
-        if not params.enterprise_ec_key:
-            return
+        def skip_update():
+            # Allow for enterprise users only
+            if not params.enterprise_ec_key:
+                return True
+
+            if force_update:
+                return False
+
+            security_data = params.breach_watch_security_data.get(record_uid, {}) if params.breach_watch_security_data \
+                else {}
+            bw_data = params.breach_watch_records.get(record_uid, {}) if params.breach_watch_records \
+                else {}
+
+            # Ignore records with no password and no security data
+            if not record_pw and not security_data:
+                return True
+
+            # Check if security data is already up-to-date
+            sd_revision = security_data.get('revision', 0)
+            return (sd_revision >= bw_data.get('revision', 0)) if bw_enabled else (sd_revision >= record.revision)
 
-        record = record or vault.KeeperRecord.load(params, rec_uid)
         if not record:
-            # Record not found in vault, abort update
             return
 
-        # Check if record should have security data (must contain a password and be owned by current user)
-        password_record_obj = next(BreachWatch.get_records(params, lambda r, d: r.record_uid == rec_uid, owned=True),
-                                   None)
-        password_record = password_record_obj[0] if password_record_obj else None
-        if not password_record:
+        record_uid = record.record_uid
+        record_pw = BreachWatch.extract_password(record)
+        bw_enabled = bool(params.breach_watch)
+        if skip_update():
             return
 
-        # Check if update is needed (unless forcing an update, existing sec. data must be older than last record change)
-        old_security_data = params.breach_watch_security_data.get(rec_uid)
-        if not force_update and old_security_data:
-            record_obj = params.breach_watch_records.get(rec_uid) or params.record_cache.get(rec_uid) or {}
-            if old_security_data.get('revision', 0) >= record_obj.get('revision', 0):
-                return
-
-        if bw_result is None:
-            status = BreachWatch.get_record_status(params, rec_uid)
-            bw_result = client_pb2.BWStatus.Value(status) if status else client_pb2.BWStatus.GOOD
         update_rq = APIRequest_pb2.SecurityDataRequest()
         rec_sec_data = calculate_security_data()
         update_rq.recordSecurityData.append(rec_sec_data)
@@ -305,7 +338,7 @@ def get_record_status(params, record_uid):  # type: (KeeperParams, str) -> Optio
             return
         bw_record = params.breach_watch_records.get(record_uid)
         if bw_record:
-            data_obj = bw_record['data_unencrypted']
+            data_obj = bw_record.get('data_unencrypted')
             if data_obj and 'passwords' in data_obj:
                 record = vault.KeeperRecord.load(params, record_uid)
                 if record:
@@ -339,7 +372,7 @@ def get_records(params,            # type: KeeperParams
                 password_dict = None
                 if params.breach_watch_records:
                     bwr = params.breach_watch_records.get(record_uid)
-                    data_obj = bwr['data_unencrypted'] if bwr else None
+                    data_obj = bwr.get('data_unencrypted') if bwr else None
                     if data_obj and 'passwords' in data_obj:
                         password_dict = next((x for x in data_obj['passwords'] if x.get('value', '') == password), None)
                 if callback(record, password_dict):
@@ -374,10 +407,14 @@ def get_records_by_status(params, status, owned=False):
     def scan_and_update_security_data(params, record_uid, bw_obj=None, force_update=False, set_reused_pws=True):
         # type: (KeeperParams, Union[str, List[str]], Optional[BreachWatch], Optional[bool], Optional[bool]) -> None
         api.sync_down(params)
-        if bw_obj:
-            bw_obj.scan_and_store_record_status(params, record_uid, force_update=force_update)
-        else:
-            BreachWatch.update_security_data(params, record_uid)
+        record = vault.KeeperRecord.load(params, record_uid)
+        if not record:
+            return
+        if not isinstance(record, (vault.PasswordRecord, vault.TypedRecord)):
+            return
+
+        bw_res = bw_obj.scan_and_store_record_status(params, record, force_update) if bw_obj else None
+        BreachWatch.update_security_data(params, record, force_update=force_update, bw_result=bw_res)
         if set_reused_pws:
             BreachWatch.save_reused_pw_count(params)
-        api.sync_down(params)
+        api.sync_down(params)

keepercommander/commands/compliance.py

@@ -28,7 +28,7 @@
 compliance_parser.add_argument('--output', dest='output', action='store',
                                help='path to resulting output file (ignored for "table" format)')
 
-default_report_parser = argparse.ArgumentParser(prog='compliance report', description='Run a SOX compliance report.',
+default_report_parser = argparse.ArgumentParser(prog='compliance report', description='Run a compliance report.',
                                                 parents=[compliance_parser])
 username_opt_help = 'user(s) whose records are to be included in report (set option once per user)'
 default_report_parser.add_argument('--username', '-u', action='append', help=username_opt_help)
@@ -68,7 +68,7 @@
 aging_help = 'include record-aging data (last modified, created, and last password rotation dates)'
 access_report_parser.add_argument('--aging', action='store_true',  help=aging_help)
 
-summary_report_desc = 'Run a summary SOX compliance report'
+summary_report_desc = 'Run a summary compliance report'
 summary_report_parser = argparse.ArgumentParser(prog='compliance summary-report', description=summary_report_desc,
                                                 parents=[compliance_parser])
 sf_report_desc = 'Run an enterprise-wide shared-folder report'
@@ -84,7 +84,7 @@ def register_commands(commands):
 def register_command_info(aliases, command_info):
     aliases['cr'] = ('compliance', 'report')
     aliases['compliance-report'] = ('compliance', 'report')
-    command_info['compliance'] = 'SOX Compliance Reporting'
+    command_info['compliance'] = 'Compliance Reporting'
 
 
 def get_email(sdata, user_uid):    # type: (SoxData, int) -> str
@@ -98,7 +98,7 @@ def get_team_usernames(sdata, team):  # type: (SoxData, sox_types.Team) -> List[
 class ComplianceCommand(GroupCommand):
     def __init__(self):
         super(ComplianceCommand, self).__init__()
-        self.register_command('report', ComplianceReportCommand(), 'Run default SOX compliance report')
+        self.register_command('report', ComplianceReportCommand(), 'Run default compliance report')
         self.register_command('team-report', ComplianceTeamReportCommand(), team_report_desc, 'tr')
         self.register_command('record-access-report', ComplianceRecordAccessReportCommand(), access_report_desc, 'rar')
         self.register_command('summary-report', ComplianceSummaryReportCommand(), summary_report_desc, 'stats')

keepercommander/commands/recordv2.py

@@ -250,6 +250,7 @@ def execute(self, params, **kwargs):
         record = api.get_record(params, record_uid)
 
         changed = False
+        password_changed = False
         if kwargs.get('title') is not None:
             title = kwargs['title']
             if title:
@@ -261,12 +262,14 @@ def execute(self, params, **kwargs):
             record.login = kwargs['login']
             changed = True
         if kwargs.get('password') is not None:
+            last_password = record.unmasked_password or record.password
             record.password = kwargs['password']
+            password_changed = record.password != last_password
             changed = True
         else:
             if kwargs.get('generate'):
                 record.password = generator.generate(16)
-                changed = True
+                changed = password_changed = True
         if kwargs.get('url') is not None:
             record.login_url = kwargs['url']
             changed = True
@@ -326,5 +329,6 @@ def execute(self, params, **kwargs):
 
         if changed:
             api.update_record(params, record)
-            BreachWatch.scan_and_update_security_data(params, record_uid, params.breach_watch)
+            if password_changed:
+                BreachWatch.scan_and_update_security_data(params, record_uid, params.breach_watch)
             params.sync_data = True

keepercommander/commands/security_audit.py

@@ -30,7 +30,7 @@ def register_command_info(aliases, command_info):
 node_filter_help = 'name(s) or UID(s) of node(s) to filter results of the report by'
 report_parser.add_argument('-n', '--node', action='append', help=node_filter_help)
 report_parser.add_argument('-b', '--breachwatch', dest='breachwatch', action='store_true',
-                           help='display BreachWatch report')
+                           help='display BreachWatch report. Ignored if BreachWatch is not active.')
 save_help = 'save updated security audit reports'
 report_parser.add_argument('-s', '--save', action='store_true', help=save_help)
 report_parser.add_argument('-su', '--show-updated', action='store_true', help='show updated data')
@@ -107,6 +107,9 @@ def __init__(self):
     def get_enterprise_private_rsa_key(self, params, enterprise_priv_key):
         if not self.enterprise_private_rsa_key:
             tree_key = params.enterprise['unencrypted_tree_key']
+            if not enterprise_priv_key:
+                key = params.enterprise.get('keys', {}).get('rsa_encrypted_private_key', '')
+                enterprise_priv_key = utils.base64_url_decode(key)
             key = crypto.decrypt_aes_v2(enterprise_priv_key, tree_key)
             key = crypto.load_rsa_private_key(key)
             self.enterprise_private_rsa_key = key
@@ -159,6 +162,11 @@ def execute(self, params, **kwargs):
             logging.info(security_audit_report_description)
             return
 
+        if kwargs.get('breachwatch') and not params.breach_watch:
+            msg = ('Ignoring "--breachwatch" option because BreachWatch is not active. '
+                   'Please visit the Web Vault at https://keepersecurity.com/vault')
+            logging.warning(msg)
+
         def get_node_id(name_or_id):
             nodes = params.enterprise.get('nodes') or []
             matches = [n for n in nodes if name_or_id in (str(n.get('node_id')), n.get('data', {}).get('displayname'))]
@@ -261,7 +269,8 @@ def get_node_id(name_or_id):
         if save_report:
             self.save_updated_security_reports(params, updated_security_reports)
 
-        fields = ('email', 'name', 'at_risk', 'passed', 'ignored') if kwargs.get('breachwatch') else \
+        show_breachwatch = kwargs.get('breachwatch') and params.breach_watch
+        fields = ('email', 'name', 'at_risk', 'passed', 'ignored') if show_breachwatch else \
             ('email', 'name', 'weak', 'medium', 'strong', 'reused', 'unique', 'securityScore', 'twoFactorChannel',
              'node')
         field_descriptions = fields
@@ -270,7 +279,7 @@ def get_node_id(name_or_id):
         if fmt == 'table':
             field_descriptions = (field_to_title(x) for x in fields)
 
-        report_title = f'Security Audit Report{" (BreachWatch)" if kwargs.get("breachwatch") else ""}'
+        report_title = f'Security Audit Report{" (BreachWatch)" if show_breachwatch else ""}'
         table = []
         for raw in rows:
             row = []