Sure, I've made the change and will send out a new patch.
Can you push this one?
Thanks,
Pavithra
----- Original Message -----
From: "Ben Pfaff" <b...@nicira.com>
To: "Pavithra Ramesh" <param...@vmware.com>
Cc: dev@openvswitch.org
Sent: Friday, February 8, 2013 11:18:38 AM
Subject: Re: [ovs-dev] [PATCH] stream-unix: append ovs_rundir to socket
Yes, would you mind taking a first shot?
On Fri, Feb 08, 2013 at 09:51:33AM -0800, Pavithra Ramesh wrote:
> 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
