2
0
Эх сурвалжийг харах

Fixes #22985: Exempt data file content from cache (#22986)

Jeremy Stretch 1 өдөр өмнө
parent
commit
e49e6afbbe

+ 19 - 0
netbox/core/tests/test_views.py

@@ -105,6 +105,25 @@ class DataFileTestCase(
         )
         )
         DataFile.objects.bulk_create(data_files)
         DataFile.objects.bulk_create(data_files)
 
 
+    def test_content_is_not_cacheable(self):
+        """
+        The detail view renders file content inline, which may include plaintext secrets, so the
+        response must instruct the browser not to persist it to its local cache.
+        """
+        datafile = DataFile.objects.first()
+        datafile.data = b'super-secret-password'
+        datafile.save()
+
+        self.add_permissions('core.view_datafile')
+        response = self.client.get(datafile.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the content is in fact rendered in the response
+        self.assertIn('super-secret-password', str(response.content))
+
+        # Confirm the response is not cacheable
+        self.assertNotCacheable(response)
+
 
 
 class JobTestCase(
 class JobTestCase(
     ViewTestCases.GetObjectViewTestCase,
     ViewTestCases.GetObjectViewTestCase,

+ 3 - 0
netbox/core/views.py

@@ -12,8 +12,10 @@ from django.db import DatabaseError, connection
 from django.http import Http404, HttpResponse, HttpResponseForbidden
 from django.http import Http404, HttpResponse, HttpResponseForbidden
 from django.shortcuts import get_object_or_404, redirect, render
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse
 from django.urls import reverse
+from django.utils.decorators import method_decorator
 from django.utils.http import content_disposition_header
 from django.utils.http import content_disposition_header
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_lazy as _
+from django.views.decorators.cache import never_cache
 from django.views.generic import View
 from django.views.generic import View
 from django_rq.queues import get_queue_by_index, get_redis_connection
 from django_rq.queues import get_queue_by_index, get_redis_connection
 from django_rq.settings import get_queues_list, get_queues_map
 from django_rq.settings import get_queues_list, get_queues_map
@@ -195,6 +197,7 @@ class DataFileListView(generic.ObjectListView):
 
 
 
 
 @register_model_view(DataFile)
 @register_model_view(DataFile)
+@method_decorator(never_cache, name='dispatch')
 class DataFileView(generic.ObjectView):
 class DataFileView(generic.ObjectView):
     queryset = DataFile.objects.all()
     queryset = DataFile.objects.all()
     actions = (DeleteObject,)
     actions = (DeleteObject,)

+ 49 - 1
netbox/dcim/tests/test_views.py

@@ -16,7 +16,7 @@ from core.models import ObjectChange, ObjectType
 from dcim.choices import *
 from dcim.choices import *
 from dcim.constants import *
 from dcim.constants import *
 from dcim.models import *
 from dcim.models import *
-from extras.models import ConfigTemplate
+from extras.models import ConfigContext, ConfigTemplate
 from ipam.models import ASN, RIR, VLAN, VRF
 from ipam.models import ASN, RIR, VLAN, VRF
 from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, WeightUnitChoices
 from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, WeightUnitChoices
 from tenancy.models import Tenant
 from tenancy.models import Tenant
@@ -2538,6 +2538,54 @@ class DeviceTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         self.assertHttpStatus(response, 200)
         self.assertHttpStatus(response, 200)
         self.assertIn(b'Error rendering template', response.content)
         self.assertIn(b'Error rendering template', response.content)
 
 
+    def test_device_configcontext_is_not_cacheable(self):
+        """
+        The config context tab renders the merged context data, which may contain sensitive
+        values, so the response must not be cached by the browser.
+        """
+        ConfigContext.objects.create(name='Config Context 1', data={'password': 'super-secret-password'})
+        device = Device.objects.first()
+
+        self.add_permissions('dcim.view_device', 'extras.view_configcontext')
+        url = reverse('dcim:device_configcontext', kwargs={'pk': device.pk})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the context data is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
+    def test_device_renderconfig_is_not_cacheable(self):
+        """
+        The render config tab renders the config template with context data substituted into it,
+        which may contain sensitive values, so the response must not be cached by the browser.
+        """
+        configtemplate = ConfigTemplate.objects.create(
+            name='Test Config Template',
+            template_code='enable secret super-secret-password'
+        )
+        device = Device.objects.first()
+        device.config_template = configtemplate
+        device.save()
+
+        self.add_permissions('dcim.view_device', 'dcim.render_config_device')
+        url = reverse('dcim:device_render-config', kwargs={'pk': device.pk})
+
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the rendered config is in fact present in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
+        # The direct export of the rendered config must not be cached either
+        response = self.client.get(url, {'export': 1})
+        self.assertHttpStatus(response, 200)
+        self.assertIn(b'super-secret-password', response.content)
+        self.assertNotCacheable(response)
+
     def test_device_role_display_colored(self):
     def test_device_role_display_colored(self):
         parent_role = DeviceRole.objects.create(name='Parent Role', slug='parent-role', color='111111')
         parent_role = DeviceRole.objects.create(name='Parent Role', slug='parent-role', color='111111')
         child_role = DeviceRole.objects.create(name='Child Role', slug='child-role', parent=parent_role, color='aa00bb')
         child_role = DeviceRole.objects.create(name='Child Role', slug='child-role', parent=parent_role, color='aa00bb')

+ 79 - 0
netbox/extras/tests/test_views.py

@@ -589,6 +589,24 @@ class ExportTemplateTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             'as_attachment': True,
             'as_attachment': True,
         }
         }
 
 
+    def test_content_is_not_cacheable(self):
+        """
+        The detail view renders the template code inline, which may have been synced from a data
+        file containing sensitive values, so the response must not be cached by the browser.
+        """
+        export_template = ExportTemplate.objects.first()
+        export_template.template_code = 'super-secret-password'
+        export_template.save()
+
+        self.add_permissions('extras.view_exporttemplate')
+        response = self.client.get(export_template.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the template code is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
 
 
 class ExportTemplateExportFlowTestCase(TestCase):
 class ExportTemplateExportFlowTestCase(TestCase):
     """
     """
@@ -852,6 +870,31 @@ class ConfigContextProfileTestCase(ViewTestCases.PrimaryObjectViewTestCase):
             f"{profiles[2].pk},New description",
             f"{profiles[2].pk},New description",
         )
         )
 
 
