@@ -90,9 +90,13 @@ int main(int argc, char **argv)

     time_t currtime;

     mode_t umsk;

     pid_t parentPid = getpid();

+#ifndef _WIN32

+    int dropPrivRet = 0;

+#endif /* _WIN32 */

     sigset_t sigset;

     struct sigaction act;

+    const char * user_name = NULL;

     cl_initialize_crypto();

@@ -154,6 +158,10 @@ int main(int argc, char **argv)

     }

     free(pt);

+    if ((opt = optget(opts, "User"))->enabled){

+        user_name = opt->strarg;

+    }

+

     if ((opt = optget(opts, "Chroot"))->enabled) {

         if (chdir(opt->strarg) != 0) {

             logg("!Cannot change directory to %s\n", opt->strarg);

@@ -252,11 +260,11 @@ int main(int argc, char **argv)

             }

         }

-        if ((opt = optget(opts, "User"))->enabled) {

+        if (NULL != user_name) {

             struct passwd *user;

-            if ((user = getpwnam(opt->strarg)) == NULL) {

+            if ((user = getpwnam(user_name)) == NULL) {

                 logg("ERROR: Can't get information about user %s.\n",

-                     opt->strarg);

+                     user_name);

                 logg_close();

                 optfree(opts);

                 return 1;

@@ -360,7 +368,7 @@ int main(int argc, char **argv)

 #ifndef _WIN32

     if (!optget(opts, "Foreground")->enabled) {

-        if (-1 == daemonize_parent_wait()) {

+        if (-1 == daemonize_parent_wait(user_name, logg_file)) {

             logg("!daemonize() failed\n");

             localnets_free();

             whitelist_free();

@@ -425,6 +433,22 @@ int main(int argc, char **argv)

         }

         umask(old_umask);

+#ifndef _WIN32

+        if (0 == err){

+            /*If the file has already been created by a different user, it will just be

+             * rewritten by us, but not change the ownership, so do that explicitly.

+             */

+            if (0 == geteuid()){

+                struct passwd * pw = getpwuid(0);

+                int ret = lchown(opt->strarg, pw->pw_uid, pw->pw_gid);

+                if (ret){

+                    logg("!Can't change ownership of PID file %s '%s'\n", opt->strarg, strerror(errno));

+                    err = 1;

+                }

+            }

+        }

+#endif /*_WIN32*/

+

         if (err){

             localnets_free();

             whitelist_free();

@@ -434,42 +458,13 @@ int main(int argc, char **argv)

         }

     }

-

-    if (geteuid() == 0 && (opt = optget(opts, "User"))->enabled) {

-        struct passwd *user = NULL;

-        if ((user = getpwnam(opt->strarg)) == NULL) {

-            fprintf(stderr, "ERROR: Can't get information about user %s.\n", opt->strarg);

-            optfree(opts);

-            return 1;

-        }

-

-#ifdef HAVE_INITGROUPS

-        if (initgroups(opt->strarg, user->pw_gid)) {

-            fprintf(stderr, "ERROR: initgroups() failed.\n");

-            optfree(opts);

-            return 1;

-        }

-#elif HAVE_SETGROUPS

-        if (setgroups(1, &user->pw_gid)) {

-            fprintf(stderr, "ERROR: setgroups() failed.\n");

-            optfree(opts);

-            return 1;

-        }

-#endif

-        if (setgid(user->pw_gid)) {

-            fprintf(stderr, "ERROR: setgid(%d) failed.\n", (int)user->pw_gid);

-            optfree(opts);

-            return 1;

-        }

-

-        if (setuid(user->pw_uid)) {

-            fprintf(stderr, "ERROR: setuid(%d) failed.\n", (int)user->pw_uid);

-            optfree(opts);

-            return 1;

-        }

+#ifndef _WIN32

+    dropPrivRet = drop_privileges(user_name, logg_file);

+    if (dropPrivRet){

+        optfree(opts);

+        return dropPrivRet;

     }

-#ifndef _WIN32

     /* We have been daemonized, and initialization is done.  Signal

      * the parent process so that it can exit cleanly.

      */

@@ -126,6 +126,7 @@ int main(int argc, char **argv)

     struct passwd *user = NULL;

     struct sigaction sa;

     struct rlimit rlim;

+    int dropPrivRet = 0;

 #endif

     time_t currtime;

     const char *dbdir, *cfgfile;

@@ -144,6 +145,7 @@ int main(int argc, char **argv)

 #endif

     pid_t mainpid = 0;

     mode_t old_umask = 0;

+    const char * user_name = NULL;

     if (check_flevel())

         exit(1);

@@ -208,6 +210,10 @@ int main(int argc, char **argv)

     }

     free(pt);

+    if ((opt = optget(opts, "User"))->enabled){

+        user_name = opt->strarg;

+    }

+

     if (optget(opts, "version")->enabled) {

         print_version(optget(opts, "DatabaseDirectory")->strarg);

         optfree(opts);

@@ -267,7 +273,7 @@ int main(int argc, char **argv)

 #endif

             gengine = engine;

             atexit(free_engine);

-            daemonizeRet = daemonize_parent_wait();

+            daemonizeRet = daemonize_parent_wait(user_name, logg_file);

             if (daemonizeRet < 0){

                 logg("!daemonize() failed: %s\n", strerror(errno));

                 return 1;

@@ -306,45 +312,30 @@ int main(int argc, char **argv)

             fclose(fd);

         }

         umask(old_umask);

-    }

-    /* drop privileges */

 #ifndef _WIN32

-    if (geteuid() == 0 && (opt = optget(opts, "User"))->enabled) {

-        if ((user = getpwnam(opt->strarg)) == NULL) {

-            fprintf(stderr, "ERROR: Can't get information about user %s.\n", opt->strarg);

-            optfree(opts);

-            return 1;

-        }

-

-#ifdef HAVE_INITGROUPS

-        if (initgroups(opt->strarg, user->pw_gid)) {

-            fprintf(stderr, "ERROR: initgroups() failed.\n");

-            optfree(opts);

-            return 1;

-        }

-#elif HAVE_SETGROUPS

-        if (setgroups(1, &user->pw_gid)) {

-            fprintf(stderr, "ERROR: setgroups() failed.\n");

-            optfree(opts);

-            return 1;

-        }

-#endif

-

-        if (setgid(user->pw_gid)) {

-            fprintf(stderr, "ERROR: setgid(%d) failed.\n", (int)user->pw_gid);

-            optfree(opts);

-            return 1;

-        }

-

-        if (setuid(user->pw_uid)) {

-            fprintf(stderr, "ERROR: setuid(%d) failed.\n", (int)user->pw_uid);

-            optfree(opts);

-            return 1;

+        /*If the file has already been created by a different user, it will just be

+         * rewritten by us, but not change the ownership, so do that explicitly.

+         */

+        if (0 == geteuid()){

+            struct passwd * pw = getpwuid(0);

+            int ret = lchown(opt->strarg, pw->pw_uid, pw->pw_gid);

+            if (ret){

+                logg("!Can't change ownership of PID file %s '%s'\n", opt->strarg, strerror(errno));

+                exit(2);

+            }

         }

+#endif /* _WIN32 */

     }

-#endif

+    /* drop privileges */

+#ifndef _WIN32

+    dropPrivRet = drop_privileges(user_name, logg_file);

+    if (dropPrivRet) {

+        optfree(opts);

+        return dropPrivRet;

+    }

+#endif /* _WIN32 */

     do { /* logger initialized */

@@ -59,6 +59,9 @@ Example

 # This option allows you to save a process identifier of the listening

 # daemon (main thread).

+# This file will be owned by root, as long as clamav-milter was started by 

+# root.  It is recommended that the directory where this file is stored is

+# also owned by root to keep other users from tampering with it.

 #

 # Default: disabled

 #PidFile /var/run/clamav-milter.pid

@@ -70,6 +70,9 @@ Example

 # This option allows you to save a process identifier of the listening

 # daemon (main thread).

+# This file will be owned by root, as long as clamd was started by root.

+# It is recommended that the directory where this file is stored is

+# also owned by root to keep other users from tampering with it.

 # Default: disabled

 #PidFile /var/run/clamd.pid

@@ -47,6 +47,9 @@ Example

 #LogRotate yes

 # This option allows you to save the process identifier of the daemon

+# This file will be owned by root, as long as freshclam was started by root.

+# It is recommended that the directory where this file is stored is

+# also owned by root to keep other users from tampering with it.

 # Default: disabled

 #PidFile /var/run/freshclam.pid

@@ -135,6 +135,21 @@ static int writepid(const char *pidfile)

         fclose(fd);

     }

     umask(old_umask);

+

+#ifndef _WIN32

+    /*If the file has already been created by a different user, it will just be

+     * rewritten by us, but not change the ownership, so do that explicitly.

+     */

+    if (0 == geteuid()){

+        struct passwd * pw = getpwuid(0);

+        int ret = lchown(pidfile, pw->pw_uid, pw->pw_gid);

+        if (ret){

+            logg("!Can't change ownership of PID file %s '%s'\n", pidfile, strerror(errno));

+            return 1;

+        }

+    }

+#endif /*_WIN32 */

+

     return 0;

 }

@@ -629,68 +644,6 @@ done:

 }

 /**

- * @brief Switch users to the DatabaseOwner, if specified.

- *

- * @param dbowner     A user account that will have write permissions in the database directory.

- * @return fc_error_t FC_SUCCESS if success.

- * @return fc_error_t FC_EARG if success.

- */

-static fc_error_t switch_user(const char *dbowner)

-{

-    fc_error_t status = FC_EARG;

-

-#ifdef HAVE_PWD_H

-    struct passwd *user;

-

-    if (NULL == dbowner) {

-        logg("*No DatabaseOwner specified. Freshclam will run as the current user.\n");

-        status = FC_SUCCESS;

-        goto done;

-    }

-

-    if (!geteuid()) {

-        if ((user = getpwnam(dbowner)) == NULL) {

-            logg("^Can't get information about user %s.\n", dbowner);

-            status = FC_ECONFIG;

-            goto done;

-        }

-

-#ifdef HAVE_INITGROUPS

-        if (initgroups(dbowner, user->pw_gid)) {

-            logg("^initgroups() failed.\n");

-            status = FC_ECONFIG;

-            goto done;

-        }

-#elif HAVE_SETGROUPS

-        if (setgroups(1, &user->pw_gid)) {

-            logg("^setgroups() failed.\n");

-            status = FC_ECONFIG;

-            goto done;

-        }

-#endif

-

-        if (setgid(user->pw_gid)) {

-            logg("^setgid(%d) failed.\n", (int)user->pw_gid);

-            status = FC_ECONFIG;

-            goto done;

-        }

-

-        if (setuid(user->pw_uid)) {

-            logg("^setuid(%d) failed.\n", (int)user->pw_uid);

-            status = FC_ECONFIG;

-            goto done;

-        }

-    }

-#endif /* HAVE_PWD_H */

-

-    status = FC_SUCCESS;

-

-done:

-

-    return status;

-}

-

-/**

  * @brief Get a list of strings for a given repeatable opt argument.

  *

  * @param opt           optstruct of repeatable argument to collect in a list.

@@ -826,11 +779,12 @@ static fc_error_t initialize(struct optstruct *opts)

         /*

          * freshclam shouldn't work with root privileges.

          * Drop privileges to the DatabaseOwner user, if specified.

+         * Pass NULL for the log file name, because it hasn't been created yet.

          */

-        ret = switch_user(optget(opts, "DatabaseOwner")->strarg);

-        if (FC_SUCCESS != ret) {

+        ret = drop_privileges(optget(opts, "DatabaseOwner")->strarg, NULL);

+        if (ret) {

             logg("!Failed to switch to %s user.\n", optget(opts, "DatabaseOwner")->strarg);

-            status = ret;

+            status = FC_ECONFIG;

             goto done;

         }

     }

@@ -1580,6 +1534,11 @@ int main(int argc, char **argv)

     int bPrune = 1;

+#ifdef HAVE_PWD_H

+    const struct optstruct *logFileOpt = NULL;

+    const char * logFileName = NULL;

+#endif /* HAVE_PWD_H */

+

     fc_ctx fc_context = {0};

 #ifndef _WIN32

@@ -1889,7 +1848,7 @@ int main(int argc, char **argv)

 #ifndef _WIN32

         /* fork into background */

         if (g_foreground == 0) {

-            if (-1 == daemonize_parent_wait()) {

+            if (-1 == daemonize_parent_wait(NULL, NULL)) {

                 logg("!daemonize() failed\n");

                 status = FC_EFAILEDUPDATE;

                 goto done;

@@ -1918,14 +1877,20 @@ int main(int argc, char **argv)

 #endif

 #ifdef HAVE_PWD_H

+        /*  Get the log file name to pass it into drop_privileges.  */

+        logFileOpt = optget(opts, "UpdateLogFile");

+        if (logFileOpt->enabled) {

+           logFileName  = logFileOpt->strarg;

+        }

+

         /*

          * freshclam shouldn't work with root privileges.

          * Drop privileges to the DatabaseOwner user, if specified.

          */

-        ret = switch_user(optget(opts, "DatabaseOwner")->strarg);

-        if (FC_SUCCESS != ret) {

+        ret = drop_privileges(optget(opts, "DatabaseOwner")->strarg, logFileName);

+        if (0 != ret) {

             logg("!Failed to switch to %s user.\n", optget(opts, "DatabaseOwner")->strarg);

-            status = ret;

+            status = FC_ECONFIG;

             goto done;

         }

 #endif /* HAVE_PWD_H */

@@ -34,6 +34,9 @@

 #include <sys/stat.h>

 #ifndef _WIN32

 #include <sys/socket.h>

+#include <pwd.h>

+#include <grp.h>

+#include <sys/types.h>

 #endif

 #include <dirent.h>

 #include <fcntl.h>

@@ -325,7 +328,7 @@ static void daemonize_child_initialized_handler(int sig)

     exit(0);

 }

-int daemonize_parent_wait()

+int daemonize_parent_wait(const char * const user, const char * const log_file)

 {

     int daemonizePid = daemonize_all_return();

     if (daemonizePid == -1) {

@@ -349,6 +352,12 @@ int daemonize_parent_wait()

             return -1;

         }

+        if (NULL != user){

+            if (drop_privileges(user, log_file)){

+                return -1;

+            }

+        }

+

         int exitStatus;

         wait(&exitStatus);

         if (WIFEXITED(exitStatus)) { //error

@@ -364,8 +373,68 @@ void daemonize_signal_parent(pid_t parentPid)

     close_std_descriptors();

     kill(parentPid, SIGINT);

 }

+

+int drop_privileges( const char * const user_name, const char * const log_file) {

+    int ret = 1;

+

+    /*This function is called in a bunch of places, and rather than change the error checking

+     * in every function, we are just going to return success if there is no work to do.

+     */

+    if ((0 == geteuid()) && (NULL != user_name)){

+        struct passwd *user = NULL;

+

+        if ((user = getpwnam(user_name)) == NULL) {

+            logg("^Can't get information about user %s.\n", user_name);

+            fprintf(stderr, "ERROR: Can't get information about user %s.\n", user_name);

+            goto done;

+        }

+

+#ifdef HAVE_INITGROUPS

+        if (initgroups(user_name, user->pw_gid)) {

+            fprintf(stderr, "ERROR: initgroups() failed.\n");

+            logg("^initgroups() failed.\n");

+            goto done;

+        }

+#elif HAVE_SETGROUPS

+        if (setgroups(1, &user->pw_gid)) {

+            fprintf(stderr, "ERROR: setgroups() failed.\n");

+            logg("^setgroups() failed.\n");

+            goto done;

+        }

 #endif

+        /*Change ownership of the log file to the user we are going to switch to.*/

+        if (NULL != log_file){

+            int ret = lchown(log_file, user->pw_uid, user->pw_gid);

+            if (ret){

+                fprintf(stderr, "ERROR: lchown to user '%s' failed on\n", user->pw_name);

+                fprintf(stderr, "log file '%s'.\n", log_file);

+                fprintf(stderr, "Error was '%s'\n", strerror(errno));

+                logg("^lchown to user '%s' failed on log file '%s'.  Error was '%s'\n", 

+                        user->pw_name, log_file, strerror(errno));

+                goto done;

+            }

+        }

+

+        if (setgid(user->pw_gid)) {

+            fprintf(stderr, "ERROR: setgid(%d) failed.\n", (int)user->pw_gid);

+            logg("^setgid(%d) failed.\n", (int)user->pw_gid);

+            goto done;

+        }

+

+        if (setuid(user->pw_uid)) {

+            fprintf(stderr, "ERROR: setuid(%d) failed.\n", (int)user->pw_uid);

+            logg("^setuid(%d) failed.\n", (int)user->pw_uid);

+            goto done;

+        }

+    }

+    ret = 0;

+

+done:

+    return ret;

+}

+#endif /*_WIN32*/

+

 int match_regex(const char *filename, const char *pattern)

 {

     regex_t reg;

@@ -26,6 +26,8 @@

 #include <netdb.h>

 #include <netinet/in.h>

 #endif

+#include <stdbool.h>

+

 #include "platform.h"

 #include "optparser.h"

 /* Maximum filenames under various systems - njh */

@@ -77,13 +79,26 @@ int daemonize_all_return(void);

 /*Parent waits for a SIGINT or the child process to exit.  If

  * it receives a SIGINT, it exits with exit code 0.  If the child

- * exits (error), it exits with the child process's exit code.*/

-int daemonize_parent_wait();

+ * exits (error), it exits with the child process's exit code.

+ *

+ * @param user If user is supplied and this function is being called

+ * as root, daemonize_parent_wait will change the parent process

+ * to user before calling wait so that the child process can signal

+ * the parent when it is time to exit.  The child process will still

+ * return as root.

+ *

+ * @param log_file If user AND log_file are both supplied and this

+ * function is being called as root, the ownership of log_file will

+ * be changed to user.

+ */

+int daemonize_parent_wait(const char * const user, const char * const log_file);

 /*Sends a SIGINT to the parent process.  It also closes stdin, stdout, 

  * and stderr.*/

 void daemonize_signal_parent(pid_t parentPid);

-#endif

+

+int drop_privileges( const char * const user, const char * const log_file);

+#endif /* _WIN32 */

 const char *get_version(void);

 int match_regex(const char *filename, const char *pattern);