From 32f3268f51479989ec29790f7ba9ec03a2cacdcc Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@opensourcerouting.org>
Date: Sun, 27 Aug 2017 20:38:54 +0200
Subject: [PATCH] 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>
---
 vtysh/vtysh.h      |  3 ++
 vtysh/vtysh_main.c | 78 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h
index b0866ec7f..e10ab11da 100644
--- 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 8145bf364..b9909b493 100644
--- 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. */
-- 
2.39.2