Thanks Ben. The patch looks good.
Do we also want to change the python stream implementation
to behave the same way?
-Pavithra
----- Original Message -----
From: "Ben Pfaff" <b...@nicira.com>
To: "Pavithra Ramesh" <param...@vmware.com>
Cc: dev@openvswitch.org
Sent: Friday, February 8, 2013 9:14:13 AM
Subject: Re: [ovs-dev] [PATCH] stream-unix: append ovs_rundir to socket
On Thu, Feb 07, 2013 at 03:48:14PM -0800, Pavithra Ramesh wrote:
> If socket path specified is relative to ovs_rundir(),
> append the directory name to in unix_open and punix_open.
> Freed the new newly allocated strings.
> Also included the change in bridge.c to relax the whitelist
> check, only if there is no /.
>
> Signed-off-by: Pavithra Ramesh <param...@vmware.com>
Here's a revised version that passes all tests.
--8<--------------------------cut here-------------------------->8--
From: Pavithra Ramesh <param...@vmware.com>
Date: Thu, 7 Feb 2013 15:48:14 -0800
Subject: [PATCH] stream-unix: append ovs_rundir to socket
If socket path specified is relative to ovs_rundir(),
append the directory name to in unix_open and punix_open.
Freed the new newly allocated strings.
Also included the change in bridge.c to relax the whitelist
check, only if there is no /.
Signed-off-by: Pavithra Ramesh <param...@vmware.com>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
lib/stream-unix.c | 20 ++++++++++++++------
tests/jsonrpc.at | 3 +++
tests/ovsdb-execution.at | 1 +
tests/ovsdb-idl.at | 3 +++
tests/ovsdb-macros.at | 3 ++-
tests/ovsdb-monitor.at | 1 +
tests/ovsdb-server.at | 9 +++++++++
tests/ovsdb-tool.at | 4 ++++
tests/vconn.at | 1 +
vswitchd/bridge.c | 6 ++++--
10 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/lib/stream-unix.c b/lib/stream-unix.c
index 6ed7648..dbee135 100644
--- a/lib/stream-unix.c
+++ b/lib/stream-unix.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
#include "packets.h"
#include "poll-loop.h"
#include "socket-util.h"
+#include "dirs.h"
#include "util.h"
#include "stream-provider.h"
#include "stream-fd.h"
@@ -42,15 +43,19 @@ static int
unix_open(const char *name, char *suffix, struct stream **streamp,
uint8_t dscp OVS_UNUSED)
{
- const char *connect_path = suffix;
+ char *connect_path;
int fd;
+ connect_path = abs_file_name(ovs_rundir(), suffix);
fd = make_unix_socket(SOCK_STREAM, true, NULL, connect_path);
+
if (fd < 0) {
VLOG_DBG("%s: connection failed (%s)", connect_path, strerror(-fd));
+ free(connect_path);
return -fd;
}
+ free(connect_path);
return new_fd_stream(name, fd, check_connection_completion(fd), streamp);
}
@@ -76,11 +81,14 @@ static int
punix_open(const char *name OVS_UNUSED, char *suffix,
struct pstream **pstreamp, uint8_t dscp OVS_UNUSED)
{
+ char *bind_path;
int fd, error;
- fd = make_unix_socket(SOCK_STREAM, true, suffix, NULL);
+ bind_path = abs_file_name(ovs_rundir(), suffix);
+ fd = make_unix_socket(SOCK_STREAM, true, bind_path, NULL);
if (fd < 0) {
- VLOG_ERR("%s: binding failed: %s", suffix, strerror(errno));
+ VLOG_ERR("%s: binding failed: %s", bind_path, strerror(errno));
+ free(bind_path);
return errno;
}
@@ -88,11 +96,11 @@ punix_open(const char *name OVS_UNUSED, char *suffix,
error = errno;
VLOG_ERR("%s: listen: %s", name, strerror(error));
close(fd);
+ free(bind_path);
return error;
}
- return new_fd_pstream(name, fd, punix_accept, NULL,
- xstrdup(suffix), pstreamp);
+ return new_fd_pstream(name, fd, punix_accept, NULL, bind_path, pstreamp);
}
static int
diff --git a/tests/jsonrpc.at b/tests/jsonrpc.at
index 2a7f91b..664debe 100644
--- a/tests/jsonrpc.at
+++ b/tests/jsonrpc.at
@@ -1,6 +1,7 @@
AT_BANNER([JSON-RPC - C])
AT_SETUP([JSON-RPC request and successful reply])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen
punix:socket])
AT_CHECK([test -s pid])
AT_CHECK([kill -0 `cat pid`])
@@ -12,6 +13,7 @@ AT_CHECK([kill `cat pid`])
AT_CLEANUP
AT_SETUP([JSON-RPC request and error reply])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen
punix:socket])
AT_CHECK([test -s pid])
AT_CHECK([kill -0 `cat pid`])
@@ -23,6 +25,7 @@ AT_CHECK([kill `cat pid`])
AT_CLEANUP
AT_SETUP([JSON-RPC notification])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen
punix:socket])
AT_CHECK([test -s pid])
# When a daemon dies it deletes its pidfile, so make a copy.
diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index 6a3b5d1..eec2a04 100644
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -138,6 +138,7 @@ m4_divert_pop([PREPARE_TESTS])
m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb execute execution positive $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([test-ovsdb execute "`$2`" m4_foreach([txn], [$3], [ 'txn'])],
[0], [stdout], [])
AT_CHECK([perl $srcdir/uuidfilt.pl stdout], [0], [$4])
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index ce22010..a206f62 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -19,6 +19,7 @@ AT_BANNER([OVSDB -- interface description language (IDL)])
m4_define([OVSDB_CHECK_IDL_C],
[AT_SETUP([$1 - C])
AT_KEYWORDS([ovsdb server idl positive $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
[0], [stdout], [ignore])
AT_CHECK([ovsdb-server '-vPATTERN:console:ovsdb-server|%c|%m' --detach
--no-chdir --pidfile="`pwd`"/pid --remote=punix:socket
--unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
@@ -36,6 +37,7 @@ m4_define([OVSDB_CHECK_IDL_PY],
[AT_SETUP([$1 - Python])
AT_SKIP_IF([test $HAVE_PYTHON = no])
AT_KEYWORDS([ovsdb server idl positive Python $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
[0], [stdout], [ignore])
AT_CHECK([ovsdb-server '-vPATTERN:console:ovsdb-server|%c|%m' --detach
--no-chdir --pidfile="`pwd`"/pid --remote=punix:socket
--unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
@@ -53,6 +55,7 @@ m4_define([OVSDB_CHECK_IDL_TCP_PY],
[AT_SETUP([$1 - Python tcp])
AT_SKIP_IF([test $HAVE_PYTHON = no])
AT_KEYWORDS([ovsdb server idl positive Python with tcp socket $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
[0], [stdout], [ignore])
AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
diff --git a/tests/ovsdb-macros.at b/tests/ovsdb-macros.at
index c1aa619..2aa752b 100644
--- a/tests/ovsdb-macros.at
+++ b/tests/ovsdb-macros.at
@@ -2,7 +2,8 @@ dnl OVSDB_INIT([$1])
dnl
dnl Creates an empty database named $1.
m4_define([OVSDB_INIT],
- [AT_CHECK(
+ [OVS_RUNDIR=`pwd`; export OVS_RUNDIR
+ AT_CHECK(
[ovsdb-tool create $1 $abs_top_srcdir/vswitchd/vswitch.ovsschema],
[0], [stdout], [ignore])
AT_CHECK(
diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at
index 167b44c..aff5854 100644
--- a/tests/ovsdb-monitor.at
+++ b/tests/ovsdb-monitor.at
@@ -19,6 +19,7 @@ AT_BANNER([OVSDB -- ovsdb-server monitors])
m4_define([OVSDB_CHECK_MONITOR],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb server monitor positive $9])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
m4_foreach([txn], [$3],
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 6dcf2f5..c5feec9 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -22,6 +22,7 @@ m4_define([OVSDB_SERVER_SHUTDOWN],
m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb server positive unix $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid
--remote=punix:socket --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
@@ -39,6 +40,7 @@ EXECUTION_EXAMPLES
AT_SETUP([truncating corrupted database log])
AT_KEYWORDS([ovsdb server positive unix])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
dnl Do one transaction and save the output.
@@ -85,6 +87,7 @@ AT_CLEANUP
AT_SETUP([truncating database log with bad transaction])
AT_KEYWORDS([ovsdb server positive unix])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
dnl Do one transaction and save the output.
@@ -132,6 +135,7 @@ AT_CLEANUP
AT_SETUP([ovsdb-client get-schema-version])
AT_KEYWORDS([ovsdb server positive])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid
--unixctl="`pwd`"/unixctl --remote=punix:socket db], [0], [ignore], [ignore])
@@ -142,6 +146,7 @@ AT_CLEANUP
AT_SETUP([database multiplexing implementation])
AT_KEYWORDS([ovsdb server positive])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema1
constraint_schema > schema2
AT_CHECK([ovsdb-tool create db1 schema1], [0], [ignore], [ignore])
@@ -280,6 +285,7 @@ AT_CLEANUP
AT_SETUP([compacting online])
AT_KEYWORDS([ovsdb server compact])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
dnl Make sure that "ovsdb-tool create" works with a dangling symlink for
dnl the database and the lockfile, creating the target of each symlink rather
@@ -430,6 +436,7 @@ m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb server positive ssl $5])
AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
SSL_PORT=`cat stdout`
@@ -479,6 +486,7 @@ AT_CLEANUP])
m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb server positive tcp $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
TCP_PORT=`cat stdout`
@@ -519,6 +527,7 @@ AT_BANNER([OVSDB -- transactions on transient ovsdb-server])
m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb server positive transient $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
m4_foreach([txn], [$3],
diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index e4f4a29..286d700 100644
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -16,6 +16,7 @@ AT_BANNER([OVSDB -- ovsdb-tool])
m4_define([OVSDB_CHECK_EXECUTION],
[AT_SETUP([$1])
AT_KEYWORDS([ovsdb file positive $5])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
$2 > schema
touch .db.~lock~
AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
@@ -48,6 +49,7 @@ AT_CLEANUP
AT_SETUP([ovsdb-tool compact])
AT_KEYWORDS([ovsdb file positive])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
dnl Make sure that "ovsdb-tool create" works with a dangling symlink,
dnl creating the target of the symlink rather than replacing the symlink
@@ -155,6 +157,7 @@ AT_CLEANUP
AT_SETUP([ovsdb-tool convert -- removing a column])
AT_KEYWORDS([ovsdb file positive])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
ordinal_schema > schema
AT_DATA([new-schema],
[[{"name": "ordinals",
@@ -218,6 +221,7 @@ AT_CLEANUP
AT_SETUP([ovsdb-tool convert -- adding a column])
AT_KEYWORDS([ovsdb file positive])
+OVS_RUNDIR=`pwd`; export OVS_RUNDIR
AT_DATA([schema],
[[{"name": "ordinals",
"tables": {
diff --git a/tests/vconn.at b/tests/vconn.at
index ae095b0..67a4224 100644
--- a/tests/vconn.at
+++ b/tests/vconn.at
@@ -11,6 +11,7 @@ m4_define([TEST_VCONN_CLASS],
[send-short-hello],
[send-invalid-version-hello]],
[AT_SETUP([$1 vconn - m4_bpatsubst(testname, [-], [ ])])
+ OVS_RUNDIR=`pwd`; export OVS_RUNDIR
m4_if([$1], [ssl], [
AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
AT_CHECK([cp $abs_top_builddir/tests/testpki*.pem .])])
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f5a4366..fdd7c64 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2799,8 +2799,10 @@ bridge_configure_remotes(struct bridge *br,
if (!strncmp(c->target, "unix:", 5)) {
/* Connect to a listening socket */
whitelist = xasprintf("unix:%s/", ovs_rundir());
- if (!equal_pathnames(c->target, whitelist,
- strlen(whitelist))) {
+ if (strchr(c->target, '/') &&
+ !equal_pathnames(c->target, whitelist,
+ strlen(whitelist))) {
+ /* Absolute path specified, but not in ovs_rundir */
VLOG_ERR_RL(&rl, "bridge %s: Not connecting to socket "
"controller \"%s\" due to possibility for "
"remote exploit. Instead, specify socket "
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
