From: Jim Fehlig <[email protected]> libvirt chown()s the swtpm log file to the swtpm user:group when starting a VM. The swtpm log directory is writable by swtmp user, who could replace the logfile with a symlink to a root-owned path. At next VM start, libvirt will chown() that path to the swtpm user:group, which breaks the intended separation between the confined swtpm account and root-owned files.
Use fchown() on an fd opened with O_NOFOLLOW to avoid the potential symlink attack. Signed-off-by: Jim Fehlig <[email protected]> --- src/qemu/qemu_tpm.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 660410bcba..34e11cc02f 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -1030,6 +1030,8 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm, uid_t qemu_user, const char *shortName) { + VIR_AUTOCLOSE logfd = -1; + /* create log dir ... allow 'tss' user to cd into it */ if (g_mkdir_with_parents(logDir, 0711) < 0) return -1; @@ -1039,13 +1041,20 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDef *tpm, VIR_DIR_CREATE_ALLOW_EXIST) < 0) return -1; - if (!virFileExists(tpm->data.emulator.logfile) && - virFileTouch(tpm->data.emulator.logfile, 0644) < 0) { + /* Open (creating if necessary) the logfile without following a + * symlink. The log directory is writable by swtpm_user, so we want + * to avoid chown'ing a symlink to an arbitrary path. + */ + if ((logfd = open(tpm->data.emulator.logfile, + O_WRONLY | O_CREAT | O_NOFOLLOW | O_CLOEXEC, 0644)) < 0) { + virReportSystemError(errno, + _("Could not open swtpm logfile %1$s"), + tpm->data.emulator.logfile); return -1; } /* ... and make sure it can be accessed by swtpm_user */ - if (chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { + if (fchown(logfd, swtpm_user, swtpm_group) < 0) { virReportSystemError(errno, _("Could not chown on swtpm logfile %1$s"), tpm->data.emulator.logfile); -- 2.51.0
