From 5f3274417c0cfa54841f2817eb9a3c5168846ec1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Sat, 3 Aug 2024 19:07:38 +0200
Subject: [PATCH] Avoid opening log file for getting SELinux context

Currently setSecCtxByName() uses open_logfile() to get a file descriptor
to the current log file to retrieve its security context.
open_logfile() performs additional checks, like whether the file is a
regular file, which alter the control flow between systems with SELinux
enabled and disabled.  This can be observed in the reported issue #632.
Use lgetfilecon_raw() instead to have the same behavior for SELinux
enabled and disabled systems and delay the checks for invalid log files
to code executed in both cases.

Closes: #632
(cherry picked from commit e4a833015eb5590cf86a6a78895ff399174ade4f)
---
 logrotate.c | 89 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 51 insertions(+), 38 deletions(-)

diff --git a/logrotate.c b/logrotate.c
index 1292b46..40b5295 100644
--- a/logrotate.c
+++ b/logrotate.c
@@ -360,27 +360,9 @@ static int movefd(int oldfd, int newfd)
     return rc;
 }
 
-static int setSecCtx(int fdSrc, const char *src, char **pPrevCtx)
-{
 #ifdef WITH_SELINUX
-    char *srcCtx;
-    *pPrevCtx = NULL;
-
-    if (!selinux_enabled)
-        /* pretend success */
-        return 0;
-
-    /* read security context of fdSrc */
-    if (fgetfilecon_raw(fdSrc, &srcCtx) < 0) {
-        if (errno == ENOTSUP)
-            /* pretend success */
-            return 0;
-
-        message(MESS_ERROR, "getting file context %s: %s\n", src,
-                strerror(errno));
-        return selinux_enforce;
-    }
-
+static int setSecCtx(char *srcCtx, char **pPrevCtx)
+{
     /* save default security context for restoreSecCtx() */
     if (getfscreatecon_raw(pPrevCtx) < 0) {
         message(MESS_ERROR, "getting default context: %s\n", strerror(errno));
@@ -400,37 +382,68 @@ static int setSecCtx(int fdSrc, const char *src, char **pPrevCtx)
 
     message(MESS_DEBUG, "set default create context to %s\n", srcCtx);
     freecon(srcCtx);
+
+    return 0;
+}
+#endif /* WITH_SELINUX */
+
+static int setSecCtxByFd(int fdSrc, const char *src, char **pPrevCtx)
+{
+#ifdef WITH_SELINUX
+    char *srcCtx;
+    *pPrevCtx = NULL;
+
+    if (!selinux_enabled)
+        /* pretend success */
+        return 0;
+
+    /* read security context of fdSrc */
+    if (fgetfilecon_raw(fdSrc, &srcCtx) < 0) {
+        if (errno == ENOTSUP)
+            /* pretend success */
+            return 0;
+
+        message(MESS_ERROR, "getting file context %s: %s\n", src,
+                strerror(errno));
+        return selinux_enforce;
+    }
+
+    return setSecCtx(srcCtx, pPrevCtx);
 #else
     (void) fdSrc;
     (void) src;
     (void) pPrevCtx;
-#endif
     return 0;
+#endif /* WITH_SELINUX */
 }
 
-static int setSecCtxByName(const char *src, const struct logInfo *log, char **pPrevCtx)
+static int setSecCtxByName(const char *src, char **pPrevCtx)
 {
-    int hasErrors = 0;
 #ifdef WITH_SELINUX
-    int fd;
+    char *srcCtx;
+    *pPrevCtx = NULL;
 
     if (!selinux_enabled)
         /* pretend success */
         return 0;
 
-    fd = open_logfile(src, log, 0);
-    if (fd < 0) {
-        message(MESS_ERROR, "error opening %s: %s\n", src, strerror(errno));
-        return 1;
+    /* read security context of src */
+    if (lgetfilecon_raw(src, &srcCtx) < 0) {
+        if (errno == ENOTSUP)
+            /* pretend success */
+            return 0;
+
+        message(MESS_ERROR, "getting file context %s: %s\n", src,
+                strerror(errno));
+        return selinux_enforce;
     }
-    hasErrors = setSecCtx(fd, src, pPrevCtx);
-    close(fd);
+
+    return setSecCtx(srcCtx, pPrevCtx);
 #else
     (void) src;
-    (void) log;
     (void) pPrevCtx;
-#endif
-    return hasErrors;
+    return 0;
+#endif /* WITH_SELINUX */
 }
 
 static void restoreSecCtx(char **pPrevCtx)
@@ -853,7 +866,7 @@ static int compressLogFile(const char *name, const struct logInfo *log, const st
         return 1;
     }
 
-    if (setSecCtx(inFile, name, &prevCtx) != 0) {
+    if (setSecCtxByFd(inFile, name, &prevCtx) != 0) {
         /* error msg already printed */
         close(inFile);
         return 1;
@@ -1290,7 +1303,7 @@ static int copyTruncate(const char *currLog, const char *saveLog, const struct s
         if (!skip_copy) {
             char *prevCtx;
 
-            if (setSecCtx(fdcurr, currLog, &prevCtx) != 0) {
+            if (setSecCtxByFd(fdcurr, currLog, &prevCtx) != 0) {
                 /* error msg already printed */
                 goto fail;
             }
@@ -1888,7 +1901,7 @@ static int prerotateSingleLog(const struct logInfo *log, unsigned logNum,
     message(MESS_DEBUG, "dateext suffix '%s'\n", dext_str);
     message(MESS_DEBUG, "glob pattern '%s'\n", dext_pattern);
 
-    if (setSecCtxByName(log->files[logNum], log, &prev_context) != 0) {
+    if (setSecCtxByName(log->files[logNum], &prev_context) != 0) {
         /* error msg already printed */
         return 1;
     }
@@ -2169,7 +2182,7 @@ static int rotateSingleLog(const struct logInfo *log, unsigned logNum,
     if (!hasErrors) {
 
         if (!(log->flags & (LOG_FLAG_COPYTRUNCATE | LOG_FLAG_COPY))) {
-            if (setSecCtxByName(log->files[logNum], log, &savedContext) != 0) {
+            if (setSecCtxByName(log->files[logNum], &savedContext) != 0) {
                 /* error msg already printed */
                 return 1;
             }
@@ -2711,7 +2724,7 @@ static int writeState(const char *stateFilename)
 
     /* get attributes, to assign them to the new state file */
 
-    if (setSecCtx(fdcurr, stateFilename, &prevCtx) != 0) {
+    if (setSecCtxByFd(fdcurr, stateFilename, &prevCtx) != 0) {
         /* error msg already printed */
         free(tmpFilename);
         close(fdcurr);
-- 
2.46.2