From 2da59394ec858ae5aecf2b26ab8d9fefdae17bb8 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 20:46:05 +0100 Subject: [PATCH 01/16] lib: add and use set_cloexec() watchquagga is already leaking an open file descriptor on its pid file on fork+exec() invocations; next up is adding vtysh support with even more fds. Mark things CLOEXEC before going there. Signed-off-by: David Lamparter --- lib/network.c | 14 ++++++++++++++ lib/network.h | 2 ++ lib/pid_output.c | 3 +++ lib/vty.c | 8 +++++++- 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/network.c b/lib/network.c index 5379ecb5a6..506e019136 100644 --- a/lib/network.c +++ b/lib/network.c @@ -94,6 +94,20 @@ set_nonblocking(int fd) return 0; } +int +set_cloexec(int fd) +{ + int flags; + flags = fcntl(fd, F_GETFD, 0); + if (flags == -1) + return -1; + + flags |= FD_CLOEXEC; + if (fcntl(fd, F_SETFD, flags) == -1) + return -1; + return 0; +} + float htonf (float host) { diff --git a/lib/network.h b/lib/network.h index 0fcb575d1c..a9126caf7f 100644 --- a/lib/network.h +++ b/lib/network.h @@ -33,6 +33,8 @@ extern int writen (int, const u_char *, int); -1 on error. */ extern int set_nonblocking(int fd); +extern int set_cloexec(int fd); + /* Does the I/O error indicate that the operation should be retried later? */ #define ERRNO_IO_RETRY(EN) \ (((EN) == EAGAIN) || ((EN) == EWOULDBLOCK) || ((EN) == EINTR)) diff --git a/lib/pid_output.c b/lib/pid_output.c index 5261babc6d..de4c2fba99 100644 --- a/lib/pid_output.c +++ b/lib/pid_output.c @@ -24,6 +24,7 @@ #include #include #include "version.h" +#include "network.h" #define PIDFILE_MASK 0644 #ifndef HAVE_FCNTL @@ -84,6 +85,8 @@ pid_output (const char *path) umask(oldumask); memset (&lock, 0, sizeof(lock)); + set_cloexec(fd); + lock.l_type = F_WRLCK; lock.l_whence = SEEK_SET; diff --git a/lib/vty.c b/lib/vty.c index 171aca1739..10ae99db07 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -1841,6 +1841,7 @@ vty_accept (struct thread *thread) return -1; } set_nonblocking(vty_sock); + set_cloexec(vty_sock); sockunion2hostprefix (&su, &p); @@ -1939,6 +1940,7 @@ vty_serv_sock_addrinfo (const char *hostname, unsigned short port) sockopt_v6only (ainfo->ai_family, sock); sockopt_reuseaddr (sock); sockopt_reuseport (sock); + set_cloexec (sock); ret = bind (sock, ainfo->ai_addr, ainfo->ai_addrlen); if (ret < 0) @@ -2006,6 +2008,7 @@ vty_serv_sock_family (const char* addr, unsigned short port, int family) /* This is server, so reuse address. */ sockopt_reuseaddr (accept_sock); sockopt_reuseport (accept_sock); + set_cloexec (accept_sock); /* Bind socket to universal address and given port. */ ret = sockunion_bind (accept_sock, &su, port, naddr); @@ -2068,6 +2071,8 @@ vty_serv_un (const char *path) len = sizeof (serv.sun_family) + strlen (serv.sun_path); #endif /* HAVE_STRUCT_SOCKADDR_UN_SUN_LEN */ + set_cloexec (sock); + ret = bind (sock, (struct sockaddr *) &serv, len); if (ret < 0) { @@ -2135,7 +2140,8 @@ vtysh_accept (struct thread *thread) close (sock); return -1; } - + set_cloexec(sock); + #ifdef VTYSH_DEBUG printf ("VTY shell accept\n"); #endif /* VTYSH_DEBUG */ From e227e24231b4a7ef892ccd8ad11c424c6a2d0c43 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 19:42:01 +0100 Subject: [PATCH 02/16] lib: privs: always look up VTY group Even if we're running without user switch, we should still try to honor the VTY group. This applies both to watchquagga (which always runs as root) as well as "no-userswitch" configurations for other daemons. Signed-off-by: David Lamparter --- lib/privs.c | 51 ++++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/privs.c b/lib/privs.c index 6cf87c18d4..ac2a8454c5 100644 --- a/lib/privs.c +++ b/lib/privs.c @@ -679,6 +679,15 @@ zprivs_init(struct zebra_privs_t *zprivs) exit (1); } + if (zprivs->vty_group) + { + /* in a "NULL" setup, this is allowed to fail too, but still try. */ + if ((grentry = getgrnam (zprivs->vty_group))) + zprivs_state.vtygrp = grentry->gr_gid; + else + zprivs_state.vtygrp = (gid_t)-1; + } + /* NULL privs */ if (! (zprivs->user || zprivs->group || zprivs->cap_num_p || zprivs->cap_num_i) ) @@ -731,34 +740,30 @@ zprivs_init(struct zebra_privs_t *zprivs) if (zprivs->vty_group) /* Add the vty_group to the supplementary groups so it can be chowned to */ { - if ( (grentry = getgrnam (zprivs->vty_group)) ) - { - zprivs_state.vtygrp = grentry->gr_gid; - - for ( i = 0; i < ngroups; i++ ) - if ( groups[i] == zprivs_state.vtygrp ) - { - found++; - break; - } - - if (!found) - { - fprintf (stderr, "privs_init: user(%s) is not part of vty group specified(%s)\n", - zprivs->user, zprivs->vty_group); - exit (1); - } - if ( i >= ngroups && ngroups < (int) ZEBRA_NUM_OF(groups) ) - { - groups[i] = zprivs_state.vtygrp; - } - } - else + if (zprivs_state.vtygrp == (gid_t)-1) { fprintf (stderr, "privs_init: could not lookup vty group %s\n", zprivs->vty_group); exit (1); } + + for ( i = 0; i < ngroups; i++ ) + if ( groups[i] == zprivs_state.vtygrp ) + { + found++; + break; + } + + if (!found) + { + fprintf (stderr, "privs_init: user(%s) is not part of vty group specified(%s)\n", + zprivs->user, zprivs->vty_group); + exit (1); + } + if ( i >= ngroups && ngroups < (int) ZEBRA_NUM_OF(groups) ) + { + groups[i] = zprivs_state.vtygrp; + } } if (ngroups) From cb947ba3aeb154ea2ad7a55ac09ac624d05978ae Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 18:22:30 +0100 Subject: [PATCH 03/16] vtysh: detangle configuration writes vtysh has a very convoluted and confusing setup where it isn't even clear which files are written where (since some filenames come indirectly from loading config). Detangle. This also removes writing vtysh.conf. The file is intended to be manually edited since it has some vague security concerns (if PAM is used). Signed-off-by: David Lamparter --- vtysh/vtysh.c | 68 +++++++------------------------------------- vtysh/vtysh.h | 2 ++ vtysh/vtysh_config.c | 1 - vtysh/vtysh_main.c | 15 +++++----- 4 files changed, 21 insertions(+), 65 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 29697b564f..cb98a11e90 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -77,8 +77,6 @@ struct vtysh_client vtysh_client[] = enum vtysh_write_integrated vtysh_write_integrated = WRITE_INTEGRATED_UNSPECIFIED; -extern char config_default[]; - static void vclient_close (struct vtysh_client *vclient) { @@ -2465,33 +2463,19 @@ write_config_integrated(void) { u_int i; char line[] = "write terminal\n"; - FILE *fp, *fp1; + FILE *fp; fprintf (stdout,"Building Configuration...\n"); - backup_config_file(integrate_default); - backup_config_file(host.config); - - fp = fopen (integrate_default, "w"); + backup_config_file(quagga_config); + fp = fopen (quagga_config, "w"); if (fp == NULL) { fprintf (stdout,"%% Can't open configuration file %s due to '%s'\n", - integrate_default, safe_strerror(errno)); + quagga_config, safe_strerror(errno)); return CMD_SUCCESS; } - fp1 = fopen (host.config, "w"); - if (fp1 == NULL) - { - fprintf (stdout,"%% Can't open configuration file %s due to '%s'\n", - host.config, safe_strerror(errno)); - return CMD_SUCCESS; - } - - vtysh_config_write (); - vtysh_config_dump (fp1); - - fclose (fp1); for (i = 0; i < array_size(vtysh_client); i++) vtysh_client_config (&vtysh_client[i], line); @@ -2500,20 +2484,14 @@ write_config_integrated(void) fclose (fp); - if (chmod (integrate_default, CONFIGFILE_MASK) != 0) + if (chmod (quagga_config, CONFIGFILE_MASK) != 0) { fprintf (stdout,"%% Can't chmod configuration file %s: %s\n", - integrate_default, safe_strerror(errno)); + quagga_config, safe_strerror(errno)); return CMD_WARNING; } - if (chmod (host.config, CONFIGFILE_MASK) != 0) - { - fprintf (stdout,"%% Can't chmod configuration file %s: %s (%d)\n", - integrate_default, safe_strerror(errno), errno); - return CMD_WARNING; - } - fprintf(stdout,"Integrated configuration saved to %s\n",integrate_default); + fprintf(stdout,"Integrated configuration saved to %s\n", quagga_config); fprintf (stdout,"[OK]\n"); @@ -2527,7 +2505,7 @@ static bool vtysh_writeconfig_integrated(void) switch (vtysh_write_integrated) { case WRITE_INTEGRATED_UNSPECIFIED: - if (stat(integrate_default, &s) && errno == ENOENT) + if (stat(quagga_config, &s) && errno == ENOENT) return false; return true; case WRITE_INTEGRATED_NO: @@ -2547,42 +2525,18 @@ DEFUN (vtysh_write_memory, int ret = CMD_SUCCESS; char line[] = "write memory\n"; u_int i; - FILE *fp; + + fprintf (stdout, "Note: this version of vtysh never writes vtysh.conf\n"); /* If integrated Quagga.conf explicitely set. */ if (vtysh_writeconfig_integrated()) return write_config_integrated(); - else - backup_config_file(integrate_default); fprintf (stdout,"Building Configuration...\n"); - + for (i = 0; i < array_size(vtysh_client); i++) ret = vtysh_client_execute (&vtysh_client[i], line, stdout); - - fp = fopen(host.config, "w"); - if (fp == NULL) - { - fprintf (stdout,"%% Can't open configuration file %s due to '%s'\n", - host.config, safe_strerror(errno)); - return CMD_SUCCESS; - } - - vtysh_config_write (); - vtysh_config_dump (fp); - - fclose (fp); - - if (chmod (host.config, CONFIGFILE_MASK) != 0) - { - fprintf (stdout,"%% Can't chmod configuration file %s: %s\n", - integrate_default, safe_strerror(errno)); - return CMD_WARNING; - } - - fprintf (stdout,"[OK]\n"); - return ret; } diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index 7241b4c125..08b24731bc 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -53,6 +53,8 @@ enum vtysh_write_integrated { extern enum vtysh_write_integrated vtysh_write_integrated; +extern char *quagga_config; + void vtysh_init_vty (void); void vtysh_init_cmd (void); extern int vtysh_connect_all (const char *optional_daemon_name); diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c index 7ad457ee7b..4ec0e00286 100644 --- a/vtysh/vtysh_config.c +++ b/vtysh/vtysh_config.c @@ -376,7 +376,6 @@ vtysh_read_config (const char *config_default_dir) FILE *confp = NULL; int ret; - host_config_set (config_default_dir); confp = fopen (config_default_dir, "r"); if (confp == NULL) { diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index 751152e911..a82acda17d 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -67,8 +67,9 @@ struct zebra_privs_t vtysh_privs = }; /* Configuration file name and directory. */ -char config_default[] = SYSCONFDIR VTYSH_DEFAULT_CONFIG; -char quagga_config_default[] = SYSCONFDIR QUAGGA_DEFAULT_CONFIG; +static char vtysh_config_always[] = SYSCONFDIR VTYSH_DEFAULT_CONFIG; +static char quagga_config_default[] = SYSCONFDIR QUAGGA_DEFAULT_CONFIG; +char *quagga_config = quagga_config_default; char history_file[MAXPATHLEN]; /* Flag for indicate executing child command. */ @@ -373,7 +374,7 @@ main (int argc, char **argv, char **env) vty_init_vtysh (); /* Read vtysh configuration file before connecting to daemons. */ - vtysh_read_config(config_default); + vtysh_read_config(vtysh_config_always); if (markfile) { @@ -512,17 +513,17 @@ main (int argc, char **argv, char **env) history_truncate_file(history_file,1000); exit (0); } - + /* Boot startup configuration file. */ if (boot_flag) { - vtysh_flock_config (integrate_default); - int ret = vtysh_read_config (integrate_default); + vtysh_flock_config (quagga_config); + int ret = vtysh_read_config (quagga_config); vtysh_unflock_config (); if (ret) { fprintf (stderr, "Configuration file[%s] processing failure: %d\n", - integrate_default, ret); + quagga_config, ret); if (no_error) exit (0); else From a68f861653da95eeddf96f2afabd481db2798613 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 19:01:06 +0100 Subject: [PATCH 04/16] vtysh: add -w option for integrated-config write This new option is intended to be used both by watchquagga as well as directly by users. It performs the collect-configuration operation and writes out Quagga.conf, regardless of whether integrated-config is enabled or not. Signed-off-by: David Lamparter --- vtysh/vtysh.c | 10 +++++----- vtysh/vtysh.h | 1 + vtysh/vtysh_main.c | 25 ++++++++++++++++++++++++- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index cb98a11e90..671eba58eb 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2458,8 +2458,8 @@ backup_config_file (const char *fbackup) free (integrate_sav); } -static int -write_config_integrated(void) +int +vtysh_write_config_integrated(void) { u_int i; char line[] = "write terminal\n"; @@ -2498,7 +2498,7 @@ write_config_integrated(void) return CMD_SUCCESS; } -static bool vtysh_writeconfig_integrated(void) +static bool want_config_integrated(void) { struct stat s; @@ -2529,8 +2529,8 @@ DEFUN (vtysh_write_memory, fprintf (stdout, "Note: this version of vtysh never writes vtysh.conf\n"); /* If integrated Quagga.conf explicitely set. */ - if (vtysh_writeconfig_integrated()) - return write_config_integrated(); + if (want_config_integrated()) + return vtysh_write_config_integrated(); fprintf (stdout,"Building Configuration...\n"); diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index 08b24731bc..d515918e03 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -75,6 +75,7 @@ void config_add_line (struct list *, const char *); int vtysh_mark_file(const char *filename); int vtysh_read_config (const char *); +int vtysh_write_config_integrated (void); void vtysh_config_parse_line (const char *); diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index a82acda17d..f2e62ba815 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -167,6 +167,7 @@ usage (int status) "-E, --echo Echo prompt and command in -c mode\n" \ "-C, --dryrun Check configuration for validity and exit\n" \ "-m, --markfile Mark input file with context end\n" + "-w, --writeconfig Write integrated config (Quagga.conf) and exit\n" "-h, --help Display this help and exit\n\n" \ "Note that multiple commands may be executed from the command\n" \ "line by passing multiple -c args, or by embedding linefeed\n" \ @@ -190,6 +191,7 @@ struct option longopts[] = { "help", no_argument, NULL, 'h'}, { "noerror", no_argument, NULL, 'n'}, { "mark", no_argument, NULL, 'm'}, + { "writeconfig", no_argument, NULL, 'w'}, { 0 } }; @@ -290,6 +292,7 @@ main (int argc, char **argv, char **env) int echo_command = 0; int no_error = 0; int markfile = 0; + int writeconfig = 0; int ret = 0; char *homedir = NULL; @@ -303,7 +306,7 @@ main (int argc, char **argv, char **env) /* Option handling. */ while (1) { - opt = getopt_long (argc, argv, "be:c:d:nf:mEhC", longopts, 0); + opt = getopt_long (argc, argv, "be:c:d:nf:mEhCw", longopts, 0); if (opt == EOF) break; @@ -347,6 +350,9 @@ main (int argc, char **argv, char **env) case 'C': dryrun = 1; break; + case 'w': + writeconfig = 1; + break; case 'h': usage (0); break; @@ -356,6 +362,18 @@ main (int argc, char **argv, char **env) } } + if (markfile + writeconfig + dryrun + boot_flag > 1) + { + fprintf (stderr, "Invalid combination of arguments. Please specify at " + "most one of:\n\t-b, -C, -m, -w\n"); + return 1; + } + if (inputfile && (writeconfig || boot_flag)) + { + fprintf (stderr, "WARNING: Combinining the -f option with -b or -w is " + "NOT SUPPORTED since its\nresults are inconsistent!\n"); + } + /* Initialize user input buffer. */ line_read = NULL; setlinebuf(stdout); @@ -423,6 +441,11 @@ main (int argc, char **argv, char **env) exit(1); } + if (writeconfig) + { + return vtysh_write_config_integrated (); + } + if (inputfile) { vtysh_flock_config (inputfile); From 367988eeb307846fd173caae36905ba645a5305b Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 23:36:01 +0100 Subject: [PATCH 05/16] vtysh: set config file permissions As vtysh may hopefully be running as root from watchquagga here, let's try to fix up ownership and permissions for Quagga.conf. Doing chown/chmod instead of changing the process's user/group IDs has the advantage of fixing up preexisting misconfigurations. Note errors in chmod/chown will print a message but the config is already written at that point. Signed-off-by: David Lamparter --- vtysh/vtysh.c | 62 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 671eba58eb..ed6c892ba4 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2464,6 +2464,13 @@ vtysh_write_config_integrated(void) u_int i; char line[] = "write terminal\n"; FILE *fp; + int fd; + struct passwd *pwentry; + struct group *grentry; + uid_t uid = -1; + gid_t gid = -1; + struct stat st; + int err = 0; fprintf (stdout,"Building Configuration...\n"); @@ -2475,6 +2482,7 @@ vtysh_write_config_integrated(void) quagga_config, safe_strerror(errno)); return CMD_SUCCESS; } + fd = fileno (fp); for (i = 0; i < array_size(vtysh_client); i++) vtysh_client_config (&vtysh_client[i], line); @@ -2482,17 +2490,57 @@ vtysh_write_config_integrated(void) vtysh_config_write (); vtysh_config_dump (fp); - fclose (fp); - - if (chmod (quagga_config, CONFIGFILE_MASK) != 0) + if (fchmod (fd, CONFIGFILE_MASK) != 0) { - fprintf (stdout,"%% Can't chmod configuration file %s: %s\n", - quagga_config, safe_strerror(errno)); - return CMD_WARNING; + printf ("%% Warning: can't chmod configuration file %s: %s\n", + quagga_config, safe_strerror(errno)); + err++; } - fprintf(stdout,"Integrated configuration saved to %s\n", quagga_config); + pwentry = getpwnam (QUAGGA_USER); + if (pwentry) + uid = pwentry->pw_uid; + else + { + printf ("%% Warning: could not look up user \"%s\"\n", QUAGGA_USER); + err++; + } + grentry = getgrnam (QUAGGA_GROUP); + if (grentry) + gid = grentry->gr_gid; + else + { + printf ("%% Warning: could not look up group \"%s\"\n", QUAGGA_GROUP); + err++; + } + + if (!fstat (fd, &st)) + { + if (st.st_uid == uid) + uid = -1; + if (st.st_gid == gid) + gid = -1; + if ((uid != (uid_t)-1 || gid != (gid_t)-1) && fchown (fd, uid, gid)) + { + printf ("%% Warning: can't chown configuration file %s: %s\n", + quagga_config, safe_strerror(errno)); + err++; + } + } + else + { + printf ("%% Warning: stat() failed on %s: %s\n", + quagga_config, safe_strerror(errno)); + err++; + } + + fclose (fp); + + if (err) + return CMD_WARNING; + + fprintf (stdout,"Integrated configuration saved to %s\n", quagga_config); fprintf (stdout,"[OK]\n"); return CMD_SUCCESS; From 46d5d8ec829a3b2daba6fe86463ece550851fc38 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 23:56:34 +0100 Subject: [PATCH 06/16] watchquagga: add ZLOG_WATCHQUAGGA watchquagga logs as "NONE", which will also become visible in vtysh for error messages. Add "WATCHQUAGGA" log target. Signed-off-by: David Lamparter --- lib/log.c | 3 ++- lib/log.h | 1 + watchquagga/watchquagga.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/log.c b/lib/log.c index cd1f0bb771..f9877300b4 100644 --- a/lib/log.c +++ b/lib/log.c @@ -58,7 +58,8 @@ const char *zlog_proto_names[] = "LDP", "ISIS", "PIM", - "RFP", + "RFP", + "WATCHQUAGGA", NULL, }; diff --git a/lib/log.h b/lib/log.h index d71b85df1a..feb1e1852d 100644 --- a/lib/log.h +++ b/lib/log.h @@ -59,6 +59,7 @@ typedef enum ZLOG_ISIS, ZLOG_PIM, ZLOG_RFP, + ZLOG_WATCHQUAGGA, } zlog_proto_t; /* If maxlvl is set to ZLOG_DISABLED, then no messages will be sent diff --git a/watchquagga/watchquagga.c b/watchquagga/watchquagga.c index e882653e38..aa5d1bcd80 100644 --- a/watchquagga/watchquagga.c +++ b/watchquagga/watchquagga.c @@ -1335,7 +1335,7 @@ main(int argc, char **argv) return usage(progname,1); } - zlog_default = openzlog(progname, ZLOG_NONE, 0, + zlog_default = openzlog(progname, ZLOG_WATCHQUAGGA, 0, LOG_CONS|LOG_NDELAY|LOG_PID, LOG_DAEMON); zlog_set_level(NULL, ZLOG_DEST_MONITOR, ZLOG_DISABLED); if (daemon_mode) From 95c4aff294f6c1bf1450cbebc4133cad3560dfc9 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 9 Nov 2016 14:15:34 +0100 Subject: [PATCH 07/16] watchquagga: add "write integrated" This new command - available for internal use by vtysh and explicit usage by users - calls "vtysh -w" from watchquagga. This ensures vtysh is run with privileges to actually write the integrated-config file. Signed-off-by: David Lamparter --- configure.ac | 11 +++ lib/command.h | 1 + lib/vty.c | 7 ++ vtysh/Makefile.am | 1 + watchquagga/Makefile.am | 4 +- watchquagga/watchquagga.c | 29 +++++++- watchquagga/watchquagga.h | 29 ++++++++ watchquagga/watchquagga_vty.c | 128 ++++++++++++++++++++++++++++++++++ 8 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 watchquagga/watchquagga.h create mode 100644 watchquagga/watchquagga_vty.c diff --git a/configure.ac b/configure.ac index 3ca5fe8523..c99ba222d6 100755 --- a/configure.ac +++ b/configure.ac @@ -1737,6 +1737,17 @@ AC_DEFINE_UNQUOTED(ISIS_VTYSH_PATH, "$quagga_statedir/isisd.vty",isisd vty socke AC_DEFINE_UNQUOTED(PIM_VTYSH_PATH, "$quagga_statedir/pimd.vty",pimd vty socket) AC_DEFINE_UNQUOTED(DAEMON_VTY_DIR, "$quagga_statedir",daemon vty directory) +dnl autoconf does this, but it does it too late... +test "x$prefix" = xNONE && prefix=$ac_default_prefix +test "x$exec_prefix" = xNONE && exec_prefix='${prefix}' + +dnl get the full path, recursing through variables... +vtysh_bin="$bindir/vtysh" +for I in 1 2 3 4 5 6 7 8 9 10; do + eval vtysh_bin="\"$vtysh_bin\"" +done +AC_DEFINE_UNQUOTED(VTYSH_BIN_PATH, "$vtysh_bin",path to vtysh binary) + dnl ------------------------------- dnl Quagga sources should always be dnl current wrt interfaces. Dont diff --git a/lib/command.h b/lib/command.h index e4aa10196f..ba40770ba3 100644 --- a/lib/command.h +++ b/lib/command.h @@ -226,6 +226,7 @@ struct cmd_token #define CMD_COMPLETE_LIST_MATCH 9 #define CMD_SUCCESS_DAEMON 10 #define CMD_ERR_NO_FILE 11 +#define CMD_SUSPEND 12 /* Argc max counts. */ #define CMD_ARGC_MAX 25 diff --git a/lib/vty.c b/lib/vty.c index 10ae99db07..26d0b67ecf 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -2235,6 +2235,13 @@ vtysh_read (struct thread *thread) printf ("vtysh node: %d\n", vty->node); #endif /* VTYSH_DEBUG */ + /* hack for asynchronous "write integrated" + * - other commands in "buf" will be ditched + * - input during pending config-write is "unsupported" */ + if (ret == CMD_SUSPEND) + break; + + /* warning: watchquagga hardcodes this result write */ header[3] = ret; buffer_put(vty->obuf, header, 4); diff --git a/vtysh/Makefile.am b/vtysh/Makefile.am index 58ffdfca26..0ff2d738f0 100644 --- a/vtysh/Makefile.am +++ b/vtysh/Makefile.am @@ -86,6 +86,7 @@ vtysh_cmd_FILES = $(vtysh_scan) \ $(top_srcdir)/zebra/zebra_fpm.c \ $(top_srcdir)/zebra/zebra_ptm.c \ $(top_srcdir)/zebra/zebra_mpls_vty.c \ + $(top_srcdir)/watchquagga/watchquagga_vty.c \ $(BGP_VNC_RFAPI_SRC) $(BGP_VNC_RFP_SRC) vtysh_cmd.c: $(vtysh_cmd_FILES) extract.pl diff --git a/watchquagga/Makefile.am b/watchquagga/Makefile.am index 1f05f26ced..43f743eba2 100644 --- a/watchquagga/Makefile.am +++ b/watchquagga/Makefile.am @@ -7,5 +7,7 @@ AM_CFLAGS = $(WERROR) sbin_PROGRAMS = watchquagga -watchquagga_SOURCES = watchquagga.c +noinst_HEADERS = watchquagga.h + +watchquagga_SOURCES = watchquagga.c watchquagga_vty.c watchquagga_LDADD = ../lib/libzebra.la @LIBCAP@ diff --git a/watchquagga/watchquagga.c b/watchquagga/watchquagga.c index aa5d1bcd80..70b35f775d 100644 --- a/watchquagga/watchquagga.c +++ b/watchquagga/watchquagga.c @@ -24,12 +24,16 @@ #include #include #include +#include "command.h" + #include #include #include #include #include +#include "watchquagga.h" + #ifndef MIN #define MIN(X,Y) (((X) <= (Y)) ? (X) : (Y)) #endif @@ -437,6 +441,12 @@ sigchild(void) return; } + if (child == integrated_write_pid) + { + integrated_write_sigchld(status); + return; + } + if ((restart = find_child(child)) != NULL) { name = restart->name; @@ -775,9 +785,9 @@ try_connect(struct daemon *dmn) return -1; } - if (set_nonblocking(sock) < 0) + if (set_nonblocking(sock) < 0 || set_cloexec(sock) < 0) { - zlog_err("%s(%s): set_nonblocking(%d) failed", + zlog_err("%s(%s): set_nonblocking/cloexec(%d) failed", __func__, addr.sun_path, sock); close(sock); return -1; @@ -1028,6 +1038,13 @@ translate_blanks(const char *cmd, const char *blankstr) return res; } +struct zebra_privs_t watchquagga_privs = +{ +#ifdef VTY_GROUP + .vty_group = VTY_GROUP, +#endif +}; + int main(int argc, char **argv) { @@ -1283,7 +1300,15 @@ main(int argc, char **argv) } gs.restart.interval = gs.min_restart_interval; + + zprivs_init (&watchquagga_privs); + master = thread_master_create(); + cmd_init(1); + vty_init(master); + watchquagga_vty_init(); + vty_serv_sock(NULL, 0, WATCHQUAGGA_VTYSH_PATH); + systemd_send_started (master, 0); signal_init (master, array_size(my_signals), my_signals); srandom(time(NULL)); diff --git a/watchquagga/watchquagga.h b/watchquagga/watchquagga.h new file mode 100644 index 0000000000..ecadf22c56 --- /dev/null +++ b/watchquagga/watchquagga.h @@ -0,0 +1,29 @@ +/* + Common definitions for watchquagga API socket. + + Copyright (C) 2016 David Lamparter for NetDEF, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef QUAGGA_WATCHQUAGGA_H +#define QUAGGA_WATCHQUAGGA_H + +extern void watchquagga_vty_init(void); + +extern pid_t integrated_write_pid; +extern void integrated_write_sigchld(int status); + +#endif /* QUAGGA_WATCHQUAGGA_H */ diff --git a/watchquagga/watchquagga_vty.c b/watchquagga/watchquagga_vty.c new file mode 100644 index 0000000000..bf413376af --- /dev/null +++ b/watchquagga/watchquagga_vty.c @@ -0,0 +1,128 @@ +/* + watchquagga CLI functions. + + Copyright (C) 2016 David Lamparter for NetDEF, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include +#include + +#include "memory.h" +#include "log.h" +#include "vty.h" +#include "command.h" + +#include "watchquagga.h" + +pid_t integrated_write_pid; +static int integrated_result_fd; + +DEFUN (config_write_integrated, + config_write_integrated_cmd, + "write integrated", + "Write running configuration to memory, network, or terminal\n" + "Write integrated all-daemon Quagga.conf file\n") +{ + pid_t child; + sigset_t oldmask, sigmask; + + if (integrated_write_pid != -1) { + vty_out(vty, "%% configuration write already in progress.%s", + VTY_NEWLINE); + return CMD_WARNING; + } + + fflush(stdout); + fflush(stderr); + + /* need to temporarily block SIGCHLD because it could arrive between + * fork() call and setting the integrated_write_pid variable. This + * would mean the completion call gets lost and this hangs forever. + */ + sigemptyset(&oldmask); + sigemptyset(&sigmask); + sigaddset(&sigmask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sigmask, &oldmask); + + child = fork(); + if (child == -1) { + vty_out(vty, "%% configuration write fork() failed: %s.%s", + safe_strerror(errno), VTY_NEWLINE); + sigprocmask(SIG_SETMASK, &oldmask, NULL); + return CMD_WARNING; + } + if (child != 0) { + /* note: the VTY won't write a command return value to vtysh; the + * session temporarily enters an intentional "hang" state. This is + * to make sure latency in vtysh doing the config write (several + * seconds is not rare to see) does not interfere with watchquagga's + * supervisor job. + * + * The fd is duplicated here so we don't need to hold a vty pointer + * (which could become invalid in the meantime). + */ + integrated_write_pid = child; + integrated_result_fd = dup(vty->wfd); + sigprocmask(SIG_SETMASK, &oldmask, NULL); + return CMD_SUSPEND; + } + + /* redirect stdout/stderr to vty session. Note vty->wfd is marked + * CLOEXEC, but dup2 will clear that flag. */ + dup2(vty->wfd, 1); + dup2(vty->wfd, 2); + + /* don't allow the user to pass parameters, we're root here! + * should probably harden vtysh at some point too... */ + execl(VTYSH_BIN_PATH, "vtysh", "-w", NULL); + + /* unbuffered write; we just messed with stdout... */ + char msg[512]; + snprintf(msg, sizeof(msg), "error executing %s: %s\n", + VTYSH_BIN_PATH, safe_strerror(errno)); + write(1, msg, strlen(msg)); + exit(1); +} + +void integrated_write_sigchld(int status) +{ + uint8_t reply[4] = { 0, 0, 0, CMD_WARNING }; + + if (WIFEXITED(status)) { + zlog_info("configuration write completed with exit code %d", + WEXITSTATUS(status)); + reply[3] = WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + zlog_warn("configuration write terminated by signal %d", + WTERMSIG(status)); + } else { + zlog_warn("configuration write terminated"); + } + + /* don't care about failures here, if the connection is broken the + * return value will just be lost. */ + write(integrated_result_fd, reply, sizeof(reply)); + close(integrated_result_fd); + + integrated_write_pid = -1; +} + +void watchquagga_vty_init(void) +{ + integrated_write_pid = -1; + install_element(ENABLE_NODE, &config_write_integrated_cmd); +} From 4a96e94474ec52767a18df7e84873d752d863834 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 19:41:48 +0100 Subject: [PATCH 08/16] vtysh: add watchquagga to target list Also tag some commands as VTYSH_REALLYALL; these are absolutely neccessary for correct vtysh operation and will cause "interesting" breakage if not present on all daemons. Signed-off-by: David Lamparter --- configure.ac | 1 + vtysh/vtysh.c | 15 ++++++++------- vtysh/vtysh.h | 6 ++++++ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index c99ba222d6..4032a447cc 100755 --- a/configure.ac +++ b/configure.ac @@ -1735,6 +1735,7 @@ AC_DEFINE_UNQUOTED(OSPF6_VTYSH_PATH, "$quagga_statedir/ospf6d.vty",ospf6d vty so AC_DEFINE_UNQUOTED(LDP_VTYSH_PATH, "$quagga_statedir/ldpd.vty",ldpd vty socket) AC_DEFINE_UNQUOTED(ISIS_VTYSH_PATH, "$quagga_statedir/isisd.vty",isisd vty socket) AC_DEFINE_UNQUOTED(PIM_VTYSH_PATH, "$quagga_statedir/pimd.vty",pimd vty socket) +AC_DEFINE_UNQUOTED(WATCHQUAGGA_VTYSH_PATH, "$quagga_statedir/watchquagga.vty",watchquagga vty socket) AC_DEFINE_UNQUOTED(DAEMON_VTY_DIR, "$quagga_statedir",daemon vty directory) dnl autoconf does this, but it does it too late... diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index ed6c892ba4..9888c35346 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -73,6 +73,7 @@ struct vtysh_client vtysh_client[] = { .fd = -1, .name = "bgpd", .flag = VTYSH_BGPD, .path = BGP_VTYSH_PATH, .next = NULL}, { .fd = -1, .name = "isisd", .flag = VTYSH_ISISD, .path = ISIS_VTYSH_PATH, .next = NULL}, { .fd = -1, .name = "pimd", .flag = VTYSH_PIMD, .path = PIM_VTYSH_PATH, .next = NULL}, + { .fd = -1, .name = "watchquagga", .flag = VTYSH_WATCHQUAGGA, .path = WATCHQUAGGA_VTYSH_PATH, .next = NULL}, }; enum vtysh_write_integrated vtysh_write_integrated = WRITE_INTEGRATED_UNSPECIFIED; @@ -1073,7 +1074,7 @@ vtysh_end (void) return CMD_SUCCESS; } -DEFUNSH (VTYSH_ALL, +DEFUNSH (VTYSH_REALLYALL, vtysh_end_all, vtysh_end_all_cmd, "end", @@ -1480,8 +1481,8 @@ DEFUNSH (VTYSH_ALL, return CMD_SUCCESS; } -DEFUNSH (VTYSH_ALL, - vtysh_enable, +DEFUNSH (VTYSH_REALLYALL, + vtysh_enable, vtysh_enable_cmd, "enable", "Turn on privileged mode command\n") @@ -1490,8 +1491,8 @@ DEFUNSH (VTYSH_ALL, return CMD_SUCCESS; } -DEFUNSH (VTYSH_ALL, - vtysh_disable, +DEFUNSH (VTYSH_REALLYALL, + vtysh_disable, vtysh_disable_cmd, "disable", "Turn off privileged mode command\n") @@ -1501,7 +1502,7 @@ DEFUNSH (VTYSH_ALL, return CMD_SUCCESS; } -DEFUNSH (VTYSH_ALL, +DEFUNSH (VTYSH_REALLYALL, vtysh_config_terminal, vtysh_config_terminal_cmd, "configure terminal", @@ -1582,7 +1583,7 @@ vtysh_exit (struct vty *vty) return CMD_SUCCESS; } -DEFUNSH (VTYSH_ALL, +DEFUNSH (VTYSH_REALLYALL, vtysh_exit_all, vtysh_exit_all_cmd, "exit", diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index d515918e03..dade049ad7 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -34,7 +34,13 @@ DECLARE_MGROUP(MVTYSH) #define VTYSH_ISISD 0x40 #define VTYSH_PIMD 0x100 #define VTYSH_LDPD 0x200 +#define VTYSH_WATCHQUAGGA 0x400 +/* commands in REALLYALL are crucial to correct vtysh operation */ +#define VTYSH_REALLYALL ~0U +/* watchquagga is not in ALL since library CLI functions should not be + * run on it (logging & co. should stay in a fixed/frozen config, and + * things like prefix lists are not even initialised) */ #define VTYSH_ALL VTYSH_ZEBRA|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_OSPFD|VTYSH_OSPF6D|VTYSH_LDPD|VTYSH_BGPD|VTYSH_ISISD|VTYSH_PIMD #define VTYSH_RMAP VTYSH_ZEBRA|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_OSPFD|VTYSH_OSPF6D|VTYSH_BGPD|VTYSH_PIMD #define VTYSH_INTERFACE VTYSH_ZEBRA|VTYSH_RIPD|VTYSH_RIPNGD|VTYSH_OSPFD|VTYSH_OSPF6D|VTYSH_LDPD|VTYSH_ISISD|VTYSH_PIMD From e10ca9b6b8789c7178dc24241d7ee158d3e757dd Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 23:36:16 +0100 Subject: [PATCH 09/16] vtysh: funnel integrated write through watchquagga Running vtysh as normal user won't have permissions to write Quagga.conf. If we're connected to watchquagga, try "write integrated" first. In all cases if something fails, try directly. Signed-off-by: David Lamparter --- vtysh/vtysh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 9888c35346..6a5a97bd2a 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2579,7 +2579,23 @@ DEFUN (vtysh_write_memory, /* If integrated Quagga.conf explicitely set. */ if (want_config_integrated()) - return vtysh_write_config_integrated(); + { + ret = CMD_WARNING; + for (i = 0; i < array_size(vtysh_client); i++) + if (vtysh_client[i].flag == VTYSH_WATCHQUAGGA) + break; + if (i < array_size(vtysh_client) && vtysh_client[i].fd != -1) + ret = vtysh_client_execute (&vtysh_client[i], "write integrated", stdout); + + if (ret != CMD_SUCCESS) + { + printf("Warning: attempting direct configuration write without " + "watchquagga.\nFile permissions and ownership may be " + "incorrect, or write may fail.\n"); + ret = vtysh_write_config_integrated(); + } + return ret; + } fprintf (stdout,"Building Configuration...\n"); From 1f8df88720e8a93fb1a5b05bda435df1dcd5c96e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Nov 2016 19:02:26 +0100 Subject: [PATCH 10/16] Revert "vtysh: Make vtysh run as quagga user" This reverts commit 5dd58b08299e85735f19fba1ee307c509fb19de7. Changing vtysh uid/gid is now actually counterproductive. Signed-off-by: David Lamparter --- vtysh/vtysh_main.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index f2e62ba815..d9c0e3fbfd 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -35,7 +35,6 @@ #include "getopt.h" #include "command.h" #include "memory.h" -#include "privs.h" #include "linklist.h" #include "memory_vty.h" @@ -45,27 +44,6 @@ /* VTY shell program name. */ char *progname; -static zebra_capabilities_t _caps_p [] = -{ - ZCAP_BIND, - ZCAP_NET_RAW, - ZCAP_NET_ADMIN, -}; - -struct zebra_privs_t vtysh_privs = -{ -#if defined(QUAGGA_USER) && defined(QUAGGA_GROUP) - .user = QUAGGA_USER, - .group = QUAGGA_GROUP, -#endif -#ifdef VTY_GROUP - .vty_group = VTY_GROUP, -#endif - .caps_p = _caps_p, - .cap_num_p = array_size(_caps_p), - .cap_num_i = 0, -}; - /* Configuration file name and directory. */ static char vtysh_config_always[] = SYSCONFDIR VTYSH_DEFAULT_CONFIG; static char quagga_config_default[] = SYSCONFDIR QUAGGA_DEFAULT_CONFIG; @@ -378,8 +356,6 @@ main (int argc, char **argv, char **env) line_read = NULL; setlinebuf(stdout); - zprivs_init (&vtysh_privs); - /* Signal and others. */ vtysh_signal_init (); From 207e0d7a7909b75b2cb9d7bf8f8e817698bf55c9 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 9 Nov 2016 10:22:22 -0500 Subject: [PATCH 11/16] watchquagga: Signal when we are actually up and running When Quagga is starting up, it is returning immediately. This is leaving us in a state where systemd believes Quagga is up and running, while the sytem might actually not have restarted the code yet. Modify the code so that when watchquagga starts up it doesn't start communicating with systemd until such time that it detects that all daemons are running. Additionally modify watchquagga to touch a file in /var/run/quagga/ that the /usr/lib/quagga/quagga script looks for for 10 seconds. If it finds this Quagga started file then we know watchquagga has successfully communicated with all daemons. If after 10 seconds we haven't communicated with Quagga, continue on for the start and let the normal start failure code work. Signed-off-by: Donald Sharp --- tools/quagga | 14 ++++++++++++-- watchquagga/watchquagga.c | 24 +++++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/tools/quagga b/tools/quagga index 83dfb63029..e8595d7877 100755 --- a/tools/quagga +++ b/tools/quagga @@ -33,7 +33,6 @@ else SSD=`which start-stop-daemon` fi -echo ${SSD} # Print the name of the pidfile. pidfile() { @@ -114,12 +113,23 @@ start() echo -n " $1" fi + if [ -e /var/run/quagga/watchquagga.started ] ; then + rm /var/run/quagga/watchquagga.started + fi ${SSD} \ --start \ --pidfile=`pidfile $1` \ --exec "$D_PATH/$1" \ -- \ "${watchquagga_options[@]}" + for i in `seq 1 10`; + do + if [ -e /var/run/quagga/watchquagga.started ] ; then + break + else + sleep 1 + fi + done elif [ -n "$2" ]; then echo -n " $1-$2" if ! check_daemon $1 $2 ; then @@ -502,8 +512,8 @@ case "$1" in if [ "$2" != "watchquagga" ]; then start_prio 10 $dmn fi - vtysh_b start_watchquagga + vtysh_b ;; 1|2|3|4|5|6|7|8|9|10) diff --git a/watchquagga/watchquagga.c b/watchquagga/watchquagga.c index e882653e38..cb9d50ff88 100644 --- a/watchquagga/watchquagga.c +++ b/watchquagga/watchquagga.c @@ -682,6 +682,28 @@ handle_read(struct thread *t_read) return 0; } +/* + * Wait till we notice that all daemons are ready before + * we send we are ready to systemd + */ +static void +daemon_send_ready (void) +{ + static int sent = 0; + if (!sent && gs.numdown == 0) + { +#if defined (HAVE_CUMULUS) + FILE *fp; + + fp = fopen("/var/run/quagga/watchquagga.started", "w"); + fclose(fp); +#endif + zlog_notice ("Watchquagga: Notifying Systemd we are up and running"); + systemd_send_started(master, 0); + sent = 1; + } +} + static void daemon_up(struct daemon *dmn, const char *why) { @@ -689,6 +711,7 @@ daemon_up(struct daemon *dmn, const char *why) gs.numdown--; dmn->connect_tries = 0; zlog_notice("%s state -> up : %s",dmn->name,why); + daemon_send_ready(); if (gs.do_ping) SET_WAKEUP_ECHO(dmn); phase_check(); @@ -1284,7 +1307,6 @@ main(int argc, char **argv) gs.restart.interval = gs.min_restart_interval; master = thread_master_create(); - systemd_send_started (master, 0); signal_init (master, array_size(my_signals), my_signals); srandom(time(NULL)); From 87f44e2f0b1251c9d1dfe492ab1f37d809ee0a7c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 9 Nov 2016 14:42:47 +0100 Subject: [PATCH 12/16] lib: add minimal no-config VTY mode This silences the following warning from watchquagga: "Can't save to configuration file, using vtysh." which otherwise appears when doing a "write file" in vtysh when no integrated-config is in use. Also make "show memory" available in watchquagga. Signed-off-by: David Lamparter --- lib/command.c | 30 +++++++++++++++++++++++------- lib/command.h | 1 + watchquagga/watchquagga.c | 4 +++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/command.c b/lib/command.c index 56c262a647..e8ba637623 100644 --- a/lib/command.c +++ b/lib/command.c @@ -3146,6 +3146,9 @@ DEFUN (config_write_file, struct vty *file_vty; struct stat conf_stat; + if (host.noconfig) + return CMD_SUCCESS; + /* Check and see if we are operating under vtysh configuration */ if (host.config == NULL) { @@ -3270,6 +3273,9 @@ DEFUN (config_write_terminal, unsigned int i; struct cmd_node *node; + if (host.noconfig) + return CMD_SUCCESS; + if (vty->type == VTY_SHELL_SERV) { for (i = 0; i < vector_active (cmdvec); i++) @@ -3313,6 +3319,11 @@ DEFUN (show_startup_config, char buf[BUFSIZ]; FILE *confp; + if (host.noconfig) + return CMD_SUCCESS; + if (host.config == NULL) + return CMD_WARNING; + confp = fopen (host.config, "r"); if (confp == NULL) { @@ -4203,7 +4214,11 @@ install_default (enum node_type node) install_element (node, &show_running_config_cmd); } -/* Initialize command interface. Install basic nodes and commands. */ +/* Initialize command interface. Install basic nodes and commands. + * + * terminal = 0 -- vtysh / no logging, no config control + * terminal = 1 -- normal daemon + * terminal = -1 -- watchquagga / no logging, but minimal config control */ void cmd_init (int terminal) { @@ -4224,6 +4239,7 @@ cmd_init (int terminal) host.enable = NULL; host.logfile = NULL; host.config = NULL; + host.noconfig = (terminal < 0); host.lines = -1; host.motd = default_motd; host.motdfile = NULL; @@ -4269,12 +4285,17 @@ cmd_init (int terminal) { install_element (ENABLE_NODE, &config_logmsg_cmd); install_default (CONFIG_NODE); + + install_element (VIEW_NODE, &show_thread_cpu_cmd); + install_element (ENABLE_NODE, &clear_thread_cpu_cmd); + + install_element (VIEW_NODE, &show_work_queues_cmd); } install_element (CONFIG_NODE, &hostname_cmd); install_element (CONFIG_NODE, &no_hostname_cmd); - if (terminal) + if (terminal > 0) { install_element (CONFIG_NODE, &password_cmd); install_element (CONFIG_NODE, &password_text_cmd); @@ -4313,11 +4334,6 @@ cmd_init (int terminal) install_element (CONFIG_NODE, &service_terminal_length_cmd); install_element (CONFIG_NODE, &no_service_terminal_length_cmd); - install_element (VIEW_NODE, &show_thread_cpu_cmd); - - install_element (ENABLE_NODE, &clear_thread_cpu_cmd); - install_element (VIEW_NODE, &show_work_queues_cmd); - vrf_install_commands (); } srandom(time(NULL)); diff --git a/lib/command.h b/lib/command.h index ba40770ba3..d2fc969d70 100644 --- a/lib/command.h +++ b/lib/command.h @@ -56,6 +56,7 @@ struct host /* config file name of this host */ char *config; + int noconfig; /* Flags for services */ int advanced; diff --git a/watchquagga/watchquagga.c b/watchquagga/watchquagga.c index 70b35f775d..93bbb0429a 100644 --- a/watchquagga/watchquagga.c +++ b/watchquagga/watchquagga.c @@ -25,6 +25,7 @@ #include #include #include "command.h" +#include "memory_vty.h" #include #include @@ -1304,7 +1305,8 @@ main(int argc, char **argv) zprivs_init (&watchquagga_privs); master = thread_master_create(); - cmd_init(1); + cmd_init(-1); + memory_init(); vty_init(master); watchquagga_vty_init(); vty_serv_sock(NULL, 0, WATCHQUAGGA_VTYSH_PATH); From c10c5926cbe10abcc5b4b4655e2b13c5b4622f5f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 9 Nov 2016 15:05:14 +0100 Subject: [PATCH 13/16] vtysh: improve config-write error reporting Signed-off-by: David Lamparter --- vtysh/vtysh.c | 13 ++++++------- watchquagga/watchquagga_vty.c | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 6a5a97bd2a..17dd58a4a8 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2479,9 +2479,9 @@ vtysh_write_config_integrated(void) fp = fopen (quagga_config, "w"); if (fp == NULL) { - fprintf (stdout,"%% Can't open configuration file %s due to '%s'\n", + fprintf (stdout,"%% Error: failed to open configuration file %s: %s\n", quagga_config, safe_strerror(errno)); - return CMD_SUCCESS; + return CMD_WARNING; } fd = fileno (fp); @@ -2538,12 +2538,11 @@ vtysh_write_config_integrated(void) fclose (fp); + printf ("Integrated configuration saved to %s\n", quagga_config); if (err) return CMD_WARNING; - fprintf (stdout,"Integrated configuration saved to %s\n", quagga_config); - fprintf (stdout,"[OK]\n"); - + printf ("[OK]\n"); return CMD_SUCCESS; } @@ -2589,9 +2588,9 @@ DEFUN (vtysh_write_memory, if (ret != CMD_SUCCESS) { - printf("Warning: attempting direct configuration write without " + printf("\nWarning: attempting direct configuration write without " "watchquagga.\nFile permissions and ownership may be " - "incorrect, or write may fail.\n"); + "incorrect, or write may fail.\n\n"); ret = vtysh_write_config_integrated(); } return ret; diff --git a/watchquagga/watchquagga_vty.c b/watchquagga/watchquagga_vty.c index bf413376af..b96011b760 100644 --- a/watchquagga/watchquagga_vty.c +++ b/watchquagga/watchquagga_vty.c @@ -113,6 +113,12 @@ void integrated_write_sigchld(int status) zlog_warn("configuration write terminated"); } + if (reply[3] != CMD_SUCCESS) { + /* failure might be silent in vtysh without this */ + static const char msg[] = "% Configuration write failed.\n"; + write(integrated_result_fd, msg, strlen(msg)); + } + /* don't care about failures here, if the connection is broken the * return value will just be lost. */ write(integrated_result_fd, reply, sizeof(reply)); From 9b7f18cf6b5cb8eddbb9cf6a2ad0908a7ee355a5 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 9 Nov 2016 13:28:40 +0100 Subject: [PATCH 14/16] doc: generic updates --- doc/install.texi | 27 ++++++--------------------- doc/ipv6.texi | 3 +++ 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/doc/install.texi b/doc/install.texi index 811ad9ae89..3ace7d6a0c 100644 --- a/doc/install.texi +++ b/doc/install.texi @@ -44,15 +44,10 @@ commands: @cindex Options to @code{./configure} Quagga has an excellent configure script which automatically detects most -host configurations. There are several additional configure options you can -use to turn off IPv6 support, to disable the compilation of specific -daemons, and to enable SNMP support. +host configurations. There are several additional configure options to +customize the build to include or exclude specific features and dependencies. @table @option -@item --disable-ipv6 -Turn off IPv6 related features and daemons. Quagga configure script -automatically detects IPv6 stack. But sometimes you might want to -disable IPv6 support of Quagga. @item --disable-zebra Do not build zebra daemon. @item --disable-ripd @@ -68,28 +63,18 @@ Do not build bgpd. @item --disable-bgp-announce Make @command{bgpd} which does not make bgp announcements at all. This feature is good for using @command{bgpd} as a BGP announcement listener. -@item --enable-netlink -Force to enable @sc{gnu}/Linux netlink interface. Quagga configure -script detects netlink interface by checking a header file. When the header -file does not match to the current running kernel, configure script will -not turn on netlink support. @item --enable-snmp Enable SNMP support. By default, SNMP support is disabled. -@item --disable-opaque-lsa -Disable support for Opaque LSAs (RFC2370) in ospfd. @item --disable-ospfapi Disable support for OSPF-API, an API to interface directly with ospfd. OSPF-API is enabled if --enable-opaque-lsa is set. @item --disable-ospfclient Disable building of the example OSPF-API client. -@item --disable-ospf-te -Disable support for OSPF Traffic Engineering Extension (RFC3630) this -requires support for Opaque LSAs. @item --disable-ospf-ri Disable support for OSPF Router Information (RFC4970 & RFC5088) this requires support for Opaque LSAs and Traffic Engineering. -@item --enable-isisd -Build isisd. +@item --disable-isisd +Do not build isisd. @item --enable-isis-topology Enable IS-IS topology generator. @item --enable-isis-te @@ -104,7 +89,7 @@ Pass the @command{-rdynamic} option to the linker driver. This is in most cases neccessary for getting usable backtraces. This option defaults to on if the compiler is detected as gcc, but giving an explicit enable/disable is suggested. -@item --enable-backtrace +@item --disable-backtrace Controls backtrace support for the crash handlers. This is autodetected by default. Using the switch will enforce the requested behaviour, failing with an error if support is requested but not available. On BSD systems, this @@ -129,7 +114,7 @@ as pid files and unix sockets. @end table @example -% ./configure --disable-ipv6 +% ./configure --disable-snmp @end example This command will configure zebra and the routing daemons. diff --git a/doc/ipv6.texi b/doc/ipv6.texi index 5be2babed4..e08759ab03 100644 --- a/doc/ipv6.texi +++ b/doc/ipv6.texi @@ -8,6 +8,9 @@ automatic address configuration via a feature called @code{address auto configuration}. To do it, the router must send router advertisement messages to the all nodes that exist on the network. +Previous versions of Quagga could be built without IPv6 support. This is +no longer possible. + @menu * Router Advertisement:: @end menu From e68ab6bb0f199a04e27e54fce583e56494042751 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 9 Nov 2016 13:29:00 +0100 Subject: [PATCH 15/16] doc: vtysh doc updates --- doc/defines.texi.in | 1 + doc/vtysh.texi | 176 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 139 insertions(+), 38 deletions(-) diff --git a/doc/defines.texi.in b/doc/defines.texi.in index a4badef0ee..5436f20c36 100644 --- a/doc/defines.texi.in +++ b/doc/defines.texi.in @@ -13,3 +13,4 @@ @set INSTALL_PREFIX_ETC /etc/quagga @set INSTALL_PREFIX_SBIN /usr/sbin @set INSTALL_PREFIX_STATE /var/run/quagga +@set INSTALL_VTY_GROUP @enable_vty_group@ diff --git a/doc/vtysh.texi b/doc/vtysh.texi index 66562a96cd..69b7acd3b7 100644 --- a/doc/vtysh.texi +++ b/doc/vtysh.texi @@ -1,56 +1,105 @@ @node VTY shell @chapter VTY shell -@command{vtysh} is integrated shell of Quagga software. - -To use vtysh please specify ---enable-vtysh to configure script. To use -PAM for authentication use ---with-libpam option to configure script. - -vtysh only searches @value{INSTALL_PREFIX_ETC} path for vtysh.conf which -is the vtysh configuration file. Vtysh does not search current -directory for configuration file because the file includes user -authentication settings. - -Currently, vtysh.conf has only two commands. - @menu -* VTY shell username:: -* VTY shell integrated configuration:: +* Integrated configuration mode:: @end menu -@node VTY shell username -@section VTY shell username +@command{vtysh} provides a combined frontend to all Quagga daemons in a +single combined session. It is enabled by default at build time, but can +be disabled through the @option{--disable-vtysh} option to +@command{./configure}. + +@command{vtysh} has a configuration file, @file{vtysh.conf}. The location +of that file cannot be changed from @file{@value{INSTALL_PREFIX_ETC}} since +it contains options controlling authentication behavior. This file will +also not be written by configuration-save commands, it is intended to be +updated manually by an administrator with an external editor. + +@quotation Warning +This also means the @command{hostname} and @command{banner motd} commands +(which both do have effect for vtysh) need to be manually updated in +@file{vtysh.conf}. +@end quotation + +@section Permissions and setup requirements + +@command{vtysh} connects to running daemons through Unix sockets located in +@file{@value{INSTALL_PREFIX_STATE}}. Running vtysh thus requires access to +that directory, plus membership in the @emph{@value{INSTALL_VTY_GROUP}} +group (which is the group that the daemons will change ownership of their +sockets to). + +To restrict access to Quagga configuration, make sure no unauthorized users +are members of the @emph{@value{INSTALL_VTY_GROUP}} group. + +@subsection PAM support (experimental) + +vtysh has working (but rather useless) PAM support. It will perform +an "authenticate" PAM call using @emph{@value{PACKAGE_NAME}} as service +name. No other (accounting, session, password change) calls will be +performed by vtysh. + +Users using vtysh still need to have appropriate access to the daemons' +VTY sockets, usually by being member of the @emph{@value{INSTALL_VTY_GROUP}} +group. If they have this membership, PAM support is useless since they can +connect to daemons and issue commands using some other tool. Alternatively, +the @command{vtysh} binary could be made SGID (set group ID) to the +@emph{@value{INSTALL_VTY_GROUP}} group. @strong{No security guarantees are +made for this configuration}. @deffn {Command} {username @var{username} nopassword} {} -With this set, user foo does not need password authentication for user vtysh. -With PAM vtysh uses PAM authentication mechanism. - -If vtysh is compiled without PAM authentication, every user can use vtysh -without authentication. vtysh requires read/write permission -to the various daemons vty sockets, this can be accomplished through use -of unix groups and the --enable-vty-group configure option. +If PAM support is enabled at build-time, this command allows disabling the +use of PAM on a per-user basis. If vtysh finds that an user is trying to +use vtysh and a "nopassword" entry is found, no calls to PAM will be made +at all. @end deffn -@node VTY shell integrated configuration -@section VTY shell integrated configuration +@node Integrated configuration mode +@section Integrated configuration mode -@deffn {Command} {service integrated-vtysh-config} {} -Write out integrated Quagga.conf file when 'write file' is issued. +Integrated configuration mode uses a single configuration file, +@file{Quagga.conf}, for all daemons. This replaces the individual files like +@file{zebra.conf} or @file{bgpd.conf}. -This command controls the behaviour of vtysh when it is told to write out -the configuration. Per default, vtysh will instruct each daemon to write -out their own config files when @command{write file} is issued. However, if -@command{service integrated-vtysh-config} is set, when @command{write file} -is issued, vtysh will instruct the daemons will write out a Quagga.conf with -all daemons' commands integrated into it. +@file{Quagga.conf} is located in @file{@value{INSTALL_PREFIX_ETC}}. All +daemons check for the existence of this file at startup, and if it exists +will not load their individual configuration files. Instead, +@command{vtysh -b} must be invoked to process @file{Quagga.conf} and apply +its settings to the individual daemons. -Vtysh per default behaves as if @command{write-conf daemon} is set. Note -that both may be set at same time if one wishes to have both Quagga.conf and -daemon specific files written out. Further, note that the daemons are -hard-coded to first look for the integrated Quagga.conf file before looking -for their own file. +@quotation Warning +@command{vtysh -b} must also be executed after restarting any daemon. +@end quotation + +@subsection Configuration saving, file ownership and permissions + +The @file{Quagga.conf} file is not written by any of the daemons; instead +@command{vtysh} contains the neccessary logic to collect configuration from +all of the daemons, combine it and write it out. + +@quotation Warning +Daemons must be running for @command{vtysh} to be able to collect their +configuration. Any configuration from non-running daemons is permanently +lost after doing a configuration save. +@end quotation + +Since the @command{vtysh} command may be running as ordinary user on the +system, configuration writes will be tried through @command{watchquagga}, +using the @command{write integrated} command internally. Since +@command{watchquagga} is running as superuser, @command{vtysh} is able to +ensure correct ownership and permissions on @file{Quagga.conf}. + +If @command{watchquagga} is not running or the configuration write fails, +@command{vtysh} will attempt to directly write to the file. This is likely +to fail if running as unprivileged user; alternatively it may leave the +file with incorrect owner or permissions. + +Writing the configuration can be triggered directly by invoking +@command{vtysh -w}. This may be useful for scripting. Note this command +should be run as either the superuser or the Quagga user. We recommend you do not mix the use of the two types of files. Further, it is better not to use the integrated Quagga.conf file, as any syntax error in @@ -58,4 +107,55 @@ it can lead to /all/ of your daemons being unable to start up. Per daemon files are more robust as impact of errors in configuration are limited to the daemon in whose file the error is made. +@deffn {Command} {service integrated-vtysh-config} {} +@deffnx {Command} {no service integrated-vtysh-config} {} + +Control whether integrated @file{Quagga.conf} file is written when +'write file' is issued. + +These commands need to be placed in @file{vtysh.conf} to have any effect. +Note that since @file{vtysh.conf} is not written by Quagga itself, they +therefore need to be manually placed in that file. + +This command has 3 states: +@itemize @bullet +@item +@command{service integrated-vtysh-config} + +@command{vtysh} will always write @file{Quagga.conf}. + +@item +@command{no service integrated-vtysh-config} + +@command{vtysh} will never write @file{Quagga.conf}; instead it will ask +daemons to write their individual configuration files. + +@item +Neither option present (default) + +@command{vtysh} will check whether @file{Quagga.conf} exists. If it does, +configuration writes will update that file. Otherwise, writes are performed +through the individual daemons. +@end itemize + +This command is primarily intended for packaging/distribution purposes, to +preset one of the two operating modes and ensure consistent operation across +installations. @end deffn + +@deffn {Command} {write integrated} {} + +Unconditionally (regardless of @command{service integrated-vtysh-config} +setting) write out integrated @file{Quagga.conf} file through +@command{watchquagga}. If @command{watchquagga} is not running, this command +is unavailable. + +@end deffn + +@section Caveats + +Configuration changes made while some daemon is not running will be invisible +to that daemon. The daemon will start up with its saved configuration +(either in its individual configuration file, or in @file{Quagga.conf}). +This is particularly troublesome for route-maps and prefix lists, which would +otherwise be synchronized between daemons. From 9f1f8df328271d3cd47a28c74efaeeefabc680ec Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 10 Nov 2016 13:59:54 +0100 Subject: [PATCH 16/16] vtysh: fix config write --- vtysh/vtysh_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index d9c0e3fbfd..999d90ab22 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -419,6 +419,7 @@ main (int argc, char **argv, char **env) if (writeconfig) { + vtysh_execute ("enable"); return vtysh_write_config_integrated (); }