153 lines
5.3 KiB
Diff
153 lines
5.3 KiB
Diff
From 13f70e3710b2df49a923cc6450ff4a8f86e65666 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <13f70e3710b2df49a923cc6450ff4a8f86e65666.1555050140.git.pmatilai@redhat.com>
|
|
From: Panu Matilainen <pmatilai@redhat.com>
|
|
Date: Wed, 20 Mar 2019 12:38:00 +0200
|
|
Subject: [PATCH] Fix FA_TOUCH on files with suid/sgid bits and/or capabilities
|
|
|
|
FA_TOUCH used to set suffix to "" instead of NULL which causes fsmCommit()
|
|
to rename the file onto itself, which is a bit dumb but mostly harmless
|
|
with regular permission. On suid/sgid/capabilities we strip any extra
|
|
privileges on rename to make sure hardlinks are neutered, and because
|
|
rename occurs after other permissions etc setting, on FA_TOUCH those
|
|
extra privileges are stripped and much brokenness will follow.
|
|
|
|
A more minimal fix would be a strategically placed strcmp(), but NULL
|
|
is what the rest of the fsm expects for no suffix and differentiating
|
|
between empty and NULL suffix is too subtle for its own good as
|
|
witnessed here. So now, NULL suffix is no suffix again and the rest
|
|
of the code will do the right thing except where related to creation,
|
|
and creation is what FA_TOUCH wont do so lets just explicitly skip it
|
|
and restore the original code otherwise. The goto is ugly but reindenting
|
|
gets even uglier, shrug. Add a test-case to go with it.
|
|
|
|
This has been broken since its introduction in commit
|
|
79ca74e15e15c1d91a9a31a9ee90abc91736f390 so all current 4.14.x versions
|
|
are affected.
|
|
---
|
|
lib/fsm.c | 17 ++++++++++----
|
|
tests/data/SPECS/replacetest.spec | 2 +-
|
|
tests/rpmverify.at | 38 ++++++++++++++++++++++++++++++-
|
|
3 files changed, 50 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/lib/fsm.c b/lib/fsm.c
|
|
index 8eb2c185c..432bcbd90 100644
|
|
--- a/lib/fsm.c
|
|
+++ b/lib/fsm.c
|
|
@@ -898,12 +898,12 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
|
|
|
|
action = rpmfsGetAction(fs, rpmfiFX(fi));
|
|
skip = XFA_SKIPPING(action);
|
|
- suffix = S_ISDIR(rpmfiFMode(fi)) ? NULL : tid;
|
|
if (action != FA_TOUCH) {
|
|
- fpath = fsmFsPath(fi, suffix);
|
|
+ suffix = S_ISDIR(rpmfiFMode(fi)) ? NULL : tid;
|
|
} else {
|
|
- fpath = fsmFsPath(fi, "");
|
|
+ suffix = NULL;
|
|
}
|
|
+ fpath = fsmFsPath(fi, suffix);
|
|
|
|
/* Remap file perms, owner, and group. */
|
|
rc = rpmfiStat(fi, 1, &sb);
|
|
@@ -926,6 +926,10 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
|
|
if (!skip) {
|
|
int setmeta = 1;
|
|
|
|
+ /* When touching we don't need any of this... */
|
|
+ if (action == FA_TOUCH)
|
|
+ goto touch;
|
|
+
|
|
/* Directories replacing something need early backup */
|
|
if (!suffix) {
|
|
rc = fsmBackup(fi, action);
|
|
@@ -934,7 +938,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
|
|
if (!suffix) {
|
|
rc = fsmVerify(fpath, fi);
|
|
} else {
|
|
- rc = (action == FA_TOUCH) ? 0 : RPMERR_ENOENT;
|
|
+ rc = RPMERR_ENOENT;
|
|
}
|
|
|
|
if (S_ISREG(sb.st_mode)) {
|
|
@@ -970,11 +974,14 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
|
|
if (!IS_DEV_LOG(fpath))
|
|
rc = RPMERR_UNKNOWN_FILETYPE;
|
|
}
|
|
+
|
|
+touch:
|
|
/* Set permissions, timestamps etc for non-hardlink entries */
|
|
if (!rc && setmeta) {
|
|
rc = fsmSetmeta(fpath, fi, plugins, action, &sb, nofcaps);
|
|
}
|
|
} else if (firsthardlink >= 0 && rpmfiArchiveHasContent(fi)) {
|
|
+ /* On FA_TOUCH no hardlinks are created thus this is skipped. */
|
|
/* we skip the hard linked file containing the content */
|
|
/* write the content to the first used instead */
|
|
char *fn = rpmfilesFN(files, firsthardlink);
|
|
@@ -987,7 +994,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
|
|
if (rc) {
|
|
if (!skip) {
|
|
/* XXX only erase if temp fn w suffix is in use */
|
|
- if (suffix && (action != FA_TOUCH)) {
|
|
+ if (suffix) {
|
|
(void) fsmRemove(fpath, sb.st_mode);
|
|
}
|
|
errno = saveerrno;
|
|
diff --git a/tests/data/SPECS/replacetest.spec b/tests/data/SPECS/replacetest.spec
|
|
index 54974567b..d5a1729d3 100644
|
|
--- a/tests/data/SPECS/replacetest.spec
|
|
+++ b/tests/data/SPECS/replacetest.spec
|
|
@@ -46,4 +46,4 @@ rm -rf $RPM_BUILD_ROOT
|
|
|
|
%files
|
|
%defattr(-,%{user},%{grp},-)
|
|
-/opt/*
|
|
+%{?fileattr} /opt/*
|
|
diff --git a/tests/rpmverify.at b/tests/rpmverify.at
|
|
index 52ee2abfb..f7dd57531 100644
|
|
--- a/tests/rpmverify.at
|
|
+++ b/tests/rpmverify.at
|
|
@@ -575,3 +575,39 @@
|
|
],
|
|
[])
|
|
AT_CLEANUP
|
|
+
|
|
+AT_SETUP([Upgraded verification with min_writes 5 (suid files)])
|
|
+AT_KEYWORDS([upgrade verify min_writes])
|
|
+AT_CHECK([
|
|
+RPMDB_CLEAR
|
|
+RPMDB_INIT
|
|
+tf="${RPMTEST}"/opt/foo
|
|
+rm -rf "${tf}" "${tf}".rpm*
|
|
+rm -rf "${TOPDIR}"
|
|
+
|
|
+for v in "1.0" "2.0"; do
|
|
+ runroot rpmbuild --quiet -bb \
|
|
+ --define "ver $v" \
|
|
+ --define "filetype file" \
|
|
+ --define "filedata foo" \
|
|
+ --define "fileattr %attr(2755,-,-)" \
|
|
+ /data/SPECS/replacetest.spec
|
|
+done
|
|
+
|
|
+runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
|
|
+runroot rpm -Va --nouser --nogroup replacetest
|
|
+runroot rpm -U \
|
|
+ --define "_minimize_writes 1" \
|
|
+ /build/RPMS/noarch/replacetest-2.0-1.noarch.rpm
|
|
+runroot rpm -Va --nouser --nogroup replacetest
|
|
+chmod 777 "${tf}"
|
|
+runroot rpm -U \
|
|
+ --oldpackage \
|
|
+ --define "_minimize_writes 1" \
|
|
+ /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
|
|
+runroot rpm -Va --nouser --nogroup replacetest
|
|
+],
|
|
+[0],
|
|
+[],
|
|
+[])
|
|
+AT_CLEANUP
|
|
--
|
|
2.20.1
|
|
|