[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Update more controller commands, now that we have kvline support



commit d8b3ec865de2738144bd6bbf9f6355662e64eb25
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Mon Apr 8 10:23:21 2019 -0400

    Update more controller commands, now that we have kvline support
---
 src/feature/control/control_cmd.c | 549 +++++++++++++++++---------------------
 1 file changed, 242 insertions(+), 307 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 4f67e3163..184855535 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -184,6 +184,17 @@ control_cmd_parse_args(const char *command,
   return result;
 }
 
+/**
+ * Return true iff <b>lines</b> contains <b>flags</b> as a no-value
+ * (keyword-only) entry.
+ **/
+static bool
+config_lines_contain_flag(const config_line_t *lines, const char *flag)
+{
+  const config_line_t *line = config_line_find_case(lines, flag);
+  return line && !strcmp(line->value, "");
+}
+
 /** Called when we receive a SETCONF message: parse the body and try
  * to update our configuration.  Reply with a DONE or ERROR message.
  * Modifies the contents of body.*/
@@ -370,15 +381,19 @@ handle_control_setevents(control_connection_t *conn,
   return 0;
 }
 
+static const control_cmd_syntax_t saveconf_syntax = {
+  .max_args = 0,
+  .accept_keywords = true,
+  .kvline_flags=KV_OMIT_VALS,
+};
+
 /** Called when we get a SAVECONF command. Try to flush the current options to
  * disk, and report success or failure. */
 static int
-handle_control_saveconf(control_connection_t *conn, uint32_t len,
-                        const char *body)
+handle_control_saveconf(control_connection_t *conn,
+                        const control_cmd_args_t *args)
 {
-  (void) len;
-
-  int force = !strcmpstart(body, "FORCE");
+  bool force = config_lines_contain_flag(args->kwargs, "FORCE");
   const or_options_t *options = get_options();
   if ((!force && options->IncludeUsed) || options_save_current() < 0) {
     connection_write_str_to_buf(
@@ -620,30 +635,27 @@ address_is_invalid_mapaddress_target(const char *addr)
     return address_is_invalid_destination(addr, 1);
 }
 
+static const control_cmd_syntax_t mapaddress_syntax = {
+  .max_args=0,
+  .accept_keywords=true,
+};
+
 /** Called when we get a MAPADDRESS command; try to bind all listed addresses,
  * and report success or failure. */
 static int
-handle_control_mapaddress(control_connection_t *conn, uint32_t len,
-                          const char *body)
+handle_control_mapaddress(control_connection_t *conn,
+                          const control_cmd_args_t *args)
 {
-  smartlist_t *elts;
-  smartlist_t *lines;
   smartlist_t *reply;
   char *r;
   size_t sz;
-  (void) len; /* body is NUL-terminated, so it's safe to ignore the length. */
 
-  lines = smartlist_new();
-  elts = smartlist_new();
   reply = smartlist_new();
-  smartlist_split_string(lines, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH_BEGIN(lines, char *, line) {
-    tor_strlower(line);
-    smartlist_split_string(elts, line, "=", 0, 2);
-    if (smartlist_len(elts) == 2) {
-      const char *from = smartlist_get(elts,0);
-      const char *to = smartlist_get(elts,1);
+  const config_line_t *line;
+  for (line = args->kwargs; line; line = line->next) {
+    const char *from = line->key;
+    const char *to = line->value;
+    {
       if (address_is_invalid_mapaddress_target(to)) {
         smartlist_add_asprintf(reply,
                      "512-syntax error: invalid address '%s'", to);
@@ -658,10 +670,10 @@ handle_control_mapaddress(control_connection_t *conn, uint32_t len,
                                                      type, tor_strdup(to));
         if (!address) {
           smartlist_add_asprintf(reply,
-                       "451-resource exhausted: skipping '%s'", line);
+                   "451-resource exhausted: skipping '%s=%s'", from,to);
           log_warn(LD_CONTROL,
                    "Unable to allocate address for '%s' in MapAddress msg",
-                   safe_str_client(line));
+                   safe_str_client(to));
         } else {
           smartlist_add_asprintf(reply, "250-%s=%s", address, to);
         }
@@ -671,27 +683,16 @@ handle_control_mapaddress(control_connection_t *conn, uint32_t len,
                                      ADDRMAPSRC_CONTROLLER, &msg) < 0) {
           smartlist_add_asprintf(reply,
                                  "512-syntax error: invalid address mapping "
-                                 " '%s': %s", line, msg);
+                                 " '%s=%s': %s", from, to, msg);
           log_warn(LD_CONTROL,
-                   "Skipping invalid argument '%s' in MapAddress msg: %s",
-                   line, msg);
+                   "Skipping invalid argument '%s=%s' in MapAddress msg: %s",
+                   from, to, msg);
         } else {
-          smartlist_add_asprintf(reply, "250-%s", line);
+          smartlist_add_asprintf(reply, "250-%s=%s", from, to);
         }
       }
-    } else {
-      smartlist_add_asprintf(reply, "512-syntax error: mapping '%s' is "
-                   "not of expected form 'foo=bar'.", line);
-      log_info(LD_CONTROL, "Skipping MapAddress '%s': wrong "
-                           "number of items.",
-                           safe_str_client(line));
     }
-    SMARTLIST_FOREACH(elts, char *, cp, tor_free(cp));
-    smartlist_clear(elts);
-  } SMARTLIST_FOREACH_END(line);
-  SMARTLIST_FOREACH(lines, char *, cp, tor_free(cp));
-  smartlist_free(lines);
-  smartlist_free(elts);
+  }
 
   if (smartlist_len(reply)) {
     ((char*)smartlist_get(reply,smartlist_len(reply)-1))[3] = ' ';
@@ -931,36 +932,36 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t setcircuitpurpose_syntax = {
+  .max_args=1,
+  .accept_keywords=true,
+};
+
 /** Called when we get a SETCIRCUITPURPOSE message. If we can find the
  * circuit and it's a valid purpose, change it. */
 static int
 handle_control_setcircuitpurpose(control_connection_t *conn,
-                                 uint32_t len, const char *body)
+                                 const control_cmd_args_t *args)
 {
   origin_circuit_t *circ = NULL;
   uint8_t new_purpose;
-  smartlist_t *args;
-  (void) len; /* body is NUL-terminated, so it's safe to ignore the length. */
+  const char *circ_id = smartlist_get(args->args,0);
 
-  args = getargs_helper("SETCIRCUITPURPOSE", conn, body, 2, -1);
-  if (!args)
-    goto done;
-
-  if (!(circ = get_circ(smartlist_get(args,0)))) {
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
+  if (!(circ = get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     goto done;
   }
 
   {
-    const char *purp = find_element_starting_with(args,1,"PURPOSE=");
+    const config_line_t *purp = config_line_find_case(args->kwargs, "PURPOSE");
     if (!purp) {
       connection_write_str_to_buf("552 No purpose given\r\n", conn);
       goto done;
     }
-    new_purpose = circuit_purpose_from_string(purp);
+    new_purpose = circuit_purpose_from_string(purp->value);
     if (new_purpose == CIRCUIT_PURPOSE_UNKNOWN) {
-      connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", purp);
+      connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
+                               purp->value);
       goto done;
     }
   }
@@ -969,54 +970,50 @@ handle_control_setcircuitpurpose(control_connection_t *conn,
   connection_write_str_to_buf("250 OK\r\n", conn);
 
  done:
-  if (args) {
-    SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-    smartlist_free(args);
-  }
   return 0;
 }
 
+static const char *attachstream_keywords[] = {
+  "HOP", NULL
+};
+static const control_cmd_syntax_t attachstream_syntax = {
+  .min_args=2, .max_args=2,
+  .accept_keywords=true,
+  .allowed_keywords=attachstream_keywords
+};
+
 /** Called when we get an ATTACHSTREAM message.  Try to attach the requested
  * stream, and report success or failure. */
 static int
-handle_control_attachstream(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_attachstream(control_connection_t *conn,
+                            const control_cmd_args_t *args)
 {
   entry_connection_t *ap_conn = NULL;
   origin_circuit_t *circ = NULL;
-  int zero_circ;
-  smartlist_t *args;
   crypt_path_t *cpath=NULL;
   int hop=0, hop_line_ok=1;
-  (void) len;
+  const char *stream_id = smartlist_get(args->args, 0);
+  const char *circ_id = smartlist_get(args->args, 1);
+  int zero_circ = !strcmp(circ_id, "0");
+  const config_line_t *hoparg = config_line_find_case(args->kwargs, "HOP");
 
-  args = getargs_helper("ATTACHSTREAM", conn, body, 2, -1);
-  if (!args)
+  if (!(ap_conn = get_stream(stream_id))) {
+    connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n", stream_id);
+    return 0;
+  } else if (!zero_circ && !(circ = get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     return 0;
-
-  zero_circ = !strcmp("0", (char*)smartlist_get(args,1));
-
-  if (!(ap_conn = get_stream(smartlist_get(args, 0)))) {
-    connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
-  } else if (!zero_circ && !(circ = get_circ(smartlist_get(args, 1)))) {
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 1));
   } else if (circ) {
-    const char *hopstring = find_element_starting_with(args,2,"HOP=");
-    if (hopstring) {
-      hopstring += strlen("HOP=");
-      hop = (int) tor_parse_ulong(hopstring, 10, 0, INT_MAX,
+    if (hoparg) {
+      hop = (int) tor_parse_ulong(hoparg->value, 10, 0, INT_MAX,
                                   &hop_line_ok, NULL);
       if (!hop_line_ok) { /* broken hop line */
-        connection_printf_to_buf(conn, "552 Bad value hop=%s\r\n", hopstring);
+        connection_printf_to_buf(conn, "552 Bad value hop=%s\r\n",
+                                 hoparg->value);
+        return 0;
       }
     }
   }
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
-  if (!ap_conn || (!zero_circ && !circ) || !hop_line_ok)
-    return 0;
 
   if (ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONTROLLER_WAIT &&
       ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONNECT_WAIT &&
@@ -1071,59 +1068,49 @@ handle_control_attachstream(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const char *postdescriptor_keywords[] = {
+  "cache", "purpose", NULL,
+};
+
+static const control_cmd_syntax_t postdescriptor_syntax = {
+  .max_args = 0,
+  .accept_keywords = true,
+  .allowed_keywords = postdescriptor_keywords,
+  .want_object = true,
+};
+
 /** Called when we get a POSTDESCRIPTOR message.  Try to learn the provided
  * descriptor, and report success or failure. */
 static int
-handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
-                              const char *body)
+handle_control_postdescriptor(control_connection_t *conn,
+                              const control_cmd_args_t *args)
 {
-  char *desc;
   const char *msg=NULL;
   uint8_t purpose = ROUTER_PURPOSE_GENERAL;
   int cache = 0; /* eventually, we may switch this to 1 */
+  const config_line_t *line;
 
-  const char *cp = memchr(body, '\n', len);
-
-  if (cp == NULL) {
-    connection_printf_to_buf(conn, "251 Empty body\r\n");
-    return 0;
+  line = config_line_find_case(args->kwargs, "purpose");
+  if (line) {
+    purpose = router_purpose_from_string(line->value);
+    connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
+                             line->value);
+    goto done;
   }
-  ++cp;
-
-  char *cmdline = tor_memdup_nulterm(body, cp-body);
-  smartlist_t *args = smartlist_new();
-  smartlist_split_string(args, cmdline, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH_BEGIN(args, char *, option) {
-    if (!strcasecmpstart(option, "purpose=")) {
-      option += strlen("purpose=");
-      purpose = router_purpose_from_string(option);
-      if (purpose == ROUTER_PURPOSE_UNKNOWN) {
-        connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n",
-                                 option);
-        goto done;
-      }
-    } else if (!strcasecmpstart(option, "cache=")) {
-      option += strlen("cache=");
-      if (!strcasecmp(option, "no"))
-        cache = 0;
-      else if (!strcasecmp(option, "yes"))
-        cache = 1;
-      else {
-        connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n",
-                                 option);
-        goto done;
-      }
-    } else { /* unrecognized argument? */
-      connection_printf_to_buf(conn,
-        "512 Unexpected argument \"%s\" to postdescriptor\r\n", option);
+  line = config_line_find_case(args->kwargs, "cache");
+  if (line) {
+    if (!strcasecmp(line->value, "no"))
+      cache = 0;
+    else if (!strcasecmp(line->value, "yes"))
+      cache = 1;
+    else {
+      connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n",
+                               line->value);
       goto done;
     }
-  } SMARTLIST_FOREACH_END(option);
-
-  read_escaped_data(cp, len-(cp-body), &desc);
+  }
 
-  switch (router_load_single_router(desc, purpose, cache, &msg)) {
+  switch (router_load_single_router(args->object, purpose, cache, &msg)) {
   case -1:
     if (!msg) msg = "Could not parse descriptor";
     connection_printf_to_buf(conn, "554 %s\r\n", msg);
@@ -1137,11 +1124,7 @@ handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
     break;
   }
 
-  tor_free(desc);
  done:
-  SMARTLIST_FOREACH(args, char *, arg, tor_free(arg));
-  smartlist_free(args);
-  tor_free(cmdline);
   return 0;
 }
 
@@ -1230,38 +1213,29 @@ handle_control_closestream(control_connection_t *conn,
   return 0;
 }
 
+static const control_cmd_syntax_t closecircuit_syntax = {
+  .min_args=1, .max_args=1,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS,
+  // XXXX we might want to exclude unrecognized flags, but for now we
+  // XXXX just ignore them for backward compatibility.
+};
+
 /** Called when we get a CLOSECIRCUIT command; try to close the named circuit
  * and report success or failure. */
 static int
-handle_control_closecircuit(control_connection_t *conn, uint32_t len,
-                            const char *body)
+handle_control_closecircuit(control_connection_t *conn,
+                            const control_cmd_args_t *args)
 {
+  const char *circ_id = smartlist_get(args->args, 0);
   origin_circuit_t *circ = NULL;
-  int safe = 0;
-  smartlist_t *args;
-  (void) len;
 
-  args = getargs_helper("CLOSECIRCUIT", conn, body, 1, -1);
-  if (!args)
+  if (!(circ=get_circ(circ_id))) {
+    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id);
     return 0;
-
-  if (!(circ=get_circ(smartlist_get(args, 0))))
-    connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n",
-                             (char*)smartlist_get(args, 0));
-  else {
-    int i;
-    for (i=1; i < smartlist_len(args); ++i) {
-      if (!strcasecmp(smartlist_get(args, i), "IfUnused"))
-        safe = 1;
-      else
-        log_info(LD_CONTROL, "Skipping unknown option %s",
-                 (char*)smartlist_get(args,i));
-    }
   }
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
-  if (!circ)
-    return 0;
+
+  bool safe =  config_lines_contain_flag(args->kwargs, "IfUnused");
 
   if (!safe || !circ->p_streams) {
     circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_REQUESTED);
@@ -1271,36 +1245,43 @@ handle_control_closecircuit(control_connection_t *conn, uint32_t len,
   return 0;
 }
 
+static const control_cmd_syntax_t resolve_syntax = {
+  .max_args=0,
+  .accept_keywords=true,
+  .kvline_flags=KV_OMIT_VALS,
+};
+
 /** Called when we get a RESOLVE command: start trying to resolve
  * the listed addresses. */
 static int
-handle_control_resolve(control_connection_t *conn, uint32_t len,
-                       const char *body)
+handle_control_resolve(control_connection_t *conn,
+                       const control_cmd_args_t *args)
 {
-  smartlist_t *args, *failed;
+  smartlist_t *failed;
   int is_reverse = 0;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
 
   if (!(conn->event_mask & (((event_mask_t)1)<<EVENT_ADDRMAP))) {
     log_warn(LD_CONTROL, "Controller asked us to resolve an address, but "
              "isn't listening for ADDRMAP events.  It probably won't see "
              "the answer.");
   }
-  args = smartlist_new();
-  smartlist_split_string(args, body, " ",
-                         SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
+
   {
-    const char *modearg = find_element_starting_with(args, 0, "mode=");
-    if (modearg && !strcasecmp(modearg, "mode=reverse"))
+    const config_line_t *modearg = config_line_find_case(args->kwargs, "mode");
+    if (modearg && !strcasecmp(modearg->value, "reverse"))
       is_reverse = 1;
   }
   failed = smartlist_new();
-  SMARTLIST_FOREACH(args, const char *, arg, {
-      if (!is_keyval_pair(arg)) {
-          if (dnsserv_launch_request(arg, is_reverse, conn)<0)
-            smartlist_add(failed, (char*)arg);
-      }
-  });
+  for (const config_line_t *line = args->kwargs; line; line = line->next) {
+    if (!strlen(line->value)) {
+      const char *addr = line->key;
+      if (dnsserv_launch_request(addr, is_reverse, conn)<0)
+        smartlist_add(failed, (char*)addr);
+    } else {
+      // XXXX arguably we should reject unrecognized keyword arguments,
+      // XXXX but the old implementation didn't do that.
+    }
+  }
 
   send_control_done(conn);
   SMARTLIST_FOREACH(failed, const char *, arg, {
@@ -1308,8 +1289,6 @@ handle_control_resolve(control_connection_t *conn, uint32_t len,
                                    "internal", 0);
   });
 
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   smartlist_free(failed);
   return 0;
 }
@@ -1443,30 +1422,33 @@ handle_control_dropguards(control_connection_t *conn,
   return 0;
 }
 
+static const char *hsfetch_keywords[] = {
+  "SERVER", NULL,
+};
+static const control_cmd_syntax_t hsfetch_syntax = {
+  .min_args = 1, .max_args = 1,
+  .accept_keywords = true,
+  .allowed_keywords = hsfetch_keywords,
+  .want_object = true,
+};
+
 /** Implementation for the HSFETCH command. */
 static int
-handle_control_hsfetch(control_connection_t *conn, uint32_t len,
-                       const char *body)
+handle_control_hsfetch(control_connection_t *conn,
+                       const control_cmd_args_t *args)
+
 {
-  int i;
-  char digest[DIGEST_LEN], *hsaddress = NULL, *arg1 = NULL, *desc_id = NULL;
-  smartlist_t *args = NULL, *hsdirs = NULL;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  static const char *hsfetch_command = "HSFETCH";
+  char digest[DIGEST_LEN], *desc_id = NULL;
+  smartlist_t *hsdirs = NULL;
   static const char *v2_str = "v2-";
   const size_t v2_str_len = strlen(v2_str);
   rend_data_t *rend_query = NULL;
   ed25519_public_key_t v3_pk;
   uint32_t version;
-
-  /* Make sure we have at least one argument, the HSAddress. */
-  args = getargs_helper(hsfetch_command, conn, body, 1, -1);
-  if (!args) {
-    goto exit;
-  }
+  const char *hsaddress = NULL;
 
   /* Extract the first argument (either HSAddress or DescID). */
-  arg1 = smartlist_get(args, 0);
+  const char *arg1 = smartlist_get(args->args, 0);
   /* Test if it's an HS address without the .onion part. */
   if (rend_valid_v2_service_id(arg1)) {
     hsaddress = arg1;
@@ -1490,18 +1472,11 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
     goto done;
   }
 
-  static const char *opt_server = "SERVER=";
-
-  /* Skip first argument because it's the HSAddress or DescID. */
-  for (i = 1; i < smartlist_len(args); ++i) {
-    const char *arg = smartlist_get(args, i);
-    const node_t *node;
+  for (const config_line_t *line = args->kwargs; line; line = line->next) {
+    if (!strcasecmp(line->key, "SERVER")) {
+      const char *server = line->value;
 
-    if (!strcasecmpstart(arg, opt_server)) {
-      const char *server;
-
-      server = arg + strlen(opt_server);
-      node = node_get_by_hex_id(server, 0);
+      const node_t *node = node_get_by_hex_id(server, 0);
       if (!node) {
         connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
                                  server);
@@ -1514,9 +1489,7 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
       /* Valid server, add it to our local list. */
       smartlist_add(hsdirs, node->rs);
     } else {
-      connection_printf_to_buf(conn, "513 Unexpected argument \"%s\"\r\n",
-                               arg);
-      goto done;
+      tor_assert_nonfatal_unreached();
     }
   }
 
@@ -1532,9 +1505,8 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   /* Using a descriptor ID, we force the user to provide at least one
    * hsdir server using the SERVER= option. */
   if (desc_id && (!hsdirs || !smartlist_len(hsdirs))) {
-      connection_printf_to_buf(conn, "512 %s option is required\r\n",
-                               opt_server);
-      goto done;
+    connection_printf_to_buf(conn, "512 SERVER option is required\r\n");
+    goto done;
   }
 
   /* We are about to trigger HSDir fetch so send the OK now because after
@@ -1552,96 +1524,75 @@ handle_control_hsfetch(control_connection_t *conn, uint32_t len,
   }
 
  done:
-  SMARTLIST_FOREACH(args, char *, cp, tor_free(cp));
-  smartlist_free(args);
   /* Contains data pointer that we don't own thus no cleanup. */
   smartlist_free(hsdirs);
   rend_data_free(rend_query);
- exit:
   return 0;
 }
 
+static const char *hspost_keywords[] = {
+  "SERVER", "HSADDRESS", NULL
+};
+static const control_cmd_syntax_t hspost_syntax = {
+  .min_args = 0, .max_args = 0,
+  .accept_keywords = true,
+  .want_object = true,
+  .allowed_keywords = hspost_keywords
+};
+
 /** Implementation for the HSPOST command. */
 static int
 handle_control_hspost(control_connection_t *conn,
-                      uint32_t len,
-                      const char *body)
+                      const control_cmd_args_t *args)
 {
-  static const char *opt_server = "SERVER=";
-  static const char *opt_hsaddress = "HSADDRESS=";
   smartlist_t *hs_dirs = NULL;
-  const char *encoded_desc = body;
-  size_t encoded_desc_len = len;
+  const char *encoded_desc = args->object;
+  size_t encoded_desc_len = args->object_len;
   const char *onion_address = NULL;
+  const config_line_t *line;
 
-  char *cp = memchr(body, '\n', len);
-  if (cp == NULL) {
-    connection_printf_to_buf(conn, "251 Empty body\r\n");
-    return 0;
-  }
-  char *argline = tor_strndup(body, cp-body);
-
-  smartlist_t *args = smartlist_new();
+  for (line = args->kwargs; line; line = line->next) {
+    if (!strcasecmpstart(line->key, "SERVER")) {
+      const char *server = line->value;
+      const node_t *node = node_get_by_hex_id(server, 0);
 
-  /* If any SERVER= or HSADDRESS= options were specified, try to parse
-   * the options line. */
-  if (!strcasecmpstart(argline, opt_server) ||
-      !strcasecmpstart(argline, opt_hsaddress)) {
-    /* encoded_desc begins after a newline character */
-    cp = cp + 1;
-    encoded_desc = cp;
-    encoded_desc_len = len-(cp-body);
-
-    smartlist_split_string(args, argline, " ",
-                           SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-    SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {
-      if (!strcasecmpstart(arg, opt_server)) {
-        const char *server = arg + strlen(opt_server);
-        const node_t *node = node_get_by_hex_id(server, 0);
-
-        if (!node || !node->rs) {
-          connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
-                                   server);
-          goto done;
-        }
-        /* Valid server, add it to our local list. */
-        if (!hs_dirs)
-          hs_dirs = smartlist_new();
-        smartlist_add(hs_dirs, node->rs);
-      } else if (!strcasecmpstart(arg, opt_hsaddress)) {
-        const char *address = arg + strlen(opt_hsaddress);
-        if (!hs_address_is_valid(address)) {
-          connection_printf_to_buf(conn, "512 Malformed onion address\r\n");
-          goto done;
-        }
-        onion_address = address;
-      } else {
-        connection_printf_to_buf(conn, "512 Unexpected argument \"%s\"\r\n",
-                                 arg);
+      if (!node || !node->rs) {
+        connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n",
+                                 server);
         goto done;
       }
-    } SMARTLIST_FOREACH_END(arg);
+      /* Valid server, add it to our local list. */
+      if (!hs_dirs)
+        hs_dirs = smartlist_new();
+      smartlist_add(hs_dirs, node->rs);
+    } else if (!strcasecmpstart(line->key, "HSADDRESS")) {
+      const char *address = line->value;
+      if (!hs_address_is_valid(address)) {
+        connection_printf_to_buf(conn, "512 Malformed onion address\r\n");
+        goto done;
+      }
+      onion_address = address;
+    } else {
+      tor_assert_nonfatal_unreached();
+    }
   }
 
   /* Handle the v3 case. */
   if (onion_address) {
-    char *desc_str = NULL;
-    read_escaped_data(encoded_desc, encoded_desc_len, &desc_str);
-    if (hs_control_hspost_command(desc_str, onion_address, hs_dirs) < 0) {
+    if (hs_control_hspost_command(encoded_desc, onion_address, hs_dirs) < 0) {
       connection_printf_to_buf(conn, "554 Invalid descriptor\r\n");
     } else {
       send_control_done(conn);
     }
-    tor_free(desc_str);
     goto done;
   }
 
   /* From this point on, it is only v2. */
 
-  /* Read the dot encoded descriptor, and parse it. */
+  /*  parse it. */
   rend_encoded_v2_service_descriptor_t *desc =
       tor_malloc_zero(sizeof(rend_encoded_v2_service_descriptor_t));
-  read_escaped_data(encoded_desc, encoded_desc_len, &desc->desc_str);
+  desc->desc_str = tor_memdup_nulterm(encoded_desc, encoded_desc_len);
 
   rend_service_descriptor_t *parsed = NULL;
   char *intro_content = NULL;
@@ -1675,10 +1626,7 @@ handle_control_hspost(control_connection_t *conn,
   tor_free(intro_content);
   rend_encoded_v2_service_descriptor_free(desc);
  done:
-  tor_free(argline);
   smartlist_free(hs_dirs); /* Contents belong to the rend service code. */
-  SMARTLIST_FOREACH(args, char *, arg, tor_free(arg));
-  smartlist_free(args);
   return 0;
 }
 
@@ -1742,21 +1690,21 @@ get_detached_onion_services(void)
   return detached_onion_services;
 }
 
+static const char *add_onion_keywords[] = {
+   "Port", "Flags", "MaxStreams", "ClientAuth", NULL
+};
+static const control_cmd_syntax_t add_onion_syntax = {
+  .min_args = 1, .max_args = 1,
+  .accept_keywords = true,
+  .allowed_keywords = add_onion_keywords
+};
+
 /** Called when we get a ADD_ONION command; parse the body, and set up
  * the new ephemeral Onion Service. */
 static int
 handle_control_add_onion(control_connection_t *conn,
-                         uint32_t len,
-                         const char *body)
+                         const control_cmd_args_t *args)
 {
-  smartlist_t *args;
-  int arg_len;
-  (void) len; /* body is nul-terminated; it's safe to ignore the length */
-  args = getargs_helper("ADD_ONION", conn, body, 2, -1);
-  if (!args)
-    return 0;
-  arg_len = smartlist_len(args);
-
   /* Parse all of the arguments that do not involve handling cryptographic
    * material first, since there's no reason to touch that at all if any of
    * the other arguments are malformed.
@@ -1769,36 +1717,28 @@ handle_control_add_onion(control_connection_t *conn,
   int max_streams = 0;
   int max_streams_close_circuit = 0;
   rend_auth_type_t auth_type = REND_NO_AUTH;
-  /* Default to adding an anonymous hidden service if no flag is given */
   int non_anonymous = 0;
-  for (int i = 1; i < arg_len; i++) {
-    static const char *port_prefix = "Port=";
-    static const char *flags_prefix = "Flags=";
-    static const char *max_s_prefix = "MaxStreams=";
-    static const char *auth_prefix = "ClientAuth=";
-
-    const char *arg = smartlist_get(args, (int)i);
-    if (!strcasecmpstart(arg, port_prefix)) {
-      /* "Port=VIRTPORT[,TARGET]". */
-      const char *port_str = arg + strlen(port_prefix);
+  const config_line_t *arg;
 
+  for (arg = args->kwargs; arg; arg = arg->next) {
+    if (!strcasecmp(arg->key, "Port")) {
+      /* "Port=VIRTPORT[,TARGET]". */
       rend_service_port_config_t *cfg =
-          rend_service_parse_port_config(port_str, ",", NULL);
+          rend_service_parse_port_config(arg->value, ",", NULL);
       if (!cfg) {
         connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n");
         goto out;
       }
       smartlist_add(port_cfgs, cfg);
-    } else if (!strcasecmpstart(arg, max_s_prefix)) {
+    } else if (!strcasecmp(arg->key, "MaxStreams")) {
       /* "MaxStreams=[0..65535]". */
-      const char *max_s_str = arg + strlen(max_s_prefix);
       int ok = 0;
-      max_streams = (int)tor_parse_long(max_s_str, 10, 0, 65535, &ok, NULL);
+      max_streams = (int)tor_parse_long(arg->value, 10, 0, 65535, &ok, NULL);
       if (!ok) {
         connection_printf_to_buf(conn, "512 Invalid MaxStreams\r\n");
         goto out;
       }
-    } else if (!strcasecmpstart(arg, flags_prefix)) {
+    } else if (!strcasecmp(arg->key, "Flags")) {
       /* "Flags=Flag[,Flag]", where Flag can be:
        *   * 'DiscardPK' - If tor generates the keypair, do not include it in
        *                   the response.
@@ -1821,8 +1761,7 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_t *flags = smartlist_new();
       int bad = 0;
 
-      smartlist_split_string(flags, arg + strlen(flags_prefix), ",",
-                             SPLIT_IGNORE_BLANK, 0);
+      smartlist_split_string(flags, arg->value, ",", SPLIT_IGNORE_BLANK, 0);
       if (smartlist_len(flags) < 1) {
         connection_printf_to_buf(conn, "512 Invalid 'Flags' argument\r\n");
         bad = 1;
@@ -1851,11 +1790,12 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_free(flags);
       if (bad)
         goto out;
-    } else if (!strcasecmpstart(arg, auth_prefix)) {
+
+    } else if (!strcasecmp(arg->key, "ClientAuth")) {
       char *err_msg = NULL;
       int created = 0;
       rend_authorized_client_t *client =
-        add_onion_helper_clientauth(arg + strlen(auth_prefix),
+        add_onion_helper_clientauth(arg->value,
                                     &created, &err_msg);
       if (!client) {
         if (err_msg) {
@@ -1888,7 +1828,7 @@ handle_control_add_onion(control_connection_t *conn,
         smartlist_add(auth_created_clients, client);
       }
     } else {
-      connection_printf_to_buf(conn, "513 Invalid argument\r\n");
+      tor_assert_nonfatal_unreached();
       goto out;
     }
   }
@@ -1929,7 +1869,8 @@ handle_control_add_onion(control_connection_t *conn,
   char *key_new_blob = NULL;
   char *err_msg = NULL;
 
-  if (add_onion_helper_keyarg(smartlist_get(args, 0), discard_pk,
+  const char *onionkey = smartlist_get(args->args, 0);
+  if (add_onion_helper_keyarg(onionkey, discard_pk,
                               &key_new_alg, &key_new_blob, &pk, &hs_version,
                               &err_msg) < 0) {
     if (err_msg) {
@@ -2031,12 +1972,6 @@ handle_control_add_onion(control_connection_t *conn,
     // Do not free entries; they are the same as auth_clients
     smartlist_free(auth_created_clients);
   }
-
-  SMARTLIST_FOREACH(args, char *, cp, {
-    memwipe(cp, 0, strlen(cp));
-    tor_free(cp);
-  });
-  smartlist_free(args);
   return 0;
 }
 
@@ -2471,28 +2406,28 @@ static const control_cmd_def_t CONTROL_COMMANDS[] =
   MULTLINE_PARSED(loadconf, 0),
   ONE_LINE_PARSED(setevents, 0),
   ONE_LINE(authenticate, legacy, CMD_FL_WIPE),
-  ONE_LINE(saveconf, legacy, 0),
+  ONE_LINE_PARSED(saveconf, 0),
   ONE_LINE_PARSED(signal, 0),
   ONE_LINE_PARSED(takeownership, 0),
   ONE_LINE_PARSED(dropownership, 0),
-  ONE_LINE(mapaddress, legacy, 0),
+  ONE_LINE_PARSED(mapaddress, 0),
   ONE_LINE_PARSED(getinfo, 0),
   ONE_LINE(extendcircuit, legacy, 0),
-  ONE_LINE(setcircuitpurpose, legacy, 0),
+  ONE_LINE_PARSED(setcircuitpurpose, 0),
   OBSOLETE(setrouterpurpose),
-  ONE_LINE(attachstream, legacy, 0),
-  MULTLINE(postdescriptor, legacy, 0),
+  ONE_LINE_PARSED(attachstream, 0),
+  MULTLINE_PARSED(postdescriptor, 0),
   ONE_LINE_PARSED(redirectstream, 0),
   ONE_LINE_PARSED(closestream, 0),
-  ONE_LINE(closecircuit, legacy, 0),
+  ONE_LINE_PARSED(closecircuit, 0),
   ONE_LINE_PARSED(usefeature, 0),
-  ONE_LINE(resolve, legacy, 0),
+  ONE_LINE_PARSED(resolve, 0),
   ONE_LINE_PARSED(protocolinfo, 0),
   ONE_LINE(authchallenge, legacy, CMD_FL_WIPE),
   ONE_LINE_PARSED(dropguards, 0),
-  ONE_LINE(hsfetch, legacy, 0),
-  MULTLINE(hspost, legacy, 0),
-  ONE_LINE(add_onion, legacy, CMD_FL_WIPE),
+  ONE_LINE_PARSED(hsfetch, 0),
+  MULTLINE_PARSED(hspost, 0),
+  ONE_LINE_PARSED(add_onion, CMD_FL_WIPE),
   ONE_LINE_PARSED(del_onion, CMD_FL_WIPE),
 };
 



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits