*: remove --enable-tcp-zebra, rework ZAPI path

This adds "@tcp" as new choice on the -z option present in zebra and the
protocol daemons.  The --enable-tcp-zebra option on configure is no
longer needed, both UNIX and TCP socket support is always available.

Note that @tcp should not be used by default (e.g. in an init script),
and --enable-tcp-zebra should never have been in any distro package
builds, because

**** TCP-ZEBRA IS A SECURITY PROBLEM ****

It allows arbitrary local users to mess with the routing table and
inject bogus data -- and also ZAPI is not designed to be robust against
attacks.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

configure.ac

@@ -321,8 +321,6 @@ AC_ARG_ENABLE(snmp,
   AS_HELP_STRING([--enable-snmp=ARG], [enable SNMP support (smux or agentx)]))
 AC_ARG_WITH(libpam,
   AS_HELP_STRING([--with-libpam], [use libpam for PAM support in vtysh]))
-AC_ARG_ENABLE(tcp-zebra,
-  AS_HELP_STRING([--enable-tcp-zebra], [enable TCP/IP socket connection between zebra and protocol daemon]))
 AC_ARG_ENABLE(ospfapi,
   AS_HELP_STRING([--disable-ospfapi], [do not build OSPFAPI to access the OSPF LSA Database]))
 AC_ARG_ENABLE(ospfclient,
@@ -559,10 +557,6 @@ AM_CONDITIONAL([HAVE_PROTOBUF], [test "x$have_protobuf" = "xyes"])
 # End of logic for protobuf support.
 #
 
-if test "${enable_tcp_zebra}" = "yes"; then
-  AC_DEFINE(HAVE_TCP_ZEBRA,,Use TCP for zebra communication)
-fi
-
 if test "${enable_linux24_tcp_md5}" = "yes"; then
   AC_DEFINE(HAVE_TCP_MD5_LINUX24,,Old Linux 2.4 TCP MD5 Signature Patch)
 fi

doc/Building_FRR_on_CentOS6.md

@@ -109,7 +109,6 @@ an example.)
         --enable-rtadv \
         --disable-exampledir \
         --enable-watchfrr \
-        --enable-tcp-zebra \
         --disable-ldpd \
         --enable-fpm \
         --enable-nhrpd \

doc/Building_FRR_on_CentOS7.md

@@ -61,7 +61,6 @@ an example.)
         --enable-rtadv \
         --disable-exampledir \
         --enable-watchfrr \
-        --enable-tcp-zebra \
         --disable-ldpd \
         --enable-fpm \
         --enable-nhrpd \

doc/Building_FRR_on_Debian8.md

@@ -60,7 +60,6 @@ an example.)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --enable-ldpd \
         --with-pkg-git-version \

doc/Building_FRR_on_Fedora24.md

@@ -54,7 +54,6 @@ an example.)
         --enable-rtadv \
         --disable-exampledir \
         --enable-watchfrr \
-        --enable-tcp-zebra \
         --enable-ldpd \
         --enable-fpm \
         --enable-nhrpd \

doc/Building_FRR_on_FreeBSD10.md

@@ -61,7 +61,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_FreeBSD11.md

@@ -61,7 +61,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_FreeBSD9.md

@@ -69,7 +69,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_NetBSD6.md

@@ -65,7 +65,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_NetBSD7.md

@@ -59,7 +59,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_OmniOS.md

@@ -104,7 +104,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_OpenBSD6.md

@@ -54,7 +54,6 @@ an example)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_Ubuntu1204.md

@@ -93,7 +93,6 @@ an example.)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --with-pkg-git-version \
         --with-pkg-extra-version=-MyOwnFRRVersion   

doc/Building_FRR_on_Ubuntu1404.md

@@ -53,7 +53,6 @@ an example.)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
         --enable-ldpd \
         --with-pkg-git-version \

doc/Building_FRR_on_Ubuntu1604.md

@@ -54,7 +54,6 @@ an example.)
         --enable-configfile-mask=0640 \
         --enable-logfile-mask=0640 \
         --enable-rtadv \
-        --enable-tcp-zebra \
         --enable-fpm \
 	--enable-systemd=yes \
         --with-pkg-git-version \

doc/pimd.8.in

@@ -60,7 +60,9 @@ restart pimd.  The default is \fB\fI@CFG_STATE@/pimd.pid\fR.
 .TP
 \fB\-z\fR, \fB\-\-socket \fR\fIpath\fR
 Specify the socket path for contacting the zebra daemon.
