From 3d07b7f4e8544844e8bc220b0978b08f3d6730f5 Mon Sep 17 00:00:00 2001 From: Roman P Date: Wed, 18 Sep 2019 12:12:10 +0400 Subject: [PATCH 1/3] Add checksum reporting to critical system files Closes #400 --- backend/device_registry/api_views.py | 4 +- .../migrations/0059_device_audit_files.py | 19 ++++++ backend/device_registry/models.py | 1 + .../templates/device_info_security.html | 67 ++++++++++++++----- backend/device_registry/templatetags/misc.py | 9 +++ 5 files changed, 83 insertions(+), 17 deletions(-) create mode 100644 backend/device_registry/migrations/0059_device_audit_files.py create mode 100644 backend/device_registry/templatetags/misc.py diff --git a/backend/device_registry/api_views.py b/backend/device_registry/api_views.py index ac72f55be..fa90bf435 100644 --- a/backend/device_registry/api_views.py +++ b/backend/device_registry/api_views.py @@ -86,6 +86,7 @@ def post(self, request, *args, **kwargs): device = Device.objects.get(device_id=request.device_id) device.last_ping = timezone.now() device.agent_version = data.get('agent_version') + device.audit_files = data.get('audit_files', []) if 'deb_packages' in data: deb_packages = data['deb_packages'] device.deb_packages_hash = deb_packages['hash'] @@ -132,7 +133,8 @@ def post(self, request, *args, **kwargs): firewall_state.save() device.update_trust_score = True - device.save(update_fields=['last_ping', 'agent_version', 'deb_packages_hash', 'update_trust_score']) + device.save(update_fields=['last_ping', 'agent_version', 'audit_files', 'deb_packages_hash', + 'update_trust_score']) if datastore_client: task_key = datastore_client.key('Ping') diff --git a/backend/device_registry/migrations/0059_device_audit_files.py b/backend/device_registry/migrations/0059_device_audit_files.py new file mode 100644 index 000000000..4588b31bb --- /dev/null +++ b/backend/device_registry/migrations/0059_device_audit_files.py @@ -0,0 +1,19 @@ +# Generated by Django 2.2.5 on 2019-09-18 04:43 + +import django.contrib.postgres.fields.jsonb +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('device_registry', '0058_auto_20190917_1245'), + ] + + operations = [ + migrations.AddField( + model_name='device', + name='audit_files', + field=django.contrib.postgres.fields.jsonb.JSONField(blank=True, default=list), + ), + ] diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 76a4e389c..2f0430c02 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -97,6 +97,7 @@ class Device(models.Model): update_trust_score = models.BooleanField(default=False, db_index=True) deb_packages = models.ManyToManyField(DebPackage) deb_packages_hash = models.CharField(max_length=32, blank=True) + audit_files = JSONField(blank=True, default=list) @property def certificate_expired(self): diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index 2bc4e165f..ead0d7a38 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -1,5 +1,6 @@ {% extends "admin_base.html" %} {% load split_string %} +{% load misc %} {% block title %}WoTT - Device Info{% endblock title %} @@ -58,7 +59,7 @@

Security

{% endif %} - + Default Credentials Security - - - OpenSSH Audit - + + + OpenSSH Audit + Coming soon! - - - Insecure Services - + + + Insecure Services + {% with object.insecure_services as services %} {% if services %} @@ -105,13 +106,13 @@

Security

Logins - - {% if object.deviceinfo.logins %} - 
{{ object.deviceinfo.beautified_logins }}
- {% else %} - No recent login attempts detected. - {% endif %} - + + {% if object.deviceinfo.logins %} + 
{{ object.deviceinfo.beautified_logins }}
+ {% else %} + No recent login attempts detected. + {% endif %} + @@ -400,6 +401,40 @@

Security

{% endif %} + + Audited files + + {% if object.audit_files %} + + + + + + + + + + {% for file in object.audit_files %} + + + + + + + {% endfor %} +
PathChecksum (SHA-256)Last modifiedIssues
{{ file.name }}{{ file.sha256 }} + {% if file.last_modified %}{{ file.last_modified|fromunix|timesince }} ago{% endif %} + {% if file.issues %} +
    + {% for issue in file.issues.items %} +
  • {{ issue.0 }}={{ issue.1 }}
    • + {% endfor %} +
