fixed a race condition when getgrnam() which is not thread safe, could be used in...

diff --git a/src/daemon.c b/src/daemon.c
index d091f7730216b8ae13354a599ed58ef5511e7b7e..9dcf32f0ba6b7482c0840b29b405c4a43ba9d6aa 100644 (file)
--- a/src/daemon.c
+++ b/src/daemon.c
@@ -82,9 +82,12 @@ int become_user(const char *username)
                return -1;
        }
 
-       if(pidfile[0] && getuid() != pw->pw_uid) {
+       uid_t uid = pw->pw_uid;
+       gid_t gid = pw->pw_gid;
+
+       if(pidfile[0] && getuid() != uid) {
                // we are dropping privileges
-               if(chown(pidfile, pw->pw_uid, pw->pw_gid) != 0)
+               if(chown(pidfile, uid, gid) != 0)
                        error("Cannot chown pidfile '%s' to user '%s'", pidfile, username);
 
                else if(pidfd != -1) {
@@ -99,30 +102,30 @@ int become_user(const char *username)
                pidfd = -1;
        }
 
-       if(setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) {
-               error("Cannot switch to user's %s group (gid: %d).", username, pw->pw_gid);
+       if(setresgid(gid, gid, gid) != 0) {
+               error("Cannot switch to user's %s group (gid: %d).", username, gid);
                return -1;
        }
 
-       if(setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) != 0) {
-               error("Cannot switch to user %s (uid: %d).", username, pw->pw_uid);
+       if(setresuid(uid, uid, uid) != 0) {
+               error("Cannot switch to user %s (uid: %d).", username, uid);
                return -1;
        }
 
-       if(setgid(pw->pw_gid) != 0) {
-               error("Cannot switch to user's %s group (gid: %d).", username, pw->pw_gid);
+       if(setgid(gid) != 0) {
+               error("Cannot switch to user's %s group (gid: %d).", username, gid);
                return -1;
        }
-       if(setegid(pw->pw_gid) != 0) {
-               error("Cannot effectively switch to user's %s group (gid: %d).", username, pw->pw_gid);
+       if(setegid(gid) != 0) {
+               error("Cannot effectively switch to user's %s group (gid: %d).", username, gid);
                return -1;
        }
-       if(setuid(pw->pw_uid) != 0) {
-               error("Cannot switch to user %s (uid: %d).", username, pw->pw_uid);
+       if(setuid(uid) != 0) {
+               error("Cannot switch to user %s (uid: %d).", username, uid);
                return -1;
        }
-       if(seteuid(pw->pw_uid) != 0) {
-               error("Cannot effectively switch to user %s (uid: %d).", username, pw->pw_uid);
+       if(seteuid(uid) != 0) {
+               error("Cannot effectively switch to user %s (uid: %d).", username, uid);
                return -1;
        }
 

diff --git a/src/main.c b/src/main.c
index 4005516f85a84b81ed2ae7f0bf6d94571953f091..c670d9c064e4e07e33267d37b17b534f01d7f4fe 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -420,8 +420,13 @@ int main(int argc, char **argv)
 
                // --------------------------------------------------------------------
 
+               // get the user we should run
+               // IMPORTANT: this is required before web_files_uid()
                user = config_get("global", "run as user"    , (getuid() == 0)?NETDATA_USER:"");
-               web_files_uid();
+
+               // IMPORTANT: these have to run once, while single threaded
+               web_files_uid(); // IMPORTANT: web_files_uid() before web_files_gid()
+               web_files_gid();
 
                // --------------------------------------------------------------------
 

diff --git a/src/web_client.c b/src/web_client.c
index 9d7e9487d0ed653180fbb1ab562296594f9c205d..6500a59b2e7446f59d7c323f9ebe9bbbe594a056 100644 (file)
--- a/src/web_client.c
+++ b/src/web_client.c
@@ -28,6 +28,7 @@
 #include "rrd2json.h"
 
 #include "web_client.h"
+#include "../config.h"
 
 #define INITIAL_WEB_DATA_LENGTH 16384
 #define WEB_REQUEST_LENGTH 16384
@@ -232,10 +233,13 @@ uid_t web_files_uid(void)
        static uid_t owner_uid = 0;
 
        if(unlikely(!web_owner)) {
-               web_owner = config_get("global", "web files owner", NETDATA_USER);
+               web_owner = config_get("global", "web files owner", config_get("global", "run as user", ""));
                if(!web_owner || !*web_owner)
                        owner_uid = geteuid();
                else {
+                       // getpwnam() is not thread safe,
+                       // but we have called this function once
+                       // while single threaded
                        struct passwd *pw = getpwnam(web_owner);
                        if(!pw) {
                                error("User %s is not present. Ignoring option.", web_owner);
@@ -257,10 +261,13 @@ gid_t web_files_gid(void)
        static gid_t owner_gid = 0;
 
        if(unlikely(!web_group)) {
-               web_group = config_get("global", "web files group", config_get("global", "web files owner", NETDATA_USER));
+               web_group = config_get("global", "web files group", config_get("global", "web files owner", ""));
                if(!web_group || !*web_group)
                        owner_gid = getegid();
                else {
+                       // getgrnam() is not thread safe,
+                       // but we have called this function once
+                       // while single threaded
                        struct group *gr = getgrnam(web_group);
                        if(!gr) {
                                error("Group %s is not present. Ignoring option.", web_group);

diff --git a/src/web_client.h b/src/web_client.h
index 14cd91bc6b84cd0af985bc9f5c268418a58f18bf..3823dbc9120c094ac87e3d77bb4cf6b5a812b1e5 100644 (file)
--- a/src/web_client.h
+++ b/src/web_client.h
@@ -81,6 +81,7 @@ struct web_client {
 extern struct web_client *web_clients;
 
 extern uid_t web_files_uid(void);
+extern uid_t web_files_gid(void);
 
 extern struct web_client *web_client_create(int listener);
 extern struct web_client *web_client_free(struct web_client *w);

diff --git a/src/web_server.c b/src/web_server.c
index ab3e4357a6c8ac6d7b8bfd028fca0a65e511b8f9..10bf39a783428ef4fba1c088d3a9ca6f886abee2 100644 (file)
--- a/src/web_server.c
+++ b/src/web_server.c
@@ -24,6 +24,7 @@
 #include "global_statistics.h"
 #include "rrd.h"
 #include "rrd2json.h"
+#include "../config.h"
 
 int listen_backlog = LISTEN_BACKLOG;