vtysh: cleanup SUID handling

Eliminate several more SUID problems (VTYSH_LOG, history file) and make
the whole SUID approach more robust.  Still possibly unsafe to use, but
much better.

[v2: wrap seteuid/setegid calls]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h
index b0866ec7f3fa20e22ce051041a6c79755e700229..e10ab11da962620f1a586a05365f57cecd806703 100644 (file)
--- a/vtysh/vtysh.h
+++ b/vtysh/vtysh.h
@@ -93,6 +93,9 @@ void vtysh_config_init(void);
 
 void vtysh_pager_init(void);
 
+void suid_on(void);
+void suid_off(void);
+
 /* Child process execution flag. */
 extern int execute_flag;
 

diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c
index 8145bf364115bd9abfc3897f4b42acccdb852f7d..b9909b4930955a2aa7db4be197c6c81e38b523eb 100644 (file)
--- a/vtysh/vtysh_main.c
+++ b/vtysh/vtysh_main.c
@@ -44,6 +44,10 @@
 /* VTY shell program name. */
 char *progname;
 
+/* SUID mode */
+static uid_t elevuid, realuid;
+static gid_t elevgid, realgid;
+
 /* Configuration file name and directory. */
 static char vtysh_config_always[MAXPATHLEN] = SYSCONFDIR VTYSH_DEFAULT_CONFIG;
 static char quagga_config_default[MAXPATHLEN] = SYSCONFDIR FRR_DEFAULT_CONFIG;
@@ -249,6 +253,30 @@ static void vtysh_unflock_config(void)
        close(flock_fd);
 }
 
+void suid_on(void)
+{
+       if (elevuid != realuid && seteuid(elevuid)) {
+               perror("seteuid(on)");
+               exit(1);
+       }
+       if (elevgid != realgid && setegid(elevgid)) {
+               perror("setegid(on)");
+               exit(1);
+       }
+}
+
+void suid_off(void)
+{
+       if (elevuid != realuid && seteuid(realuid)) {
+               perror("seteuid(off)");
+               exit(1);
+       }
+       if (elevgid != realgid && setegid(realgid)) {
+               perror("setegid(off)");
+               exit(1);
+       }
+}
+
 /* VTY shell main routine. */
 int main(int argc, char **argv, char **env)
 {
@@ -270,17 +298,18 @@ int main(int argc, char **argv, char **env)
        int writeconfig = 0;
        int ret = 0;
        char *homedir = NULL;
+       int ditch_suid = 0;
 
-       /* check for restricted functionality if vtysh is run setuid */
-       int restricted = (getuid() != geteuid()) || (getgid() != getegid());
+       /* SUID: drop down to calling user & go back up when needed */
+       elevuid = geteuid();
+       elevgid = getegid();
+       realuid = getuid();
+       realgid = getgid();
+       suid_off();
 
        /* Preserve name of myself. */
        progname = ((p = strrchr(argv[0], '/')) ? ++p : argv[0]);
 
-       /* if logging open now */
-       if ((p = getenv("VTYSH_LOG")) != NULL)
-               logfile = fopen(p, "a");
-
        /* Option handling. */
        while (1) {
                opt = getopt_long(argc, argv, "be:c:d:nf:mEhCw", longopts, 0);
@@ -307,17 +336,11 @@ int main(int argc, char **argv, char **env)
                        tail = cr;
                } break;
                case OPTION_VTYSOCK:
+                       ditch_suid = 1; /* option disables SUID */
                        vty_sock_path = optarg;
                        break;
                case OPTION_CONFDIR:
-                       /*
-                        * Skip option for Config Directory if setuid
-                        */
-                       if (restricted) {
-                               fprintf(stderr,
-                                       "Overriding of Config Directory blocked for vtysh with setuid");
-                               return 1;
-                       }
+                       ditch_suid = 1; /* option disables SUID */
                        /*
                         * Overwrite location for vtysh.conf
                         */
@@ -395,6 +418,11 @@ int main(int argc, char **argv, char **env)
                }
        }
 
+       if (ditch_suid) {
+               elevuid = realuid;
+               elevgid = realgid;
+       }
+
        if (!vty_sock_path)
                vty_sock_path = frr_vtydir;
 
@@ -425,8 +453,11 @@ int main(int argc, char **argv, char **env)
 
        vty_init_vtysh();
 
-       /* Read vtysh configuration file before connecting to daemons. */
+       /* Read vtysh configuration file before connecting to daemons.
+        * (file may not be readable to calling user in SUID mode) */
+       suid_on();
        vtysh_read_config(vtysh_config_always);
+       suid_off();
 
        if (markfile) {
                if (!inputfile) {
@@ -486,6 +517,9 @@ int main(int argc, char **argv, char **env)
                }
        }
 
+       /* SUID: go back up elevated privs */
+       suid_on();
+
        /* Make sure we pass authentication before proceeding. */
        vtysh_auth();
 
@@ -498,6 +532,9 @@ int main(int argc, char **argv, char **env)
                        exit(1);
        }
 
+       /* SUID: back down, don't need privs further on */
+       suid_off();
+
        if (writeconfig) {
                vtysh_execute("enable");
                return vtysh_write_config_integrated();
@@ -531,6 +568,17 @@ int main(int argc, char **argv, char **env)
                }
        }
 
+       if (getenv("VTYSH_LOG")) {
+               const char *logpath = getenv("VTYSH_LOG");
+
+               logfile = fopen(logpath, "a");
+               if (!logfile) {
+                       fprintf(stderr, "Failed to open logfile (%s): %s\n",
+                               logpath, strerror(errno));
+                       exit(1);
+               }
+       }
+
        /* If eval mode. */
        if (cmd) {
                /* Enter into enable node. */