Merge "kolla_container: add systempaths_unconfined support"

10 files changed, 105 insertions, 0 deletions

diff --git a/ansible/library/kolla_container.py b/ansible/library/kolla_container.py
index 675170a44..a87686c2c 100644
--- a/ansible/library/kolla_container.py
+++ b/ansible/library/kolla_container.py
@@ -170,6 +170,14 @@ options:
     required: False
     default: False
     type: bool
+  systempaths_unconfined:
+    description:
+      - Clear Docker's default MaskedPaths/ReadonlyPaths container hardening
+        (equivalent to C(docker run --security-opt systempaths=unconfined))
+      - Docker only, ignored under Podman
+    required: False
+    default: False
+    type: bool
   remove_on_exit:
     description:
       - When not detaching from container, remove on successful exit
@@ -315,6 +323,8 @@ def generate_module():
         cgroupns_mode=dict(required=False, type='str',
                            choices=['private', 'host']),
         privileged=dict(required=False, type='bool', default=False),
+        systempaths_unconfined=dict(required=False, type='bool',
+                                    default=False),
         graceful_timeout=dict(required=False, type='int'),
         remove_on_exit=dict(required=False, type='bool', default=True),
         restart_policy=dict(required=False, type='str', choices=[
diff --git a/ansible/module_utils/kolla_container_worker.py b/ansible/module_utils/kolla_container_worker.py
index 4b10bb268..477725dc5 100644
--- a/ansible/module_utils/kolla_container_worker.py
+++ b/ansible/module_utils/kolla_container_worker.py
@@ -112,6 +112,7 @@ class ContainerWorker(ABC):
             'privileged',
             'pid_mode',
             'cgroupns_mode',
+            'systempaths_unconfined',
             'tmpfs',
             'volumes',
             'volumes_from',
@@ -198,6 +199,14 @@ class ContainerWorker(ABC):
                    container_info['HostConfig'].get('CgroupMode') or 'host')
         return current, self.params.get('cgroupns_mode')
 
+    def diff_systempaths_unconfined(self, container_info):
+        current_masked = self._get_host_config_list(
+            container_info, 'MaskedPaths')
+        current_readonly = self._get_host_config_list(
+            container_info, 'ReadonlyPaths')
+        current = not current_masked and not current_readonly
+        return current, self.params.get('systempaths_unconfined', False)
+
     def diff_tmpfs(self, container_info):
         current = list(container_info['HostConfig'].get('Tmpfs') or [])
         desired = list(self.generate_tmpfs() or [])
@@ -337,6 +346,19 @@ class ContainerWorker(ABC):
         if new_privileged != current_privileged:
             return True
 
+    def compare_systempaths_unconfined(self, container_info):
+        if self.params.get('privileged', False):
+            # MaskedPaths/ReadonlyPaths are not applied to privileged
+            # containers, so this setting is meaningless there
+            return False
+        new_value = self.params.get('systempaths_unconfined', False)
+        current_masked = self._get_host_config_list(
+            container_info, 'MaskedPaths')
+        current_readonly = self._get_host_config_list(
+            container_info, 'ReadonlyPaths')
+        current_value = not current_masked and not current_readonly
+        return new_value != current_value
+
     @abstractmethod
     def compare_image(self, container_info=None):
         pass
diff --git a/ansible/module_utils/kolla_docker_worker.py b/ansible/module_utils/kolla_docker_worker.py
index f31c77f9e..172f8570c 100644
--- a/ansible/module_utils/kolla_docker_worker.py
+++ b/ansible/module_utils/kolla_docker_worker.py
@@ -317,6 +317,13 @@ class DockerWorker(ContainerWorker):
         if cgroupns_mode is not None:
             host_config['CgroupnsMode'] = cgroupns_mode
 
+        # NOTE: python-docker does not support clearing MaskedPaths/
+        # ReadonlyPaths (docker run --security-opt systempaths=unconfined)
+        # natively so we stuff it in manually.
+        if self.params.get('systempaths_unconfined'):
+            host_config['MaskedPaths'] = []
+            host_config['ReadonlyPaths'] = []
+
         # detached containers should only log to journald
         if self.params.get('detach'):
             options['log_config'] = docker.types.LogConfig(
diff --git a/ansible/module_utils/kolla_podman_worker.py b/ansible/module_utils/kolla_podman_worker.py
index faeff105f..ca06b100a 100644
--- a/ansible/module_utils/kolla_podman_worker.py
+++ b/ansible/module_utils/kolla_podman_worker.py
@@ -356,6 +356,11 @@ class PodmanWorker(ContainerWorker):
         if new_pid_mode != current_pid_mode:
             return True
 
+    def compare_systempaths_unconfined(self, container_info):
+        # Podman has no MaskedPaths/ReadonlyPaths HostConfig equivalent,
+        # so this Docker-only setting is never considered to differ.
+        return False
+
     def compare_image(self, container_info=None):
         def parse_tag(tag):
             splits = tag.rsplit('/', 1)
diff --git a/ansible/roles/service-check-containers/tasks/iterated.yml b/ansible/roles/service-check-containers/tasks/iterated.yml
index c4f28e6d4..a98c670fa 100644
--- a/ansible/roles/service-check-containers/tasks/iterated.yml
+++ b/ansible/roles/service-check-containers/tasks/iterated.yml
@@ -22,6 +22,7 @@
     labels: "{{ service.labels | default(omit) }}"
     command: "{{ service.command | default(omit) }}"
     cgroupns_mode: "{{ service.cgroupns_mode | default(omit) }}"
+    systempaths_unconfined: "{{ service.systempaths_unconfined | default(omit) }}"
   loop: "{{ range(1, (iterate_count | int) + 1) | list }}"
   when:
     # NOTE(dougszu): Due to issues with nova-compute-ironic HA, we can't have
diff --git a/ansible/roles/service-check-containers/tasks/main.yml b/ansible/roles/service-check-containers/tasks/main.yml
index 5a150d4d5..1f975c3b8 100644
--- a/ansible/roles/service-check-containers/tasks/main.yml
+++ b/ansible/roles/service-check-containers/tasks/main.yml
@@ -24,6 +24,7 @@
     labels: "{{ service.labels | default(omit) }}"
     command: "{{ service.command | default(omit) }}"
     cgroupns_mode: "{{ service.cgroupns_mode | default(omit) }}"
+    systempaths_unconfined: "{{ service.systempaths_unconfined | default(omit) }}"
   with_dict: "{{ lookup('vars', (kolla_role_name | default(project_name)) + '_services') | select_services_enabled_and_mapped_to_host }}"
   when: not (service.iterate | default(False)) | bool
   register: container_check
diff --git a/releasenotes/notes/kolla-docker-systempaths-unconfined-0b442bee04e88214.yaml b/releasenotes/notes/kolla-docker-systempaths-unconfined-0b442bee04e88214.yaml
new file mode 100644
index 000000000..9474451d3
--- /dev/null
+++ b/releasenotes/notes/kolla-docker-systempaths-unconfined-0b442bee04e88214.yaml
@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    Adds support in the ``kolla_docker`` module to clear Docker's default
+    ``MaskedPaths``/``ReadonlyPaths`` container hardening (equivalent to
+    ``docker run --security-opt systempaths=unconfined``) via the new
+    ``systempaths_unconfined`` module parameter. Docker only;
+    ignored under Podman.
diff --git a/tests/kolla_container_tests/test_docker_worker.py b/tests/kolla_container_tests/test_docker_worker.py
index e912bf5d7..1675d6126 100644
--- a/tests/kolla_container_tests/test_docker_worker.py
+++ b/tests/kolla_container_tests/test_docker_worker.py
@@ -356,6 +356,27 @@ class TestContainer(base.BaseTestCase):
         self.dw.dc.create_container.assert_called_once_with(
             **{k: self.fake_data['params'][k] for k in expected_args})
 
+    def test_create_container_with_systempaths_unconfined(self):
+        self.fake_data['params']['systempaths_unconfined'] = True
+        self.dw = get_DockerWorker(self.fake_data['params'])
+        self.dw.dc.create_host_config = mock.MagicMock(
+            return_value=self.fake_data['params']['host_config'])
+        self.dw.create_container()
+        self.assertEqual(
+            [], self.fake_data['params']['host_config']['MaskedPaths'])
+        self.assertEqual(
+            [], self.fake_data['params']['host_config']['ReadonlyPaths'])
+
+    def test_create_container_without_systempaths_unconfined(self):
+        self.dw = get_DockerWorker(self.fake_data['params'])
+        self.dw.dc.create_host_config = mock.MagicMock(
+            return_value=self.fake_data['params']['host_config'])
+        self.dw.create_container()
+        self.assertNotIn(
+            'MaskedPaths', self.fake_data['params']['host_config'])
+        self.assertNotIn(
+            'ReadonlyPaths', self.fake_data['params']['host_config'])
+
     def test_create_container_with_None_healthcheck(self):
         self.fake_data['params']['healthcheck'] = \
             {'test': ['NONE']}
@@ -1366,6 +1387,28 @@ class TestAttrComp(base.BaseTestCase):
         self.dw = get_DockerWorker({'privileged': False})
         self.assertTrue(self.dw.compare_privileged(container_info))
 
+    def test_compare_systempaths_unconfined_neg(self):
+        container_info = {'HostConfig': dict(MaskedPaths=[],
+                                             ReadonlyPaths=[])}
+        self.dw = get_DockerWorker({'systempaths_unconfined': True})
+        self.assertFalse(self.dw.compare_systempaths_unconfined(
+            container_info))
+
+    def test_compare_systempaths_unconfined_pos(self):
+        container_info = {'HostConfig': dict(MaskedPaths=['/proc/acpi'],
+                                             ReadonlyPaths=['/proc/sys'])}
+        self.dw = get_DockerWorker({'systempaths_unconfined': True})
+        self.assertTrue(self.dw.compare_systempaths_unconfined(
+            container_info))
+
+    def test_compare_systempaths_unconfined_ignore_when_privileged(self):
+        container_info = {'HostConfig': dict(MaskedPaths=['/proc/acpi'],
+                                             ReadonlyPaths=['/proc/sys'])}
+        self.dw = get_DockerWorker({'systempaths_unconfined': True,
+                                   'privileged': True})
+        self.assertFalse(self.dw.compare_systempaths_unconfined(
+            container_info))
+
     def test_compare_labels_image_missing(self):
         container_info = {'Config': dict(Labels={'kolla_version': '1.0.1'})}
         self.dw = get_DockerWorker({'labels': {}})
diff --git a/tests/kolla_container_tests/test_podman_worker.py b/tests/kolla_container_tests/test_podman_worker.py
index f05088981..18363f350 100644
--- a/tests/kolla_container_tests/test_podman_worker.py
+++ b/tests/kolla_container_tests/test_podman_worker.py
@@ -1292,6 +1292,12 @@ class TestAttrComp(base.BaseTestCase):
         self.pw = get_PodmanWorker({'pid_mode': 'host2'})
         self.assertTrue(self.pw.compare_pid_mode(container_info))
 
+    def test_compare_systempaths_unconfined_noop(self):
+        container_info = {'HostConfig': dict()}
+        self.pw = get_PodmanWorker({'systempaths_unconfined': True})
+        self.assertFalse(self.pw.compare_systempaths_unconfined(
+            container_info))
+
     def test_compare_cgroupns_mode_neg(self):
         container_info = {'HostConfig': dict(CgroupMode='host')}
         self.pw = get_PodmanWorker({'cgroupns_mode': 'host'})
diff --git a/tests/test_kolla_container.py b/tests/test_kolla_container.py
index 317d7880a..99892d7b0 100644
--- a/tests/test_kolla_container.py
+++ b/tests/test_kolla_container.py
@@ -77,6 +77,8 @@ class ModuleArgsTest(base.BaseTestCase):
             cgroupns_mode=dict(required=False, type='str',
                                choices=['private', 'host']),
             privileged=dict(required=False, type='bool', default=False),
+            systempaths_unconfined=dict(required=False, type='bool',
+                                        default=False),
             graceful_timeout=dict(required=False, type='int'),
             remove_on_exit=dict(required=False, type='bool', default=True),
             restart_policy=dict(