Merge branch 'feature6411_v4'

13 files changed, 980 insertions, 45 deletions

diff --git a/changes/feature6411 b/changes/feature6411
new file mode 100644
index 0000000000..6024364319
--- /dev/null
+++ b/changes/feature6411
@@ -0,0 +1,7 @@
+  o Major features (controller):
+    - Add the ADD_ONION and DEL_ONION commands that allows the creation
+      and management of hidden services via the controller. Closes
+      ticket 6411.
+    - New "GETINFO onions/current" and "GETINFO onions/detached" to get
+      information about hidden services created via the controller.
+      Part of ticket 6411.
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 7857a54951..8038631f18 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -1397,6 +1397,78 @@ crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out)
   return 0;
 }
 
+/** Given a crypto_pk_t <b>pk</b>, allocate a new buffer containing the
+ * Base64 encoding of the DER representation of the private key as a NUL
+ * terminated string, and return it via <b>priv_out</b>.  Return 0 on
+ * sucess, -1 on failure.
+ *
+ * It is the caller's responsibility to sanitize and free the resulting buffer.
+ */
+int
+crypto_pk_base64_encode(const crypto_pk_t *pk, char **priv_out)
+{
+  unsigned char *der = NULL;
+  int der_len;
+  int ret = -1;
+
+  *priv_out = NULL;
+
+  der_len = i2d_RSAPrivateKey(pk->key, &der);
+  if (der_len < 0 || der == NULL)
+    return ret;
+
+  size_t priv_len = base64_encode_size(der_len, 0) + 1;
+  char *priv = tor_malloc_zero(priv_len);
+  if (base64_encode(priv, priv_len, (char *)der, der_len, 0) >= 0) {
+    *priv_out = priv;
+    ret = 0;
+  } else {
+    tor_free(priv);
+  }
+
+  memwipe(der, 0, der_len);
+  OPENSSL_free(der);
+  return ret;
+}
+
+/** Given a string containing the Base64 encoded DER representation of the
+ * private key <b>str</b>, decode and return the result on success, or NULL
+ * on failure.
+ */
+crypto_pk_t *
+crypto_pk_base64_decode(const char *str, size_t len)
+{
+  crypto_pk_t *pk = NULL;
+
+  char *der = tor_malloc_zero(len + 1);
+  int der_len = base64_decode(der, len, str, len);
+  if (der_len <= 0) {
+    log_warn(LD_CRYPTO, "Stored RSA private key seems corrupted (base64).");
+    goto out;
+  }
+
+  const unsigned char *dp = (unsigned char*)der; /* Shut the compiler up. */
+  RSA *rsa = d2i_RSAPrivateKey(NULL, &dp, der_len);
+  if (!rsa) {
+    crypto_log_errors(LOG_WARN, "decoding private key");
+    goto out;
+  }
+
+  pk = crypto_new_pk_from_rsa_(rsa);
+
+  /* Make sure it's valid. */
+  if (crypto_pk_check_key(pk) <= 0) {
+    crypto_pk_free(pk);
+    pk = NULL;
+    goto out;
+  }
+
+ out:
+  memwipe(der, 0, len + 1);
+  tor_free(der);
+  return pk;
+}
+
 /* symmetric crypto */
 
 /** Return a pointer to the key set for the cipher in <b>env</b>.
diff --git a/src/common/crypto.h b/src/common/crypto.h
index 1ac02ea7a5..8b620d910c 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -184,6 +184,9 @@ int crypto_pk_get_all_digests(crypto_pk_t *pk, digests_t *digests_out);
 int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space);
 int crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out);
 
+int crypto_pk_base64_encode(const crypto_pk_t *pk, char **priv_out);
+crypto_pk_t *crypto_pk_base64_decode(const char *str, size_t len);
+
 /* symmetric crypto */
 const char *crypto_cipher_get_key(crypto_cipher_t *env);
 
diff --git a/src/or/connection.c b/src/or/connection.c
index 5610815589..24d47ccd18 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -586,6 +586,13 @@ connection_free_(connection_t *conn)
     control_connection_t *control_conn = TO_CONTROL_CONN(conn);
     tor_free(control_conn->safecookie_client_hash);
     tor_free(control_conn->incoming_cmd);
+    if (control_conn->ephemeral_onion_services) {
+      SMARTLIST_FOREACH(control_conn->ephemeral_onion_services, char *, cp, {
+        memwipe(cp, 0, strlen(cp));
+        tor_free(cp);
+      });
+      smartlist_free(control_conn->ephemeral_onion_services);
+    }
   }
 
   /* Probably already freed by connection_free. */
diff --git a/src/or/control.c b/src/or/control.c
index 8c95b056b2..7c8a6c4aa7 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -36,6 +36,8 @@
 #include "networkstatus.h"
 #include "nodelist.h"
 #include "policies.h"
+#include "rendcommon.h"
+#include "rendservice.h"
 #include "reasons.h"
 #include "rendclient.h"
 #include "rendcommon.h"
@@ -96,6 +98,11 @@ static uint8_t *authentication_cookie = NULL;
   "Tor safe cookie authentication controller-to-server hash"
 #define SAFECOOKIE_SERVER_NONCE_LEN DIGEST256_LEN
 
+/** The list of onion services that have been added via ADD_ONION that do not
+ * belong to any particular control connection.
+ */
+static smartlist_t *detached_onion_services = NULL;
+
 /** A sufficiently large size to record the last bootstrap phase string. */
 #define BOOTSTRAP_MSG_LEN 1024
 
@@ -163,6 +170,10 @@ static int handle_control_usefeature(control_connection_t *conn,
                                      const char *body);
 static int handle_control_hsfetch(control_connection_t *conn, uint32_t len,
                                   const char *body);
