From 9caa0d20bb293b977b3a0c2bee6fdce9a6de055a Mon Sep 17 00:00:00 2001 From: Michal Domonkos Date: Mon, 13 Jan 2025 12:19:06 +0100 Subject: [PATCH] Add patches for next release Resolves: RHEL-33393 RHEL-63070 --- ...mlinks-during-installation-as-a-spec.patch | 215 ++++++++++++++++++ ...files-getting-removed-on-failed-upda.patch | 46 ++++ rpm.spec | 6 +- 3 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 0001-Report-unsafe-symlinks-during-installation-as-a-spec.patch create mode 100644 0002-Fix-FA_TOUCH-ed-files-getting-removed-on-failed-upda.patch diff --git a/0001-Report-unsafe-symlinks-during-installation-as-a-spec.patch b/0001-Report-unsafe-symlinks-during-installation-as-a-spec.patch new file mode 100644 index 0000000..3da0ec0 --- /dev/null +++ b/0001-Report-unsafe-symlinks-during-installation-as-a-spec.patch @@ -0,0 +1,215 @@ +From fb192da52f3f390793dbaffe66c6148f497a8023 Mon Sep 17 00:00:00 2001 +From: Panu Matilainen +Date: Mon, 19 Aug 2024 11:03:10 +0300 +Subject: [PATCH 1/2] Report unsafe symlinks during installation as a specific + case + +RPM refuses to follow non root owned symlinks pointing to files owned by +another user for security reasons. This case was lumped in with +O_DIRECTORY behavior, leading to confusing error message as the symlink +often indeed points at a directory. Emit a more meaningful error message +when encountering unsafe symlinks. + +We already detect the error condition in the main if block here, might +as well set the error code right there and then so we don't need to +redetect later. We previously only tested for the unsafe link condition +when our O_DIRECTORY equivalent was set, but that seems wrong. Probably +doesn't matter with the existing callers, but we really must not +follow those unsafe symlinks no matter what. + +Co-authored-by: Florian Festi + +Backported from commits: +14516542c113560dc0070df2f9102568a7a71b58 +535eacc96ae6fe5289a2917bb0af43e491b0f4f4 + +Tests are excluded from this backport since they would need significant +rework, the use case will be covered by Beaker. + +Fixes: RHEL-33393 +--- + lib/fsm.c | 70 +++++++++++++++++++++++++++-------------------------- + lib/rpmfi.c | 1 + + 2 files changed, 37 insertions(+), 34 deletions(-) + +diff --git a/lib/fsm.c b/lib/fsm.c +index 9dd50b784..720d4a2ec 100644 +--- a/lib/fsm.c ++++ b/lib/fsm.c +@@ -65,7 +65,7 @@ struct filedata_s { + * things around needlessly + */ + static const char * fileActionString(rpmFileAction a); +-static int fsmOpenat(int dirfd, const char *path, int flags, int dir); ++static int fsmOpenat(int *fdp, int dirfd, const char *path, int flags, int dir); + static int fsmClose(int *wfdp); + + /** \ingroup payload +@@ -98,9 +98,9 @@ static int fsmLink(int odirfd, const char *opath, int dirfd, const char *path) + #ifdef WITH_CAP + static int cap_set_fileat(int dirfd, const char *path, cap_t fcaps) + { +- int rc = -1; +- int fd = fsmOpenat(dirfd, path, O_RDONLY|O_NOFOLLOW, 0); +- if (fd >= 0) { ++ int fd = -1; ++ int rc = fsmOpenat(&fd, dirfd, path, O_RDONLY|O_NOFOLLOW, 0); ++ if (!rc) { + rc = cap_set_fd(fd, fcaps); + fsmClose(&fd); + } +@@ -299,12 +299,12 @@ static int fsmMkdir(int dirfd, const char *path, mode_t mode) + return rc; + } + +-static int fsmOpenat(int dirfd, const char *path, int flags, int dir) ++static int fsmOpenat(int *wfdp, int dirfd, const char *path, int flags, int dir) + { + struct stat lsb, sb; + int sflags = flags | O_NOFOLLOW; + int fd = openat(dirfd, path, sflags); +- int ffd = fd; ++ int rc = 0; + + /* + * Only ever follow symlinks by root or target owner. Since we can't +@@ -313,7 +313,7 @@ static int fsmOpenat(int dirfd, const char *path, int flags, int dir) + * it could've only been the link owner or root. + */ + if (fd < 0 && errno == ELOOP && flags != sflags) { +- ffd = openat(dirfd, path, flags); ++ int ffd = openat(dirfd, path, flags); + if (ffd >= 0) { + if (fstatat(dirfd, path, &lsb, AT_SYMLINK_NOFOLLOW) == 0) { + if (fstat(ffd, &sb) == 0) { +@@ -322,17 +322,26 @@ static int fsmOpenat(int dirfd, const char *path, int flags, int dir) + } + } + } +- if (ffd != fd) ++ /* Symlink with non-matching owners */ ++ if (ffd != fd) { + close(ffd); ++ rc = RPMERR_INVALID_SYMLINK; ++ } + } + } + + /* O_DIRECTORY equivalent */ +- if (dir && ((fd != ffd) || (fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)))) { +- errno = ENOTDIR; ++ if (!rc && dir && fd >= 0 && fstat(fd, &sb) == 0 && !S_ISDIR(sb.st_mode)) ++ rc = RPMERR_ENOTDIR; ++ ++ if (!rc && fd < 0) ++ rc = RPMERR_OPEN_FAILED; ++ ++ if (rc) + fsmClose(&fd); +- } +- return fd; ++ ++ *wfdp = fd; ++ return rc; + } + + static int fsmDoMkDir(rpmPlugins plugins, int dirfd, const char *dn, +@@ -351,9 +360,7 @@ static int fsmDoMkDir(rpmPlugins plugins, int dirfd, const char *dn, + rc = fsmMkdir(dirfd, dn, mode); + + if (!rc) { +- *fdp = fsmOpenat(dirfd, dn, O_RDONLY|O_NOFOLLOW, 1); +- if (*fdp == -1) +- rc = RPMERR_ENOTDIR; ++ rc = fsmOpenat(fdp, dirfd, dn, O_RDONLY|O_NOFOLLOW, 1); + } + + if (!rc) { +@@ -378,47 +385,44 @@ static int ensureDir(rpmPlugins plugins, const char *p, int owned, int create, + char *sp = NULL, *bn; + char *apath = NULL; + int oflags = O_RDONLY; +- int rc = 0; + + if (*dirfdp >= 0) +- return rc; ++ return 0; + +- int dirfd = fsmOpenat(-1, "/", oflags, 1); ++ int dirfd = -1; ++ int rc = fsmOpenat(&dirfd, -1, "/", oflags, 1); + int fd = dirfd; /* special case of "/" */ + + char *path = xstrdup(p); + char *dp = path; + + while ((bn = strtok_r(dp, "/", &sp)) != NULL) { +- fd = fsmOpenat(dirfd, bn, oflags, 1); ++ rc = fsmOpenat(&fd, dirfd, bn, oflags, 1); + /* assemble absolute path for plugins benefit, sigh */ + apath = rstrscat(&apath, "/", bn, NULL); + +- if (fd < 0 && errno == ENOENT && create) { ++ if (rc && errno == ENOENT && create) { + mode_t mode = S_IFDIR | (_dirPerms & 07777); + rc = fsmDoMkDir(plugins, dirfd, bn, apath, owned, mode, &fd); + } + + fsmClose(&dirfd); +- if (fd >= 0) { +- dirfd = fd; +- } else { +- if (!quiet) { +- rpmlog(RPMLOG_ERR, _("failed to open dir %s of %s: %s\n"), +- bn, p, strerror(errno)); +- } +- rc = RPMERR_OPEN_FAILED; ++ if (rc) + break; +- } + ++ dirfd = fd; + dp = NULL; + } + + if (rc) { ++ if (!quiet) { ++ char *msg = rpmfileStrerror(rc); ++ rpmlog(RPMLOG_ERR, _("failed to open dir %s of %s: %s\n"), ++ bn, p, msg); ++ free(msg); ++ } + fsmClose(&fd); + fsmClose(&dirfd); +- } else { +- rc = 0; + } + *dirfdp = dirfd; + +@@ -1025,10 +1029,8 @@ setmeta: + /* Only follow safe symlinks, and never on temporary files */ + if (fp->suffix) + flags |= AT_SYMLINK_NOFOLLOW; +- fd = fsmOpenat(di.dirfd, fp->fpath, flags, ++ rc = fsmOpenat(&fd, di.dirfd, fp->fpath, flags, + S_ISDIR(fp->sb.st_mode)); +- if (fd < 0) +- rc = RPMERR_OPEN_FAILED; + } + + if (!rc && fp->setmeta) { +diff --git a/lib/rpmfi.c b/lib/rpmfi.c +index b5223a43f..e8e3ea294 100644 +--- a/lib/rpmfi.c ++++ b/lib/rpmfi.c +@@ -2436,6 +2436,7 @@ char * rpmfileStrerror(int rc) + case RPMERR_DIGEST_MISMATCH: s = _("Digest mismatch"); break; + case RPMERR_INTERNAL: s = _("Internal error"); break; + case RPMERR_UNMAPPED_FILE: s = _("Archive file not in header"); break; ++ case RPMERR_INVALID_SYMLINK: s = _("Unsafe symlink"); break; + case RPMERR_ENOENT: s = strerror(ENOENT); break; + case RPMERR_ENOTEMPTY: s = strerror(ENOTEMPTY); break; + case RPMERR_EXIST_AS_DIR: +-- +2.47.1 + diff --git a/0002-Fix-FA_TOUCH-ed-files-getting-removed-on-failed-upda.patch b/0002-Fix-FA_TOUCH-ed-files-getting-removed-on-failed-upda.patch new file mode 100644 index 0000000..a690b52 --- /dev/null +++ b/0002-Fix-FA_TOUCH-ed-files-getting-removed-on-failed-upda.patch @@ -0,0 +1,46 @@ +From 9eedcd6b770154a8d853db30b49d411823d64c44 Mon Sep 17 00:00:00 2001 +From: Panu Matilainen +Date: Fri, 18 Oct 2024 14:50:35 +0300 +Subject: [PATCH 2/2] Fix FA_TOUCH'ed files getting removed on failed update + +On install/update, most files are laid down with a temporary suffix +and if the update fails, removing those at the end of the loop is +the right thing to do. However FA_TOUCH'ed files were already there, +we only update their metadata, and we better not remove them! + +AFAICS this all versions since rpm >= 4.14 in one way or the other. +If %_minimize_writes is enabled then it affects way more than just +unmodified config files. + +The test is a simplified version of pam update failing in the original +report. Technically, --nomtime should not be needed for the test +verification but we don't even try to restore the metadata on failure, +and fixing that is way out of scope here. + +Backported from commits: +027ef640b33b38ca257bb301bb302e9c71d43c27 + +Tests are excluded from this backport since they would need significant +rework, the use case will be covered by Beaker. + +Fixes: RHEL-63070 +--- + lib/fsm.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/lib/fsm.c b/lib/fsm.c +index 720d4a2ec..91155c13c 100644 +--- a/lib/fsm.c ++++ b/lib/fsm.c +@@ -1093,7 +1093,7 @@ setmeta: + if (ensureDir(NULL, rpmfiDN(fi), 0, 0, 1, &di.dirfd)) + continue; + +- if (fp->stage > FILE_NONE && !fp->skip) { ++ if (fp->stage > FILE_NONE && !fp->skip && fp->action != FA_TOUCH) { + (void) fsmRemove(di.dirfd, fp->fpath, fp->sb.st_mode); + } + } +-- +2.47.1 + diff --git a/rpm.spec b/rpm.spec index 3c9ba0d..a0aa8c6 100644 --- a/rpm.spec +++ b/rpm.spec @@ -115,6 +115,8 @@ Patch146: 0001-Fix-potential-use-of-uninitialized-pgp-struct.patch Patch147: 0001-Add-SourceLicense-tag-to-spec-syntax.patch Patch148: 0001-Talk-about-rpmsign-in-the-rpmsign-man-page.patch Patch149: 0001-Allow-parametric-macros-to-opt-out-of-option-process.patch +Patch150: 0001-Report-unsafe-symlinks-during-installation-as-a-spec.patch +Patch151: 0002-Fix-FA_TOUCH-ed-files-getting-removed-on-failed-upda.patch # These are not yet upstream Patch906: rpm-4.7.1-geode-i686.patch @@ -667,8 +669,10 @@ fi %doc doc/librpm/html/* %changelog -* Tue Nov 12 2024 Miro Hrončok - 4.16.1.3-37 +* Mon Jan 13 2025 Michal Domonkos - 4.16.1.3-37 - Allow parametric macros to opt out of option processing (RHEL-67161) +- Report unsafe symlinks during installation as a specific case (RHEL-33393) +- Fix FA_TOUCH'ed files getting removed on failed update (RHEL-63070) * Wed Nov 06 2024 Michal Domonkos - 4.16.1.3-36 - Improve newly added %%patch warning/error messages (RHEL-6294)