Change where we do suid up/down.
authorAlan T. DeKok <aland@freeradius.org>
Sun, 14 Dec 2008 09:31:17 +0000 (10:31 +0100)
committerAlan T. DeKok <aland@freeradius.org>
Sun, 14 Dec 2008 09:31:17 +0000 (10:31 +0100)
If the server starts as root, but it supposed to run as another
user, we want to *temporarily* drop permissions very early.  Then,
when binding to privileged sockets, we re-gain permissions.
Once all of the sockets are open, we drop them permanently.

However, if we suid up for *all* sockets, then the control socket
will be created as root, rather than as the unprivileged user.
To fix that, we put suid up/down just around the 2 calls that
need it.

src/include/radiusd.h
src/main/event.c
src/main/listen.c
src/main/mainconfig.c

index bf126ba..52b5cbf 100644 (file)
@@ -590,6 +590,9 @@ extern struct main_config_t mainconfig;
 
 int read_mainconfig(int reload);
 int free_mainconfig(void);
+void fr_suid_down(void);
+void fr_suid_up(void);
+void fr_suid_down_permanent(void);
 
 /* listen.c */
 void listen_free(rad_listen_t **head);
index f50a899..0084545 100644 (file)
@@ -2795,67 +2795,6 @@ static void event_status(struct timeval *wake)
 
 }
 
-#if defined(HAVE_SETRESUID) && defined (HAVE_GETRESUID)
-static void fr_suid_up(void)
-{
-       uid_t ruid, euid, suid;
-       
-       if (getresuid(&ruid, &euid, &suid) < 0) {
-               radlog(L_ERR, "Failed getting saved UID's");
-               _exit(1);
-       }
-
-       if (setresuid(-1, suid, -1) < 0) {
-               radlog(L_ERR, "Failed switching to privileged user");
-               _exit(1);
-       }
-
-       if (geteuid() != suid) {
-               radlog(L_ERR, "Switched to unknown UID");
-               _exit(1);
-       }
-}
-
-extern uid_t server_uid;
-extern int did_setuid;
-static void fr_suid_down(void)
-{
-       uid_t ruid, euid, suid;
-
-       if (!did_setuid) return;
-
-       if (getresuid(&ruid, &euid, &suid) < 0) {
-               radlog(L_ERR, "Failed getting saved UID's");
-               _exit(1);
-       }
-
-       if (setresuid(server_uid, server_uid, server_uid) < 0) {
-               radlog(L_ERR, "Failed to permanently switch UID to %u: %s",
-                      server_uid, strerror(errno));
-               _exit(1);
-       }
-
-       if (geteuid() != server_uid) {
-               radlog(L_ERR, "Switched to unknown UID");
-               _exit(1);
-       }
-
-
-       if (getresuid(&ruid, &euid, &suid) < 0) {
-               radlog(L_ERR, "Failed getting saved UID's: %s",
-                      strerror(errno));
-               _exit(1);
-       }
-}
-#else
-/*
- *     Much less secure...
- */
-#define fr_suid_up()
-#define fr_suid_down()
-#endif
-
-
 /*
  *     Externally-visibly functions.
  */
@@ -2966,13 +2905,24 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
        DEBUG("%s: #### Opening IP addresses and Ports ####",
               mainconfig.name);
 
-       fr_suid_up();           /* sockets may bind to privileged ports */
-
+       /*
+       *       The server temporarily switches to an unprivileged
+       *       user very early in the bootstrapping process.
+       *       However, some sockets MAY require privileged access
+       *       (bind to device, or to port < 1024, or to raw
+       *       sockets).  Those sockets need to call suid up/down
+       *       themselves around the functions that need a privileged
+       *       uid.
+       */
        if (listen_init(cs, &head) < 0) {
                _exit(1);
        }
        