+static int handle_control_add_onion(control_connection_t *conn, uint32_t len,
+                                    const char *body);
+static int handle_control_del_onion(control_connection_t *conn, uint32_t len,
+                                    const char *body);
 static int write_stream_target_to_buf(entry_connection_t *conn, char *buf,
                                       size_t len);
 static void orconn_target_get_name(char *buf, size_t len,
@@ -2170,6 +2181,31 @@ getinfo_helper_events(control_connection_t *control_conn,
   return 0;
 }
 
+/** Implementation helper for GETINFO: knows how to enumerate hidden services
+ * created via the control port. */
+static int
+getinfo_helper_onions(control_connection_t *control_conn,
+                      const char *question, char **answer,
+                      const char **errmsg)
+{
+  smartlist_t *onion_list = NULL;
+
+  if (!strcmp(question, "onions/current")) {
+    onion_list = control_conn->ephemeral_onion_services;
+  } else if (!strcmp(question, "onions/detached")) {
+    onion_list = detached_onion_services;
+  } else {
+    return 0;
+  }
+  if (!onion_list || smartlist_len(onion_list) == 0) {
+    *errmsg = "No onion services of the specified type.";
+    return -1;
+  }
+  *answer = smartlist_join_strings(onion_list, "\r\n", 0, NULL);
+
+  return 0;
+}
+
 /** Callback function for GETINFO: on a given control connection, try to
  * answer the question <b>q</b> and store the newly-allocated answer in
  * *<b>a</b>. If an internal error occurs, return -1 and optionally set
@@ -2306,6 +2342,10 @@ static const getinfo_item_t getinfo_items[] = {
   ITEM("exit-policy/ipv4", policies, "IPv4 parts of exit policy"),
   ITEM("exit-policy/ipv6", policies, "IPv6 parts of exit policy"),
   PREFIX("ip-to-country/", geoip, "Perform a GEOIP lookup"),
+  ITEM("onions/current", onions,
+       "Onion services owned by the current control connection."),
+  ITEM("onions/detached", onions,
+       "Onion services detached from the control connection."),
   { NULL, NULL, NULL, 0 }
 };
 
@@ -3388,6 +3428,348 @@ exit:
   return 0;
 }
 
+/** 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)
+{
+  smartlist_t *args;
+  size_t 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.
+   */
+  smartlist_t *port_cfgs = smartlist_new();
+  int discard_pk = 0;
+  int detach = 0;
+  for (size_t i = 1; i < arg_len; i++) {
+    static const char *port_prefix = "Port=";
+    static const char *flags_prefix = "Flags=";
+
+    const char *arg = smartlist_get(args, i);
+    if (!strcasecmpstart(arg, port_prefix)) {
+      /* "Port=VIRTPORT[,TARGET]". */
+      const char *port_str = arg + strlen(port_prefix);
+
+      rend_service_port_config_t *cfg =
+          rend_service_parse_port_config(port_str, ",", 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, flags_prefix)) {
+      /* "Flags=Flag[,Flag]", where Flag can be:
+       *   * 'DiscardPK' - If tor generates the keypair, do not include it in
+       *                   the response.
+       *   * 'Detach' - Do not tie this onion service to any particular control
+       *                connection.
+       */
+      static const char *discard_flag = "DiscardPK";
+      static const char *detach_flag = "Detach";
+
+      smartlist_t *flags = smartlist_new();
+      int bad = 0;
+
+      smartlist_split_string(flags, arg + strlen(flags_prefix), ",",
+                             SPLIT_IGNORE_BLANK, 0);
+      if (smartlist_len(flags) < 1) {
+        connection_printf_to_buf(conn, "512 Invalid 'Flags' argument\r\n");
+        bad = 1;
+      }
+      SMARTLIST_FOREACH_BEGIN(flags, const char *, flag)
+      {
+        if (!strcasecmp(flag, discard_flag)) {
+          discard_pk = 1;
+        } else if (!strcasecmp(flag, detach_flag)) {
+          detach = 1;
+        } else {
+          connection_printf_to_buf(conn,
+                                   "512 Invalid 'Flags' argument: %s\r\n",
+                                   escaped(flag));
+          bad = 1;
+          break;
+        }
+      } SMARTLIST_FOREACH_END(flag);
+      SMARTLIST_FOREACH(flags, char *, cp, tor_free(cp));
+      smartlist_free(flags);
+      if (bad)
+        goto out;
+    } else {
+      connection_printf_to_buf(conn, "513 Invalid argument\r\n");
+      goto out;
+    }
+  }
+  if (smartlist_len(port_cfgs) == 0) {
+    connection_printf_to_buf(conn, "512 Missing 'Port' argument\r\n");
+    goto out;
+  }
+
+  /* Parse the "keytype:keyblob" argument. */
+  crypto_pk_t *pk = NULL;
+  const char *key_new_alg = NULL;
+  char *key_new_blob = NULL;
+  char *err_msg = NULL;
+
+  pk = add_onion_helper_keyarg(smartlist_get(args, 0), discard_pk,
+                               &key_new_alg, &key_new_blob,
+                               &err_msg);
+  if (!pk) {
+    if (err_msg) {
+      connection_write_str_to_buf(err_msg, conn);
+      tor_free(err_msg);
+    }
+    goto out;
+  }
+  tor_assert(!err_msg);
+
+  /* Create the HS, using private key pk, and port config port_cfg.
+   * rend_service_add_ephemeral() will take ownership of pk and port_cfg,
+   * regardless of success/failure.
+   */
+  char *service_id = NULL;
+  int ret = rend_service_add_ephemeral(pk, port_cfgs, &service_id);
+  port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */
+  switch (ret) {
+  case RSAE_OKAY:
+  {
+    char *buf = NULL;
+    tor_assert(service_id);
+    if (key_new_alg) {
+      tor_assert(key_new_blob);
+      tor_asprintf(&buf,
+                   "250-ServiceID=%s\r\n"
+                   "250-PrivateKey=%s:%s\r\n"
+                   "250 OK\r\n",
+                   service_id,
+                   key_new_alg,
+                   key_new_blob);
+    } else {
+      tor_asprintf(&buf,
+                   "250-ServiceID=%s\r\n"
+                   "250 OK\r\n",
+                   service_id);
+    }
+    if (detach) {
+      if (!detached_onion_services)
+        detached_onion_services = smartlist_new();
+      smartlist_add(detached_onion_services, service_id);
+    } else {
+      if (!conn->ephemeral_onion_services)
+        conn->ephemeral_onion_services = smartlist_new();
+      smartlist_add(conn->ephemeral_onion_services, service_id);
+    }
+
+    connection_write_str_to_buf(buf, conn);
+    memwipe(buf, 0, strlen(buf));
+    tor_free(buf);
+    break;
+  }
+  case RSAE_BADPRIVKEY:
+    connection_printf_to_buf(conn, "551 Failed to generate onion address\r\n");
+    break;
+  case RSAE_ADDREXISTS:
+    connection_printf_to_buf(conn, "550 Onion address collision\r\n");
+    break;
+  case RSAE_BADVIRTPORT:
+    connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n");
+    break;
+  case RSAE_INTERNAL: /* FALLSTHROUGH */
+  default:
+    connection_printf_to_buf(conn, "551 Failed to add Onion Service\r\n");
+  }
+  if (key_new_blob) {
+    memwipe(key_new_blob, 0, strlen(key_new_blob));
+    tor_free(key_new_blob);
+  }
+
+ out:
+  if (port_cfgs) {
+    SMARTLIST_FOREACH(port_cfgs, rend_service_port_config_t*, p,
+                      rend_service_port_config_free(p));
+    smartlist_free(port_cfgs);
+  }
+
+  SMARTLIST_FOREACH(args, char *, cp, {
+    memwipe(cp, 0, strlen(cp));
+    tor_free(cp);
+  });
+  smartlist_free(args);
+  return 0;
+}
+
+/** Helper function to handle parsing the KeyType:KeyBlob argument to the
+ * ADD_ONION command. Return a new crypto_pk_t and if a new key was generated
+ * and the private key not discarded, the algorithm and serialized private key,
+ * or NULL and an optional control protocol error message on failure.  The
+ * caller is responsible for freeing the returned key_new_blob and err_msg.
+ *
+ * Note: The error messages returned are deliberately vague to avoid echoing
+ * key material.
+ */
+STATIC crypto_pk_t *
+add_onion_helper_keyarg(const char *arg, int discard_pk,
+                        const char **key_new_alg_out, char **key_new_blob_out,
+                        char **err_msg_out)
+{
+  smartlist_t *key_args = smartlist_new();
+  crypto_pk_t *pk = NULL;
+  const char *key_new_alg = NULL;
+  char *key_new_blob = NULL;
+  char *err_msg = NULL;
+  int ok = 0;
+
+  smartlist_split_string(key_args, arg, ":", SPLIT_IGNORE_BLANK, 0);
+  if (smartlist_len(key_args) != 2) {
+    err_msg = tor_strdup("512 Invalid key type/blob\r\n");
+    goto err;
+  }
+
+  /* The format is "KeyType:KeyBlob". */
+  static const char *key_type_new = "NEW";
+  static const char *key_type_best = "BEST";
+  static const char *key_type_rsa1024 = "RSA1024";
+
+  const char *key_type = smartlist_get(key_args, 0);
+  const char *key_blob = smartlist_get(key_args, 1);
+
+  if (!strcasecmp(key_type_rsa1024, key_type)) {
+    /* "RSA:<Base64 Blob>" - Loading a pre-existing RSA1024 key. */
+    pk = crypto_pk_base64_decode(key_blob, strlen(key_blob));
+    if (!pk) {
+      err_msg = tor_strdup("512 Failed to decode RSA key\r\n");
+      goto err;
+    }
+    if (crypto_pk_num_bits(pk) != PK_BYTES*8) {
+      err_msg = tor_strdup("512 Invalid RSA key size\r\n");
+      goto err;
+    }
+  } else if (!strcasecmp(key_type_new, key_type)) {
+    /* "NEW:<Algorithm>" - Generating a new key, blob as algorithm. */
+    if (!strcasecmp(key_type_rsa1024, key_blob) ||
+        !strcasecmp(key_type_best, key_blob)) {
+      /* "RSA1024", RSA 1024 bit, also currently "BEST" by default. */
+      pk = crypto_pk_new();
+      if (crypto_pk_generate_key(pk)) {
+        tor_asprintf(&err_msg, "551 Failed to generate %s key\r\n",
+                     key_type_rsa1024);
+        goto err;
+      }
+      if (!discard_pk) {
+        if (crypto_pk_base64_encode(pk, &key_new_blob)) {
+          tor_asprintf(&err_msg, "551 Failed to encode %s key\r\n",
+                       key_type_rsa1024);
+          goto err;
+        }
+        key_new_alg = key_type_rsa1024;
+      }
+    } else {
+      err_msg = tor_strdup("513 Invalid key type\r\n");
+      goto err;
+    }
+  } else {
+    err_msg = tor_strdup("513 Invalid key type\r\n");
+    goto err;
+  }
+
+  /* Succeded in loading or generating a private key. */
+  tor_assert(pk);
+  ok = 1;
+
+ err:
+  SMARTLIST_FOREACH(key_args, char *, cp, {
+    memwipe(cp, 0, strlen(cp));
+    tor_free(cp);
+  });
+
+  if (!ok) {
+    crypto_pk_free(pk);
+    pk = NULL;
+  }
+  if (err_msg_out) *err_msg_out = err_msg;
+  *key_new_alg_out = key_new_alg;
+  *key_new_blob_out = key_new_blob;
+
+  return pk;
+}
+
+/** Called when we get a DEL_ONION command; parse the body, and remove
+ * the existing ephemeral Onion Service. */
+static int
+handle_control_del_onion(control_connection_t *conn,
+                          uint32_t len,
+                          const char *body)
+{
+  smartlist_t *args;
+  (void) len; /* body is nul-terminated; it's safe to ignore the length */
+  args = getargs_helper("DEL_ONION", conn, body, 1, 1);
+  if (!args)
+    return 0;
+
+  const char *service_id = smartlist_get(args, 0);
+  if (!rend_valid_service_id(service_id)) {
+    connection_printf_to_buf(conn, "512 Malformed Onion Service id\r\n");
+    goto out;
+  }
+
+  /* Determine if the onion service belongs to this particular control
+   * connection, or if it is in the global list of detached services.  If it
+   * is in neither, either the service ID is invalid in some way, or it
+   * explicitly belongs to a different control connection, and an error
+   * should be returned.
+   */
+  smartlist_t *services[2] = {
+    conn->ephemeral_onion_services,
+    detached_onion_services
+  };
+  smartlist_t *onion_services = NULL;
+  int idx = -1;
+  for (size_t i = 0; i < ARRAY_LENGTH(services); i++) {
+    idx = smartlist_string_pos(services[i], service_id);
+    if (idx != -1) {
+      onion_services = services[i];
+      break;
+    }
+  }
+  if (onion_services == NULL) {
+    connection_printf_to_buf(conn, "552 Unknown Onion Service id\r\n");
+  } else {
+    int ret = rend_service_del_ephemeral(service_id);
+    if (ret) {
+      /* This should *NEVER* fail, since the service is on either the
+       * per-control connection list, or the global one.
+       */
+      log_warn(LD_BUG, "Failed to remove Onion Service %s.",
+               escaped(service_id));
+      tor_fragile_assert();
+    }
+
+    /* Remove/scrub the service_id from the appropriate list. */
+    char *cp = smartlist_get(onion_services, idx);
+    smartlist_del(onion_services, idx);
+    memwipe(cp, 0, strlen(cp));
+    tor_free(cp);
+
+    send_control_done(conn);
+  }
+
+ out:
+  SMARTLIST_FOREACH(args, char *, cp, {
+    memwipe(cp, 0, strlen(cp));
+    tor_free(cp);
+  });
+  smartlist_free(args);
+  return 0;
+}
+
 /** Called when <b>conn</b> has no more bytes left on its outbuf. */
 int
 connection_control_finished_flushing(control_connection_t *conn)