-The default is \fB\fI@CFG_STATE@/zserv.api\fR.
+The default is \fB\fI@CFG_STATE@/zserv.api\fR.  The value of this option
+must be the same as the one given when starting zebra.  Refer to the \fBzebra
+(8)\fR man page for more information.
 .TP
 \fB\-P\fR, \fB\-\-vty_port \fR\fIport-number\fR 
 Specify the port that the pimd VTY will listen on. This defaults to

doc/zebra.8.in

@@ -26,6 +26,9 @@ zebra \- a routing manager for use with associated @PACKAGE_FULLNAME@ components
 ] [
 .B \-M
 .I module:options
+] [
+.B \-z
+.I socketpath
 ]
 .SH DESCRIPTION
 .B zebra 
@@ -97,6 +100,19 @@ respectively.  The \fBfpm\fR module takes an additional colon-separated
 argument specifying the encapsulation, either \fBnetlink\fR or \fBprotobuf\fR.
 It should thus be loaded with \fB-M fpm:netlink\fR or \fB-M fpm:protobuf\fR.
 .TP
+\fB\-z\fR, \fB\-\-socket \fR\fIsocketpath\fR
+Use the specified path to open the zebra API socket on.
+The default is \fB\fI@CFG_STATE@/zserv.api\fR.  This option must be given with
+the same value to all FRR protocol daemons.
+
+For debugging purposes (using tcpdump or wireshark to trace cross-daemon
+communication), a TCP socket can be used by specifying \fI@tcp[46][:port]\fR.
+It is intentionally not possible to bind this to anything other than localhost
+since zebra and the other daemons need to be running on the same host.  Using
+this feature \fBCREATES A SECURITY ISSUE\fR since nothing prevents other users
+on the local system from connecting to zebra and injecting bogus routing
+information.
+.TP
 \fB\-v\fR, \fB\-\-version\fR
 Print the version and exit.
 .SH FILES

ldpd/lde.c

@@ -37,6 +37,7 @@
 #include "zclient.h"
 #include "stream.h"
 #include "network.h"
+#include "libfrr.h"
 
 static void		 lde_shutdown(void);
 static int		 lde_dispatch_imsg(struct thread *);
@@ -170,7 +171,8 @@ lde_init(struct ldpd_init *init)
 	lde_gc_start_timer();
 
 	/* Init synchronous zclient and label list */
-	zclient_serv_path_set(init->zclient_serv_path);
+	frr_zclient_addr(&zclient_addr, &zclient_addr_len,
+			 init->zclient_serv_path);
 	zclient_sync_init(init->instance);
 	lde_label_list_init();
 }

ldpd/ldpd.c

@@ -256,7 +256,7 @@ main(int argc, char *argv[])
 	strlcpy(init.user, ldpd_privs.user, sizeof(init.user));
 	strlcpy(init.group, ldpd_privs.group, sizeof(init.group));
 	strlcpy(init.ctl_sock_path, ctl_sock_path, sizeof(init.ctl_sock_path));
