From: Ben Pfaff Date: Tue, 22 Dec 2009 01:02:17 +0000 (-0800) Subject: ofproto: Drop remote command execution feature. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2280e7223;p=openvswitch ofproto: Drop remote command execution feature. At one point Nicira had deployment plans for which adding a remote command execution feature to the OpenFlow stack made a lot of sense. We no longer have those plans, as far as I know, and leaving the feature in seems like a huge potential security hole. So this commit blows away the entire feature. --- diff --git a/Makefile.am b/Makefile.am index 5888e85f..bfba76f8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,7 +39,6 @@ EXTRA_DIST = INSTALL.bridge \ bin_PROGRAMS = sbin_PROGRAMS = bin_SCRIPTS = -dist_commands_DATA = dist_man_MANS = dist_pkgdata_SCRIPTS = dist_sbin_SCRIPTS = diff --git a/debian/automake.mk b/debian/automake.mk index 80b66ccb..1b649453 100644 --- a/debian/automake.mk +++ b/debian/automake.mk @@ -1,7 +1,5 @@ EXTRA_DIST += \ debian/changelog \ - debian/commands/reconfigure \ - debian/commands/update \ debian/compat \ debian/control \ debian/control.modules.in \ @@ -60,5 +58,6 @@ EXTRA_DIST += \ debian/ovs-switch-setup.8 \ debian/po/POTFILES.in \ debian/po/templates.pot \ + debian/reconfigure \ debian/rules \ debian/rules.modules diff --git a/debian/commands/reconfigure b/debian/commands/reconfigure deleted file mode 100755 index dc493a18..00000000 --- a/debian/commands/reconfigure +++ /dev/null @@ -1,128 +0,0 @@ -#! /usr/bin/perl - -use POSIX; -use strict; -use warnings; - -my $default = '/etc/default/openvswitch-switch'; - -my (%config) = load_config($default); -if (@ARGV) { - foreach my $arg (@ARGV) { - my ($key, $value) = $arg =~ /^([^=]+)=(.*)/ - or die "bad argument '$arg'\n"; - if ($value ne '') { - $config{$key} = $value; - } else { - delete $config{$key}; - } - } - save_config($default, %config); -} -print "$_=$config{$_}\n" foreach sort(keys(%config)); - -sub load_config { - my ($file) = @_; - - # Get the list of the variables that the shell sets automatically. - my (%auto_vars) = read_vars("set -a && env"); - - # Get the variables from $default. - my (%config) = read_vars("set -a && . '$default' && env"); - - # Subtract. - delete @config{keys %auto_vars}; - - return %config; -} - -sub read_vars { - my ($cmd) = @_; - local @ENV; - if (!open(VARS, '-|', $cmd)) { - print STDERR "$cmd: failed to execute: $!\n"; - return (); - } - my (%config); - while () { - my ($var, $value) = /^([^=]+)=(.*)$/ or next; - $config{$var} = $value; - } - close(VARS); - return %config; -} - -sub shell_escape { - local $_ = $_[0]; - if ($_ eq '') { - return '""'; - } elsif (m&^[-a-zA-Z0-9:./%^_+,]*$&) { - return $_; - } else { - s/'/'\\''/; - return "'$_'"; - } -} - -sub shell_assign { - my ($var, $value) = @_; - return $var . '=' . shell_escape($value); -} - -sub save_config { - my ($file, %config) = @_; - my (@lines); - if (open(FILE, '<', $file)) { - @lines = ; - chomp @lines; - close(FILE); - } - - # Replace all existing variable assignments. - for (my ($i) = 0; $i <= $#lines; $i++) { - local $_ = $lines[$i]; - my ($var, $value) = /^\s*([^=#]+)=(.*)$/ or next; - if (exists($config{$var})) { - $lines[$i] = shell_assign($var, $config{$var}); - delete $config{$var}; - } else { - $lines[$i] = "#$lines[$i]"; - } - } - - # Find a place to put any remaining variable assignments. - VAR: - for my $var (keys(%config)) { - my $assign = shell_assign($var, $config{$var}); - - # Replace the last commented-out variable assignment to $var, if any. - for (my ($i) = $#lines; $i >= 0; $i--) { - local $_ = $lines[$i]; - if (/^\s*#\s*$var=/) { - $lines[$i] = $assign; - next VAR; - } - } - - # Find a place to add the var: after the final commented line - # just after a line that contains "$var:". - for (my ($i) = 0; $i <= $#lines; $i++) { - if ($lines[$i] =~ /^\s*#\s*$var:/) { - for (my ($j) = $i + 1; $j <= $#lines; $j++) { - if ($lines[$j] !~ /^\s*#/) { - splice(@lines, $j, 0, $assign); - next VAR; - } - } - } - } - - # Just append it. - push(@lines, $assign); - } - - open(NEWFILE, '>', "$file.tmp") or die "$file.tmp: create: $!\n"; - print NEWFILE join('', map("$_\n", @lines)); - close(NEWFILE); - rename("$file.tmp", $file) or die "$file.tmp: rename to $file: $!\n"; -} diff --git a/debian/commands/update b/debian/commands/update deleted file mode 100755 index 545e3c23..00000000 --- a/debian/commands/update +++ /dev/null @@ -1,4 +0,0 @@ -#! /bin/sh -set -e -apt-get update -qy -apt-get upgrade -qy diff --git a/debian/openvswitch-switch.install b/debian/openvswitch-switch.install index 8e9b6c11..7b988da1 100644 --- a/debian/openvswitch-switch.install +++ b/debian/openvswitch-switch.install @@ -4,5 +4,3 @@ _debian/utilities/ovs-dpctl usr/sbin _debian/utilities/ovs-kill usr/sbin _debian/utilities/ovs-vsctl usr/sbin _debian/vswitchd/ovs-vswitchd usr/sbin -debian/commands/* usr/share/openvswitch/commands -debian/openvswitch/usr/share/openvswitch/commands/* usr/share/openvswitch/commands diff --git a/debian/openvswitch-switchui.install b/debian/openvswitch-switchui.install index f2872c83..0cfb2904 100644 --- a/debian/openvswitch-switchui.install +++ b/debian/openvswitch-switchui.install @@ -1,2 +1,3 @@ _debian/extras/ezio/ezio-term usr/sbin _debian/extras/ezio/ovs-switchui usr/bin +debian/reconfigure usr/share/openvswitch-switchui diff --git a/debian/reconfigure b/debian/reconfigure new file mode 100755 index 00000000..dc493a18 --- /dev/null +++ b/debian/reconfigure @@ -0,0 +1,128 @@ +#! /usr/bin/perl + +use POSIX; +use strict; +use warnings; + +my $default = '/etc/default/openvswitch-switch'; + +my (%config) = load_config($default); +if (@ARGV) { + foreach my $arg (@ARGV) { + my ($key, $value) = $arg =~ /^([^=]+)=(.*)/ + or die "bad argument '$arg'\n"; + if ($value ne '') { + $config{$key} = $value; + } else { + delete $config{$key}; + } + } + save_config($default, %config); +} +print "$_=$config{$_}\n" foreach sort(keys(%config)); + +sub load_config { + my ($file) = @_; + + # Get the list of the variables that the shell sets automatically. + my (%auto_vars) = read_vars("set -a && env"); + + # Get the variables from $default. + my (%config) = read_vars("set -a && . '$default' && env"); + + # Subtract. + delete @config{keys %auto_vars}; + + return %config; +} + +sub read_vars { + my ($cmd) = @_; + local @ENV; + if (!open(VARS, '-|', $cmd)) { + print STDERR "$cmd: failed to execute: $!\n"; + return (); + } + my (%config); + while () { + my ($var, $value) = /^([^=]+)=(.*)$/ or next; + $config{$var} = $value; + } + close(VARS); + return %config; +} + +sub shell_escape { + local $_ = $_[0]; + if ($_ eq '') { + return '""'; + } elsif (m&^[-a-zA-Z0-9:./%^_+,]*$&) { + return $_; + } else { + s/'/'\\''/; + return "'$_'"; + } +} + +sub shell_assign { + my ($var, $value) = @_; + return $var . '=' . shell_escape($value); +} + +sub save_config { + my ($file, %config) = @_; + my (@lines); + if (open(FILE, '<', $file)) { + @lines = ; + chomp @lines; + close(FILE); + } + + # Replace all existing variable assignments. + for (my ($i) = 0; $i <= $#lines; $i++) { + local $_ = $lines[$i]; + my ($var, $value) = /^\s*([^=#]+)=(.*)$/ or next; + if (exists($config{$var})) { + $lines[$i] = shell_assign($var, $config{$var}); + delete $config{$var}; + } else { + $lines[$i] = "#$lines[$i]"; + } + } + + # Find a place to put any remaining variable assignments. + VAR: + for my $var (keys(%config)) { + my $assign = shell_assign($var, $config{$var}); + + # Replace the last commented-out variable assignment to $var, if any. + for (my ($i) = $#lines; $i >= 0; $i--) { + local $_ = $lines[$i]; + if (/^\s*#\s*$var=/) { + $lines[$i] = $assign; + next VAR; + } + } + + # Find a place to add the var: after the final commented line + # just after a line that contains "$var:". + for (my ($i) = 0; $i <= $#lines; $i++) { + if ($lines[$i] =~ /^\s*#\s*$var:/) { + for (my ($j) = $i + 1; $j <= $#lines; $j++) { + if ($lines[$j] !~ /^\s*#/) { + splice(@lines, $j, 0, $assign); + next VAR; + } + } + } + } + + # Just append it. + push(@lines, $assign); + } + + open(NEWFILE, '>', "$file.tmp") or die "$file.tmp: create: $!\n"; + print NEWFILE join('', map("$_\n", @lines)); + close(NEWFILE); + rename("$file.tmp", $file) or die "$file.tmp: rename to $file: $!\n"; +} diff --git a/extras/ezio/ovs-switchui.c b/extras/ezio/ovs-switchui.c index bd07e869..14dca49f 100644 --- a/extras/ezio/ovs-switchui.c +++ b/extras/ezio/ovs-switchui.c @@ -2079,7 +2079,7 @@ save_config(const struct svec *settings) } svec_init(&argv); - svec_add(&argv, "/usr/share/openvswitch/commands/reconfigure"); + svec_add(&argv, "/usr/share/openvswitch-switchui/reconfigure"); svec_append(&argv, settings); svec_terminate(&argv); ok = run_and_report_failure(argv.names, "Save failed"); diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index c4c60622..8434a30a 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -44,14 +44,11 @@ enum nicira_type { /* Get configuration of action. */ NXT_ACT_GET_CONFIG, - /* Remote command execution. The request body is a sequence of strings - * delimited by null bytes. The first string is a command name. - * Subsequent strings are command arguments. */ - NXT_COMMAND_REQUEST, + /* No longer used. */ + NXT_COMMAND_REQUEST__OBSOLETE, - /* Remote command execution reply, sent when the command's execution - * completes. The reply body is struct nx_command_reply. */ - NXT_COMMAND_REPLY, + /* No longer used. */ + NXT_COMMAND_REPLY__OBSOLETE, /* No longer used. */ NXT_FLOW_END_CONFIG__OBSOLETE, @@ -97,24 +94,4 @@ struct nx_action_header { }; OFP_ASSERT(sizeof(struct nx_action_header) == 16); -/* Status bits for NXT_COMMAND_REPLY. */ -enum { - NXT_STATUS_EXITED = 1 << 31, /* Exited normally. */ - NXT_STATUS_SIGNALED = 1 << 30, /* Exited due to signal. */ - NXT_STATUS_UNKNOWN = 1 << 29, /* Exited for unknown reason. */ - NXT_STATUS_COREDUMP = 1 << 28, /* Exited with core dump. */ - NXT_STATUS_ERROR = 1 << 27, /* Command could not be executed. */ - NXT_STATUS_STARTED = 1 << 26, /* Command was started. */ - NXT_STATUS_EXITSTATUS = 0xff, /* Exit code mask if NXT_STATUS_EXITED. */ - NXT_STATUS_TERMSIG = 0xff, /* Signal number if NXT_STATUS_SIGNALED. */ -}; - -/* NXT_COMMAND_REPLY. */ -struct nx_command_reply { - struct nicira_header nxh; - uint32_t status; /* Status bits defined above. */ - /* Followed by any number of bytes of process output. */ -}; -OFP_ASSERT(sizeof(struct nx_command_reply) == 20); - #endif /* openflow/nicira-ext.h */ diff --git a/lib/vlog-modules.def b/lib/vlog-modules.def index b33daf62..f68e9d82 100644 --- a/lib/vlog-modules.def +++ b/lib/vlog-modules.def @@ -32,7 +32,6 @@ VLOG_MODULE(dpif) VLOG_MODULE(dpif_linux) VLOG_MODULE(dpif_netdev) VLOG_MODULE(dpctl) -VLOG_MODULE(executer) VLOG_MODULE(ezio_term) VLOG_MODULE(fail_open) VLOG_MODULE(fatal_signal) diff --git a/ofproto/automake.mk b/ofproto/automake.mk index 87a0fa68..0f05e535 100644 --- a/ofproto/automake.mk +++ b/ofproto/automake.mk @@ -11,8 +11,6 @@ ofproto_libofproto_a_SOURCES = \ ofproto/collectors.h \ ofproto/discovery.c \ ofproto/discovery.h \ - ofproto/executer.c \ - ofproto/executer.h \ ofproto/fail-open.c \ ofproto/fail-open.h \ ofproto/in-band.c \ @@ -27,5 +25,3 @@ ofproto_libofproto_a_SOURCES = \ ofproto/pinsched.h \ ofproto/status.c \ ofproto/status.h - -include ofproto/commands/automake.mk diff --git a/ofproto/commands/automake.mk b/ofproto/commands/automake.mk deleted file mode 100644 index 96d165f5..00000000 --- a/ofproto/commands/automake.mk +++ /dev/null @@ -1,3 +0,0 @@ -commandsdir = ${pkgdatadir}/commands -dist_commands_SCRIPTS = \ - ofproto/commands/reboot diff --git a/ofproto/commands/reboot b/ofproto/commands/reboot deleted file mode 100755 index 42fd10c1..00000000 --- a/ofproto/commands/reboot +++ /dev/null @@ -1,3 +0,0 @@ -#! /bin/sh -ovs-kill --force --signal=USR1 ovs-switchui.pid -reboot diff --git a/ofproto/executer.c b/ofproto/executer.c deleted file mode 100644 index f78ec338..00000000 --- a/ofproto/executer.c +++ /dev/null @@ -1,512 +0,0 @@ -/* - * Copyright (c) 2008, 2009 Nicira Networks. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at: - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include "executer.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "dirs.h" -#include "dynamic-string.h" -#include "fatal-signal.h" -#include "openflow/nicira-ext.h" -#include "ofpbuf.h" -#include "openflow/openflow.h" -#include "poll-loop.h" -#include "rconn.h" -#include "socket-util.h" -#include "util.h" -#include "vconn.h" - -#define THIS_MODULE VLM_executer -#include "vlog.h" - -#define MAX_CHILDREN 8 - -struct child { - /* Information about child process. */ - char *name; /* argv[0] passed to child. */ - pid_t pid; /* Child's process ID. */ - - /* For sending a reply to the controller when the child dies. */ - struct rconn *rconn; - uint32_t xid; /* Transaction ID used by controller. */ - - /* We read up to MAX_OUTPUT bytes of output and send them back to the - * controller when the child dies. */ -#define MAX_OUTPUT 4096 - int output_fd; /* FD from which to read child's output. */ - uint8_t *output; /* Output data. */ - size_t output_size; /* Number of bytes of output data so far. */ -}; - -struct executer { - /* Settings. */ - char *command_acl; /* Command white/blacklist, as shell globs. */ - char *command_dir; /* Directory that contains commands. */ - - /* Children. */ - struct child children[MAX_CHILDREN]; - size_t n_children; -}; - -/* File descriptors for waking up when a child dies. */ -static int signal_fds[2] = {-1, -1}; - -static void send_child_status(struct rconn *, uint32_t xid, uint32_t status, - const void *data, size_t size); -static void send_child_message(struct rconn *, uint32_t xid, uint32_t status, - const char *message); - -/* Returns true if 'cmd' is allowed by 'acl', which is a command-separated - * access control list in the format described for --command-acl in - * ovs-openflowd(8). */ -static bool -executer_is_permitted(const char *acl_, const char *cmd) -{ - char *acl, *save_ptr, *pattern; - bool allowed, denied; - - /* Verify that 'cmd' consists only of alphanumerics plus _ or -. */ - if (cmd[strspn(cmd, "abcdefghijklmnopqrstuvwxyz" - "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-")] != '\0') { - VLOG_WARN("rejecting command name \"%s\" that contain forbidden " - "characters", cmd); - return false; - } - - /* Check 'cmd' against 'acl'. */ - acl = xstrdup(acl_); - save_ptr = acl; - allowed = denied = false; - while ((pattern = strsep(&save_ptr, ",")) != NULL && !denied) { - if (pattern[0] != '!' && !fnmatch(pattern, cmd, 0)) { - allowed = true; - } else if (pattern[0] == '!' && !fnmatch(pattern + 1, cmd, 0)) { - denied = true; - } - } - free(acl); - - /* Check the command white/blacklisted state. */ - if (allowed && !denied) { - VLOG_INFO("permitting command execution: \"%s\" is whitelisted", cmd); - } else if (allowed && denied) { - VLOG_WARN("denying command execution: \"%s\" is both blacklisted " - "and whitelisted", cmd); - } else if (!allowed) { - VLOG_WARN("denying command execution: \"%s\" is not whitelisted", cmd); - } else if (denied) { - VLOG_WARN("denying command execution: \"%s\" is blacklisted", cmd); - } - return allowed && !denied; -} - -int -executer_handle_request(struct executer *e, struct rconn *rconn, - struct nicira_header *request) -{ - char **argv; - char *args; - char *exec_file = NULL; - int max_fds; - struct stat s; - size_t args_size; - size_t argc; - size_t i; - pid_t pid; - int output_fds[2]; - - /* Verify limit on children not exceeded. - * XXX should probably kill children when the connection drops? */ - if (e->n_children >= MAX_CHILDREN) { - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "too many child processes"); - return 0; - } - - /* Copy argument buffer, adding a null terminator at the end. Now every - * argument is null-terminated, instead of being merely null-delimited. */ - args_size = ntohs(request->header.length) - sizeof *request; - args = xmemdup0((const void *) (request + 1), args_size); - - /* Count arguments. */ - argc = 0; - for (i = 0; i <= args_size; i++) { - argc += args[i] == '\0'; - } - - /* Set argv[*] to point to each argument. */ - argv = xmalloc((argc + 1) * sizeof *argv); - argv[0] = args; - for (i = 1; i < argc; i++) { - argv[i] = strchr(argv[i - 1], '\0') + 1; - } - argv[argc] = NULL; - - /* Check permissions. */ - if (!executer_is_permitted(e->command_acl, argv[0])) { - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "command not allowed"); - goto done; - } - - /* Find the executable. */ - exec_file = xasprintf("%s/%s", e->command_dir, argv[0]); - if (stat(exec_file, &s)) { - VLOG_WARN("failed to stat \"%s\": %s", exec_file, strerror(errno)); - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "command not allowed"); - goto done; - } - if (!S_ISREG(s.st_mode)) { - VLOG_WARN("\"%s\" is not a regular file", exec_file); - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "command not allowed"); - goto done; - } - argv[0] = exec_file; - - /* Arrange to capture output. */ - if (pipe(output_fds)) { - VLOG_WARN("pipe failed: %s", strerror(errno)); - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "internal error (pipe)"); - goto done; - } - - pid = fork(); - if (!pid) { - /* Running in child. - * XXX should run in new process group so that we can signal all - * subprocesses at once? Would also want to catch fatal signals and - * kill them at the same time though. */ - fatal_signal_fork(); - dup2(get_null_fd(), 0); - dup2(output_fds[1], 1); - dup2(get_null_fd(), 2); - max_fds = get_max_fds(); - for (i = 3; i < max_fds; i++) { - close(i); - } - if (chdir(e->command_dir)) { - printf("could not change directory to \"%s\": %s", - e->command_dir, strerror(errno)); - exit(EXIT_FAILURE); - } - execv(argv[0], argv); - printf("failed to start \"%s\": %s\n", argv[0], strerror(errno)); - exit(EXIT_FAILURE); - } else if (pid > 0) { - /* Running in parent. */ - struct child *child; - - VLOG_INFO("started \"%s\" subprocess", argv[0]); - send_child_status(rconn, request->header.xid, NXT_STATUS_STARTED, - NULL, 0); - child = &e->children[e->n_children++]; - child->name = xstrdup(argv[0]); - child->pid = pid; - child->rconn = rconn; - child->xid = request->header.xid; - child->output_fd = output_fds[0]; - child->output = xmalloc(MAX_OUTPUT); - child->output_size = 0; - set_nonblocking(output_fds[0]); - close(output_fds[1]); - } else { - VLOG_WARN("fork failed: %s", strerror(errno)); - send_child_message(rconn, request->header.xid, NXT_STATUS_ERROR, - "internal error (fork)"); - close(output_fds[0]); - close(output_fds[1]); - } - -done: - free(exec_file); - free(args); - free(argv); - return 0; -} - -static void -send_child_status(struct rconn *rconn, uint32_t xid, uint32_t status, - const void *data, size_t size) -{ - if (rconn) { - struct nx_command_reply *r; - struct ofpbuf *buffer; - - r = make_openflow_xid(sizeof *r, OFPT_VENDOR, xid, &buffer); - r->nxh.vendor = htonl(NX_VENDOR_ID); - r->nxh.subtype = htonl(NXT_COMMAND_REPLY); - r->status = htonl(status); - ofpbuf_put(buffer, data, size); - update_openflow_length(buffer); - if (rconn_send(rconn, buffer, NULL)) { - ofpbuf_delete(buffer); - } - } -} - -static void -send_child_message(struct rconn *rconn, uint32_t xid, uint32_t status, - const char *message) -{ - send_child_status(rconn, xid, status, message, strlen(message)); -} - -/* 'child' died with 'status' as its return code. Deal with it. */ -static void -child_terminated(struct child *child, int status) -{ - struct ds ds; - uint32_t ofp_status; - - /* Log how it terminated. */ - ds_init(&ds); - if (WIFEXITED(status)) { - ds_put_format(&ds, "normally with status %d", WEXITSTATUS(status)); - } else if (WIFSIGNALED(status)) { - const char *name = NULL; -#ifdef HAVE_STRSIGNAL - name = strsignal(WTERMSIG(status)); -#endif - ds_put_format(&ds, "by signal %d", WTERMSIG(status)); - if (name) { - ds_put_format(&ds, " (%s)", name); - } - } - if (WCOREDUMP(status)) { - ds_put_cstr(&ds, " (core dumped)"); - } - VLOG_INFO("child process \"%s\" with pid %ld terminated %s", - child->name, (long int) child->pid, ds_cstr(&ds)); - ds_destroy(&ds); - - /* Send a status message back to the controller that requested the - * command. */ - if (WIFEXITED(status)) { - ofp_status = WEXITSTATUS(status) | NXT_STATUS_EXITED; - } else if (WIFSIGNALED(status)) { - ofp_status = WTERMSIG(status) | NXT_STATUS_SIGNALED; - } else { - ofp_status = NXT_STATUS_UNKNOWN; - } - if (WCOREDUMP(status)) { - ofp_status |= NXT_STATUS_COREDUMP; - } - send_child_status(child->rconn, child->xid, ofp_status, - child->output, child->output_size); -} - -/* Read output from 'child' and append it to its output buffer. */ -static void -poll_child(struct child *child) -{ - ssize_t n; - - if (child->output_fd < 0) { - return; - } - - do { - n = read(child->output_fd, child->output + child->output_size, - MAX_OUTPUT - child->output_size); - } while (n < 0 && errno == EINTR); - if (n > 0) { - child->output_size += n; - if (child->output_size < MAX_OUTPUT) { - return; - } - } else if (n < 0 && errno == EAGAIN) { - return; - } - close(child->output_fd); - child->output_fd = -1; -} - -void -executer_run(struct executer *e) -{ - char buffer[MAX_CHILDREN]; - size_t i; - - if (!e->n_children) { - return; - } - - /* Read output from children. */ - for (i = 0; i < e->n_children; i++) { - struct child *child = &e->children[i]; - poll_child(child); - } - - /* If SIGCHLD was received, reap dead children. */ - if (read(signal_fds[0], buffer, sizeof buffer) <= 0) { - return; - } - for (;;) { - int status; - pid_t pid; - - /* Get dead child in 'pid' and its return code in 'status'. */ - pid = waitpid(WAIT_ANY, &status, WNOHANG); - if (pid < 0 && errno == EINTR) { - continue; - } else if (pid <= 0) { - return; - } - - /* Find child with given 'pid' and drop it from the list. */ - for (i = 0; i < e->n_children; i++) { - struct child *child = &e->children[i]; - if (child->pid == pid) { - poll_child(child); - child_terminated(child, status); - free(child->name); - free(child->output); - *child = e->children[--e->n_children]; - goto found; - } - } - VLOG_WARN("child with unknown pid %ld terminated", (long int) pid); - found:; - } - -} - -void -executer_wait(struct executer *e) -{ - if (e->n_children) { - size_t i; - - /* Wake up on SIGCHLD. */ - poll_fd_wait(signal_fds[0], POLLIN); - - /* Wake up when we get output from a child. */ - for (i = 0; i < e->n_children; i++) { - struct child *child = &e->children[i]; - if (child->output_fd >= 0) { - poll_fd_wait(child->output_fd, POLLIN); - } - } - } -} - -void -executer_rconn_closing(struct executer *e, struct rconn *rconn) -{ - size_t i; - - /* If any of our children was connected to 'r', then disconnect it so we - * don't try to reference a dead connection when the process terminates - * later. - * XXX kill the children started by 'r'? */ - for (i = 0; i < e->n_children; i++) { - if (e->children[i].rconn == rconn) { - e->children[i].rconn = NULL; - } - } -} - -static void -sigchld_handler(int signr UNUSED) -{ - ignore(write(signal_fds[1], "", 1)); -} - -int -executer_create(const char *command_acl, const char *command_dir, - struct executer **executerp) -{ - struct executer *e; - struct sigaction sa; - - *executerp = NULL; - if (signal_fds[0] == -1) { - /* Make sure we can get a fd for /dev/null. */ - int null_fd = get_null_fd(); - if (null_fd < 0) { - return -null_fd; - } - - /* Create pipe for notifying us that SIGCHLD was invoked. */ - if (pipe(signal_fds)) { - VLOG_ERR("pipe failed: %s", strerror(errno)); - return errno; - } - set_nonblocking(signal_fds[0]); - set_nonblocking(signal_fds[1]); - } - - /* Set up signal handler. */ - memset(&sa, 0, sizeof sa); - sa.sa_handler = sigchld_handler; - sigemptyset(&sa.sa_mask); - sa.sa_flags = SA_NOCLDSTOP | SA_RESTART; - if (sigaction(SIGCHLD, &sa, NULL)) { - VLOG_ERR("sigaction(SIGCHLD) failed: %s", strerror(errno)); - return errno; - } - - e = xzalloc(sizeof *e); - e->command_acl = xstrdup(command_acl); - e->command_dir = (command_dir - ? xstrdup(command_dir) - : xasprintf("%s/commands", ovs_pkgdatadir)); - e->n_children = 0; - *executerp = e; - return 0; -} - -void -executer_destroy(struct executer *e) -{ - if (e) { - size_t i; - - free(e->command_acl); - free(e->command_dir); - for (i = 0; i < e->n_children; i++) { - struct child *child = &e->children[i]; - - free(child->name); - kill(child->pid, SIGHUP); - /* We don't own child->rconn. */ - free(child->output); - free(child); - } - free(e); - } -} - -void -executer_set_acl(struct executer *e, const char *acl, const char *dir) -{ - free(e->command_acl); - e->command_acl = xstrdup(acl); - free(e->command_dir); - e->command_dir = xstrdup(dir); -} diff --git a/ofproto/executer.h b/ofproto/executer.h deleted file mode 100644 index fbed85b8..00000000 --- a/ofproto/executer.h +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (c) 2008, 2009 Nicira Networks. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at: - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef EXECUTER_H -#define EXECUTER_H 1 - -struct executer; -struct nicira_header; -struct rconn; - -int executer_create(const char *acl, const char *dir, struct executer **); -void executer_set_acl(struct executer *, const char *acl, const char *dir); -void executer_destroy(struct executer *); -void executer_run(struct executer *); -void executer_wait(struct executer *); -void executer_rconn_closing(struct executer *, struct rconn *); -int executer_handle_request(struct executer *, struct rconn *, - struct nicira_header *); - -#endif /* executer.h */ diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 09388ba2..ea6a8177 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -27,7 +27,6 @@ #include "discovery.h" #include "dpif.h" #include "dynamic-string.h" -#include "executer.h" #include "fail-open.h" #include "in-band.h" #include "mac-learning.h" @@ -207,7 +206,6 @@ struct ofproto { struct discovery *discovery; struct fail_open *fail_open; struct pinsched *miss_sched, *action_sched; - struct executer *executer; struct netflow *netflow; /* Flow table. */ @@ -314,7 +312,6 @@ ofproto_create(const char *datapath, const struct ofhooks *ofhooks, void *aux, p->discovery = NULL; p->fail_open = NULL; p->miss_sched = p->action_sched = NULL; - p->executer = NULL; p->netflow = NULL; /* Initialize flow table. */ @@ -603,24 +600,6 @@ ofproto_set_stp(struct ofproto *ofproto UNUSED, bool enable_stp) } } -int -ofproto_set_remote_execution(struct ofproto *ofproto, const char *command_acl, - const char *command_dir) -{ - if (command_acl) { - if (!ofproto->executer) { - return executer_create(command_acl, command_dir, - &ofproto->executer); - } else { - executer_set_acl(ofproto->executer, command_acl, command_dir); - } - } else { - executer_destroy(ofproto->executer); - ofproto->executer = NULL; - } - return 0; -} - uint64_t ofproto_get_datapath_id(const struct ofproto *ofproto) { @@ -716,7 +695,6 @@ ofproto_destroy(struct ofproto *p) fail_open_destroy(p->fail_open); pinsched_destroy(p->miss_sched); pinsched_destroy(p->action_sched); - executer_destroy(p->executer); netflow_destroy(p->netflow); switch_status_unregister(p->ss_cat); @@ -812,9 +790,6 @@ ofproto_run1(struct ofproto *p) } pinsched_run(p->miss_sched, send_packet_in_miss, p); pinsched_run(p->action_sched, send_packet_in_action, p); - if (p->executer) { - executer_run(p->executer); - } LIST_FOR_EACH_SAFE (ofconn, next_ofconn, struct ofconn, node, &p->all_conns) { @@ -923,9 +898,6 @@ ofproto_wait(struct ofproto *p) } pinsched_wait(p->miss_sched); pinsched_wait(p->action_sched); - if (p->executer) { - executer_wait(p->executer); - } if (!tag_set_is_empty(&p->revalidate_set)) { poll_immediate_wake(); } @@ -1330,10 +1302,6 @@ ofconn_create(struct ofproto *p, struct rconn *rconn) static void ofconn_destroy(struct ofconn *ofconn, struct ofproto *p) { - if (p->executer) { - executer_rconn_closing(p->executer, ofconn->rconn); - } - list_remove(&ofconn->node); rconn_destroy(ofconn->rconn); rconn_packet_counter_destroy(ofconn->packet_in_counter); @@ -3030,12 +2998,6 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg) case NXT_ACT_GET_CONFIG: return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE); /* XXX */ - case NXT_COMMAND_REQUEST: - if (p->executer) { - return executer_handle_request(p->executer, ofconn->rconn, msg); - } - break; - case NXT_MGMT: return handle_ofmp(p, ofconn, msg); } diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 50dd5d5b..5fe8d774 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -65,8 +65,6 @@ int ofproto_set_netflow(struct ofproto *, void ofproto_set_failure(struct ofproto *, bool fail_open); void ofproto_set_rate_limit(struct ofproto *, int rate_limit, int burst_limit); int ofproto_set_stp(struct ofproto *, bool enable_stp); -int ofproto_set_remote_execution(struct ofproto *, const char *command_acl, - const char *command_dir); /* Configuration querying. */ uint64_t ofproto_get_datapath_id(const struct ofproto *); diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 439d3f5b..5b34a549 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -164,15 +164,6 @@ flow expirations. This command may be useful for debugging switch or controller implementations. -.TP -\fBexecute \fIswitch command \fR[\fIarg\fR...] -Sends a request to \fIswitch\fR to execute \fIcommand\fR along with -each \fIarg\fR, if any, then waits for the command to complete and -reports its completion status on \fBstderr\fR and its output, if any, -on \fBstdout\fR. The set of available commands and their argument is -switch-dependent. (This command uses a Nicira extension to OpenFlow -that may not be available on all switches.) - .SS "OpenFlow Switch and Controller Commands" The following commands, like those in the previous section, may be diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 89af18ed..ab4e9b55 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -164,7 +164,6 @@ usage(void) " mod-flows SWITCH FLOW modify actions of matching FLOWs\n" " del-flows SWITCH [FLOW] delete matching FLOWs\n" " monitor SWITCH MISSLEN EXP print packets received from SWITCH\n" - " execute SWITCH CMD [ARG...] execute CMD with ARGS on SWITCH\n" "\nFor OpenFlow switches and controllers:\n" " probe VCONN probe whether VCONN is up\n" " ping VCONN [N] latency of N-byte echos\n" @@ -1163,71 +1162,6 @@ do_benchmark(int argc UNUSED, char *argv[]) count * message_size / (duration / 1000.0)); } -static void -do_execute(int argc, char *argv[]) -{ - struct vconn *vconn; - struct ofpbuf *request; - struct nicira_header *nicira; - struct nx_command_reply *ncr; - uint32_t xid; - int i; - - nicira = make_openflow(sizeof *nicira, OFPT_VENDOR, &request); - xid = nicira->header.xid; - nicira->vendor = htonl(NX_VENDOR_ID); - nicira->subtype = htonl(NXT_COMMAND_REQUEST); - ofpbuf_put(request, argv[2], strlen(argv[2])); - for (i = 3; i < argc; i++) { - ofpbuf_put_zeros(request, 1); - ofpbuf_put(request, argv[i], strlen(argv[i])); - } - update_openflow_length(request); - - open_vconn(argv[1], &vconn); - run(vconn_send_block(vconn, request), "send"); - - for (;;) { - struct ofpbuf *reply; - uint32_t status; - - run(vconn_recv_xid(vconn, xid, &reply), "recv_xid"); - if (reply->size < sizeof *ncr) { - ovs_fatal(0, "reply is too short (%zu bytes < %zu bytes)", - reply->size, sizeof *ncr); - } - ncr = reply->data; - if (ncr->nxh.header.type != OFPT_VENDOR - || ncr->nxh.vendor != htonl(NX_VENDOR_ID) - || ncr->nxh.subtype != htonl(NXT_COMMAND_REPLY)) { - ovs_fatal(0, "reply is invalid"); - } - - status = ntohl(ncr->status); - if (status & NXT_STATUS_STARTED) { - /* Wait for a second reply. */ - continue; - } else if (status & NXT_STATUS_EXITED) { - fprintf(stderr, "process terminated normally with exit code %d", - status & NXT_STATUS_EXITSTATUS); - } else if (status & NXT_STATUS_SIGNALED) { - fprintf(stderr, "process terminated by signal %d", - status & NXT_STATUS_TERMSIG); - } else if (status & NXT_STATUS_ERROR) { - fprintf(stderr, "error executing command"); - } else { - fprintf(stderr, "process terminated for unknown reason"); - } - if (status & NXT_STATUS_COREDUMP) { - fprintf(stderr, " (core dumped)"); - } - putc('\n', stderr); - - fwrite(ncr + 1, reply->size - sizeof *ncr, 1, stdout); - break; - } -} - static void do_help(int argc UNUSED, char *argv[] UNUSED) { @@ -1251,7 +1185,6 @@ static const struct command all_commands[] = { { "probe", 1, 1, do_probe }, { "ping", 1, 2, do_ping }, { "benchmark", 3, 3, do_benchmark }, - { "execute", 2, INT_MAX, do_execute }, { "help", 0, INT_MAX, do_help }, { NULL, 0, 0, NULL }, }; diff --git a/utilities/ovs-openflowd.8.in b/utilities/ovs-openflowd.8.in index 25b222f4..ed21fa5b 100644 --- a/utilities/ovs-openflowd.8.in +++ b/utilities/ovs-openflowd.8.in @@ -388,34 +388,6 @@ specified on \fB--rate-limit\fR. This option takes effect only when \fB--rate-limit\fR is also specified. -.SS "Remote Command Execution Options" - -.TP -\fB--command-acl=\fR[\fB!\fR]\fIglob\fR[\fB,\fR[\fB!\fR]\fIglob\fR...] -Configures the commands that remote OpenFlow connections are allowed -to invoke using (e.g.) \fBovs\-ofctl execute\fR. The argument is a -comma-separated sequence of shell glob patterns. A glob pattern -specified without a leading \fB!\fR is a ``whitelist'' that specifies -a set of commands that are that may be invoked, whereas a pattern that -does begin with \fB!\fR is a ``blacklist'' that specifies commands -that may not be invoked. To be permitted, a command name must be -whitelisted and must not be blacklisted; -e.g. \fB--command-acl=up*,!upgrade\fR would allow any command whose name -begins with \fBup\fR except for the command named \fBupgrade\fR. -Command names that include characters other than upper- and lower-case -English letters, digits, and the underscore and hyphen characters are -unconditionally disallowed. - -When the whitelist and blacklist permit a command name, \fBovs\-openflowd\fR -looks for a program with the same name as the command in the commands -directory (see below). Other directories are not searched. - -.TP -\fB--command-dir=\fIdirectory\fR -Sets the directory searched for remote command execution to -\fBdirectory\fR. The default directory is -\fB@pkgdatadir@/commands\fR. - .SS "Datapath Options" . .IP "\fB\-\-ports=\fIport\fR[\fB,\fIport\fR...]" diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c index ba97faf6..0b0580df 100644 --- a/utilities/ovs-openflowd.c +++ b/utilities/ovs-openflowd.c @@ -94,10 +94,6 @@ struct ofsettings { /* Spanning tree protocol. */ bool enable_stp; - /* Remote command execution. */ - char *command_acl; /* Command white/blacklist, as shell globs. */ - char *command_dir; /* Directory that contains commands. */ - /* Management. */ uint64_t mgmt_id; /* Management ID. */ @@ -206,11 +202,6 @@ main(int argc, char *argv[]) if (error) { ovs_fatal(error, "failed to configure STP"); } - error = ofproto_set_remote_execution(ofproto, s.command_acl, - s.command_dir); - if (error) { - ovs_fatal(error, "failed to configure remote command execution"); - } if (!s.discovery) { error = ofproto_set_controller(ofproto, s.controller_name); if (error) { @@ -265,8 +256,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) OPT_NO_STP, OPT_OUT_OF_BAND, OPT_IN_BAND, - OPT_COMMAND_ACL, - OPT_COMMAND_DIR, OPT_NETFLOW, OPT_MGMT_ID, OPT_PORTS, @@ -295,8 +284,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) {"no-stp", no_argument, 0, OPT_NO_STP}, {"out-of-band", no_argument, 0, OPT_OUT_OF_BAND}, {"in-band", no_argument, 0, OPT_IN_BAND}, - {"command-acl", required_argument, 0, OPT_COMMAND_ACL}, - {"command-dir", required_argument, 0, OPT_COMMAND_DIR}, {"netflow", required_argument, 0, OPT_NETFLOW}, {"mgmt-id", required_argument, 0, OPT_MGMT_ID}, {"ports", required_argument, 0, OPT_PORTS}, @@ -332,8 +319,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) s->accept_controller_re = NULL; s->enable_stp = false; s->in_band = true; - s->command_acl = ""; - s->command_dir = NULL; svec_init(&s->netflow); s->mgmt_id = 0; svec_init(&s->ports); @@ -449,16 +434,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) s->in_band = true; break; - case OPT_COMMAND_ACL: - s->command_acl = (s->command_acl[0] - ? xasprintf("%s,%s", s->command_acl, optarg) - : optarg); - break; - - case OPT_COMMAND_DIR: - s->command_dir = optarg; - break; - case OPT_NETFLOW: svec_add(&s->netflow, optarg); break; @@ -584,10 +559,7 @@ usage(void) " --netflow=HOST:PORT configure NetFlow output target\n" "\nRate-limiting of \"packet-in\" messages to the controller:\n" " --rate-limit[=PACKETS] max rate, in packets/s (default: 1000)\n" - " --burst-limit=BURST limit on packet credit for idle time\n" - "\nRemote command execution options:\n" - " --command-acl=[!]GLOB[,[!]GLOB...] set allowed/denied commands\n" - " --command-dir=DIR set command dir (default: %s/commands)\n", + " --burst-limit=BURST limit on packet credit for idle time\n", ovs_pkgdatadir); daemon_usage(); vlog_usage(); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 040c7f38..4cf1b86f 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -1456,8 +1456,6 @@ bridge_reconfigure_controller(const struct ovsrec_open_vswitch *ovs_cfg, rate_limit = c->controller_rate_limit ? *c->controller_rate_limit : 0; burst_limit = c->controller_burst_limit ? *c->controller_burst_limit : 0; ofproto_set_rate_limit(br->ofproto, rate_limit, burst_limit); - - ofproto_set_remote_execution(br->ofproto, NULL, NULL); /* XXX */ } else { union ofp_action action; flow_t flow;