From 689f5a8c84b95dbd31ecab481f8f2977965fe741 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 6 Aug 2017 07:35:50 +0200 Subject: *: 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 --- lib/libfrr.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) (limited to 'lib/libfrr.c') diff --git a/lib/libfrr.c b/lib/libfrr.c index 022296b3fb..e92456cf77 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -19,6 +19,7 @@ */ #include +#include #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; -- cgit v1.2.3 From 5d13cd091a183601eb8ebedeeeed2121ce4c3781 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 8 Aug 2017 20:11:10 +0200 Subject: lib: thoroughly disable tcp-zebra Disable this in the code to make it hard for people to shoot themselves in the foot. It's only left as a remnant for development use. Signed-off-by: David Lamparter --- doc/zebra.8.in | 8 -------- lib/libfrr.c | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) (limited to 'lib/libfrr.c') diff --git a/doc/zebra.8.in b/doc/zebra.8.in index 333e66fcf6..7f4a81b1a0 100644 --- a/doc/zebra.8.in +++ b/doc/zebra.8.in @@ -104,14 +104,6 @@ It should thus be loaded with \fB-M fpm:netlink\fR or \fB-M fpm:protobuf\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. diff --git a/lib/libfrr.c b/lib/libfrr.c index e92456cf77..c901dcc229 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -138,6 +138,7 @@ bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len, path = ZEBRA_SERV_PATH; if (!strncmp(path, ZAPI_TCP_PATHNAME, strlen(ZAPI_TCP_PATHNAME))) { + /* note: this functionality is disabled at bottom */ int af; int port = ZEBRA_PORT; char *err = NULL; @@ -193,6 +194,21 @@ bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len, #endif break; } + +#if 1 + /* force-disable this path, because tcp-zebra is a + * SECURITY ISSUE. there are no checks at all against + * untrusted users on the local system connecting on TCP + * and injecting bogus routing data into the entire routing + * domain. + * + * The functionality is only left here because it may be + * useful during development, in order to be able to get + * tcpdump or wireshark watching ZAPI as TCP. If you want + * to do that, flip the #if 1 above to #if 0. */ + memset(sa, 0, sizeof(*sa)); + return false; +#endif } else { /* "sun" is a #define on solaris */ struct sockaddr_un *suna = (struct sockaddr_un *)sa; -- cgit v1.2.3