From a2b63dae13b3d7fd3f5b75b02ac6c1021b792bb5 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Wed, 4 Oct 2023 14:02:06 -0600 Subject: [PATCH 1/3] util: ensure log file is root owned --- subiquitycore/file_util.py | 2 +- subiquitycore/tests/test_file_util.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/subiquitycore/file_util.py b/subiquitycore/file_util.py index 3e67194c9..25701a9c4 100644 --- a/subiquitycore/file_util.py +++ b/subiquitycore/file_util.py @@ -44,7 +44,7 @@ def set_log_perms(target, *, group_write=False, mode=None, group=_DEF_GROUP): if group_write: mode |= 0o020 os.chmod(target, mode) - os.chown(target, -1, grp.getgrnam(group).gr_gid) + os.chown(target, 0, grp.getgrnam(group).gr_gid) @contextlib.contextmanager diff --git a/subiquitycore/tests/test_file_util.py b/subiquitycore/tests/test_file_util.py index 2d522eef0..4702fa46c 100644 --- a/subiquitycore/tests/test_file_util.py +++ b/subiquitycore/tests/test_file_util.py @@ -64,52 +64,52 @@ def test_defaults_file(self): Path(target).touch() set_log_perms(target) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_defaults_dir(self): target = self.tmp_dir() set_log_perms(target) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE | 0o110) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_group_write_file(self): target = self.tmp_path("file") Path(target).touch() set_log_perms(target, group_write=True) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE | 0o020) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_group_write_dir(self): target = self.tmp_dir() set_log_perms(target, group_write=True) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE | 0o130) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_nogroup_write_file(self): target = self.tmp_path("file") Path(target).touch() set_log_perms(target, group_write=False) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_nogroup_write_dir(self): target = self.tmp_dir() set_log_perms(target, group_write=False) self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE | 0o110) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_mode_file(self): target = self.tmp_path("file") Path(target).touch() set_log_perms(target, mode=0o510) self.chmod.assert_called_once_with(target, 0o510) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_mode_dir(self): target = self.tmp_dir() set_log_perms(target, mode=0o510) self.chmod.assert_called_once_with(target, 0o510) - self.chown.assert_called_once_with(target, -1, self.mock_gid) + self.chown.assert_called_once_with(target, 0, self.mock_gid) def test_group_file(self): self.getgrnam.return_value = Mock(gr_gid=11) @@ -117,11 +117,11 @@ def test_group_file(self): Path(target).touch() set_log_perms(target, group="group1") self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE) - self.chown.assert_called_once_with(target, -1, 11) + self.chown.assert_called_once_with(target, 0, 11) def test_group_dir(self): self.getgrnam.return_value = Mock(gr_gid=11) target = self.tmp_dir() set_log_perms(target, group="group1") self.chmod.assert_called_once_with(target, _DEF_PERMS_FILE | 0o110) - self.chown.assert_called_once_with(target, -1, 11) + self.chown.assert_called_once_with(target, 0, 11) From d3debfcea3bb3bd420af6ad1f24714bd74dff23e Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Wed, 4 Oct 2023 14:24:46 -0600 Subject: [PATCH 2/3] shutdown: refactor cloud init logs logic --- subiquity/server/controllers/shutdown.py | 27 ++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/subiquity/server/controllers/shutdown.py b/subiquity/server/controllers/shutdown.py index 503d0ce31..95a9e959a 100644 --- a/subiquity/server/controllers/shutdown.py +++ b/subiquity/server/controllers/shutdown.py @@ -83,6 +83,19 @@ async def _run(self): elif self.app.state == ApplicationState.DONE: await self.shutdown() + async def copy_cloud_init_logs(self, target_logs): + # Preserve ephemeral boot cloud-init logs if applicable + cloudinit_logs = ( + "/var/log/cloud-init.log", + "/var/log/cloud-init-output.log", + ) + for logfile in cloudinit_logs: + if not os.path.exists(logfile): + continue + await self.app.command_runner.run( + ["cp", "-a", logfile, "/var/log/installer"] + ) + @with_context() async def copy_logs_to_target(self, context): if self.opts.dry_run and "copy-logs-fail" in self.app.debug_flags: @@ -96,19 +109,7 @@ async def copy_logs_to_target(self, context): if self.opts.dry_run: os.makedirs(target_logs, exist_ok=True) else: - # Preserve ephemeral boot cloud-init logs if applicable - cloudinit_logs = [ - cloudinit_log - for cloudinit_log in ( - "/var/log/cloud-init.log", - "/var/log/cloud-init-output.log", - ) - if os.path.exists(cloudinit_log) - ] - if cloudinit_logs: - await self.app.command_runner.run( - ["cp", "-a"] + cloudinit_logs + ["/var/log/installer"] - ) + await self.copy_cloud_init_logs(target_logs) await self.app.command_runner.run( ["cp", "-aT", "/var/log/installer", target_logs] ) From ab0af6375e7075fd21eece2de8752bf090347358 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Wed, 4 Oct 2023 14:01:51 -0600 Subject: [PATCH 3/3] shutdown: fix owner on cloud-init logs These have owner syslog at install time, but that is uid remapped on the target system which may end up with a different owning user. --- subiquity/server/controllers/shutdown.py | 1 + 1 file changed, 1 insertion(+) diff --git a/subiquity/server/controllers/shutdown.py b/subiquity/server/controllers/shutdown.py index 95a9e959a..5e27e5c9c 100644 --- a/subiquity/server/controllers/shutdown.py +++ b/subiquity/server/controllers/shutdown.py @@ -92,6 +92,7 @@ async def copy_cloud_init_logs(self, target_logs): for logfile in cloudinit_logs: if not os.path.exists(logfile): continue + set_log_perms(logfile) await self.app.command_runner.run( ["cp", "-a", logfile, "/var/log/installer"] )