@@ -3434,6 +3816,15 @@ connection_control_closed(control_connection_t *conn)
   conn->event_mask = 0;
   control_update_global_event_mask();
 
+  /* Close all ephemeral Onion Services if any.
+   * The list and it's contents are scrubbed/freed in connection_free_.
+   */
+  if (conn->ephemeral_onion_services) {
+    SMARTLIST_FOREACH(conn->ephemeral_onion_services, char *, cp, {
+      rend_service_del_ephemeral(cp);
+    });
+  }
+
   if (conn->is_owning_control_connection) {
     lost_owning_controller("connection", "closed");
   }
@@ -3688,6 +4079,16 @@ connection_control_process_inbuf(control_connection_t *conn)
   } else if (!strcasecmp(conn->incoming_cmd, "HSFETCH")) {
     if (handle_control_hsfetch(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);
@@ -5520,6 +5921,10 @@ control_free_all(void)
 {
   if (authentication_cookie) /* Free the auth cookie */
     tor_free(authentication_cookie);
+  if (detached_onion_services) { /* Free the detached onion services */
+    SMARTLIST_FOREACH(detached_onion_services, char *, cp, tor_free(cp));
+    smartlist_free(detached_onion_services);
+  }
 }
 
 #ifdef TOR_UNIT_TESTS
diff --git a/src/or/control.h b/src/or/control.h
index 93bcb8ef3d..cb0468fa07 100644
--- a/src/or/control.h
+++ b/src/or/control.h
@@ -233,6 +233,11 @@ void append_cell_stats_by_command(smartlist_t *event_parts,
 void format_cell_stats(char **event_string, circuit_t *circ,
                        cell_stats_t *cell_stats);
 STATIC char *get_bw_samples(void);
+
+STATIC crypto_pk_t *add_onion_helper_keyarg(const char *arg, int discard_pk,
+                                            const char **key_new_alg_out,
+                                            char **key_new_blob_out,
+                                            char **err_msg_out);
 #endif
 
 #endif
diff --git a/src/or/or.h b/src/or/or.h
index 1c00423166..dee1881034 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1735,6 +1735,9 @@ typedef struct control_connection_t {
    * connection. */
   unsigned int is_owning_control_connection:1;
 
+  /** List of ephemeral onion services belonging to this connection. */
+  smartlist_t *ephemeral_onion_services;
+
   /** If we have sent an AUTHCHALLENGE reply on this connection and
    * have not received a successful AUTHENTICATE command, points to
    * the value which the client must send to authenticate itself;
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 41bbd69c93..f257f6e89f 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -42,9 +42,15 @@ static int intro_point_accepted_intro_count(rend_intro_point_t *intro);
 static int intro_point_should_expire_now(rend_intro_point_t *intro,
                                          time_t now);
 struct rend_service_t;
+static int rend_service_derive_key_digests(struct rend_service_t *s);
 static int rend_service_load_keys(struct rend_service_t *s);
 static int rend_service_load_auth_keys(struct rend_service_t *s,
                                        const char *hfname);
+static struct rend_service_t *rend_service_get_by_pk_digest(
+    const char* digest);
+static struct rend_service_t *rend_service_get_by_service_id(const char *id);
+static const char *rend_service_escaped_dir(
+    const struct rend_service_t *s);
 
 static ssize_t rend_service_parse_intro_for_v0_or_v1(
     rend_intro_cell_t *intro,
@@ -65,7 +71,7 @@ static ssize_t rend_service_parse_intro_for_v3(
 /** Represents the mapping from a virtual port of a rendezvous service to
  * a real port on some IP.
  */
-typedef struct rend_service_port_config_t {
+struct rend_service_port_config_s {
   /* The incoming HS virtual port we're mapping */
   uint16_t virtual_port;
   /* Is this an AF_UNIX port? */
@@ -76,7 +82,7 @@ typedef struct rend_service_port_config_t {
   tor_addr_t real_addr;
   /* The socket path to connect to, if is_unix_addr */
   char unix_addr[FLEXIBLE_ARRAY_MEMBER];
-} rend_service_port_config_t;
+};
 
 /** Try to maintain this many intro points per service by default. */
 #define NUM_INTRO_POINTS_DEFAULT 3
@@ -102,7 +108,8 @@ typedef struct rend_service_port_config_t {
 /** Represents a single hidden service running at this OP. */
 typedef struct rend_service_t {
   /* Fields specified in config file */
-  char *directory; /**< where in the filesystem it stores it */
+  char *directory; /**< where in the filesystem it stores it. Will be NULL if
+                    * this service is ephemeral. */
   int dir_group_readable; /**< if 1, allow group read
                              permissions on directory */
   smartlist_t *ports; /**< List of rend_service_port_config_t */
@@ -141,6 +148,14 @@ typedef struct rend_service_t {
   int allow_unknown_ports;
 } rend_service_t;
 
+/** Returns a escaped string representation of the service, <b>s</b>.
+ */
+static const char *
+rend_service_escaped_dir(const struct rend_service_t *s)
+{
+  return (s->directory) ? escaped(s->directory) : "[EPHEMERAL]";
+}
+
 /** A list of rend_service_t's for services run on this OP.
  */
 static smartlist_t *rend_service_list = NULL;
@@ -195,7 +210,8 @@ rend_service_free(rend_service_t *service)
     return;
 
   tor_free(service->directory);
-  SMARTLIST_FOREACH(service->ports, void*, p, tor_free(p));
+  SMARTLIST_FOREACH(service->ports, rend_service_port_config_t*, p,
+                    rend_service_port_config_free(p));
   smartlist_free(service->ports);
   if (service->private_key)
     crypto_pk_free(service->private_key);
@@ -232,8 +248,9 @@ rend_service_free_all(void)
 }
 
 /** Validate <b>service</b> and add it to rend_service_list if possible.
+ * Return 0 on success and -1 on failure.
  */
-static void
+static int
 rend_add_service(rend_service_t *service)
 {
   int i;
@@ -245,16 +262,17 @@ rend_add_service(rend_service_t *service)
       smartlist_len(service->clients) == 0) {
     log_warn(LD_CONFIG, "Hidden service (%s) with client authorization but no "
                         "clients; ignoring.",
-             escaped(service->directory));
+             rend_service_escaped_dir(service));
     rend_service_free(service);
-    return;
+    return -1;
   }
 
   if (!smartlist_len(service->ports)) {
     log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured; "
              "ignoring.",
-             escaped(service->directory));
+             rend_service_escaped_dir(service));
     rend_service_free(service);
+    return -1;
   } else {
     int dupe = 0;
     /* XXX This duplicate check has two problems:
@@ -272,14 +290,17 @@ rend_add_service(rend_service_t *service)
      * lock file.  But this is enough to detect a simple mistake that
      * at least one person has actually made.
      */
-    SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr,
-                      dupe = dupe ||
-                             !strcmp(ptr->directory, service->directory));
-    if (dupe) {
-      log_warn(LD_REND, "Another hidden service is already configured for "
-               "directory %s, ignoring.", service->directory);
-      rend_service_free(service);
-      return;
+    if (service->directory != NULL) { /* Skip dupe for ephemeral services. */
+      SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr,
+                        dupe = dupe ||
+                               !strcmp(ptr->directory, service->directory));
+      if (dupe) {
+        log_warn(LD_REND, "Another hidden service is already configured for "
+                 "directory %s, ignoring.",
+                 rend_service_escaped_dir(service));
+        rend_service_free(service);
+        return -1;
+      }
     }
     smartlist_add(rend_service_list, service);
     log_debug(LD_REND,"Configuring service with directory \"%s\"",
@@ -305,7 +326,9 @@ rend_add_service(rend_service_t *service)
 #endif /* defined(HAVE_SYS_UN_H) */
       }
     }
+    return 0;
   }
+  /* NOTREACHED */
 }
 
 /** Return a new rend_service_port_config_t with its path set to
@@ -324,15 +347,17 @@ rend_service_port_config_new(const char *socket_path)
   return conf;
 }
 
-/** Parses a real-port to virtual-port mapping and returns a new
- * rend_service_port_config_t.
+/** Parses a real-port to virtual-port mapping separated by the provided
+ * separator and returns a new rend_service_port_config_t, or NULL and an
+ * optional error string on failure.
  *
- * The format is: VirtualPort (IP|RealPort|IP:RealPort|'socket':path)?
+ * The format is: VirtualPort SEP (IP|RealPort|IP:RealPort|'socket':path)?
  *
  * IP defaults to 127.0.0.1; RealPort defaults to VirtualPort.
  */