+ {% endif %} +
+ {% endif %} + + diff --git a/backend/device_registry/templatetags/misc.py b/backend/device_registry/templatetags/misc.py new file mode 100644 index 000000000..7ccbd9d5b --- /dev/null +++ b/backend/device_registry/templatetags/misc.py @@ -0,0 +1,9 @@ +from django import template +from django.utils import timezone + +register = template.Library() + + +@register.filter(name='fromunix') +def fromunix(value): + return timezone.datetime.fromtimestamp(value, timezone.get_default_timezone()) From 8af0f638e2c2fa72134eada8c5be0e45d4a41d11 Mon Sep 17 00:00:00 2001 From: Roman P Date: Fri, 20 Sep 2019 08:26:57 +0400 Subject: [PATCH 2/3] Improvements requested in the review - moved 'System File Audit' section to the proper place - added 'Configuration Audit' section with only OpenSSH issues for now --- backend/backend/settings/dev.py | 2 + ...it_files.py => 0061_device_audit_files.py} | 4 +- backend/device_registry/models.py | 14 ++++ .../templates/device_info_security.html | 73 +++++++++---------- 4 files changed, 53 insertions(+), 40 deletions(-) rename backend/device_registry/migrations/{0059_device_audit_files.py => 0061_device_audit_files.py} (78%) diff --git a/backend/backend/settings/dev.py b/backend/backend/settings/dev.py index 78fa67a90..cb1f27cc3 100644 --- a/backend/backend/settings/dev.py +++ b/backend/backend/settings/dev.py @@ -25,3 +25,5 @@ IS_DEV = True EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' + +ALLOWED_HOSTS = ['*'] diff --git a/backend/device_registry/migrations/0059_device_audit_files.py b/backend/device_registry/migrations/0061_device_audit_files.py similarity index 78% rename from backend/device_registry/migrations/0059_device_audit_files.py rename to backend/device_registry/migrations/0061_device_audit_files.py index 4588b31bb..5457daedf 100644 --- a/backend/device_registry/migrations/0059_device_audit_files.py +++ b/backend/device_registry/migrations/0061_device_audit_files.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.5 on 2019-09-18 04:43 +# Generated by Django 2.2.5 on 2019-09-24 10:39 import django.contrib.postgres.fields.jsonb from django.db import migrations @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ - ('device_registry', '0058_auto_20190917_1245'), + ('device_registry', '0060_auto_20190920_1215'), ] operations = [ diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 2f0430c02..7f356c0f9 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -99,6 +99,20 @@ class Device(models.Model): deb_packages_hash = models.CharField(max_length=32, blank=True) audit_files = JSONField(blank=True, default=list) + def sshd_issues(self): + if self.audit_files: + for file_info in self.audit_files: + if 'sshd' in file_info['name']: + issues = [] + for k, v in file_info['issues'].items(): + if v == 'yes': + secure_value = 'no' + else: + secure_value = '2' # Support only 'Protocol' now. TODO: improve this. + issues.append((k, v, secure_value)) + return issues + return None + @property def certificate_expired(self): return self.certificate_expires < timezone.now() diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index ead0d7a38..e22844cbc 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -79,10 +79,41 @@

Security

- - OpenSSH Audit - - Coming soon! + System File Audit + + {% if object.audit_files %} + + + + + + + + + {% for file in object.audit_files %} + + + + + + {% endfor %} +
PathChecksum (SHA-256)Last Modified
{{ file.name }}{{ file.sha256 }} + {% if file.last_modified %}{{ file.last_modified|fromunix|timesince }} ago{% endif %}
+ {% endif %} + + + + Configuration Audit + + {% if object.sshd_issues %} +
OpenSSH
+
    + {% for issue in object.sshd_issues %} +
  • {% include "badge.html" with icon="exclamation-circle" color="danger" %} Consider changing "{{ issue.0 }}" from "{{ issue.1 }}" to "{{ issue.2 }}"
    • + {% endfor %} +
+ {% endif %} + @@ -401,40 +432,6 @@

Security

