Hi!

I have received error report on Fedora [1] with problem of log-facility
used. Problem happens because systemd does not grant even root processes
cap_dac_override. Therefore log file created by dnsmasq has to be
writable by dnsmasq user. But it is created/opened when still did not
drop privileges under effective uid == 0. It leads to surprising
situation. If dnsmasq with log-facility=/var/log/dnsmasq.log is started
first time, it passes just fine. However when it is started second time
with /var/log/dnsmasq owned by dnsmasq:root, mode 0640, it fails opening
the log.

My attached patch fixes it. If dnsmasq were started as root, it gives
also group write right to log file. Because it does not change group of
file, it should be always root. My patch expects log file group is not
changed in mean time. If it is something different, for example adm
group, skip granting writeable flag. It fixes the problem at hand.

What do you think Simon? Others?

Cheers,
Petr

1. https://bugzilla.redhat.com/show_bug.cgi?id=2024166

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From d76d835d7c689f962102cc8bf305b141574df396 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Mon, 10 Jan 2022 12:34:42 +0100
Subject: [PATCH] Add root group writeable flag to log file

Some systems strips even root process capability of writing to different
users file. That include systemd under Fedora. When
log-facility=/tmp/log is used, log file with mode 0640 is created. But
restart then fails, because such log file can be used only when created
new. Existing file cannot be opened by root when starting, causing fatal
error. Avoid that by adding root group writeable flag.

Ensure group is always root when granting write access. If it is
anything else, administrator has to configure correct rights.
---
 src/log.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/log.c b/src/log.c
index 1ec3447..d10d17e 100644
--- a/src/log.c
+++ b/src/log.c
@@ -101,9 +101,15 @@ int log_start(struct passwd *ent_pw, int errfd)
      change the ownership here so that the file is always owned by
      the dnsmasq user. Then logrotate can just copy the owner.
      Failure of the chown call is OK, (for instance when started as non-root) */
-  if (log_to_file && !log_stderr && ent_pw && ent_pw->pw_uid != 0 && 
-      fchown(log_fd, ent_pw->pw_uid, -1) != 0)
-    ret = errno;
+  if (log_to_file && !log_stderr && ent_pw && ent_pw->pw_uid != 0)
+    {
+      struct stat ls;
+      if (getgid() == 0 && fstat(log_fd, &ls) == 0 && ls.st_gid == 0 &&
+	  (ls.st_mode & S_IWGRP) == 0)
+	(void)fchmod(log_fd, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP);
+      if (fchown(log_fd, ent_pw->pw_uid, -1) != 0)
+	ret = errno;
+    }
 
   return ret;
 }
@@ -118,7 +124,7 @@ int log_reopen(char *log_file)
       /* NOTE: umask is set to 022 by the time this gets called */
       
       if (log_file)
-	log_fd = open(log_file, O_WRONLY|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR|S_IRGRP);      
+	log_fd = open(log_file, O_WRONLY|O_CREAT|O_APPEND, S_IRUSR|S_IWUSR|S_IRGRP);
       else
 	{
 #if defined(HAVE_SOLARIS_NETWORK) || defined(__ANDROID__)
-- 
2.31.1

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

Reply via email to