Merge pull request #465 from GreatFruitOmsk/442-security-empty-cells

Display N/A if no data for Vulnerable Packages and Audit fields.
WoTTsecurity · Oct 14, 2019 · 0c65bb6 · 0c65bb6
2 parents a53bb94 + 08663ca
commit 0c65bb6
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 25 deletions.
diff --git 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:
@@ -233,7 +226,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())))
 
@@ -309,8 +302,12 @@ 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_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:
         ordering = ('created',)

diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html
@@ -23,22 +23,22 @@ <h4 class="tab-title">Security</h4>
                 <tr>
                   <th scope="row">Vulnerable Packages</th>
                   <td>
-                    {% if object.liable_for_vulnerable_packages_check %}
-                      {% with object.vulnerable_packages as vulnerable_packages %}
-                        {% if vulnerable_packages %}
-                          <ul>
-                            {% for package in vulnerable_packages %}
-                              <li>{{ package.name }} / {{ package.version }}
-                                ({% for v in package.vulnerabilities.all %}<a href="https://security-tracker.debian.org/tracker/{{ v.name }}"
-                                     target="_blank">{{ v.name }}</a>{% if not forloop.last %}, {% endif %}{% endfor %})
-                              </li>
-                            {% endfor %}
-                          </ul>
-                        {% endif %}
-                      {% endwith %}
-                    {% else %}
+                    {% with object.vulnerable_packages as vulnerable_packages %}
+                    {% if vulnerable_packages is None %}
                       N/A
+                    {% elif vulnerable_packages.exists %}
+                      <ul>
+                        {% for package in vulnerable_packages %}
+                          <li>{{ package.name }} / {{ package.version }}
+                            ({% for v in package.vulnerabilities.all %}<a href="https://security-tracker.debian.org/tracker/{{ v.name }}"
+                               target="_blank">{{ v.name }}</a>{% if not forloop.last %}, {% endif %}{% endfor %})
+                          </li>
+                        {% endfor %}
+                      </ul>
+                    {% else %}
+                     {% include "badge.html" with icon="check" color="success" %}
                     {% endif %}
+                    {% endwith %}
                   </td>
                 </tr>
                 <tr>
@@ -81,14 +81,18 @@ <h4 class="tab-title">Security</h4>
                           </tr>
                         {% endfor %}
                       </table>
+                    {% else %}
+                      N/A
                     {% endif %}
                   </td>
                 </tr>
                 <tr>
                   <th scope="row">Configuration Audit</th>
                   <td>
                     {% with object.sshd_issues as sshd_issues %}
-                      {% if sshd_issues %}
+                      {% if sshd_issues is None %}
+                        N/A
+                      {% elif sshd_issues %}
                         <h5>OpenSSH</h5>
                         <ul style="list-style-type:none; padding: 0;">
                           {% for issue in sshd_issues %}
@@ -97,6 +101,8 @@ <h5>OpenSSH</h5>
                             </li>
                           {% endfor %}
                         </ul>
+                      {% else %}
+                        {% include "badge.html" with icon="check" color="success" %}
                       {% endif %}
                     {% endwith %}
                   </td>

diff --git a/backend/device_registry/tests/test_all.py b/backend/device_registry/tests/test_all.py
@@ -645,6 +645,53 @@ 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('''
+            <th scope="row">
+                Vulnerable Packages
+            </th>
+            <td>
+                N/A
+            </td>''', 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)
+        self.assertInHTML('''
+            <th scope="row">
+                Vulnerable Packages
+            </th>
+            <td>
+                <span class="p-1 text-success">
+                    <i class="fas fa-check" ></i>
+                </span>
+            </td>
+                ''', 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):

diff --git 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'<a href="{reverse("device-detail", kwargs={"pk": dev.pk})}">{dev.get_name()}</a>' \
-                                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(