-	strlcpy(init.zclient_serv_path, zclient_serv_path_get(),
+	strlcpy(init.zclient_serv_path, frr_zclientpath,
 	    sizeof(init.zclient_serv_path));
 
 	argc -= optind;

lib/libfrr.c

@@ -19,6 +19,7 @@
  */
 
 #include <zebra.h>
+#include <sys/un.h>
 
 #include "libfrr.h"
 #include "getopt.h"
@@ -40,6 +41,7 @@ char frr_protoname[256] = "NONE";
 char frr_protonameinst[256] = "NONE";
 
 char config_default[256];
+char frr_zclientpath[256];
 static char pidfile_default[256];
 static char vtypath_default[256];
 
@@ -127,6 +129,100 @@ static const struct optspec os_user = {"u:g:",
 				       lo_user};
 
 
+bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len,
+		      const char *path)
+{
+	memset(sa, 0, sizeof(*sa));
+
+	if (!path)
+		path = ZEBRA_SERV_PATH;
+
+	if (!strncmp(path, ZAPI_TCP_PATHNAME, strlen(ZAPI_TCP_PATHNAME))) {
+		int af;
+		int port = ZEBRA_PORT;
+		char *err = NULL;
+		struct sockaddr_in *sin = NULL;
+		struct sockaddr_in6 *sin6 = NULL;
+
+		path += strlen(ZAPI_TCP_PATHNAME);
+
+		switch (path[0]) {
+		case '4':
+			path++;
+			af = AF_INET;
+			break;
+		case '6':
+			path++;
+			/* fallthrough */
+		default:
+			af = AF_INET6;
+			break;
+		}
+
+		switch (path[0]) {
+		case '\0':
+			break;
+		case ':':
+			path++;
+			port = strtoul(path, &err, 10);
+			if (*err || !*path)
+				return false;
+			break;
+		default:
+			return false;
+		}
+
+		sa->ss_family = af;
+		switch (af) {
+		case AF_INET:
+			sin = (struct sockaddr_in *)sa;
+			sin->sin_port = htons(port);
+			sin->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+			*sa_len = sizeof(struct sockaddr_in);
+#ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
+			sin->sin_len = *sa_len;
+#endif
+			break;
+		case AF_INET6:
+			sin6 = (struct sockaddr_in6 *)sa;
+			sin6->sin6_port = htons(port);
+			inet_pton(AF_INET6, "::1", &sin6->sin6_addr);
+			*sa_len = sizeof(struct sockaddr_in6);
+#ifdef SIN6_LEN
+			sin6->sin6_len = *sa_len;
+#endif
+			break;
+		}
+	} else {
+		/* "sun" is a #define on solaris */
+		struct sockaddr_un *suna = (struct sockaddr_un *)sa;
+
+		suna->sun_family = AF_UNIX;
+		strlcpy(suna->sun_path, path, sizeof(suna->sun_path));
+#ifdef HAVE_STRUCT_SOCKADDR_UN_SUN_LEN
+		*sa_len = suna->sun_len = SUN_LEN(suna);
+#else
+		*sa_len = sizeof(suna->sun_family) + strlen(suna->sun_path);
+#endif /* HAVE_STRUCT_SOCKADDR_UN_SUN_LEN */
+#if 0
+		/* this is left here for future reference;  Linux abstract
+		 * socket namespace support can be enabled by replacing
+		 * above #if 0 with #ifdef GNU_LINUX.
+		 *
+		 * THIS IS A SECURITY ISSUE, the abstract socket namespace
+		 * does not have user/group permission control on sockets.
+		 * we'd need to implement SCM_CREDENTIALS support first to
+		 * check that only proper users can connect to abstract
+		 * sockets. (same problem as tcp-zebra, except there is a
+		 * fix with SCM_CREDENTIALS.  tcp-zebra has no such fix.)
+		 */
+		if (suna->sun_path[0] == '@')
+			suna->sun_path[0] = '\0';
+#endif
+	}
+	return true;
+}
+
 static struct frr_daemon_info *di = NULL;
 
 void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
@@ -156,6 +252,8 @@ void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
 
 	strlcpy(frr_protoname, di->logname, sizeof(frr_protoname));
 	strlcpy(frr_protonameinst, di->logname, sizeof(frr_protonameinst));
+
+	strlcpy(frr_zclientpath, ZEBRA_SERV_PATH, sizeof(frr_zclientpath));
 }
 
 void frr_opt_add(const char *optstr, const struct option *longopts,
@@ -233,7 +331,7 @@ static int frr_opt(int opt)
 	case 'z':
 		if (di->flags & FRR_NO_ZCLIENT)
 			return 1;
-		zclient_serv_path_set(optarg);
+		strlcpy(frr_zclientpath, optarg, sizeof(frr_zclientpath));
 		break;
 	case 'A':
 		if (di->flags & FRR_NO_TCPVTY)
@@ -341,6 +439,13 @@ struct thread_master *frr_init(void)
 	zlog_set_level(ZLOG_DEST_SYSLOG, zlog_default->default_lvl);
 #endif
 
+	if (!frr_zclient_addr(&zclient_addr, &zclient_addr_len,
+			      frr_zclientpath)) {
+		fprintf(stderr, "Invalid zserv socket path: %s\n",
+			frr_zclientpath);
+		exit(1);
+	}
+
 	frrmod_init(di->module);
 	while (modules) {
 		modules = (oc = modules)->next;

lib/libfrr.h

@@ -100,7 +100,11 @@ extern void frr_vty_serv(void);
 /* note: contains call to frr_vty_serv() */
 extern void frr_run(struct thread_master *master);
 
+extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len,
+			     const char *path);
+
 extern char config_default[256];
+extern char frr_zclientpath[256];
 extern const char frr_sysconfdir[];
 extern const char frr_vtydir[];
 extern const char frr_moduledir[];