{% endif %} - - Audited files - - {% if object.audit_files %} - - - - - - - - - - {% for file in object.audit_files %} - - - - - - - {% endfor %} -
PathChecksum (SHA-256)Last modifiedIssues
{{ file.name }}{{ file.sha256 }} - {% if file.last_modified %}{{ file.last_modified|fromunix|timesince }} ago{% endif %} - {% if file.issues %} -
    - {% for issue in file.issues.items %} -
  • {{ issue.0 }}={{ issue.1 }}
    • - {% endfor %} -
- {% endif %} -
- {% endif %} - - From cad5cedfc83d9872f372715f2ac6b8a093b7b1fc Mon Sep 17 00:00:00 2001 From: Roman P Date: Thu, 3 Oct 2019 12:32:57 +0400 Subject: [PATCH 3/3] Fixed sshd config params secure values --- ...it_files.py => 0062_device_audit_files.py} | 4 ++-- backend/device_registry/models.py | 15 +++++++++----- .../templates/device_info_security.html | 20 +++++++++++-------- 3 files changed, 24 insertions(+), 15 deletions(-) rename backend/device_registry/migrations/{0061_device_audit_files.py => 0062_device_audit_files.py} (77%) diff --git a/backend/device_registry/migrations/0061_device_audit_files.py b/backend/device_registry/migrations/0062_device_audit_files.py similarity index 77% rename from backend/device_registry/migrations/0061_device_audit_files.py rename to backend/device_registry/migrations/0062_device_audit_files.py index 5457daedf..101d55d73 100644 --- a/backend/device_registry/migrations/0061_device_audit_files.py +++ b/backend/device_registry/migrations/0062_device_audit_files.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.5 on 2019-09-24 10:39 +# Generated by Django 2.2.6 on 2019-10-03 06:44 import django.contrib.postgres.fields.jsonb from django.db import migrations @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ - ('device_registry', '0060_auto_20190920_1215'), + ('device_registry', '0061_deviceinfo_processes'), ] operations = [ diff --git a/backend/device_registry/models.py b/backend/device_registry/models.py index 7f356c0f9..2e6305a5c 100644 --- a/backend/device_registry/models.py +++ b/backend/device_registry/models.py @@ -68,6 +68,15 @@ class Meta: unique_together = ['name', 'version', 'arch'] +SSHD_CONFIG_PARAMS_SAFE_VALUES = { + 'PermitEmptyPasswords': 'no', + 'PermitRootLogin': 'no', + 'PasswordAuthentication': 'no', + 'AllowAgentForwarding': 'no', + 'Protocol': '2' +} + + class Device(models.Model): device_id = models.CharField( max_length=128, @@ -105,11 +114,7 @@ def sshd_issues(self): if 'sshd' in file_info['name']: issues = [] for k, v in file_info['issues'].items(): - if v == 'yes': - secure_value = 'no' - else: - secure_value = '2' # Support only 'Protocol' now. TODO: improve this. - issues.append((k, v, secure_value)) + issues.append((k, v, SSHD_CONFIG_PARAMS_SAFE_VALUES[k])) return issues return None diff --git a/backend/device_registry/templates/device_info_security.html b/backend/device_registry/templates/device_info_security.html index e22844cbc..faa3e0853 100644 --- a/backend/device_registry/templates/device_info_security.html +++ b/backend/device_registry/templates/device_info_security.html @@ -105,14 +105,18 @@

Security

Configuration Audit - {% if object.sshd_issues %} -
OpenSSH
-
    - {% for issue in object.sshd_issues %} -
  • {% include "badge.html" with icon="exclamation-circle" color="danger" %} Consider changing "{{ issue.0 }}" from "{{ issue.1 }}" to "{{ issue.2 }}"
    • - {% endfor %} -
- {% endif %} + {% with object.sshd_issues as sshd_issues %} + {% if sshd_issues %} +
OpenSSH
+
    + {% for issue in sshd_issues %} +
  • {% include "badge.html" with icon="exclamation-circle" color="danger" %} Consider + changing "{{ issue.0 }}" from "{{ issue.1 }}" to "{{ issue.2 }}" +
    • + {% endfor %} +
+ {% endif %} + {% endwith %}