-       fr_suid_down();
+       /*
+        *      At this point, no one has any business *ever* going
+        *      back to root uid.
+        */
+       fr_suid_down_permanent();
 
        /*
         *      Add all of the sockets to the event loop.
index a98121a..3ee1ee3 100644 (file)
@@ -1091,6 +1091,7 @@ static const rad_listen_master_t master_listen[RAD_LISTEN_MAX] = {
  */
 static int listen_bind(rad_listen_t *this)
 {
+       int rcode;
        struct sockaddr_storage salocal;
        socklen_t       salen;
        listen_socket_t *sock = this->data;
@@ -1157,11 +1158,14 @@ static int listen_bind(rad_listen_t *this)
        if (sock->interface) {
                struct ifreq ifreq;
                strcpy(ifreq.ifr_name, sock->interface);
-               
-               if (setsockopt(this->fd, SOL_SOCKET, SO_BINDTODEVICE,
-                              (char *)&ifreq, sizeof(ifreq)) < 0) {
+
+               fr_suid_up();
+               rcode = setsockopt(this->fd, SOL_SOCKET, SO_BINDTODEVICE,
+                                  (char *)&ifreq, sizeof(ifreq));
+               fr_suid_down();
+               if (rcode < 0) {
                        close(this->fd);
-                       radlog(L_ERR, "Failed opening to interface %s: %s",
+                       radlog(L_ERR, "Failed binding to interface %s: %s",
                               sock->interface, strerror(errno));
                        return -1;
                } /* else it worked. */
@@ -1205,8 +1209,14 @@ static int listen_bind(rad_listen_t *this)
 #endif /* IPV6_V6ONLY */
        }
 #endif /* HAVE_STRUCT_SOCKADDR_IN6 */
-               
-       if (bind(this->fd, (struct sockaddr *) &salocal, salen) < 0) {
+
+       /*
+        *      May be binding to priviledged ports.
+        */
+       fr_suid_up();
+       rcode = bind(this->fd, (struct sockaddr *) &salocal, salen);
+       fr_suid_down();
+       if (rcode < 0) {
                close(this->fd);
                radlog(L_ERR, "Failed binding to socket: %s\n",
                       strerror(errno));
index bfc6df2..754a23f 100644 (file)
@@ -406,6 +406,95 @@ static int r_mkdir(const char *part)
 #ifndef __MINGW32__
 int did_setuid = FALSE;
 
+#if defined(HAVE_SETRESUID) && defined (HAVE_GETRESUID)
+void fr_suid_up(void)
+{
+       uid_t ruid, euid, suid;
+       
+       if (getresuid(&ruid, &euid, &suid) < 0) {
+               radlog(L_ERR, "Failed getting saved UID's");
+               _exit(1);
+       }
+
+       if (setresuid(-1, suid, -1) < 0) {
+               radlog(L_ERR, "Failed switching to privileged user");
+               _exit(1);
+       }
+
+       if (geteuid() != suid) {
+               radlog(L_ERR, "Switched to unknown UID");
+               _exit(1);
+       }
+}
+
+void fr_suid_down(void)
+{
+       if (!did_setuid) return;
+
+       if (setresuid(-1, server_uid, geteuid()) < 0) {
+               fprintf(stderr, "%s: Failed switching to uid %s: %s\n",
+                       progname, uid_name, strerror(errno));
+               _exit(1);
+       }
+               
+       if (geteuid() != server_uid) {
+               fprintf(stderr, "%s: Failed switching uid: UID is incorrect\n",
+                       progname);
+               _exit(1);
+       }
+}
+
+void fr_suid_down_permanent(void)
+{
+       uid_t ruid, euid, suid;
+
+       if (!did_setuid) return;
+
+       if (getresuid(&ruid, &euid, &suid) < 0) {
+               radlog(L_ERR, "Failed getting saved uid's");
+               _exit(1);
+       }
+
+       if (setresuid(server_uid, server_uid, server_uid) < 0) {
+               radlog(L_ERR, "Failed in permanent switch to uid %s: %s",
+                      uid_name, strerror(errno));
+               _exit(1);
+       }
+
+       if (geteuid() != server_uid) {
+               radlog(L_ERR, "Switched to unknown uid");
+               _exit(1);
+       }
+
+
+       if (getresuid(&ruid, &euid, &suid) < 0) {
+               radlog(L_ERR, "Failed getting saved uid's: %s",
+                      strerror(errno));
+               _exit(1);
+       }
+}
+#else
+/*
+ *     Much less secure...
+ */
+void fr_suid_up(void)
+{
+}
+void fr_suid_down(void)
+{
+       if (!uid_name) return;
+
+       if (setuid(server_uid) < 0) {
+               fprintf(stderr, "%s: Failed switching to uid %s: %s\n",
+                       progname, uid_name, strerror(errno));
+               _exit(1);
+       }
+}
+void fr_suid_down_permanent(void)
+{
+}
+#endif
+
 /*
  *  Do chroot, if requested.
  *
@@ -511,31 +600,14 @@ static int switch_users(CONF_SECTION *cs)
 
 #ifdef HAVE_PWD_H
        if (uid_name) {
+               fr_suid_down();
 
-#ifndef HAVE_SETRESUID
-/*
- *     Fake out setresuid with something that's close.
- */
-#define setresuid(_a, _b, _c) setuid(_b)
-#endif
-
-               if (setresuid(-1, server_uid, geteuid()) < 0) {
-                       fprintf(stderr, "%s: Failed switching uid: %s\n",
-                               progname, strerror(errno));
-                       return 0;
-               }
+               /*
+                *      Now core dumps are disabled on most secure systems.
+                */
                
-               if (geteuid() != server_uid) {
-                       fprintf(stderr, "%s: Failed switching uid: UID is incorrect\n",
-                               progname);
-                       return 0;
-               }
+               did_setuid = TRUE;
        }
-
-       /*
-        *      Now core dumps are disabled on most secure systems.
-        */
-       did_setuid = TRUE;
 #endif
 
        /*