From 12fb742b6f0a08590f9ef8d246cd3f57e87b57de Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 11 Jun 2009 13:02:33 -0700 Subject: [PATCH] Don't accept unix: connections, etc. in OpenFlow controller discovery. The controller discovery code has always had the capability to whitelist only certain types of controller locations. Until now, we have only taken advantage of this when SSL is enabled (so that all OpenFlow connections are authenticated with SSL if SSL is configured). However, it occurs to me that making the section of connections entirely unrestricted is too permissive. An attacker could make the vswitch connect to an arbitrary Unix domain socket, for example. I don't have a description of how this is an exploitable security vulnerability, but it seems entirely too lax. So: this commit changes the default to allowing only TCP connections to controller in the non-SSL case. --- secchan/discovery.c | 2 +- secchan/main.c | 3 ++- secchan/secchan.8.in | 4 ++-- utilities/ovs-discover.8.in | 11 ++++++----- utilities/ovs-discover.c | 2 +- vswitchd/ovs-vswitchd.conf.5.in | 4 ++-- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/secchan/discovery.c b/secchan/discovery.c index 06de6f07..92d0d928 100644 --- a/secchan/discovery.c +++ b/secchan/discovery.c @@ -168,7 +168,7 @@ discovery_set_accept_controller_re(struct discovery *d, const char *re_) int error; char *re; - re = (!re_ ? xstrdup(vconn_ssl_is_configured() ? "^ssl:.*" : ".*") + re = (!re_ ? xstrdup(vconn_ssl_is_configured() ? "^ssl:.*" : "^tcp:.*") : re_[0] == '^' ? xstrdup(re_) : xasprintf("^%s", re_)); regex = xmalloc(sizeof *regex); error = regcomp(regex, re, REG_NOSUB | REG_EXTENDED); diff --git a/secchan/main.c b/secchan/main.c index ca385766..c9cfc64e 100644 --- a/secchan/main.c +++ b/secchan/main.c @@ -497,7 +497,8 @@ parse_options(int argc, char *argv[], struct ofsettings *s) /* Set accept_controller_regex. */ if (!s->accept_controller_re) { - s->accept_controller_re = vconn_ssl_is_configured() ? "^ssl:.*" : ".*"; + s->accept_controller_re + = vconn_ssl_is_configured() ? "^ssl:.*" : "^tcp:.*"; } /* Mode of operation. */ diff --git a/secchan/secchan.8.in b/secchan/secchan.8.in index b40842a9..bbab7fba 100644 --- a/secchan/secchan.8.in +++ b/secchan/secchan.8.in @@ -182,8 +182,8 @@ names match the regular expression will be accepted. The default regular expression is \fBssl:.*\fR (meaning that only SSL controller connections will be accepted) when any of the SSL configuration options \fB--private-key\fR, \fB--certificate\fR, or -\fB--ca-cert\fR is specified. The default is \fB.*\fR otherwise -(meaning that any controller will be accepted). +\fB--ca-cert\fR is specified. The default is \fB^tcp:.*\fR otherwise +(meaning that only TCP controller connections will be accepted). The \fIregex\fR is implicitly anchored at the beginning of the controller location string, as if it begins with \fB^\fR. diff --git a/utilities/ovs-discover.8.in b/utilities/ovs-discover.8.in index d38ce9ee..eb83a5f8 100644 --- a/utilities/ovs-discover.8.in +++ b/utilities/ovs-discover.8.in @@ -28,15 +28,16 @@ itself to the background. .SH OPTIONS .TP \fB--accept-vconn=\fIregex\fR -By default, \fBovs\-discover\fR accepts any controller location -advertised over DHCP. With this option, only controllers whose names -match POSIX extended regular expression \fIregex\fR will be accepted. -Specifying \fBssl:.*\fR for \fIregex\fR, for example, would cause only -SSL controller connections to be accepted. +With this option, only controllers whose names match POSIX extended +regular expression \fIregex\fR will be accepted. Specifying +\fBssl:.*\fR for \fIregex\fR, for example, would cause only SSL +controller connections to be accepted. The \fIregex\fR is implicitly anchored at the beginning of the controller location string, as if it begins with \fB^\fR. +When this option is not given, the default \fIregex\fR is +\fBtcp:.*\fR. .TP \fB--exit-without-bind\fR By default, \fBovs\-discover\fR binds the network device that receives diff --git a/utilities/ovs-discover.c b/utilities/ovs-discover.c index b664321f..f47edc72 100644 --- a/utilities/ovs-discover.c +++ b/utilities/ovs-discover.c @@ -48,7 +48,7 @@ static int n_ifaces; /* --accept-vconn: Regular expression specifying the class of controller vconns * that we will accept during autodiscovery. */ -static const char *accept_controller_re = ".*"; +static const char *accept_controller_re = "tcp:.*"; static regex_t accept_controller_regex; /* --exit-without-bind: Exit after discovering the controller, without binding diff --git a/vswitchd/ovs-vswitchd.conf.5.in b/vswitchd/ovs-vswitchd.conf.5.in index 81e007fb..de0d6ea2 100644 --- a/vswitchd/ovs-vswitchd.conf.5.in +++ b/vswitchd/ovs-vswitchd.conf.5.in @@ -410,8 +410,8 @@ the regular expression will be accepted. .IP The default regular expression is \fBssl:.*\fR, meaning that only SSL controller connections will be accepted, when SSL is configured (see -\fBSSL Configuration\fR), and \fB.*\fR otherwise, meaning that any -controller will be accepted. +\fBSSL Configuration\fR), and \fBtcp:.*\fR otherwise, meaning that only +TCP controller connections will be accepted. .IP The regular expression is implicitly anchored at the beginning of the controller location string, as if it begins with \fB^\fR. -- 2.30.2