From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Sun, 5 Jun 2022 17:22:53 +0000 (-0400)
Subject: Print file check errors in the context of the pair that specified the file
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=899e90408e838fbb6a90dc8d223da4520ac7f4a9;p=thirdparty%2Ffreeradius-server.git

Print file check errors in the context of the pair that specified the file
---

diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c
index de45ccb5439..465b2cb8f2d 100644
--- a/src/lib/server/cf_file.c
+++ b/src/lib/server/cf_file.c
@@ -593,22 +593,22 @@ static int cf_file_open(CONF_SECTION *cs, char const *filename, bool from_dir, F
  *
  * @note Must be called with super user privileges.
  *
- * @param cs		currently being processed.
- * @param filename	to check.
+ * @param cp		currently being processed.
  * @param check_perms	If true - will return false if file is world readable,
  *			or not readable by the unprivileged user/group.
  * @return
  *	- true if permissions are OK, or the file exists.
  *	- false if the file does not exist or the permissions are incorrect.
  */
-bool cf_file_check(CONF_SECTION *cs, char const *filename, bool check_perms)
+bool cf_file_check(CONF_PAIR *cp, bool check_perms)
 {
 	cf_file_t	*file;
 	CONF_SECTION	*top;
 	fr_rb_tree_t	*tree;
+	char const 	*filename = cf_pair_value(cp);
 	int		fd = -1;
 
-	top = cf_root(cs);
+	top = cf_root(cp);
 	tree = cf_data_value(cf_data_find(top, fr_rb_tree_t, "filename"));
 	if (!tree) return false;
 
@@ -616,13 +616,13 @@ bool cf_file_check(CONF_SECTION *cs, char const *filename, bool check_perms)
 	if (!file) return false;
 
 	file->filename = talloc_strdup(file, filename);	/* The rest of the code expects this to be talloced */
-	file->cs = cs;
+	file->cs = cf_item_to_section(cf_parent(cp));
 
 	if (!check_perms) {
 		if (stat(filename, &file->buf) < 0) {
 		perm_error:
 			fr_perm_file_error(errno);	/* Write error and euid/egid to error buff */
-			PERROR("Unable to open file \"%s\"", filename);
+			cf_log_perr(cp, "Unable to open file \"%s\"", filename);
 		error:
 			if (fd >= 0) close(fd);
 			talloc_free(file);
@@ -643,31 +643,30 @@ bool cf_file_check(CONF_SECTION *cs, char const *filename, bool check_perms)
 
 		if ((conf_check_gid != (gid_t)-1) && ((egid = getegid()) != conf_check_gid)) {
 			if (setegid(conf_check_gid) < 0) {
-				ERROR("Failed setting effective group ID (%i) for file check: %s",
-				      conf_check_gid, fr_syserror(errno));
+				cf_log_perr(cp, "Failed setting effective group ID (%i) for file check: %s",
+					    conf_check_gid, fr_syserror(errno));
 				goto error;
 			}
 		}
 		if ((conf_check_uid != (uid_t)-1) && ((euid = geteuid()) != conf_check_uid)) {
 			if (seteuid(conf_check_uid) < 0) {
-				ERROR("Failed setting effective user ID (%i) for file check: %s",
-				      conf_check_uid, fr_syserror(errno));
+				cf_log_perr(cp, "Failed setting effective user ID (%i) for file check: %s",
+					    conf_check_uid, fr_syserror(errno));
 				goto error;
 			}
 		}
 		fd = open(filename, O_RDONLY);
 		if (conf_check_uid != euid) {
 			if (seteuid(euid) < 0) {
-				ERROR("Failed restoring effective user ID (%i) after file check: %s",
-				      euid, fr_syserror(errno));
-
+				cf_log_perr(cp, "Failed restoring effective user ID (%i) after file check: %s",
+					    euid, fr_syserror(errno));
 				goto error;
 			}
 		}
 		if (conf_check_gid != egid) {
 			if (setegid(egid) < 0) {
-				ERROR("Failed restoring effective group ID (%i) after file check: %s",
-				      egid, fr_syserror(errno));
+				cf_log_perr(cp, "Failed restoring effective group ID (%i) after file check: %s",
+					    egid, fr_syserror(errno));
 				goto error;
 			}
 		}
@@ -680,8 +679,8 @@ bool cf_file_check(CONF_SECTION *cs, char const *filename, bool check_perms)
 
 #ifdef S_IWOTH
 	if ((file->buf.st_mode & S_IWOTH) != 0) {
-		ERROR("Configuration file %s is globally writable.  "
-		      "Refusing to start due to insecure configuration.", filename);
+		cf_log_perr(cp, "Configuration file %s is globally writable.  "
+		            "Refusing to start due to insecure configuration.", filename);
 		talloc_free(file);
 		return false;
 	}
diff --git a/src/lib/server/cf_file.h b/src/lib/server/cf_file.h
index 9cb52ad1731..fb86ce316b7 100644
--- a/src/lib/server/cf_file.h
+++ b/src/lib/server/cf_file.h
@@ -51,7 +51,7 @@ int		cf_file_read(CONF_SECTION *cs, char const *file);
 int		cf_section_pass2(CONF_SECTION *cs);
 void		cf_file_free(CONF_SECTION *cs);
 
-bool		cf_file_check(CONF_SECTION *cs, char const *filename, bool check_perms);
+bool		cf_file_check(CONF_PAIR *cp, bool check_perms);
 void		cf_file_check_user(uid_t uid, gid_t gid);
 
 /*
diff --git a/src/lib/server/cf_parse.c b/src/lib/server/cf_parse.c
index 424a374b7e2..9be87611f32 100644
--- a/src/lib/server/cf_parse.c
+++ b/src/lib/server/cf_parse.c
@@ -145,12 +145,12 @@ int cf_pair_to_value_box(TALLOC_CTX *ctx, fr_value_box_t *out, CONF_PAIR *cp, CO
 		 *	to be caught as early as possible, during
 		 *	server startup.
 		 */
-		if (fr_rule_file_input(rule) && !cf_file_check(cs, cp->value, true)) {
+		if (fr_rule_file_input(rule) && !cf_file_check(cp, true)) {
 		error:
 			fr_value_box_clear(out);
 			return -1;
 		}
-		if (fr_rule_file_exists(rule) && !cf_file_check(cs, cp->value, false)) goto error;
+		if (fr_rule_file_exists(rule) && !cf_file_check(cp, false)) goto error;
 	}
 
 	return 0;