+    def test_content_is_not_cacheable(self):
+        """
+        The detail view renders the schema inline, which may have been synced from a data file
+        containing sensitive values, so the response must not be cached by the browser.
+        """
+        instance = ConfigContextProfile.objects.first()
+        instance.schema = {
+            'properties': {
+                'password': {
+                    'type': 'string',
+                    'default': 'super-secret-password',
+                }
+            }
+        }
+        instance.save()
+
+        self.add_permissions('extras.view_configcontextprofile')
+        response = self.client.get(instance.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the schema is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
 
 
 # TODO: Change base class to PrimaryObjectViewTestCase
 # TODO: Change base class to PrimaryObjectViewTestCase
 # Blocked by absence of standard create/edit, bulk create views
 # Blocked by absence of standard create/edit, bulk create views
@@ -902,6 +945,24 @@ class ConfigContextTestCase(
             'description': 'New description',
             'description': 'New description',
         }
         }
 
 
+    def test_content_is_not_cacheable(self):
+        """
+        The detail view renders the data inline, which may have been synced from a data
+        file containing sensitive values, so the response must not be cached by the browser.
+        """
+        instance = ConfigContext.objects.first()
+        instance.data = {'password': 'super-secret-password'}
+        instance.save()
+
+        self.add_permissions('extras.view_configcontext')
+        response = self.client.get(instance.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the context data is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
 
 
 class ConfigTemplateTestCase(
 class ConfigTemplateTestCase(
     ViewTestCases.GetObjectViewTestCase,
     ViewTestCases.GetObjectViewTestCase,
@@ -959,6 +1020,24 @@ class ConfigTemplateTestCase(
             'as_attachment': True,
             'as_attachment': True,
         }
         }
 
 
+    def test_content_is_not_cacheable(self):
+        """
+        The detail view renders the template code inline, which may have been synced from a data
+        file containing sensitive values, so the response must not be cached by the browser.
+        """
+        instance = ConfigTemplate.objects.first()
+        instance.template_code = 'super-secret-password'
+        instance.save()
+
+        self.add_permissions('extras.view_configtemplate')
+        response = self.client.get(instance.get_absolute_url())
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the template code is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
 
 
 class JournalEntryTestCase(
 class JournalEntryTestCase(
     # ViewTestCases.GetObjectViewTestCase,
     # ViewTestCases.GetObjectViewTestCase,

+ 8 - 0
netbox/extras/views.py

@@ -9,9 +9,11 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpRespo
 from django.shortcuts import get_object_or_404, redirect, render
 from django.shortcuts import get_object_or_404, redirect, render
 from django.urls import reverse
 from django.urls import reverse
 from django.utils import timezone
 from django.utils import timezone
+from django.utils.decorators import method_decorator
 from django.utils.http import content_disposition_header
 from django.utils.http import content_disposition_header
 from django.utils.module_loading import import_string
 from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_lazy as _
+from django.views.decorators.cache import never_cache
 from django.views.generic import View
 from django.views.generic import View
 
 
 from core.choices import ManagedFileRootPathChoices
 from core.choices import ManagedFileRootPathChoices
@@ -308,6 +310,7 @@ class ExportTemplateListView(generic.ObjectListView):
 
 
 
 
 @register_model_view(ExportTemplate)
 @register_model_view(ExportTemplate)
+@method_decorator(never_cache, name='dispatch')
 class ExportTemplateView(generic.ObjectView):
 class ExportTemplateView(generic.ObjectView):
     queryset = ExportTemplate.objects.all()
     queryset = ExportTemplate.objects.all()
     template_name = 'generic/object.html'
     template_name = 'generic/object.html'
@@ -994,6 +997,7 @@ class ConfigContextProfileListView(generic.ObjectListView):
 
 
 
 
 @register_model_view(ConfigContextProfile)
 @register_model_view(ConfigContextProfile)
+@method_decorator(never_cache, name='dispatch')
 class ConfigContextProfileView(generic.ObjectView):
 class ConfigContextProfileView(generic.ObjectView):
     queryset = ConfigContextProfile.objects.all()
     queryset = ConfigContextProfile.objects.all()
     template_name = 'generic/object.html'
     template_name = 'generic/object.html'
@@ -1069,6 +1073,7 @@ class ConfigContextListView(generic.ObjectListView):
 
 
 
 
 @register_model_view(ConfigContext)
 @register_model_view(ConfigContext)
+@method_decorator(never_cache, name='dispatch')
 class ConfigContextView(generic.ObjectView):
 class ConfigContextView(generic.ObjectView):
     queryset = ConfigContext.objects.all()
     queryset = ConfigContext.objects.all()
     template_name = 'generic/object.html'
     template_name = 'generic/object.html'
@@ -1155,6 +1160,7 @@ class ConfigContextBulkSyncDataView(generic.BulkSyncDataView):
     queryset = ConfigContext.objects.all()
     queryset = ConfigContext.objects.all()
 
 
 
 
+@method_decorator(never_cache, name='dispatch')
 class ObjectConfigContextView(generic.ObjectView):
 class ObjectConfigContextView(generic.ObjectView):
     base_template = None
     base_template = None
     template_name = 'extras/object_configcontext.html'
     template_name = 'extras/object_configcontext.html'
@@ -1199,6 +1205,7 @@ class ConfigTemplateListView(generic.ObjectListView):
 
 
 
 
 @register_model_view(ConfigTemplate)
 @register_model_view(ConfigTemplate)
+@method_decorator(never_cache, name='dispatch')
 class ConfigTemplateView(generic.ObjectView):
 class ConfigTemplateView(generic.ObjectView):
     queryset = ConfigTemplate.objects.all()
     queryset = ConfigTemplate.objects.all()
     template_name = 'generic/object.html'
     template_name = 'generic/object.html'
@@ -1260,6 +1267,7 @@ class ConfigTemplateBulkSyncDataView(generic.BulkSyncDataView):
     queryset = ConfigTemplate.objects.all()
     queryset = ConfigTemplate.objects.all()
 
 
 
 
+@method_decorator(never_cache, name='dispatch')
 class ObjectRenderConfigView(generic.ObjectView):
 class ObjectRenderConfigView(generic.ObjectView):
     base_template = None
     base_template = None
     template_name = 'extras/object_render_config.html'
     template_name = 'extras/object_render_config.html'

+ 14 - 0
netbox/utilities/testing/base.py

@@ -125,6 +125,20 @@ class TestCase(_TestCase):
             err_message = f"Expected HTTP status {expected_status}; received {response.status_code}: {err}"
             err_message = f"Expected HTTP status {expected_status}; received {response.status_code}: {err}"
         self.assertEqual(response.status_code, expected_status, err_message)
         self.assertEqual(response.status_code, expected_status, err_message)
 
 
+    def assertNotCacheable(self, response):
+        """
+        TestCase method. Assert that a response instructs the browser not to persist its content
+        to the local cache. Views which render potentially sensitive content (e.g. the contents of
+        a synced data file) must not be written to the browser's cache, where they would remain
+        readable after the session has ended.
+        """
+        cache_control = response.headers.get('Cache-Control', '')
+        self.assertIn(
+            'no-store',
+            cache_control,
+            f"Expected a no-store cache directive; received Cache-Control: '{cache_control}'"
+        )
+
 
 
 class ModelTestCase(TestCase):
 class ModelTestCase(TestCase):
     """
     """

+ 49 - 1
netbox/virtualization/tests/test_views.py

@@ -5,7 +5,7 @@ from django.urls import reverse
 
 
 from dcim.choices import InterfaceModeChoices
 from dcim.choices import InterfaceModeChoices
 from dcim.models import DeviceRole, Platform, Site
 from dcim.models import DeviceRole, Platform, Site
-from extras.models import ConfigTemplate
+from extras.models import ConfigContext, ConfigTemplate
 from ipam.models import VLAN, VRF
 from ipam.models import VLAN, VRF
 from utilities.testing import ViewTestCases, create_tags, create_test_device, create_test_virtualmachine
 from utilities.testing import ViewTestCases, create_tags, create_test_device, create_test_virtualmachine
 from virtualization.choices import *
 from virtualization.choices import *
@@ -556,6 +556,54 @@ class VirtualMachineTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         self.assertHttpStatus(response, 200)
         self.assertHttpStatus(response, 200)
         self.assertIn(b'Error rendering template', response.content)
         self.assertIn(b'Error rendering template', response.content)
 
 
+    def test_virtualmachine_configcontext_is_not_cacheable(self):
+        """
+        The config context tab renders the merged context data, which may contain sensitive
+        values, so the response must not be cached by the browser.
+        """
+        ConfigContext.objects.create(name='Config Context 1', data={'password': 'super-secret-password'})
+        vm = VirtualMachine.objects.first()
+
+        self.add_permissions('virtualization.view_virtualmachine', 'extras.view_configcontext')
+        url = reverse('virtualization:virtualmachine_configcontext', kwargs={'pk': vm.pk})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the context data is in fact rendered in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
+    def test_virtualmachine_renderconfig_is_not_cacheable(self):
+        """
+        The render config tab renders the config template with context data substituted into it,
+        which may contain sensitive values, so the response must not be cached by the browser.
+        """
+        configtemplate = ConfigTemplate.objects.create(
+            name='Test Config Template',
+            template_code='enable secret super-secret-password'
+        )
+        vm = VirtualMachine.objects.first()
+        vm.config_template = configtemplate
+        vm.save()
+
+        self.add_permissions('virtualization.view_virtualmachine', 'virtualization.render_config_virtualmachine')
+        url = reverse('virtualization:virtualmachine_render-config', kwargs={'pk': vm.pk})
+
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+
+        # Confirm the rendered config is in fact present in the response
+        self.assertIn(b'super-secret-password', response.content)
+
+        self.assertNotCacheable(response)
+
+        # The direct export of the rendered config must not be cached either
+        response = self.client.get(url, {'export': 1})
+        self.assertHttpStatus(response, 200)
+        self.assertIn(b'super-secret-password', response.content)
+        self.assertNotCacheable(response)
+
 
 
 class VMInterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase):
 class VMInterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase):
     model = VMInterface
     model = VMInterface