[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] Improve handling of controller commands
commit d1f5957c4ea82d1622233c0dabd1e761df5200d4
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Mon Apr 1 17:27:08 2019 -0400
Improve handling of controller commands
Use a table-based lookup to find the right command handler. This
will serve as the basement for several future improvements, as we
improve the API for parsing commands.
---
src/feature/control/control_cmd.c | 298 +++++++++++++++++++++++++-------------
1 file changed, 198 insertions(+), 100 deletions(-)
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 95cf0d561..4fcfa7310 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -82,7 +82,7 @@ handle_control_resetconf(control_connection_t *conn, uint32_t len, char *body)
* reply with a CONFVALUE or an ERROR message */
static int
handle_control_getconf(control_connection_t *conn, uint32_t body_len,
- const char *body)
+ char *body)
{
smartlist_t *questions = smartlist_new();
smartlist_t *answers = smartlist_new();
@@ -2206,110 +2206,208 @@ handle_control_del_onion(control_connection_t *conn,
return 0;
}
+/**
+ * Called when we get an obsolete command: tell the controller that it is
+ * obsolete.
+ */
+static int
+handle_control_obsolete(control_connection_t *conn,
+ uint32_t arg_len,
+ const char *args)
+{
+ (void)arg_len;
+ (void)args;
+ char *command = tor_strdup(conn->incoming_cmd);
+ tor_strupper(command);
+ connection_printf_to_buf(conn, "511 %s is obsolete.\r\n", command);
+ tor_free(command);
+ return 0;
+}
+
+/**
+ * Selects an API to a controller command. See handler_fn_t for the
+ * possible types.
+ **/
+typedef enum handler_type_t {
+ hnd_legacy,
+ hnd_legacy_mut
+} handler_type_t;
+
+/**
+ * Union: a function pointer to a handler function for a controller command.
+ *
+ * This needs to be a union (rather than just a single pointer) since not
+ * all controller commands have the same type.
+ **/
+typedef union handler_fn_t {
+ /**
+ * A "legacy" handler takes a command's arguments as a nul-terminated
+ * string, and their length. It may not change the contents of the
+ * arguments. If the command is a multiline one, then the arguments may
+ * extend across multiple lines.
+ */
+ int (*legacy)(control_connection_t *conn,
+ uint32_t arg_len,
+ const char *args);
+ /**
+ * A "legacy_mut" handler is the same as a "legacy" one, except that it may
+ * change the contents of the command's arguments -- for example, by
+ * inserting NULs. It may not deallocate them.
+ */
+ int (*legacy_mut)(control_connection_t *conn,
+ uint32_t arg_len,
+ char *args);
+} handler_fn_t;
+
+/**
+ * Definition for a controller command.
+ */
+typedef struct control_cmd_def_t {
+ /**
+ * The name of the command. If the command is multiline, the name must
+ * begin with "+". This is not case-sensitive. */
+ const char *name;
+ /**
+ * Which API to use when calling the handler function.
+ */
+ handler_type_t handler_type;
+ /**
+ * A function to execute the command.
+ */
+ handler_fn_t handler;
+ /**
+ * Zero or more CMD_FL_* flags, or'd together.
+ */
+ unsigned flags;
+} control_cmd_def_t;
+
+/**
+ * Indicates that the command's arguments are sensitive, and should be
+ * memwiped after use.
+ */
+#define CMD_FL_WIPE (1u<<0)
+
+/**
+ * Macro: declare a command with a one-line argument and a given set of
+ * flags.
+ **/
+#define ONE_LINE(name, htype, flags) \
+ { #name, \
+ hnd_ ##htype, \
+ { .htype = handle_control_ ##name }, \
+ flags \
+ }
+/**
+ * Macro: declare a command with a multi-line argument and a given set of
+ * flags.
+ **/
+#define MULTLINE(name, htype, flags) \
+ { "+"#name, \
+ hnd_ ##htype, \
+ { .htype = handle_control_ ##name }, \
+ flags \
+ }
+/**
+ * Macro: declare an obsolete command. (Obsolete commands give a different
+ * error than non-existent ones.)
+ **/
+#define OBSOLETE(name) \
+ { #name, \
+ hnd_legacy, \
+ { .legacy = handle_control_obsolete }, \
+ 0 \
+ }
+
+/**
+ * An array defining all the recognized controller commands.
+ **/
+static const control_cmd_def_t CONTROL_COMMANDS[] =
+{
+ ONE_LINE(setconf, legacy_mut, 0),
+ ONE_LINE(resetconf, legacy_mut, 0),
+ ONE_LINE(getconf, legacy_mut, 0),
+ MULTLINE(loadconf, legacy, 0),
+ ONE_LINE(setevents, legacy, 0),
+ ONE_LINE(authenticate, legacy, 0),
+ ONE_LINE(saveconf, legacy, 0),
+ ONE_LINE(signal, legacy, 0),
+ ONE_LINE(takeownership, legacy, 0),
+ ONE_LINE(dropownership, legacy, 0),
+ ONE_LINE(mapaddress, legacy, 0),
+ ONE_LINE(getinfo, legacy, 0),
+ ONE_LINE(extendcircuit, legacy, 0),
+ ONE_LINE(setcircuitpurpose, legacy, 0),
+ OBSOLETE(setrouterpurpose),
+ ONE_LINE(attachstream, legacy, 0),
+ MULTLINE(postdescriptor, legacy, 0),
+ ONE_LINE(redirectstream, legacy, 0),
+ ONE_LINE(closestream, legacy, 0),
+ ONE_LINE(closecircuit, legacy, 0),
+ ONE_LINE(usefeature, legacy, 0),
+ ONE_LINE(resolve, legacy, 0),
+ ONE_LINE(protocolinfo, legacy, 0),
+ ONE_LINE(authchallenge, legacy, 0),
+ ONE_LINE(dropguards, legacy, 0),
+ ONE_LINE(hsfetch, legacy, 0),
+ MULTLINE(hspost, legacy, 0),
+ ONE_LINE(add_onion, legacy, CMD_FL_WIPE),
+ ONE_LINE(del_onion, legacy, CMD_FL_WIPE),
+};
+
+/**
+ * The number of entries in CONTROL_COMMANDS.
+ **/
+static const size_t N_CONTROL_COMMANDS = ARRAY_LENGTH(CONTROL_COMMANDS);
+
+/**
+ * Run a single control command, as defined by a control_cmd_def_t,
+ * with a given set of arguments.
+ */
+static int
+handle_single_control_command(const control_cmd_def_t *def,
+ control_connection_t *conn,
+ uint32_t cmd_data_len,
+ char *args)
+{
+ int rv = 0;
+ switch (def->handler_type) {
+ case hnd_legacy:
+ if (def->handler.legacy(conn, cmd_data_len, args))
+ rv = -1;
+ break;
+ case hnd_legacy_mut:
+ if (def->handler.legacy_mut(conn, cmd_data_len, args))
+ rv = -1;
+ break;
+ default:
+ tor_assert_unreached();
+ }
+
+ if (def->flags & CMD_FL_WIPE)
+ memwipe(args, 0, cmd_data_len);
+
+ return rv;
+}
+
+/**
+ * Run a given controller command, as selected by the incoming_cmd field of
+ * <b>conn</b>.
+ */
int
handle_control_command(control_connection_t *conn,
- uint32_t cmd_data_len,
- char *args)
+ uint32_t cmd_data_len,
+ char *args)
{
- /* XXXX Why is this not implemented as a table like the GETINFO
- * items are? Even handling the plus signs at the beginnings of
- * commands wouldn't be very hard with proper macros. */
-
- if (!strcasecmp(conn->incoming_cmd, "SETCONF")) {
- if (handle_control_setconf(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "RESETCONF")) {
- if (handle_control_resetconf(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "GETCONF")) {
- if (handle_control_getconf(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "+LOADCONF")) {
- if (handle_control_loadconf(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "SETEVENTS")) {
- if (handle_control_setevents(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "AUTHENTICATE")) {
- if (handle_control_authenticate(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "SAVECONF")) {
- if (handle_control_saveconf(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "SIGNAL")) {
- if (handle_control_signal(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "TAKEOWNERSHIP")) {
- if (handle_control_takeownership(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "DROPOWNERSHIP")) {
- if (handle_control_dropownership(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "MAPADDRESS")) {
- if (handle_control_mapaddress(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "GETINFO")) {
- if (handle_control_getinfo(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "EXTENDCIRCUIT")) {
- if (handle_control_extendcircuit(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "SETCIRCUITPURPOSE")) {
- if (handle_control_setcircuitpurpose(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "SETROUTERPURPOSE")) {
- connection_write_str_to_buf("511 SETROUTERPURPOSE is obsolete.\r\n", conn);
- } else if (!strcasecmp(conn->incoming_cmd, "ATTACHSTREAM")) {
- if (handle_control_attachstream(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "+POSTDESCRIPTOR")) {
- if (handle_control_postdescriptor(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "REDIRECTSTREAM")) {
- if (handle_control_redirectstream(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "CLOSESTREAM")) {
- if (handle_control_closestream(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "CLOSECIRCUIT")) {
- if (handle_control_closecircuit(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "USEFEATURE")) {
- if (handle_control_usefeature(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "RESOLVE")) {
- if (handle_control_resolve(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "PROTOCOLINFO")) {
- if (handle_control_protocolinfo(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "AUTHCHALLENGE")) {
- if (handle_control_authchallenge(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "DROPGUARDS")) {
- if (handle_control_dropguards(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "HSFETCH")) {
- if (handle_control_hsfetch(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "+HSPOST")) {
- if (handle_control_hspost(conn, cmd_data_len, args))
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "ADD_ONION")) {
- int ret = handle_control_add_onion(conn, cmd_data_len, args);
- memwipe(args, 0, cmd_data_len); /* Scrub the private key. */
- if (ret)
- return -1;
- } else if (!strcasecmp(conn->incoming_cmd, "DEL_ONION")) {
- int ret = handle_control_del_onion(conn, cmd_data_len, args);
- memwipe(args, 0, cmd_data_len); /* Scrub the service id/pk. */
- if (ret)
- return -1;
- } else {
- connection_printf_to_buf(conn, "510 Unrecognized command \"%s\"\r\n",
- conn->incoming_cmd);
+ for (unsigned i = 0; i < N_CONTROL_COMMANDS; ++i) {
+ const control_cmd_def_t *def = &CONTROL_COMMANDS[i];
+ if (!strcasecmp(conn->incoming_cmd, def->name)) {
+ return handle_single_control_command(def, conn, cmd_data_len, args);
+ }
}
+ connection_printf_to_buf(conn, "510 Unrecognized command \"%s\"\r\n",
+ conn->incoming_cmd);
+
return 0;
}
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits