From e5dd8360de489cd7cc71979d47d31b903139fbc0 Mon Sep 17 00:00:00 2001 From: Artem Martynovich Date: Fri, 11 Oct 2019 22:33:29 +0600 Subject: [PATCH 1/4] Display N/A if no data for Vulnerable Packages and Audit fields. Add test for Vulnerable Packages. --- backend/device_registry/models.py | 9 +++- .../templates/device_info_security.html | 36 ++++++++------ backend/device_registry/tests/test_all.py | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 8ec0dd89d..e8961fc14 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -309,8 +309,15 @@ def set_meta_tags(self): if self.deviceinfo.get_hardware_type() == 'Raspberry Pi' and raspberry_pi_tag not in self.tags: self.tags.add(raspberry_pi_tag) + @property def vulnerable_packages(self): - return self.deb_packages.filter(vulnerabilities__isnull=False).distinct().order_by('name') + if self.deb_packages.exists(): + return self.deb_packages.filter(vulnerabilities__isnull=False).distinct().order_by('name') + + def has_vulnerable_packages(self): + packages = self.vulnerable_packages + if packages is not None: + return packages.count() class Meta: ordering = ('created',) diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index 0663d8003..7fce7231c 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -23,22 +23,22 @@

Security

Vulnerable Packages - {% if object.liable_for_vulnerable_packages_check %} - {% with object.vulnerable_packages as vulnerable_packages %} - {% if vulnerable_packages %} - - {% endif %} - {% endwith %} - {% else %} + {% with object.has_vulnerable_packages as has_vulnerable_packages %} + {% if has_vulnerable_packages is None %} N/A + {% elif has_vulnerable_packages %} + + {% else %} + {% include "badge.html" with icon="check" color="success" %} {% endif %} + {% endwith %} @@ -81,6 +81,8 @@

Security

{% endfor %} + {% else %} + N/A {% endif %} @@ -88,7 +90,9 @@

Security

Configuration Audit {% with object.sshd_issues as sshd_issues %} - {% if sshd_issues %} + {% if sshd_issues is None %} + N/A + {% elif sshd_issues %}
OpenSSH
+ {% else %} + {% include "badge.html" with icon="check" color="success" %} {% endif %} {% endwith %} diff --git a/backend/device_registry/tests/test_all.py b/backend/device_registry/tests/test_all.py index 646587b37..0e9f17c11 100644 --- a/backend/device_registry/tests/test_all.py +++ b/backend/device_registry/tests/test_all.py @@ -645,6 +645,54 @@ def test_insecure_services(self): {'name': 'fingerd', 'version': 'VERSION', 'arch': 'i386', 'os_release_codename': 'jessie'}]) + def test_vulnerable_packages_render(self): + self.client.login(username='test', password='123') + url = reverse('device-detail-security', kwargs={'pk': self.device.pk}) + + # No packages - should render N/A + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertInHTML(''' + + Vulnerable Packages + + + N/A + ''', response.rendered_content) + + self.device.set_deb_packages([ + {'name': 'python2', 'version': 'VERSION', 'source_name': 'python2', 'source_version': 'abcd', + 'arch': 'i386'}, + {'name': 'python3', 'version': 'VERSION', 'source_name': 'python3', 'source_version': 'abcd', + 'arch': 'i386'} + ]) + # No vulnerable packages - green check mark + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertInHTML(''' + + Vulnerable Packages + + + + + + + ''', response.rendered_content) + + v = Vulnerability.objects.create(name='CVE-123', package='python2', is_binary=False, other_versions=[], + urgency=Vulnerability.Urgency.NONE, fix_available=True) + self.device.deb_packages.get(name='python2').vulnerabilities.add(v) + + # Has vulnerable packages - should render them + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'python2') + self.assertContains(response, 'CVE-123') + + + + class PairingKeysView(TestCase): def setUp(self): From 4aab17ee49cc0032cf195a253e7b3d9373b5b12d Mon Sep 17 00:00:00 2001 From: Artem Martynovich Date: Fri, 11 Oct 2019 23:33:13 +0600 Subject: [PATCH 2/4] Update Vulnerable Packages display logic. --- backend/device_registry/models.py | 7 +------ .../templates/device_info_security.html | 10 +++++----- backend/device_registry/tests/test_all.py | 3 --- backend/device_registry/views.py | 2 +- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index e8961fc14..93ec4b544 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -233,7 +233,7 @@ def hostname(self): def actions_count(self): return sum((self.deviceinfo.default_password is True, self.firewallstate.policy != FirewallState.POLICY_ENABLED_BLOCK, - self.vulnerable_packages().exists(), + bool(self.vulnerable_packages and self.vulnerable_packages.exists()), bool(self.insecure_services), bool(self.sshd_issues()))) @@ -314,11 +314,6 @@ def vulnerable_packages(self): if self.deb_packages.exists(): return self.deb_packages.filter(vulnerabilities__isnull=False).distinct().order_by('name') - def has_vulnerable_packages(self): - packages = self.vulnerable_packages - if packages is not None: - return packages.count() - class Meta: ordering = ('created',) diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index 7fce7231c..173906d6c 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -23,15 +23,15 @@

