diff --git a/src/config/toml_helper.c b/src/config/toml_helper.c index 26c6b1dd..22b85dd0 100644 --- a/src/config/toml_helper.c +++ b/src/config/toml_helper.c @@ -23,6 +23,8 @@ #include // escape_json() #include "webserver/http-common.h" +// chown_pihole() +#include "files.h" // Open the TOML file for reading or writing FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *mode, const unsigned int version) @@ -96,26 +98,8 @@ void closeFTLtoml(FILE *fp) // Chown file if we are root if(geteuid() == 0) - { - // Get UID and GID of user with name "pihole" - struct passwd *pwd = getpwnam("pihole"); - if(pwd == NULL) - { - log_warn("Cannot get UID and GID of user pihole: %s", strerror(errno)); - } - else - { - const uid_t pihole_uid = pwd->pw_uid; - const gid_t pihole_gid = pwd->pw_gid; - // Chown file to pihole user - if(chown(GLOBALTOMLPATH, pihole_uid, pihole_gid) != 0) - log_warn("Cannot chown "GLOBALTOMLPATH" to pihole:pihole (%u:%u): %s", - (unsigned int)pihole_uid, (unsigned int)pihole_gid, strerror(errno)); - else - log_debug(DEBUG_CONFIG, "Chown-ed "GLOBALTOMLPATH" to pihole:pihole (%u:%u)", - (unsigned int)pihole_uid, (unsigned int)pihole_gid); - } - } + chown_pihole(GLOBALTOMLPATH, NULL); + return; } diff --git a/src/daemon.c b/src/daemon.c index 5e7cf2db..a85b0ed0 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -330,13 +330,32 @@ void set_nice(void) // Set nice value const int ret = setpriority(which, pid, config.misc.nice.v.i); if(ret == -1) - // ERROR EPERM: The calling process attempted to increase its priority - // by supplying a negative value but has insufficient privileges. - // On Linux, the RLIMIT_NICE resource limit can be used to define a limit to - // which an unprivileged process's nice value can be raised. We are not - // affected by this limit when pihole-FTL is running with CAP_SYS_NICE - log_warn("Cannot set process priority to %d: %s. Process priority remains at %d", - config.misc.nice.v.i, strerror(errno), priority); + { + if(errno == EACCES || errno == EPERM) + { + // from man 2 setpriority: + // + // ERRORS + // [...] + // EACCES The caller attempted to set a lower nice value (i.e., a higher + // process priority), but did not have the required privilege (on + // Linux: did not have the CAP_SYS_NICE capability). + // + // EPERM A process was located, but its effective user ID did not match + // either the effective or the real user ID of the caller, and was + // not privileged (on Linux: did not have the CAP_SYS_NICE capabil‐ + // ity). + // [...] + log_warn("Insufficient permissions to set process priority to %d (CAP_SYS_NICE required), process priority remains at %d", + config.misc.nice.v.i, priority); + } + else + { + // Other error + log_warn("Cannot set process priority to %d: %s. Process priority remains at %d", + config.misc.nice.v.i, strerror(errno), priority); + } + } } } diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 42caa431..cce51b45 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2963,26 +2963,16 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // we're actually dropping root (user/group may be set to root) if(ent_pw != NULL && ent_pw->pw_uid != 0) { - log_info("FTL is going to drop from root to user %s (UID %u)", - ent_pw->pw_name, ent_pw->pw_uid); + log_info("FTL is going to drop from root to user pihole"); // Change ownership of shared memory objects chown_all_shmem(ent_pw); // Configured FTL log file - if(chown(config.files.log.ftl.v.s, ent_pw->pw_uid, ent_pw->pw_gid) == -1) - { - log_warn("Setting ownership (%u:%u) of %s failed: %s (%i)", - ent_pw->pw_uid, ent_pw->pw_gid, config.files.log.ftl.v.s, strerror(errno), errno); - } + chown_pihole(config.files.log.ftl.v.s, ent_pw); // Configured FTL database file - if(chown(config.files.database.v.s, ent_pw->pw_uid, ent_pw->pw_gid) == -1) - { - log_warn("Setting ownership (%u:%u) of %s failed: %s (%i)", - ent_pw->pw_uid, ent_pw->pw_gid, config.files.database.v.s, strerror(errno), errno); - - } + chown_pihole(config.files.database.v.s, ent_pw); // Check if auxiliary files exist and change ownership char *extrafile = calloc(strlen(config.files.database.v.s) + 5, sizeof(char)); @@ -2995,20 +2985,14 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // Check -wal file (write-ahead log) strcpy(extrafile, config.files.database.v.s); strcat(extrafile, "-wal"); - if(file_exists(extrafile) && chown(extrafile, ent_pw->pw_uid, ent_pw->pw_gid) == -1) - { - log_warn("Setting ownership (%u:%u) of %s failed: %s (%i)", - ent_pw->pw_uid, ent_pw->pw_gid, extrafile, strerror(errno), errno); - } + if(file_exists(extrafile)) + chown_pihole(extrafile, ent_pw); // Check -shm file (mmapped shared memory) strcpy(extrafile, config.files.database.v.s); strcat(extrafile, "-shm"); - if(file_exists(extrafile) && chown(extrafile, ent_pw->pw_uid, ent_pw->pw_gid) == -1) - { - log_warn("Setting ownership (%u:%u) of %s failed: %s (%i)", - ent_pw->pw_uid, ent_pw->pw_gid, extrafile, strerror(errno), errno); - } + if(file_exists(extrafile)) + chown_pihole(extrafile, ent_pw); // Free allocated memory free(extrafile); diff --git a/src/files.c b/src/files.c index cb5799fd..8158f60a 100644 --- a/src/files.c +++ b/src/files.c @@ -16,8 +16,6 @@ // opendir(), readdir() #include -// getpwuid() -#include // getgrgid() #include // NAME_MAX @@ -434,29 +432,35 @@ static int copy_file(const char *source, const char *destination) } // Change ownership of file to pihole user -static bool chown_pihole(const char *path) +bool chown_pihole(const char *path, struct passwd *pwd) { - // Get pihole user's uid and gid - struct passwd *pwd = getpwnam("pihole"); + // Get pihole user's UID and GID if not provided if(pwd == NULL) { - log_warn("chown_pihole(): Failed to get pihole user's uid: %s", strerror(errno)); - return false; + pwd = getpwnam("pihole"); + if(pwd == NULL) + { + log_warn("chown_pihole(): Failed to get pihole user's UID/GID: %s", strerror(errno)); + return false; + } } - struct group *grp = getgrnam("pihole"); - if(grp == NULL) + + // Get group name + struct group *grp = getgrgid(pwd->pw_gid); + const char *grp_name = grp != NULL ? grp->gr_name : ""; + + // Change ownership of file to pihole user + if(chown(path, pwd->pw_uid, pwd->pw_gid) < 0) { - log_warn("chown_pihole(): Failed to get pihole user's gid: %s", strerror(errno)); + log_warn("Failed to change ownership of \"%s\" to %s:%s (%u:%u): %s", + path, pwd->pw_name, grp_name, pwd->pw_uid, pwd->pw_gid, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); + return false; } - // Change ownership of file to pihole user - if(chown(path, pwd->pw_uid, grp->gr_gid) < 0) - { - log_warn("chown_pihole(): Failed to change ownership of \"%s\" to %u:%u: %s", - path, pwd->pw_uid, grp->gr_gid, strerror(errno)); - return false; - } + log_debug(DEBUG_INOTIFY, "Changed ownership of \"%s\" to %s:%s (%u:%u)", + path, pwd->pw_name, grp_name, pwd->pw_uid, pwd->pw_gid); return true; } @@ -533,7 +537,7 @@ void rotate_files(const char *path, char **first_file) } // Change ownership of file to pihole user - chown_pihole(new_path); + chown_pihole(new_path, NULL); } // Free memory diff --git a/src/files.h b/src/files.h index 742e8d9a..fc10ad23 100644 --- a/src/files.h +++ b/src/files.h @@ -16,6 +16,8 @@ #include // SHA256_DIGEST_SIZE #include +// getpwuid() +#include #define MAX_ROTATIONS 15 #define BACKUP_DIR "/etc/pihole/config_backups" @@ -31,6 +33,7 @@ void ls_dir(const char* path); unsigned int get_path_usage(const char *path, char buffer[64]); struct mntent *get_filesystem_details(const char *path); bool directory_exists(const char *path); +bool chown_pihole(const char *path, struct passwd *pwd); void rotate_files(const char *path, char **first_file); bool files_different(const char *pathA, const char* pathB, unsigned int from); bool sha256sum(const char *path, uint8_t checksum[SHA256_DIGEST_SIZE]); diff --git a/src/ntp/client.c b/src/ntp/client.c index 43cce286..2359d152 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -657,7 +657,7 @@ bool ntp_start_sync_thread(pthread_attr_t *attr) // in starting the thread. if(!check_capability(CAP_SYS_TIME)) { - log_warn("Insufficient permissions to set system time, NTP client not available"); + log_warn("Insufficient permissions to set system time (CAP_SYS_TIME required), NTP client not available"); // Send SIGUSR7 to embedded dnsmasq instance to signal time is // assumed to be correct kill(main_pid(), SIGUSR7); diff --git a/src/ntp/rtc.c b/src/ntp/rtc.c index 3fe8f93b..07232711 100644 --- a/src/ntp/rtc.c +++ b/src/ntp/rtc.c @@ -82,7 +82,8 @@ static int open_rtc(void) if(chown(config.ntp.sync.rtc.device.v.s, uid, gid) == -1) { log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", - config.ntp.sync.rtc.device.v.s, uid, gid, strerror(errno)); + config.ntp.sync.rtc.device.v.s, uid, gid, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); return -1; } @@ -97,7 +98,8 @@ static int open_rtc(void) if(chown(config.ntp.sync.rtc.device.v.s, st.st_uid, st.st_gid) == -1) { log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", - config.ntp.sync.rtc.device.v.s, st.st_uid, st.st_gid, strerror(errno)); + config.ntp.sync.rtc.device.v.s, st.st_uid, st.st_gid, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); return -1; } @@ -139,7 +141,8 @@ static int open_rtc(void) if(chown(rtc_devices[i], uid, gid) == -1) { log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", - rtc_devices[i], uid, gid, strerror(errno)); + rtc_devices[i], uid, gid, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); return -1; } @@ -154,7 +157,8 @@ static int open_rtc(void) if(chown(rtc_devices[i], st.st_uid, st.st_gid) == -1) { log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", - rtc_devices[i], st.st_uid, st.st_gid, strerror(errno)); + rtc_devices[i], st.st_uid, st.st_gid, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); return -1; } diff --git a/src/shmem.c b/src/shmem.c index 44f72eb8..83e7da5c 100644 --- a/src/shmem.c +++ b/src/shmem.c @@ -190,17 +190,20 @@ static bool chown_shmem(SharedMemory *sharedMemory, struct passwd *ent_pw) // Open shared memory object const int fd = shm_open(sharedMemory->name, O_RDWR, S_IRUSR | S_IWUSR); log_debug(DEBUG_SHMEM, "Changing %s (%d) to %u:%u", sharedMemory->name, fd, ent_pw->pw_uid, ent_pw->pw_gid); + if(fd == -1) { - log_crit("chown_shmem(): Failed to open shared memory object \"%s\": %s", + log_crit("Failed to open shared memory object \"%s\" for chown: %s", sharedMemory->name, strerror(errno)); exit(EXIT_FAILURE); } + if(fchown(fd, ent_pw->pw_uid, ent_pw->pw_gid) == -1) { - log_warn("chown_shmem(%d, %u, %u): failed for %s: %s (%d)", - fd, ent_pw->pw_uid, ent_pw->pw_gid, sharedMemory->name, - strerror(errno), errno); + log_crit("Failed to change ownership of shared memory object \"%s\": %s", + sharedMemory->name, + errno == EPERM ? "Insufficient permissions (CAP_CHOWN required)" : strerror(errno)); + return false; } diff --git a/test/test_suite.bats b/test/test_suite.bats index c62c9c1d..2e0db7e5 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -481,7 +481,7 @@ } @test "No WARNING messages in FTL.log (besides known warnings)" { - run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|CAP_SYS_TIME|(Cannot set process priority)|FTLCONF_|(Insufficient permissions to set system time, NTP client not available)"' + run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|CAP_SYS_TIME|FTLCONF_"' printf "%s\n" "${lines[@]}" [[ "${lines[@]}" == "" ]] }