summaryrefslogtreecommitdiffstats
path: root/vtysh
diff options
context:
space:
mode:
authorDavid Lamparter <equinox@opensourcerouting.org>2017-08-27 20:38:54 +0200
committerDavid Lamparter <equinox@opensourcerouting.org>2017-08-28 23:49:58 +0200
commit32f3268f51479989ec29790f7ba9ec03a2cacdcc (patch)
tree67484bbb666b028e82ae1d72bf73aaa0f0b3199a /vtysh
parentMerge pull request #1054 from dslicenc/eigrp-connected (diff)
downloadfrr-32f3268f51479989ec29790f7ba9ec03a2cacdcc.tar.xz
frr-32f3268f51479989ec29790f7ba9ec03a2cacdcc.zip
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>
Diffstat (limited to 'vtysh')
-rw-r--r--vtysh/vtysh.h3
-rw-r--r--vtysh/vtysh_main.c78
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. */