-static rend_service_port_config_t *
-parse_port_config(const char *string)
+rend_service_port_config_t *
+rend_service_parse_port_config(const char *string, const char *sep,
+                               char **err_msg_out)
 {
   smartlist_t *sl;
   int virtport;
@@ -343,19 +368,24 @@ parse_port_config(const char *string)
   rend_service_port_config_t *result = NULL;
   unsigned int is_unix_addr = 0;
   char *socket_path = NULL;
+  char *err_msg = NULL;
 
   sl = smartlist_new();
-  smartlist_split_string(sl, string, " ",
+  smartlist_split_string(sl, string, sep,
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   if (smartlist_len(sl) < 1 || smartlist_len(sl) > 2) {
-    log_warn(LD_CONFIG, "Bad syntax in hidden service port configuration.");
+    if (err_msg_out)
+      err_msg = tor_strdup("Bad syntax in hidden service port configuration.");
+
     goto err;
   }
 
   virtport = (int)tor_parse_long(smartlist_get(sl,0), 10, 1, 65535, NULL,NULL);
   if (!virtport) {
-    log_warn(LD_CONFIG, "Missing or invalid port %s in hidden service port "
-             "configuration", escaped(smartlist_get(sl,0)));
+    if (err_msg_out)
+      tor_asprintf(&err_msg, "Missing or invalid port %s in hidden service "
+                   "port configuration", escaped(smartlist_get(sl,0)));
+
     goto err;
   }
 
@@ -369,10 +399,11 @@ parse_port_config(const char *string)
     addrport = smartlist_get(sl,1);
     ret = config_parse_unix_port(addrport, &socket_path);
     if (ret < 0 && ret != -ENOENT) {
-      if (ret == -EINVAL) {
-        log_warn(LD_CONFIG,
-                 "Empty socket path in hidden service port configuration.");
-      }
+      if (ret == -EINVAL)
+        if (err_msg_out)
+          err_msg = tor_strdup("Empty socket path in hidden service port "
+                               "configuration.");
+
       goto err;
     }
     if (socket_path) {
@@ -380,8 +411,10 @@ parse_port_config(const char *string)
     } else if (strchr(addrport, ':') || strchr(addrport, '.')) {
       /* else try it as an IP:port pair if it has a : or . in it */
       if (tor_addr_port_lookup(addrport, &addr, &p)<0) {
-        log_warn(LD_CONFIG,"Unparseable address in hidden service port "
-                 "configuration.");
+        if (err_msg_out)
+          err_msg = tor_strdup("Unparseable address in hidden service port "
+                               "configuration.");
+
         goto err;
       }
       realport = p?p:virtport;
@@ -389,8 +422,11 @@ parse_port_config(const char *string)
       /* No addr:port, no addr -- must be port. */
       realport = (int)tor_parse_long(addrport, 10, 1, 65535, NULL, NULL);
       if (!realport) {
-        log_warn(LD_CONFIG,"Unparseable or out-of-range port %s in hidden "
-                 "service port configuration.", escaped(addrport));
+        if (err_msg_out)
+          tor_asprintf(&err_msg, "Unparseable or out-of-range port %s in "
+                       "hidden service port configuration.",
+                       escaped(addrport));
+
         goto err;
       }
       tor_addr_from_ipv4h(&addr, 0x7F000001u); /* Default to 127.0.0.1 */
@@ -408,6 +444,7 @@ parse_port_config(const char *string)
   }
 
  err:
+  if (err_msg_out) *err_msg_out = err_msg;
   SMARTLIST_FOREACH(sl, char *, c, tor_free(c));
   smartlist_free(sl);
   if (socket_path) tor_free(socket_path);
@@ -415,6 +452,13 @@ parse_port_config(const char *string)
   return result;
 }
 
+/** Release all storage held in a rend_service_port_config_t. */
+void
+rend_service_port_config_free(rend_service_port_config_t *p)
+{
+  tor_free(p);
+}
+
 /** Set up rend_service_list, based on the values of HiddenServiceDir and
  * HiddenServicePort in <b>options</b>.  Return 0 on success and -1 on
  * failure.  (If <b>validate_only</b> is set, parse, warn and return as
@@ -456,11 +500,16 @@ rend_config_services(const or_options_t *options, int validate_only)
        return -1;
      }
      if (!strcasecmp(line->key, "HiddenServicePort")) {
-       portcfg = parse_port_config(line->value);
+       char *err_msg = NULL;
+       portcfg = rend_service_parse_port_config(line->value, " ", &err_msg);
        if (!portcfg) {
+         if (err_msg)
+           log_warn(LD_CONFIG, "%s", err_msg);
+         tor_free(err_msg);
          rend_service_free(service);
          return -1;
        }
+       tor_assert(!err_msg);
        smartlist_add(service->ports, portcfg);
      } else if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) {
        service->allow_unknown_ports = (int)tor_parse_long(line->value,
@@ -632,6 +681,28 @@ rend_config_services(const or_options_t *options, int validate_only)
   if (old_service_list && !validate_only) {
     smartlist_t *surviving_services = smartlist_new();
 
+    /* Preserve the existing ephemeral services.
+     *
+     * This is the ephemeral service equivalent of the "Copy introduction
+     * points to new services" block, except there's no copy required since
+     * the service structure isn't regenerated.
+     *
+     * After this is done, all ephemeral services will be:
+     *  * Removed from old_service_list, so the equivalent non-ephemeral code
+     *    will not attempt to preserve them.
+     *  * Added to the new rend_service_list (that previously only had the
+     *    services listed in the configuration).
+     *  * Added to surviving_services, which is the list of services that
+     *    will NOT have their intro point closed.
+     */
+    SMARTLIST_FOREACH(old_service_list, rend_service_t *, old, {
+      if (!old->directory) {
+        SMARTLIST_DEL_CURRENT(old_service_list, old);
+        smartlist_add(surviving_services, old);
+        smartlist_add(rend_service_list, old);
+      }
+    });
+
     /* Copy introduction points to new services. */
     /* XXXX This is O(n^2), but it's only called on reconfigure, so it's
      * probably ok? */
@@ -685,6 +756,118 @@ rend_config_services(const or_options_t *options, int validate_only)
   return 0;
 }
 
+/** Add the ephemeral service <b>pk</b>/<b>ports</b> if possible.
+ *
+ * Regardless of sucess/failure, callers should not touch pk/ports after
+ * calling this routine, and may assume that correct cleanup has been done
+ * on failure.
+ *
+ * Return an appropriate rend_service_add_ephemeral_status_t.
+ */
+rend_service_add_ephemeral_status_t
+rend_service_add_ephemeral(crypto_pk_t *pk,
+                           smartlist_t *ports,
+                           char **service_id_out)
+{
+  *service_id_out = NULL;
+  /* Allocate the service structure, and initialize the key, and key derived
+   * parameters.
+   */
+  rend_service_t *s = tor_malloc_zero(sizeof(rend_service_t));
+  s->directory = NULL; /* This indicates the service is ephemeral. */
+  s->private_key = pk;
+  s->auth_type = REND_NO_AUTH;
+  s->ports = ports;
+  s->intro_period_started = time(NULL);
+  s->n_intro_points_wanted = NUM_INTRO_POINTS_DEFAULT;
+  if (rend_service_derive_key_digests(s) < 0) {
+    rend_service_free(s);
+    return RSAE_BADPRIVKEY;
+  }
+
+  if (!s->ports || smartlist_len(s->ports) == 0) {
+    log_warn(LD_CONFIG, "At least one VIRTPORT/TARGET must be specified.");
+    rend_service_free(s);
+    return RSAE_BADVIRTPORT;
+  }
+
+  /* Enforcing pk/id uniqueness should be done by rend_service_load_keys(), but
+   * it's not, see #14828.
+   */
+  if (rend_service_get_by_pk_digest(s->pk_digest)) {
+    log_warn(LD_CONFIG, "Onion Service private key collides with an "
+             "existing service.");
+    rend_service_free(s);
+    return RSAE_ADDREXISTS;
+  }
+  if (rend_service_get_by_service_id(s->service_id)) {
+    log_warn(LD_CONFIG, "Onion Service id collides with an existing service.");
+    rend_service_free(s);
+    return RSAE_ADDREXISTS;
+  }
+
+  /* Initialize the service. */
+  if (rend_add_service(s)) {
+    rend_service_free(s);
+    return RSAE_INTERNAL;
+  }
+  *service_id_out = tor_strdup(s->service_id);
+
+  log_debug(LD_CONFIG, "Added ephemeral Onion Service: %s", s->service_id);
+  return RSAE_OKAY;
+}
+
+/** Remove the ephemeral service <b>service_id</b> if possible.  Returns 0 on
+ * success, and -1 on failure.
+ */
+int
+rend_service_del_ephemeral(const char *service_id)
+{
+  rend_service_t *s;
+  if (!rend_valid_service_id(service_id)) {
+    log_warn(LD_CONFIG, "Requested malformed Onion Service id for removal.");
+    return -1;
+  }
+  if ((s = rend_service_get_by_service_id(service_id)) == NULL) {
+    log_warn(LD_CONFIG, "Requested non-existent Onion Service id for "
+             "removal.");
+    return -1;
+  }
+  if (s->directory) {
+    log_warn(LD_CONFIG, "Requested non-ephemeral Onion Service for removal.");
+    return -1;
+  }
+
+  /* Kill the intro point circuit for the Onion Service, and remove it from
+   * the list.  Closing existing connections is the application's problem.
+   *
+   * XXX: As with the comment in rend_config_services(), a nice abstraction
+   * would be ideal here, but for now just duplicate the code.
+   */
+  SMARTLIST_FOREACH_BEGIN(circuit_get_global_list(), circuit_t *, circ) {
+    if (!circ->marked_for_close &&
+        circ->state == CIRCUIT_STATE_OPEN &&
+        (circ->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO ||
+         circ->purpose == CIRCUIT_PURPOSE_S_INTRO)) {
+      origin_circuit_t *oc = TO_ORIGIN_CIRCUIT(circ);
+      tor_assert(oc->rend_data);
+      if (!tor_memeq(s->pk_digest, oc->rend_data->rend_pk_digest, DIGEST_LEN))
+        continue;
+      log_debug(LD_REND, "Closing intro point %s for service %s.",
+                safe_str_client(extend_info_describe(
+                                          oc->build_state->chosen_exit)),
+                oc->rend_data->onion_address);
+      circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED);
+    }
+  } SMARTLIST_FOREACH_END(circ);
+  smartlist_remove(rend_service_list, s);
+  rend_service_free(s);
+
+  log_debug(LD_CONFIG, "Removed ephemeral Onion Service: %s", service_id);
+
+  return 0;
+}
+
 /** Replace the old value of <b>service</b>-\>desc with one that reflects
  * the other fields in service.
  */
