Skip to content

Commit

Permalink
Removed default installation of the config files for SSH and Slurm hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
Madeeks committed Sep 26, 2023
1 parent 006eb9a commit 329012b
Show file tree
Hide file tree
Showing 7 changed files with 303 additions and 194 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

### Changed

- The configuration files for the SSH hook and the Slurm sync hook are no longer generated automatically as part of the CMake installation process.
In other words, the aforementioned hooks are no longer configured and enabled by default.
- Updated recommended runc version to 1.1.9


### Fixed

- Fixed support for image manifests which are provided by registries as multi-line, not indented JSON
Expand Down
96 changes: 55 additions & 41 deletions CI/src/integration_tests/test_command_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,69 +7,83 @@

import unittest
import os
import subprocess


import common.util as util


class TestCommandHooks(unittest.TestCase):
"""
These tests verify the outputs "hooks" command
These tests verify the outputs of the "hooks" command
"""

def test_command_hooks(self):
expected_header = ["NAME", "PATH", "STAGES"]
actual_header = self._header_in_output_of_hooks_command()
actual_header = header_in_output_of_hooks_command()
self.assertEqual(actual_header, expected_header)

hook_output = self._hook_in_output_of_hooks_command("07-ssh-hook")
self.assertEqual(hook_output[1], "/opt/sarus/default/bin/ssh_hook")
self.assertEqual(hook_output[2], "prestart")

hook_output = self._hook_in_output_of_hooks_command("09-slurm-global-sync-hook")
self.assertEqual(hook_output[1], "/opt/sarus/default/bin/slurm_global_sync_hook")
self.assertEqual(hook_output[2], "prestart")
with util.temporary_hook_files((generate_dummy_hook_config(),
os.environ["CMAKE_INSTALL_PREFIX"] + "/etc/hooks.d/01-dummy-hook.json")):
hook_output = hook_in_output_of_hooks_command("01-dummy-hook")
self.assertEqual(hook_output[1], "/opt/sarus/default/bin/dummy_hook")
self.assertEqual(hook_output[2], "prestart")

def test_command_hooks_mpi(self):
expected_header = ["NAME", "MPI", "TYPE"]
actual_header = self._header_in_output_of_hooks_command(print_mpi_hooks=True)
actual_header = header_in_output_of_hooks_command(print_mpi_hooks=True)
self.assertEqual(actual_header, expected_header)

hook_config_paths = [os.environ["CMAKE_INSTALL_PREFIX"] + "/etc/hooks.d/050-mpi0-hook.json",
os.environ["CMAKE_INSTALL_PREFIX"] + "/etc/hooks.d/051-mpi1-hook.json"]

with util.temporary_hook_files((self._generate_mpi_hook_config("mpi0"), hook_config_paths[0]),
(self._generate_mpi_hook_config("mpi1"), hook_config_paths[1])):
with util.temporary_hook_files((generate_mpi_hook_config("mpi0"), hook_config_paths[0]),
(generate_mpi_hook_config("mpi1"), hook_config_paths[1])):
with util.custom_sarus_json({"defaultMPIType": "mpi0"}):
hook_output = self._hook_in_output_of_hooks_command("050-mpi0-hook", print_mpi_hooks=True)
hook_output = hook_in_output_of_hooks_command("050-mpi0-hook", print_mpi_hooks=True)
self.assertEqual(hook_output, ['050-mpi0-hook', '^mpi0$', '(default)'])

hook_output = self._hook_in_output_of_hooks_command("051-mpi1-hook", print_mpi_hooks=True)
hook_output = hook_in_output_of_hooks_command("051-mpi1-hook", print_mpi_hooks=True)
self.assertEqual(hook_output, ['051-mpi1-hook', '^mpi1$'])

def _header_in_output_of_hooks_command(self, print_mpi_hooks=False):
return util.get_hooks_command_output(print_mpi_hooks)[0]

def _hook_in_output_of_hooks_command(self, hook_name, print_mpi_hooks=False):
output = util.get_hooks_command_output(print_mpi_hooks=print_mpi_hooks)
for line in output:
if line[0] == hook_name:
return line

def _generate_mpi_hook_config(self, mpi_type):
config = {
"version": "1.0.0",
"hook": {
"path": os.environ["CMAKE_INSTALL_PREFIX"] + "/bin/mpi_hook",
"env": []
},
"when": {
"annotations": {
"^com.hooks.mpi.enabled$": "^true$",
"^com.hooks.mpi.type": f"^{mpi_type}$"
}
},
"stages": ["prestart"]
}
return config

def header_in_output_of_hooks_command(print_mpi_hooks=False):
return util.get_hooks_command_output(print_mpi_hooks)[0]


def hook_in_output_of_hooks_command(hook_name, print_mpi_hooks=False):
output = util.get_hooks_command_output(print_mpi_hooks=print_mpi_hooks)
for line in output:
if line[0] == hook_name:
return line


def generate_dummy_hook_config():
config = {
"version": "1.0.0",
"hook": {
"path": os.environ["CMAKE_INSTALL_PREFIX"] + "/bin/dummy_hook",
"env": []
},
"when": {
"always": True
},
"stages": ["prestart"]
}
return config


