From eff1827d9d8a087ca58230537e1f7c1f523e0520 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 1 Mar 2017 15:42:49 -0500 Subject: [PATCH] Batch QSS notifications after initial scan --- endpoints/notificationevent.py | 14 +++++++--- test/test_secscan.py | 17 ++++++++++++ util/secscan/analyzer.py | 50 ++++++++++++++++++++++------------ 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/endpoints/notificationevent.py b/endpoints/notificationevent.py index a662522b1..0e8e805dd 100644 --- a/endpoints/notificationevent.py +++ b/endpoints/notificationevent.py @@ -113,13 +113,14 @@ def _build_summary(event_data): class VulnerabilityFoundEvent(NotificationEvent): CONFIG_LEVEL = 'level' VULNERABILITY_KEY = 'vulnerability' + MULTIPLE_VULNERABILITY_KEY = 'vulnerabilities' @classmethod def event_name(cls): return 'vulnerability_found' def get_level(self, event_data, notification_data): - priority = event_data['vulnerability']['priority'] + priority = event_data[VulnerabilityFoundEvent.CONFIG_LEVEL]['priority'] if priority == 'Defcon1' or priority == 'Critical': return 'error' @@ -166,9 +167,14 @@ class VulnerabilityFoundEvent(NotificationEvent): return actual_level_index <= filter_level_index def get_summary(self, event_data, notification_data): - msg = '%s vulnerability detected in repository %s in %s tags' - return msg % (event_data['vulnerability']['priority'], event_data['repository'], - len(event_data['tags'])) + multiple_vulns = event_data.get(VulnerabilityFoundEvent.MULTIPLE_VULNERABILITY_KEY) + if multiple_vulns is not None: + msg = '%s vulnerabilities were detected in repository %s in %s tags' + return msg % (len(multiple_vulns), event_data['repository'], len(event_data['tags'])) + else: + msg = '%s vulnerability detected in repository %s in %s tags' + return msg % (event_data['vulnerability']['priority'], event_data['repository'], + len(event_data['tags'])) class BaseBuildEvent(NotificationEvent): diff --git a/test/test_secscan.py b/test/test_secscan.py index b1df0aa70..aec0e55fd 100644 --- a/test/test_secscan.py +++ b/test/test_secscan.py @@ -317,6 +317,15 @@ class TestSecurityScanner(unittest.TestCase): "Link": "https://security-tracker.debian.org/tracker/CVE-2014-9471", "Severity": "Low", "FixedBy": "9.23-5" + }, + + { + "Name": "CVE-2016-7530", + "Namespace": "debian:8", + "Description": "Some other service", + "Link": "https://security-tracker.debian.org/tracker/CVE-2016-7530", + "Severity": "Unknown", + "FixedBy": "19.343-2" } ]) @@ -338,6 +347,14 @@ class TestSecurityScanner(unittest.TestCase): self.assertEquals('CVE-2014-9471', body['event_data']['vulnerability']['id']) self.assertEquals('Low', body['event_data']['vulnerability']['priority']) self.assertTrue(body['event_data']['vulnerability']['has_fix']) + + self.assertEquals('CVE-2014-9471', body['event_data']['vulnerabilities'][0]['id']) + self.assertEquals(2, len(body['event_data']['vulnerabilities'])) + + # Ensure we get the correct event message out as well. + event = VulnerabilityFoundEvent() + self.assertEquals('2 vulnerabilities were detected in repository devtable/simple in 2 tags', + event.get_summary(body['event_data'], {})) else: self.assertIsNone(queue_item) diff --git a/util/secscan/analyzer.py b/util/secscan/analyzer.py index 674ae212d..141144ed6 100644 --- a/util/secscan/analyzer.py +++ b/util/secscan/analyzer.py @@ -9,6 +9,7 @@ from endpoints.notificationhelper import spawn_notification from data.database import ExternalNotificationEvent, IMAGE_NOT_SCANNED_ENGINE_VERSION, Image from data.model.tag import filter_tags_have_repository_event, get_tags_for_image from data.model.image import set_secscan_status, get_image_with_storage_and_parent_base +from util.secscan import PRIORITY_LEVELS from util.secscan.api import (APIRequestFailure, AnalyzeLayerException, MissingParentLayerException, InvalidLayerException, AnalyzeLayerRetryException) from util.morecollections import AttrDict @@ -150,30 +151,45 @@ class LayerAnalyzer(object): found_features = layer_data['Layer'].get('Features', []) for repository_id in repository_map: tags = repository_map[repository_id] + vulnerabilities = dict() + # Collect all the vulnerabilities found for the layer under each repository and send + # as a batch notification. for feature in found_features: if 'Vulnerabilities' not in feature: continue for vulnerability in feature.get('Vulnerabilities', []): - event_data = { - 'tags': [tag.name for tag in tags], - 'vulnerability': { - 'id': vulnerability['Name'], - 'description': vulnerability.get('Description', None), - 'link': vulnerability.get('Link', None), - 'has_fix': 'FixedBy' in vulnerability, + vuln_data = { + 'id': vulnerability['Name'], + 'description': vulnerability.get('Description', None), + 'link': vulnerability.get('Link', None), + 'has_fix': 'FixedBy' in vulnerability, - # TODO: Change this key name if/when we change the event format. - 'priority': vulnerability.get('Severity', 'Unknown'), - }, + # TODO: Change this key name if/when we change the event format. + 'priority': vulnerability.get('Severity', 'Unknown'), } - # TODO(jzelinskie): remove when more endpoints have been converted to using - # interfaces - repository = AttrDict({ - 'namespace_name': tags[0].repository.namespace_user.username, - 'name': tags[0].repository.name, - }) + vulnerabilities[vulnerability['Name']] = vuln_data - spawn_notification(repository, 'vulnerability_found', event_data) + # TODO(jzelinskie): remove when more endpoints have been converted to using + # interfaces + repository = AttrDict({ + 'namespace_name': tags[0].repository.namespace_user.username, + 'name': tags[0].repository.name, + }) + + repo_vulnerabilities = list(vulnerabilities.values()) + if not repo_vulnerabilities: + continue + + priority_key = lambda v: PRIORITY_LEVELS.get(v['priority'], {}).get('index', 100) + repo_vulnerabilities.sort(key=priority_key) + + event_data = { + 'tags': [tag.name for tag in tags], + 'vulnerabilities': repo_vulnerabilities, + 'vulnerability': repo_vulnerabilities[0], # For back-compat with existing events. + } + + spawn_notification(repository, 'vulnerability_found', event_data)