@@ -769,6 +952,7 @@ rend_service_add_filenames_to_list(smartlist_t *lst, const rend_service_t *s)
 {
   tor_assert(lst);
   tor_assert(s);
+  tor_assert(s->directory);
   smartlist_add_asprintf(lst, "%s"PATH_SEPARATOR"private_key",
                          s->directory);
   smartlist_add_asprintf(lst, "%s"PATH_SEPARATOR"hostname",
@@ -787,11 +971,31 @@ rend_services_add_filenames_to_lists(smartlist_t *open_lst,
   if (!rend_service_list)
     return;
   SMARTLIST_FOREACH_BEGIN(rend_service_list, rend_service_t *, s) {
-    rend_service_add_filenames_to_list(open_lst, s);
-    smartlist_add(stat_lst, tor_strdup(s->directory));
+    if (s->directory) {
+      rend_service_add_filenames_to_list(open_lst, s);
+      smartlist_add(stat_lst, tor_strdup(s->directory));
+    }
   } SMARTLIST_FOREACH_END(s);
 }
 
+/** Derive all rend_service_t internal material based on the service's key.
+ * Returns 0 on sucess, -1 on failure.
+ */
+static int
+rend_service_derive_key_digests(struct rend_service_t *s)
+{
+  if (rend_get_service_id(s->private_key, s->service_id)<0) {
+    log_warn(LD_BUG, "Internal error: couldn't encode service ID.");
+    return -1;
+  }
+  if (crypto_pk_get_digest(s->private_key, s->pk_digest)<0) {
+    log_warn(LD_BUG, "Couldn't compute hash of public key.");
+    return -1;
+  }
+
+  return 0;
+}
+
 /** Load and/or generate private keys for the hidden service <b>s</b>,
  * possibly including keys for client authorization.  Return 0 on success, -1
  * on failure. */
@@ -830,15 +1034,10 @@ rend_service_load_keys(rend_service_t *s)
   if (!s->private_key)
     return -1;
 
-  /* Create service file */
-  if (rend_get_service_id(s->private_key, s->service_id)<0) {
-    log_warn(LD_BUG, "Internal error: couldn't encode service ID.");
+  if (rend_service_derive_key_digests(s) < 0)
     return -1;
-  }
-  if (crypto_pk_get_digest(s->private_key, s->pk_digest)<0) {
-    log_warn(LD_BUG, "Couldn't compute hash of public key.");
-    return -1;
-  }
+
+  /* Create service file */
   if (strlcpy(fname,s->directory,sizeof(fname)) >= sizeof(fname) ||
       strlcat(fname,PATH_SEPARATOR"hostname",sizeof(fname))
       >= sizeof(fname)) {
@@ -1078,6 +1277,20 @@ rend_service_get_by_pk_digest(const char* digest)
   return NULL;
 }
 
+/** Return the service whose service id is <b>id</b>, or NULL if no such
+ * service exists.
+ */
+static struct rend_service_t *
+rend_service_get_by_service_id(const char *id)
+{
+  tor_assert(strlen(id) == REND_SERVICE_ID_LEN_BASE32);
+  SMARTLIST_FOREACH(rend_service_list, rend_service_t*, s, {
+    if (tor_memeq(s->service_id, id, REND_SERVICE_ID_LEN_BASE32))
+      return s;
+  });
+  return NULL;
+}
+
 /** Return 1 if any virtual port in <b>service</b> wants a circuit
  * to have good uptime. Else return 0.
  */
diff --git a/src/or/rendservice.h b/src/or/rendservice.h
index 754f7c358c..6d0973b06c 100644
--- a/src/or/rendservice.h
+++ b/src/or/rendservice.h
@@ -15,6 +15,7 @@
 #include "or.h"
 
 typedef struct rend_intro_cell_s rend_intro_cell_t;
+typedef struct rend_service_port_config_s rend_service_port_config_t;
 
 #ifdef RENDSERVICE_PRIVATE
 
@@ -101,5 +102,23 @@ int rend_service_set_connection_addr_port(edge_connection_t *conn,
 void rend_service_dump_stats(int severity);
 void rend_service_free_all(void);
 
+rend_service_port_config_t *rend_service_parse_port_config(const char *string,
+                                                           const char *sep,
+                                                           char **err_msg_out);
+void rend_service_port_config_free(rend_service_port_config_t *p);
+
+/** Return value from rend_service_add_ephemeral. */
+typedef enum {
+  RSAE_BADVIRTPORT = -4, /**< Invalid VIRTPORT/TARGET(s) */
+  RSAE_ADDREXISTS = -3, /**< Onion address collision */
+  RSAE_BADPRIVKEY = -2, /**< Invalid public key */
+  RSAE_INTERNAL = -1, /**< Internal error */
+  RSAE_OKAY = 0 /**< Service added as expected */
+} rend_service_add_ephemeral_status_t;
+rend_service_add_ephemeral_status_t rend_service_add_ephemeral(crypto_pk_t *pk,
+                               smartlist_t *ports,
+                               char **service_id_out);
+int rend_service_del_ephemeral(const char *service_id);
+
 #endif
 
diff --git a/src/test/include.am b/src/test/include.am
index 570048dc44..c36ebb777a 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -56,6 +56,7 @@ src_test_test_SOURCES = \
 	src/test/test_circuitmux.c \
 	src/test/test_config.c \
 	src/test/test_containers.c \
+	src/test/test_controller.c \
 	src/test/test_controller_events.c \
 	src/test/test_crypto.c \
 	src/test/test_data.c \
diff --git a/src/test/test.c b/src/test/test.c
index 0524a6978f..f30b8ae1af 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -1127,6 +1127,7 @@ extern struct testcase_t circuitlist_tests[];
 extern struct testcase_t circuitmux_tests[];
 extern struct testcase_t config_tests[];
 extern struct testcase_t container_tests[];
+extern struct testcase_t controller_tests[];
 extern struct testcase_t controller_event_tests[];
 extern struct testcase_t crypto_tests[];
 extern struct testcase_t dir_tests[];
@@ -1171,7 +1172,8 @@ struct testgroup_t testgroups[] = {
   { "circuitmux/", circuitmux_tests },
   { "config/", config_tests },
   { "container/", container_tests },
-  { "control/", controller_event_tests },
+  { "control/", controller_tests },
+  { "control/event/", controller_event_tests },
   { "crypto/", crypto_tests },
   { "dir/", dir_tests },
   { "dir/md/", microdesc_tests },
diff --git a/src/test/test_controller.c b/src/test/test_controller.c
new file mode 100644
index 0000000000..9039dc7677
--- /dev/null
+++ b/src/test/test_controller.c
@@ -0,0 +1,161 @@
+/* Copyright (c) 2015, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#define CONTROL_PRIVATE
+#include "or.h"
+#include "control.h"
+#include "rendservice.h"
+#include "test.h"
+
+static void
+test_add_onion_helper_keyarg(void *arg)
+{
+  crypto_pk_t *pk = NULL;
+  crypto_pk_t *pk2 = NULL;
+  const char *key_new_alg = NULL;
+  char *key_new_blob = NULL;
+  char *err_msg = NULL;
+  char *encoded = NULL;
+  char *arg_str = NULL;
+
+  (void) arg;
+
+  /* Test explicit RSA1024 key generation. */
+  pk = add_onion_helper_keyarg("NEW:RSA1024", 0, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(pk);
+  tt_str_op(key_new_alg, OP_EQ, "RSA1024");
+  tt_assert(key_new_blob);
+  tt_assert(!err_msg);
+
+  /* Test "BEST" key generation (Assumes BEST = RSA1024). */
+  crypto_pk_free(pk);
+  tor_free(key_new_blob);
+  pk = add_onion_helper_keyarg("NEW:BEST", 0, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(pk);
+  tt_str_op(key_new_alg, OP_EQ, "RSA1024");
+  tt_assert(key_new_blob);
+  tt_assert(!err_msg);
+
+  /* Test discarding the private key. */
+  crypto_pk_free(pk);
+  tor_free(key_new_blob);
+  pk = add_onion_helper_keyarg("NEW:BEST", 1, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(pk);
+  tt_assert(!key_new_alg);
+  tt_assert(!key_new_blob);
+  tt_assert(!err_msg);
+
+  /* Test generating a invalid key type. */
+  crypto_pk_free(pk);
+  pk = add_onion_helper_keyarg("NEW:RSA512", 0, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(!pk);
+  tt_assert(!key_new_alg);
+  tt_assert(!key_new_blob);
+  tt_assert(err_msg);
+
+  /* Test loading a RSA1024 key. */
+  tor_free(err_msg);
+  pk = pk_generate(0);
+  tt_int_op(0, OP_EQ, crypto_pk_base64_encode(pk, &encoded));
+  tor_asprintf(&arg_str, "RSA1024:%s", encoded);
+  pk2 = add_onion_helper_keyarg(arg_str, 0, &key_new_alg, &key_new_blob,
+                                &err_msg);
+  tt_assert(pk2);
+  tt_assert(!key_new_alg);
+  tt_assert(!key_new_blob);
+  tt_assert(!err_msg);
+  tt_assert(crypto_pk_cmp_keys(pk, pk2) == 0);
+
+  /* Test loading a invalid key type. */
+  tor_free(arg_str);
+  tor_asprintf(&arg_str, "RSA512:%s", encoded);
+  pk = add_onion_helper_keyarg(arg_str, 0, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(!pk);
+  tt_assert(!key_new_alg);
+  tt_assert(!key_new_blob);
+  tt_assert(err_msg);
+
+  /* Test loading a invalid key. */
+  tor_free(arg_str);
+  tor_free(err_msg);
+  encoded[strlen(encoded)/2] = '\0';
+  tor_asprintf(&arg_str, "RSA1024:%s", encoded);
+  pk = add_onion_helper_keyarg(arg_str, 0, &key_new_alg, &key_new_blob,
+                               &err_msg);
+  tt_assert(!pk);
+  tt_assert(!key_new_alg);
+  tt_assert(!key_new_blob);
+  tt_assert(err_msg);
+
+ done:
+  crypto_pk_free(pk);
+  crypto_pk_free(pk2);
+  tor_free(key_new_blob);
+  tor_free(err_msg);
+  tor_free(encoded);
+  tor_free(arg_str);
+}
+
+static void
+test_rend_service_parse_port_config(void *arg)
+{
+  const char *sep = ",";
+  rend_service_port_config_t *cfg = NULL;
+  char *err_msg = NULL;
+
+  (void)arg;
+
+  /* Test "VIRTPORT" only. */
+  cfg = rend_service_parse_port_config("80", sep, &err_msg);
+  tt_assert(cfg);
+  tt_assert(!err_msg);
+
+  /* Test "VIRTPORT,TARGET" (Target is port). */
+  rend_service_port_config_free(cfg);
+  cfg = rend_service_parse_port_config("80,8080", sep, &err_msg);
+  tt_assert(cfg);
+  tt_assert(!err_msg);
+
+  /* Test "VIRTPORT,TARGET" (Target is IPv4:port). */
+  rend_service_port_config_free(cfg);
+  cfg = rend_service_parse_port_config("80,192.0.2.1:8080", sep, &err_msg);
+  tt_assert(cfg);
+  tt_assert(!err_msg);
+
+  /* Test "VIRTPORT,TARGET" (Target is IPv6:port). */
+  rend_service_port_config_free(cfg);
+  cfg = rend_service_parse_port_config("80,[2001:db8::1]:8080", sep, &err_msg);
+  tt_assert(cfg);
+  tt_assert(!err_msg);
+
+  /* XXX: Someone should add tests for AF_UNIX targets if supported. */
+
+  /* Test empty config. */
+  rend_service_port_config_free(cfg);
+  cfg = rend_service_parse_port_config("", sep, &err_msg);
+  tt_assert(!cfg);
+  tt_assert(err_msg);
+
+  /* Test invalid port. */
+  tor_free(err_msg);
+  cfg = rend_service_parse_port_config("90001", sep, &err_msg);
+  tt_assert(!cfg);
+  tt_assert(err_msg);
+
+ done:
+  rend_service_port_config_free(cfg);
+  tor_free(err_msg);
+}
+
+struct testcase_t controller_tests[] = {
+  { "add_onion_helper_keyarg", test_add_onion_helper_keyarg, 0, NULL, NULL },
+  { "rend_service_parse_port_config", test_rend_service_parse_port_config, 0,
+    NULL, NULL },
+  END_OF_TESTCASES
+};
+
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index bcd7069b9d..84a90da7f8 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -597,6 +597,42 @@ test_crypto_pk_fingerprints(void *arg)
   tor_free(mem_op_hex_tmp);
 }
 
+static void
+test_crypto_pk_base64(void *arg)
+{
+  crypto_pk_t *pk1 = NULL;
+  crypto_pk_t *pk2 = NULL;
+  char *encoded = NULL;
+
+  (void)arg;
+
+  /* Test Base64 encoding a key. */
+  pk1 = pk_generate(0);
+  tt_assert(pk1);
+  tt_int_op(0, OP_EQ, crypto_pk_base64_encode(pk1, &encoded));
+  tt_assert(encoded);
+
+  /* Test decoding a valid key. */
+  pk2 = crypto_pk_base64_decode(encoded, strlen(encoded));
+  tt_assert(pk2);
+  tt_assert(crypto_pk_cmp_keys(pk1,pk2) == 0);
+  crypto_pk_free(pk2);
+
+  /* Test decoding a invalid key (not Base64). */
+  static const char *invalid_b64 = "The key is in another castle!";
+  pk2 = crypto_pk_base64_decode(invalid_b64, strlen(invalid_b64));
+  tt_assert(!pk2);
+
+  /* Test decoding a truncated Base64 blob. */
+  pk2 = crypto_pk_base64_decode(encoded, strlen(encoded)/2);
+  tt_assert(!pk2);
+
+ done:
+  crypto_pk_free(pk1);
+  crypto_pk_free(pk2);
+  tor_free(encoded);
+}
+
 /** Sanity check for crypto pk digests  */
 static void
 test_crypto_digests(void *arg)
@@ -1667,6 +1703,7 @@ struct testcase_t crypto_tests[] = {
   CRYPTO_LEGACY(sha),
   CRYPTO_LEGACY(pk),
   { "pk_fingerprints", test_crypto_pk_fingerprints, TT_FORK, NULL, NULL },
+  { "pk_base64", test_crypto_pk_base64, TT_FORK, NULL, NULL },
   CRYPTO_LEGACY(digests),
   CRYPTO_LEGACY(dh),
   { "aes_iv_AES", test_crypto_aes_iv, TT_FORK, &passthrough_setup,