def generate_mpi_hook_config(mpi_type):
config = {
"version": "1.0.0",
"hook": {
"path": os.environ["CMAKE_INSTALL_PREFIX"] + "/bin/mpi_hook",
"env": []
},
"when": {
"annotations": {
"^com.hooks.mpi.enabled$": "^true$",
"^com.hooks.mpi.type": f"^{mpi_type}$"
}
},
"stages": ["prestart"]
}
return config
108 changes: 61 additions & 47 deletions CI/src/integration_tests/test_security_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,73 +40,87 @@ class TestSecurityChecks(unittest.TestCase):

def setUp(self):
# To make sure these tests start from a sane default state
self._sarus_foo()
sarus_foo()

def tearDown(self):
# To make sure these tests are not altering config files permissions
self._sarus_foo()
sarus_foo()

def test_untamperable_config(self):
self._check_untamperable(CONFIG_FILENAME)
check_untamperable(CONFIG_FILENAME)

def test_untamperable_config_schema(self):
self._check_untamperable(SCHEMA_FILENAME)
check_untamperable(SCHEMA_FILENAME)

def test_disabled_security_checks(self):
with util.custom_sarus_json({"securityChecks": False}):
# some tamperable locations are OK...
with changed_owner("/opt/sarus/default/bin/", NONROOT_ID, NONROOT_ID):
self._sarus_foo()
sarus_foo()

# but not these two...
self._check_untamperable(CONFIG_FILENAME)
self._check_untamperable(SCHEMA_FILENAME)
check_untamperable(CONFIG_FILENAME)
check_untamperable(SCHEMA_FILENAME)

def test_untamperable_binaries(self):
INIT_PATH = self._get_parameter_from_config_file("initPath")
RUNC_PATH = self._get_parameter_from_config_file("runcPath")
init_path = get_parameter_from_config_file("initPath")
runc_path = get_parameter_from_config_file("runcPath")

self._check_untamperable(INIT_PATH)
self._check_untamperable(RUNC_PATH)
check_untamperable(init_path)
check_untamperable(runc_path)

self._check_untamperable("/opt/sarus/default/bin/")
check_untamperable("/opt/sarus/default/bin/")

def test_untamperable_hooks_and_deps(self):
# ssh is only default hook enabled
self._check_untamperable("/opt/sarus/default/etc/hooks.d/07-ssh-hook.json")
self._check_untamperable("/opt/sarus/default/bin/ssh_hook")
self._check_untamperable("/opt/sarus/default/dropbear")

def _get_parameter_from_config_file(self, parameter):
with open(CONFIG_FILENAME) as conf_file:
conf = json.load(conf_file)
return conf[parameter]

def _sarus_foo(self):
"""
Performs the simplest Sarus action so Security Checks are run.
"""
with open(os.devnull, 'wb') as devnull:
res = subprocess.run(SARUS_FOO_CMD, stdout=devnull, check=True)
assert res.returncode == 0

def _check_untamperable(self, path):
if os.path.isdir(path):
entry = path if path[-1] != '/' else path[:-1]
else:
entry = os.path.basename(path)

with changed_owner(path, NONROOT_ID, NONROOT_ID):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" must be owned by root'.format(entry))

with changed_permissions(path, stat.S_IWGRP):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" cannot be group- or world-writable'.format(entry))

with changed_permissions(path, stat.S_IWOTH):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" cannot be group- or world-writable'.format(entry))
hook_config = {
"version": "1.0.0",
"hook": {
"path": os.environ["CMAKE_INSTALL_PREFIX"] + "/bin/timestamp_hook"
},
"when": {
"always": True
},
"stages": ["prestart"]
}
hook_config_file = "/opt/sarus/default/etc/hooks.d/00-timestamp-hook.json"
with util.temporary_hook_files((hook_config, hook_config_file)):
check_untamperable(hook_config_file)
check_untamperable("/opt/sarus/default/bin/timestamp_hook")
check_untamperable("/opt/sarus/default/dropbear")


def get_parameter_from_config_file(parameter):
with open(CONFIG_FILENAME) as conf_file:
conf = json.load(conf_file)
return conf[parameter]


def sarus_foo():
"""
Performs the simplest Sarus action so Security Checks are run.
"""
with open(os.devnull, 'wb') as devnull:
res = subprocess.run(SARUS_FOO_CMD, stdout=devnull, check=True)
assert res.returncode == 0


def check_untamperable(path):
if os.path.isdir(path):
entry = path if path[-1] != '/' else path[:-1]
else:
entry = os.path.basename(path)

with changed_owner(path, NONROOT_ID, NONROOT_ID):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" must be owned by root'.format(entry))

with changed_permissions(path, stat.S_IWGRP):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" cannot be group- or world-writable'.format(entry))

with changed_permissions(path, stat.S_IWOTH):
util.assert_sarus_raises_error_containing_text(SARUS_FOO_CMD,
'{}" cannot be group- or world-writable'.format(entry))


@contextmanager
Expand Down
Loading

0 comments on commit 329012b

Please sign in to comment.