fix ownership of log file before dropping privs, don't segfault on pe…

…rmission problem with log file

refs #3521

Changelog

@@ -45,7 +45,7 @@ FIXES
 * core: fix faulty macro cleaning, replacing spaces with pluses where they shouldn't be cleaned #3397 - MF
 * core: fix macro escaping logs incorrect warning for $$escapes #3404 - MF
 * core: fix wrong escalation notification due to state based escalation range behaviour changes #3441 - MF
-* core: change ownership of debug log file before dropping privileges (Eric Stanley) #3521 - MF
+* core: change ownership of (debug) log file before dropping privileges (Andreas Ericsson) #3521 - MF
 * core: fix keep_unknown_macros still exposes wrong warnings to logs #3725 - MF
 * core: fix host_check, last_check == next_check wrong in scheduling queue #2195 - MF
 * core: fix triggered downtimes for child hosts are missing after icinga restart (thx Michael Lucka) #3390 - MF

base/logging.c

@@ -216,13 +216,37 @@ FILE *open_log_file(void) {
 
 	log_fp = fopen(log_file, "a+");
 
-	if (log_fp == NULL && daemon_mode == FALSE) {
-		printf("Warnings: Cannot open log file '%s' for writing\n", log_file);
+	if (log_fp == NULL) {
+		if (daemon_mode == FALSE) {
+			printf("Warnings: Cannot open log file '%s' for writing\n", log_file);
+		}
+		return NULL;
 	}
 
+	(void)fcntl(fileno(log_fp), F_SETFD, FD_CLOEXEC);
+
 	return log_fp;
 }
 
+int fix_log_file_owner(uid_t uid, gid_t gid) {
+
+	int r1 = 0, r2 = 0;
+
+	if (!(log_fp = open_log_file()))
+		return -1;
+
+	r1 = fchown(fileno(log_fp), uid, gid);
+
+	if (open_debug_log() != OK)
+		return -1;
+
+	if (debug_file_fp)
+		r2 = fchown(fileno(debug_file_fp), uid, gid);
+
+	/* return 0 if both are 0 and otherwise < 0 */
+	return r1 < r2 ? r1 : r2;
+}
+
 int close_log_file(void) {
 
 	if (!log_fp)
@@ -258,6 +282,9 @@ int write_to_log(char *buffer, unsigned long data_type, time_t *timestamp) {
 
 	fp = open_log_file();
 
+	if (fp == NULL)
+		return ERROR;
+
 	/* what timestamp should we use? */
 	if (timestamp == NULL)
 		time(&log_time);
@@ -590,22 +617,7 @@ int open_debug_log(void) {
 	if ((debug_file_fp = fopen(debug_file, "a+")) == NULL)
 		return ERROR;
 
-	return OK;
-}
-
-/* change ownership of the debug log file */
-int chown_debug_log(uid_t uid, gid_t gid) {
-
-	/* bail early if not running */
-	if (verify_config == TRUE || test_scheduling == TRUE)
-		return OK;
-
-	/* we do not debug anything, bail early */
-	if (debug_level == DEBUGL_NONE)
-		return OK;
-
-	if (chown(debug_file, uid, gid) < 0)
-		return ERROR;
+	(void)fcntl(fileno(debug_file_fp), F_SETFD, FD_CLOEXEC);
 
 	return OK;
 }

base/utils.c

@@ -2568,17 +2568,8 @@ int drop_privileges(char *user, char *group) {
 		else
 			gid = (gid_t)atoi(group);
 
-		/* set effective group ID if other than current EGID */
-		if (gid != getegid()) {
-
-			if (setgid(gid) == -1) {
-				logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Could not set effective GID=%d", (int)gid);
-				result = ERROR;
-			}
-		}
 	}
 
-
 	/* set effective user ID */
 	if (user != NULL) {
 
@@ -2594,37 +2585,39 @@ int drop_privileges(char *user, char *group) {
 		/* else we were passed the UID */
 		else
 			uid = (uid_t)atoi(user);
+	}
+
+	/* now that we know what to change to, we fix log file permissions */
+	fix_log_file_owner(uid, gid);
+
+	/* set effective group ID if other than current EGID */
+	if (gid != getegid()) {
+
+		if (setgid(gid) == -1) {
+			logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Could not set effective GID=%d", (int)gid);
+			result = ERROR;
+		}
+	}
 
 #ifdef HAVE_INITGROUPS
 
-		if (uid != geteuid()) {
+	if (uid != geteuid()) {
 
-			/* initialize supplementary groups */
-			if (initgroups(user, gid) == -1) {
-				if (errno == EPERM)
-					logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Unable to change supplementary groups using initgroups() -- I hope you know what you're doing");
-				else {
-					logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Possibly root user failed dropping privileges with initgroups()");
-					return ERROR;
-				}
+		/* initialize supplementary groups */
+		if (initgroups(user, gid) == -1) {
+			if (errno == EPERM)
+				logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Unable to change supplementary groups using initgroups() -- I hope you know what you're doing");
+			else {
+				logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Possibly root user failed dropping privileges with initgroups()");
+				return ERROR;
 			}
 		}
+	}
 #endif
 
-		/* change ownership of debug log file
-		 * this is required in order to re-open
-		 * the file when receiving a SIGHUP, after
-		 * creating the file with root privileges
-		 */
-		if (chown_debug_log(uid, gid) == ERROR) {
-			logit(NSLOG_RUNTIME_WARNING, TRUE, "Failed to change ownership (UID=%d, GID=%d) on debug log file '%s': %s.", (int)uid, (int)gid, debug_file, strerror(errno));
-			result = ERROR;
-		}
-
-		if (setuid(uid) == -1) {
-			logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Could not set effective UID=%d", (int)uid);
-			result = ERROR;
-		}
+	if (setuid(uid) == -1) {
+		logit(NSLOG_RUNTIME_WARNING, TRUE, "Warning: Could not set effective UID=%d", (int)uid);
+		result = ERROR;
 	}
 
 	log_debug_info(DEBUGL_PROCESS, 0, "New UID/GID: %d/%d\n", (int)getuid(), (int)getgid());