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