file-logger: Prevent potential race when opening log files and change permission

This fixes a potential TOCTOU issue with opening log files.  The use of
`chown()` instead of `fchown()` could theoretically allow modifying the
ownership of an unintended file.

The log file now also is not world-readable anymore.

Also, the patch fixes the log groups for the two `(f)chown()` errors.

Fixes: d35d6691807d ("Make syslog and file loggers configurable at runtime")

diff --git a/src/libcharon/bus/listeners/file_logger.c b/src/libcharon/bus/listeners/file_logger.c
index d5d2a3d6909068277dfd02a7d5861de1546a4d4d..f9f1f0f5121d471c2db7b70c6ebc71dc521230ae 100644 (file)
--- a/src/libcharon/bus/listeners/file_logger.c
+++ b/src/libcharon/bus/listeners/file_logger.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2024 Tobias Brunner
+ * Copyright (C) 2012-2026 Tobias Brunner
  * Copyright (C) 2006 Martin Willi
  *
  * Copyright (C) secunet Security Networks AG
@@ -19,6 +19,7 @@
 #include <string.h>
 #include <time.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <unistd.h>
 #include <sys/types.h>
 
@@ -306,32 +307,42 @@ METHOD(file_logger_t, open_, void,
        }
        else
        {
-               file = fopen(this->filename, append ? "a" : "w");
-               if (file == NULL)
+               int fd, flags = O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC);
+
+               fd = open(this->filename, flags, 0660);
+               if (fd < 0)
                {
-                       DBG1(DBG_DMN, "opening file %s for logging failed: %s",
+                       DBG1(DBG_DMN, "opening file '%s' for logging failed: %s",
                                 this->filename, strerror(errno));
                        return;
                }
 #ifdef HAVE_CHOWN
                if (lib->caps->check(lib->caps, CAP_CHOWN))
                {
-                       if (chown(this->filename, lib->caps->get_uid(lib->caps),
-                                         lib->caps->get_gid(lib->caps)) != 0)
+                       if (fchown(fd, lib->caps->get_uid(lib->caps),
+                                          lib->caps->get_gid(lib->caps)) != 0)
                        {
-                               DBG1(DBG_NET, "changing owner/group for '%s' failed: %s",
+                               DBG1(DBG_DMN, "changing owner/group for '%s' failed: %s",
                                         this->filename, strerror(errno));
                        }
                }
                else
                {
-                       if (chown(this->filename, -1, lib->caps->get_gid(lib->caps)) != 0)
+                       if (fchown(fd, -1, lib->caps->get_gid(lib->caps)) != 0)
                        {
-                               DBG1(DBG_NET, "changing group for '%s' failed: %s",
+                               DBG1(DBG_DMN, "changing group for '%s' failed: %s",
                                         this->filename, strerror(errno));
                        }
                }
 #endif /* HAVE_CHOWN */
+               file = fdopen(fd, append ? "a" : "w");
+               if (!file)
+               {
+                       DBG1(DBG_DMN, "opening file %s for logging failed: %s",
+                                this->filename, strerror(errno));
+                       close(fd);
+                       return;
+               }
 #ifdef HAVE_SETLINEBUF
                if (flush_line)
                {