Security

Vulnerable Packages - {% with object.has_vulnerable_packages as has_vulnerable_packages %} - {% if has_vulnerable_packages is None %} + {% with object.vulnerable_packages as vulnerable_packages %} + {% if vulnerable_packages is None %} N/A - {% elif has_vulnerable_packages %} + {% elif vulnerable_packages.count %} diff --git a/backend/device_registry/tests/test_all.py b/backend/device_registry/tests/test_all.py index 0e9f17c11..d0cb7bd16 100644 --- a/backend/device_registry/tests/test_all.py +++ b/backend/device_registry/tests/test_all.py @@ -691,9 +691,6 @@ def test_vulnerable_packages_render(self): self.assertContains(response, 'CVE-123') - - - class PairingKeysView(TestCase): def setUp(self): User = get_user_model() diff --git a/backend/device_registry/views.py b/backend/device_registry/views.py index 96859dc99..1f8895757 100644 --- a/backend/device_registry/views.py +++ b/backend/device_registry/views.py @@ -517,7 +517,7 @@ def actions_view(request, device_pk=None): text_blocks = [] for dev in devices_with_vuln_packages: device_text_block = f'{dev.get_name()}' \ - f'({dev.vulnerable_packages().count()} packages)' + f'({dev.vulnerable_packages.count()} packages)' text_blocks.append(device_text_block) full_string = ', '.join(text_blocks) action = Action( From 3a34dd1432c848c1f68913e1e5a473e1da2e2e58 Mon Sep 17 00:00:00 2001 From: Artem Martynovich Date: Mon, 14 Oct 2019 17:00:54 +0600 Subject: [PATCH 3/4] Remove liable_for_vulnerable_packages_check, merge it with vulnerable_packages logic. --- backend/device_registry/models.py | 11 +++-------- backend/device_registry/tests/test_all.py | 4 +++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 93ec4b544..863ac9970 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -105,13 +105,6 @@ class Device(models.Model): deb_packages_hash = models.CharField(max_length=32, blank=True) audit_files = JSONField(blank=True, default=list) - # TODO: to improve when we support saving full distro info. - def liable_for_vulnerable_packages_check(self): - if self.deb_packages_hash and self.deb_packages.exists() and \ - self.deb_packages.first().os_release_codename in DEBIAN_SUITES: - return True - return False - def sshd_issues(self): if self.audit_files: for file_info in self.audit_files: @@ -311,7 +304,9 @@ def set_meta_tags(self): @property def vulnerable_packages(self): - if self.deb_packages.exists(): + if self.deb_packages_hash and self.deb_packages.exists() and \ + self.deb_packages.first().os_release_codename in DEBIAN_SUITES: + # FIXME: should use self.os_release_codename instead of a first deb package return self.deb_packages.filter(vulnerabilities__isnull=False).distinct().order_by('name') class Meta: diff --git a/backend/device_registry/tests/test_all.py b/backend/device_registry/tests/test_all.py index d0cb7bd16..1bfb96c35 100644 --- a/backend/device_registry/tests/test_all.py +++ b/backend/device_registry/tests/test_all.py @@ -660,12 +660,14 @@ def test_vulnerable_packages_render(self): N/A ''', response.rendered_content) + self.device.deb_packages_hash = 'aabbccdd' + self.device.save() self.device.set_deb_packages([ {'name': 'python2', 'version': 'VERSION', 'source_name': 'python2', 'source_version': 'abcd', 'arch': 'i386'}, {'name': 'python3', 'version': 'VERSION', 'source_name': 'python3', 'source_version': 'abcd', 'arch': 'i386'} - ]) + ], os_info={'codename': 'stretch'}) # No vulnerable packages - green check mark response = self.client.get(url) self.assertEqual(response.status_code, 200) From 08663ca590f4136938efe4150dac20cf18e8ba0d Mon Sep 17 00:00:00 2001 From: Artem Martynovich Date: Mon, 14 Oct 2019 21:44:49 +0600 Subject: [PATCH 4/4] vulnerable_packages: count -> exists --- backend/device_registry/templates/device_info_security.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index 173906d6c..6cacff6fb 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -26,7 +26,7 @@

Security

{% with object.vulnerable_packages as vulnerable_packages %} {% if vulnerable_packages is None %} N/A - {% elif vulnerable_packages.count %} + {% elif vulnerable_packages.exists %}
    {% for package in vulnerable_packages %}
  • {{ package.name }} / {{ package.version }}