From eacf52891587cd9201407831a3112376dad9cf4d Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Sun, 15 Nov 2020 12:56:14 -0800
Subject: [PATCH 1/9] Add support for creating v3 onion services form the
 control port

---
 src/core/or/or.h                     |  1 +
 src/feature/control/control_cmd.c    | 77 ++++++++++++++++++++++++----
 src/feature/control/control_events.c |  3 ++
 src/feature/hs/hs_service.c          | 67 +++++++++++++++++-------
 src/feature/hs/hs_service.h          | 18 ++++---
 5 files changed, 129 insertions(+), 37 deletions(-)

diff --git a/src/core/or/or.h b/src/core/or/or.h
index d80c41371e..646dbf2c3a 100644
--- a/src/core/or/or.h
+++ b/src/core/or/or.h
@@ -404,6 +404,7 @@ typedef enum rend_auth_type_t {
   REND_NO_AUTH      = 0,
   REND_BASIC_AUTH   = 1,
   REND_STEALTH_AUTH = 2,
+  REND_V3_AUTH      = 3,
 } rend_auth_type_t;
 
 /** Client-side configuration of authorization for a hidden service. */
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 5b75c24692..4b02b1c5c0 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -33,6 +33,7 @@
 #include "feature/control/control_getinfo.h"
 #include "feature/control/control_proto.h"
 #include "feature/hs/hs_control.h"
+#include "feature/hs/hs_service.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerinfo.h"
 #include "feature/nodelist/routerlist.h"
@@ -1653,7 +1654,8 @@ add_onion_helper_add_service(int hs_version,
                              add_onion_secret_key_t *pk,
                              smartlist_t *port_cfgs, int max_streams,
                              int max_streams_close_circuit, int auth_type,
-                             smartlist_t *auth_clients, char **address_out)
+                             smartlist_t *auth_clients,
+                             smartlist_t *auth_clients_v3, char **address_out)
 {
   hs_service_add_ephemeral_status_t ret;
 
@@ -1669,7 +1671,8 @@ add_onion_helper_add_service(int hs_version,
     break;
   case HS_VERSION_THREE:
     ret = hs_service_add_ephemeral(pk->v3, port_cfgs, max_streams,
-                                   max_streams_close_circuit, address_out);
+                                   max_streams_close_circuit,
+                                   auth_clients_v3, address_out);
     break;
   default:
     tor_assert_unreached();
@@ -1693,7 +1696,7 @@ get_detached_onion_services(void)
 }
 
 static const char *add_onion_keywords[] = {
-   "Port", "Flags", "MaxStreams", "ClientAuth", NULL
+   "Port", "Flags", "MaxStreams", "ClientAuth", "ClientAuthV3", NULL
 };
 static const control_cmd_syntax_t add_onion_syntax = {
   .min_args = 1, .max_args = 1,
@@ -1714,6 +1717,8 @@ handle_control_add_onion(control_connection_t *conn,
   smartlist_t *port_cfgs = smartlist_new();
   smartlist_t *auth_clients = NULL;
   smartlist_t *auth_created_clients = NULL;
+  smartlist_t *auth_clients_v3 = NULL;
+  smartlist_t *auth_clients_v3_str = NULL;
   int discard_pk = 0;
   int detach = 0;
   int max_streams = 0;
@@ -1758,6 +1763,7 @@ handle_control_add_onion(control_connection_t *conn,
       static const char *detach_flag = "Detach";
       static const char *max_s_close_flag = "MaxStreamsCloseCircuit";
       static const char *basicauth_flag = "BasicAuth";
+      static const char *v3auth_flag = "V3Auth";
       static const char *non_anonymous_flag = "NonAnonymous";
 
       smartlist_t *flags = smartlist_new();
@@ -1778,6 +1784,8 @@ handle_control_add_onion(control_connection_t *conn,
           max_streams_close_circuit = 1;
         } else if (!strcasecmp(flag, basicauth_flag)) {
           auth_type = REND_BASIC_AUTH;
+        } else if (!strcasecmp(flag, v3auth_flag)) {
+          auth_type = REND_V3_AUTH;
         } else if (!strcasecmp(flag, non_anonymous_flag)) {
           non_anonymous = 1;
         } else {
@@ -1821,6 +1829,20 @@ handle_control_add_onion(control_connection_t *conn,
       if (created) {
         smartlist_add(auth_created_clients, client);
       }
+    } else if (!strcasecmp(arg->key, "ClientAuthV3")) {
+      hs_service_authorized_client_t *client_v3 =
+                                       parse_authorized_client_key(arg->value);
+      if (!client_v3) {
+        goto out;
+      }
+
+      if (auth_clients_v3 == NULL) {
+        auth_clients_v3 = smartlist_new();
+        auth_clients_v3_str = smartlist_new();
+      }
+
+      smartlist_add(auth_clients_v3, client_v3);
+      smartlist_add(auth_clients_v3_str, tor_strdup(arg->value));
     } else {
       tor_assert_nonfatal_unreached();
       goto out;
@@ -1829,10 +1851,12 @@ handle_control_add_onion(control_connection_t *conn,
   if (smartlist_len(port_cfgs) == 0) {
     control_write_endreply(conn, 512, "Missing 'Port' argument");
     goto out;
-  } else if (auth_type == REND_NO_AUTH && auth_clients != NULL) {
+  } else if (auth_type == REND_NO_AUTH &&
+             (auth_clients != NULL && auth_clients_v3 != NULL)) {
     control_write_endreply(conn, 512, "No auth type specified");
     goto out;
-  } else if (auth_type != REND_NO_AUTH && auth_clients == NULL) {
+  } else if (auth_type != REND_NO_AUTH &&
+             (auth_clients == NULL && auth_clients_v3 == NULL)) {
     control_write_endreply(conn, 512, "No auth clients specified");
     goto out;
   } else if ((auth_type == REND_BASIC_AUTH &&
@@ -1841,6 +1865,15 @@ handle_control_add_onion(control_connection_t *conn,
               smartlist_len(auth_clients) > 16)) {
     control_write_endreply(conn, 512, "Too many auth clients");
     goto out;
+  } else if ((auth_type == REND_BASIC_AUTH ||
+              auth_type == REND_STEALTH_AUTH) && auth_clients_v3) {
+    control_write_endreply(conn, 512,
+                        "ClientAuthV3 does not support basic or stealth auth");
+    goto out;
+  } else if (auth_type == REND_V3_AUTH && auth_clients) {
+    control_write_endreply(conn, 512, "ClientAuth does not support v3 auth");
+    goto out;
+
   } else if (non_anonymous != rend_service_non_anonymous_mode_enabled(
                                                               get_options())) {
     /* If we failed, and the non-anonymous flag is set, Tor must be in
@@ -1869,12 +1902,16 @@ handle_control_add_onion(control_connection_t *conn,
     goto out;
   }
 
-  /* Hidden service version 3 don't have client authentication support so if
-   * ClientAuth was given, send back an error. */
+  /* We can't mix ClientAuth and Version 3 Onion Services, or ClientAuthV3 and
+   * Version 2. If that's the case, send back an error. */
   if (hs_version == HS_VERSION_THREE && auth_clients) {
     control_write_endreply(conn, 513, "ClientAuth not supported");
     goto out;
   }
+  if (hs_version == HS_VERSION_TWO && auth_clients_v3) {
+    control_write_endreply(conn, 513, "ClientAuthV3 not supported");
+    goto out;
+  }
 
   /* Create the HS, using private key pk, client authentication auth_type,
    * the list of auth_clients, and port config port_cfg.
@@ -1882,12 +1919,13 @@ handle_control_add_onion(control_connection_t *conn,
    * regardless of success/failure.
    */
   char *service_id = NULL;
-  int ret = add_onion_helper_add_service(hs_version, &pk, port_cfgs,
-                                         max_streams,
-                                         max_streams_close_circuit, auth_type,
-                                         auth_clients, &service_id);
+  int ret =
+      add_onion_helper_add_service(hs_version, &pk, port_cfgs, max_streams,
+                                   max_streams_close_circuit, auth_type,
+                                   auth_clients, auth_clients_v3, &service_id);
   port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */
   auth_clients = NULL; /* so is auth_clients */
+  auth_clients_v3 = NULL; /* so is auth_clients_v3 */
   switch (ret) {
   case RSAE_OKAY:
   {
@@ -1919,6 +1957,11 @@ handle_control_add_onion(control_connection_t *conn,
         tor_free(encoded);
       });
     }
+    if (auth_clients_v3_str) {
+      SMARTLIST_FOREACH(auth_clients_v3_str, char *, client_str, {
+        control_printf_midreply(conn, 250, "ClientAuthV3=%s", client_str);
+      });
+    }
 
     send_control_done(conn);
     break;
@@ -1956,6 +1999,18 @@ handle_control_add_onion(control_connection_t *conn,
                       rend_authorized_client_free(ac));
     smartlist_free(auth_clients);
   }
+  if (auth_clients_v3) {
+    SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, ac,
+                      service_authorized_client_free(ac));
+    smartlist_free(auth_clients_v3);
+  }
+  if (auth_clients_v3_str) {
+    SMARTLIST_FOREACH(auth_clients_v3_str, char *, client_str,
+                      tor_free(client_str));
+    smartlist_free(auth_clients_v3_str);
+  }
+
+
   if (auth_created_clients) {
     // Do not free entries; they are the same as auth_clients
     smartlist_free(auth_created_clients);
diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c
index 0dd52659ec..c0ccb1eb26 100644
--- a/src/feature/control/control_events.c
+++ b/src/feature/control/control_events.c
@@ -1927,6 +1927,9 @@ rend_auth_type_to_string(rend_auth_type_t auth_type)
     case REND_STEALTH_AUTH:
       str = "STEALTH_AUTH";
       break;
+    case REND_V3_AUTH:
+      str = "REND_V3_AUTH";
+      break;
     default:
       str = "UNKNOWN";
   }
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index fee999cac5..0d7441ada2 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -1115,6 +1115,43 @@ client_filename_is_valid(const char *filename)
   return ret;
 }
 
+/** Parse an base32-encoded authorized client from a string.
+ *
+ * Return the key on success, return NULL, otherwise. */
+hs_service_authorized_client_t *
+parse_authorized_client_key(const char *key_str)
+{
+  hs_service_authorized_client_t *client = NULL;
+
+  /* We expect a specific length of the base32 encoded key so make sure we
+   * have that so we don't successfully decode a value with a different length
+   * and end up in trouble when copying the decoded key into a fixed length
+   * buffer. */
+  if (strlen(key_str) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
+    log_warn(LD_REND, "Client authorization encoded base32 public key "
+                      "length is invalid: %s", key_str);
+    goto err;
+  }
+
+  client = tor_malloc_zero(sizeof(hs_service_authorized_client_t));
+  if (base32_decode((char *) client->client_pk.public_key,
+                    sizeof(client->client_pk.public_key),
+                    key_str, strlen(key_str)) !=
+      sizeof(client->client_pk.public_key)) {
+    log_warn(LD_REND, "Client authorization public key cannot be decoded: %s",
+             key_str);
+    goto err;
+  }
+
+  return client;
+
+ err:
+  if (client != NULL) {
+    tor_free(client);
+  }
+  return NULL;
+}
+
 /** Parse an authorized client from a string. The format of a client string
  * looks like (see rend-spec-v3.txt):
  *
@@ -1161,23 +1198,7 @@ parse_authorized_client(const char *client_key_str)
     goto err;
   }
 
-  /* We expect a specific length of the base32 encoded key so make sure we
-   * have that so we don't successfully decode a value with a different length
-   * and end up in trouble when copying the decoded key into a fixed length
-   * buffer. */
-  if (strlen(pubkey_b32) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
-    log_warn(LD_REND, "Client authorization encoded base32 public key "
-                      "length is invalid: %s", pubkey_b32);
-    goto err;
-  }
-
-  client = tor_malloc_zero(sizeof(hs_service_authorized_client_t));
-  if (base32_decode((char *) client->client_pk.public_key,
-                    sizeof(client->client_pk.public_key),
-                    pubkey_b32, strlen(pubkey_b32)) !=
-      sizeof(client->client_pk.public_key)) {
-    log_warn(LD_REND, "Client authorization public key cannot be decoded: %s",
-             pubkey_b32);
+  if ((client = parse_authorized_client_key(pubkey_b32)) == NULL) {
     goto err;
   }
 
@@ -1301,7 +1322,7 @@ load_client_keys(hs_service_t *service)
 }
 
 /** Release all storage held in <b>client</b>. */
-STATIC void
+void
 service_authorized_client_free_(hs_service_authorized_client_t *client)
 {
   if (!client) {
@@ -3687,7 +3708,8 @@ hs_service_upload_desc_to_dir(const char *encoded_desc,
 hs_service_add_ephemeral_status_t
 hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
                          int max_streams_per_rdv_circuit,
-                         int max_streams_close_circuit, char **address_out)
+                         int max_streams_close_circuit,
+                         smartlist_t *auth_clients_v3, char **address_out)
 {
   hs_service_add_ephemeral_status_t ret;
   hs_service_t *service = NULL;
@@ -3731,6 +3753,13 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
     goto err;
   }
 
+  if (service->config.clients == NULL) {
+    service->config.clients = smartlist_new();
+  }
+  SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c,
+    smartlist_add(service->config.clients, c));
+
+
   /* Build the onion address for logging purposes but also the control port
    * uses it for the HS_DESC event. */
   hs_build_address(&service->keys.identity_pk,
diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h
index ec0e83f2c2..4d49929127 100644
--- a/src/feature/hs/hs_service.h
+++ b/src/feature/hs/hs_service.h
@@ -372,7 +372,8 @@ char *hs_service_lookup_current_desc(const ed25519_public_key_t *pk);
 hs_service_add_ephemeral_status_t
 hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
                          int max_streams_per_rdv_circuit,
-                         int max_streams_close_circuit, char **address_out);
+                         int max_streams_close_circuit,
+                         smartlist_t *auth_clients_v3, char **address_out);
 int hs_service_del_ephemeral(const char *address);
 
 /* Used outside of the HS subsystem by the control port command HSPOST. */
@@ -388,6 +389,15 @@ hs_service_exports_circuit_id(const ed25519_public_key_t *pk);
 void hs_service_dump_stats(int severity);
 void hs_service_circuit_cleanup_on_close(const circuit_t *circ);
 
+hs_service_authorized_client_t *
+parse_authorized_client_key(const char *key_str);
+
+void
+service_authorized_client_free_(hs_service_authorized_client_t *client);
+#define service_authorized_client_free(c) \
+  FREE_AND_NULL(hs_service_authorized_client_t, \
+                           service_authorized_client_free_, (c))
+
 #ifdef HS_SERVICE_PRIVATE
 
 #ifdef TOR_UNIT_TESTS
@@ -452,12 +462,6 @@ STATIC void service_descriptor_free_(hs_service_descriptor_t *desc);
   FREE_AND_NULL(hs_service_descriptor_t, \
                            service_descriptor_free_, (d))
 
-STATIC void
-service_authorized_client_free_(hs_service_authorized_client_t *client);
-#define service_authorized_client_free(c) \
-  FREE_AND_NULL(hs_service_authorized_client_t, \
-                           service_authorized_client_free_, (c))
-
 STATIC int
 write_address_to_file(const hs_service_t *service, const char *fname_);
 

From 3057671a380773167f5e8efe7b3860b88015eeab Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Sun, 15 Nov 2020 14:33:43 -0800
Subject: [PATCH 2/9] Add changes file for ticket 40084

---
 changes/ticket40084 | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 changes/ticket40084

diff --git a/changes/ticket40084 b/changes/ticket40084
new file mode 100644
index 0000000000..072af813f1
--- /dev/null
+++ b/changes/ticket40084
@@ -0,0 +1,4 @@
+  o Major features (control port, onion services):
+    - Add support for creating version 3 onion services with authorization
+      from the control port. Previously, we could only create version 2
+      services here. Closes ticket 40084. Patch by Neel Chauhan.

From af48afe667388ce1559b560c64e6416b414ada3e Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Mon, 16 Nov 2020 08:47:12 -0800
Subject: [PATCH 3/9] Unbreak build

---
 src/feature/rend/rendcommon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/feature/rend/rendcommon.c b/src/feature/rend/rendcommon.c
index 775d487805..f00dfee68a 100644
--- a/src/feature/rend/rendcommon.c
+++ b/src/feature/rend/rendcommon.c
@@ -548,6 +548,8 @@ rend_encode_v2_descriptors(smartlist_t *descs_out,
         ipos = ipos_encrypted;
         ipos_len = ipos_encrypted_len;
         break;
+      case REND_V3_AUTH:
+        break; /* v3 service, break. */
       default:
         log_warn(LD_REND|LD_BUG, "Unrecognized authorization type %d",
                  (int)auth_type);

From 157fe4597e5876cb7af4f4f467db1ffaff4bd9ce Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Thu, 19 Nov 2020 11:27:25 -0800
Subject: [PATCH 4/9] Add tests for bug #40084

---
 src/feature/control/control_cmd.c |  3 +-
 src/feature/control/control_cmd.h | 11 +++++
 src/feature/hs/hs_service.c       |  8 ++--
 src/feature/rend/rendservice.c    |  1 +
 src/test/test_hs_control.c        | 78 ++++++++++++++++++++++++++++++-
 5 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 4b02b1c5c0..739577c506 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -1649,7 +1649,7 @@ handle_control_hspost(control_connection_t *conn,
  * On success (RSAE_OKAY), the address_out points to a newly allocated string
  * containing the onion address without the .onion part. On error, address_out
  * is untouched. */
-static hs_service_add_ephemeral_status_t
+STATIC hs_service_add_ephemeral_status_t
 add_onion_helper_add_service(int hs_version,
                              add_onion_secret_key_t *pk,
                              smartlist_t *port_cfgs, int max_streams,
@@ -2010,7 +2010,6 @@ handle_control_add_onion(control_connection_t *conn,
     smartlist_free(auth_clients_v3_str);
   }
 
-
   if (auth_created_clients) {
     // Do not free entries; they are the same as auth_clients
     smartlist_free(auth_created_clients);
diff --git a/src/feature/control/control_cmd.h b/src/feature/control/control_cmd.h
index 0ff0f0755f..56ffb9f661 100644
--- a/src/feature/control/control_cmd.h
+++ b/src/feature/control/control_cmd.h
@@ -103,6 +103,17 @@ STATIC control_cmd_args_t *control_cmd_parse_args(
                                    size_t body_len,
                                    const char *body,
                                    char **error_out);
+#ifdef TOR_UNIT_TESTS
+#include "feature/hs/hs_common.h"
+
+STATIC hs_service_add_ephemeral_status_t
+add_onion_helper_add_service(int hs_version,
+                             add_onion_secret_key_t *pk,
+                             smartlist_t *port_cfgs, int max_streams,
+                             int max_streams_close_circuit, int auth_type,
+                             smartlist_t *auth_clients,
+                             smartlist_t *auth_clients_v3, char **address_out);
+#endif /* defined(TOR_UNIT_TESTS) */
 
 #endif /* defined(CONTROL_CMD_PRIVATE) */
 
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 0d7441ada2..53b90ce374 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -3756,9 +3756,11 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
   if (service->config.clients == NULL) {
     service->config.clients = smartlist_new();
   }
-  SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c,
-    smartlist_add(service->config.clients, c));
-
+  SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, {
+    if (c != NULL) {
+      smartlist_add(service->config.clients, c);
+    }
+  });
 
   /* Build the onion address for logging purposes but also the control port
    * uses it for the HS_DESC event. */
diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c
index a2be900e2a..45b1d3d822 100644
--- a/src/feature/rend/rendservice.c
+++ b/src/feature/rend/rendservice.c
@@ -3818,6 +3818,7 @@ upload_service_descriptor(rend_service_t *service)
       smartlist_clear(client_cookies);
       switch (service->auth_type) {
         case REND_NO_AUTH:
+	case REND_V3_AUTH:
           /* Do nothing here. */
           break;
         case REND_BASIC_AUTH:
diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c
index 6e41c4994f..e1a5ab4841 100644
--- a/src/test/test_hs_control.c
+++ b/src/test/test_hs_control.c
@@ -7,15 +7,17 @@
  **/
 
 #define CONTROL_EVENTS_PRIVATE
+#define CONTROL_CMD_PRIVATE
 #define HS_CLIENT_PRIVATE
+#define HS_SERVICE_PRIVATE
 
 #include "core/or/or.h"
 #include "test/test.h"
 #include "test/test_helpers.h"
 #include "core/mainloop/connection.h"
 #include "feature/control/control.h"
-#include "feature/control/control_events.h"
 #include "feature/control/control_cmd.h"
+#include "feature/control/control_events.h"
 #include "feature/control/control_fmt.h"
 #include "feature/control/control_connection_st.h"
 #include "app/config/config.h"
@@ -23,9 +25,11 @@
 #include "feature/hs/hs_client.h"
 #include "feature/hs/hs_control.h"
 #include "feature/nodelist/nodelist.h"
+#include "feature/rend/rendservice.h"
 
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerstatus_st.h"
+#include "lib/container/smartlist.h"
 #include "lib/crypt_ops/crypto_format.h"
 
 #ifdef HAVE_SYS_STAT_H
@@ -735,6 +739,76 @@ test_hs_control_add_onion_with_bad_pubkey(void *arg)
   tor_free(conn.current_cmd);
 }
 
+/** Test that add_onion_helper_add_service can add the service. */
+static void
+test_hs_add_onion_helper_add_service(void *arg)
+{
+  int hs_version_good, hs_version_bad;
+  add_onion_secret_key_t sk_good, sk_bad;
+  ed25519_public_key_t pk_good, pk_bad;
+  char *key_new_blob_good = NULL, *key_new_blob_bad = NULL;
+  const char *key_new_alg_good = NULL, *key_new_alg_bad = NULL;
+  hs_service_authorized_client_t *client_good, *client_bad;
+  smartlist_t *list_v2, *list_good, *list_bad;
+  hs_service_ht *global_map;
+  rend_service_port_config_t *portcfg;
+  smartlist_t *portcfgs;
+  char *address_out_good, *address_out_bad;
+
+  (void) arg;
+
+  hs_init();
+  global_map = get_hs_service_map();
+
+  portcfg = rend_service_parse_port_config("8080", ",", NULL);
+  portcfgs = smartlist_new();
+  smartlist_add(portcfgs, portcfg);
+
+  memset(&sk_good, 0, sizeof(sk_good));
+  memset(&sk_bad, 0, sizeof(sk_bad));
+
+  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_good,
+                         &key_new_blob_good, &sk_good, &hs_version_good, NULL);
+  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_bad,
+                         &key_new_blob_bad, &sk_bad, &hs_version_bad, NULL);
+
+  ed25519_public_key_generate(&pk_good, sk_good.v3);
+  ed25519_public_key_generate(&pk_bad, sk_bad.v3);
+
+  client_good = parse_authorized_client_key(
+                       "N2NU7BSRL6YODZCYPN4CREB54TYLKGIE2KYOQWLFYC23ZJVCE5DQ");
+  client_bad = parse_authorized_client_key("dummy");
+
+  list_v2 = smartlist_new();
+  list_good = smartlist_new();
+  smartlist_add(list_good, client_good);
+  list_bad = smartlist_new();
+  smartlist_add(list_bad, client_bad);
+
+  add_onion_helper_add_service(HS_VERSION_THREE, &sk_good, portcfgs, 1, 1,
+                          REND_V3_AUTH, list_v2, list_good, &address_out_good);
+  add_onion_helper_add_service(HS_VERSION_THREE, &sk_bad, portcfgs, 1, 1,
+                          REND_V3_AUTH, list_v2, list_bad, &address_out_bad);
+
+  hs_service_t *srv_good = find_service(global_map, &pk_good);
+  hs_service_t *srv_bad = find_service(global_map, &pk_bad);
+
+  tt_int_op(smartlist_len(srv_good->config.clients), OP_EQ, 1);
+  tt_int_op(smartlist_len(srv_bad->config.clients), OP_EQ, 0);
+
+ done:
+  tor_free(key_new_blob_good);
+  tor_free(key_new_blob_bad);
+  tor_free(address_out_good);
+  tor_free(address_out_bad);
+
+  service_authorized_client_free(client_good);
+
+  smartlist_free(list_v2);
+  smartlist_free(list_good);
+  smartlist_free(list_bad);
+}
+
 struct testcase_t hs_control_tests[] = {
   { "hs_desc_event", test_hs_desc_event, TT_FORK,
     NULL, NULL },
@@ -748,6 +822,8 @@ struct testcase_t hs_control_tests[] = {
     test_hs_control_store_permanent_creds, TT_FORK, NULL, NULL },
   { "hs_control_add_onion_with_bad_pubkey",
     test_hs_control_add_onion_with_bad_pubkey, TT_FORK, NULL, NULL },
+  { "hs_add_onion_helper_add_service",
+    test_hs_add_onion_helper_add_service, TT_FORK, NULL, NULL},
 
   END_OF_TESTCASES
 };

From be6db23d1d4ce1185a7263f8554978e0fb9ea821 Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Tue, 24 Nov 2020 19:05:27 -0800
Subject: [PATCH 5/9] Some test and logic corrections

---
 src/feature/control/control_cmd.c |  4 +-
 src/feature/hs/hs_service.c       | 32 ++++++-----
 src/feature/hs/hs_service.h       |  2 +-
 src/feature/rend/rendservice.c    |  2 +-
 src/test/test_hs_control.c        | 88 +++++++++++--------------------
 5 files changed, 54 insertions(+), 74 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 739577c506..8df9598c9f 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -1831,8 +1831,9 @@ handle_control_add_onion(control_connection_t *conn,
       }
     } else if (!strcasecmp(arg->key, "ClientAuthV3")) {
       hs_service_authorized_client_t *client_v3 =
-                                       parse_authorized_client_key(arg->value);
+                                parse_authorized_client_key(arg->value, false);
       if (!client_v3) {
+        control_write_endreply(conn, 512, "Cannot decode v3 client auth key");
         goto out;
       }
 
@@ -1925,7 +1926,6 @@ handle_control_add_onion(control_connection_t *conn,
                                    auth_clients, auth_clients_v3, &service_id);
   port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */
   auth_clients = NULL; /* so is auth_clients */
-  auth_clients_v3 = NULL; /* so is auth_clients_v3 */
   switch (ret) {
   case RSAE_OKAY:
   {
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 53b90ce374..c173dbcbfe 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -1119,17 +1119,19 @@ client_filename_is_valid(const char *filename)
  *
  * Return the key on success, return NULL, otherwise. */
 hs_service_authorized_client_t *
-parse_authorized_client_key(const char *key_str)
+parse_authorized_client_key(const char *key_str, bool log)
 {
   hs_service_authorized_client_t *client = NULL;
 
-  /* We expect a specific length of the base32 encoded key so make sure we
+  /* We expect a specific length of the base64 encoded key so make sure we
    * have that so we don't successfully decode a value with a different length
    * and end up in trouble when copying the decoded key into a fixed length
    * buffer. */
   if (strlen(key_str) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
-    log_warn(LD_REND, "Client authorization encoded base32 public key "
-                      "length is invalid: %s", key_str);
+    if (log) {
+      log_warn(LD_REND, "Client authorization encoded base32 public key "
+                        "length is invalid: %s", key_str);
+    }
     goto err;
   }
 
@@ -1138,8 +1140,10 @@ parse_authorized_client_key(const char *key_str)
                     sizeof(client->client_pk.public_key),
                     key_str, strlen(key_str)) !=
       sizeof(client->client_pk.public_key)) {
-    log_warn(LD_REND, "Client authorization public key cannot be decoded: %s",
-             key_str);
+    if (log) {
+      log_warn(LD_REND, "Client authorization public key cannot be decoded: "
+               "%s", key_str);
+    }
     goto err;
   }
 
@@ -1198,7 +1202,7 @@ parse_authorized_client(const char *client_key_str)
     goto err;
   }
 
-  if ((client = parse_authorized_client_key(pubkey_b32)) == NULL) {
+  if ((client = parse_authorized_client_key(pubkey_b32, true)) == NULL) {
     goto err;
   }
 
@@ -3753,14 +3757,14 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
     goto err;
   }
 
-  if (service->config.clients == NULL) {
-    service->config.clients = smartlist_new();
-  }
-  SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, {
-    if (c != NULL) {
-      smartlist_add(service->config.clients, c);
+  if (auth_clients_v3) {
+    if (service->config.clients == NULL) {
+      service->config.clients = smartlist_new();
     }
-  });
+    SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, {
+      smartlist_add(service->config.clients, c);
+    });
+  }
 
   /* Build the onion address for logging purposes but also the control port
    * uses it for the HS_DESC event. */
diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h
index 4d49929127..12698a483c 100644
--- a/src/feature/hs/hs_service.h
+++ b/src/feature/hs/hs_service.h
@@ -390,7 +390,7 @@ void hs_service_dump_stats(int severity);
 void hs_service_circuit_cleanup_on_close(const circuit_t *circ);
 
 hs_service_authorized_client_t *
-parse_authorized_client_key(const char *key_str);
+parse_authorized_client_key(const char *key_str, bool log);
 
 void
 service_authorized_client_free_(hs_service_authorized_client_t *client);
diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c
index 45b1d3d822..add25579b3 100644
--- a/src/feature/rend/rendservice.c
+++ b/src/feature/rend/rendservice.c
@@ -3818,7 +3818,7 @@ upload_service_descriptor(rend_service_t *service)
       smartlist_clear(client_cookies);
       switch (service->auth_type) {
         case REND_NO_AUTH:
-	case REND_V3_AUTH:
+        case REND_V3_AUTH:
           /* Do nothing here. */
           break;
         case REND_BASIC_AUTH:
diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c
index e1a5ab4841..e5401b4ce7 100644
--- a/src/test/test_hs_control.c
+++ b/src/test/test_hs_control.c
@@ -743,70 +743,46 @@ test_hs_control_add_onion_with_bad_pubkey(void *arg)
 static void
 test_hs_add_onion_helper_add_service(void *arg)
 {
-  int hs_version_good, hs_version_bad;
-  add_onion_secret_key_t sk_good, sk_bad;
-  ed25519_public_key_t pk_good, pk_bad;
-  char *key_new_blob_good = NULL, *key_new_blob_bad = NULL;
-  const char *key_new_alg_good = NULL, *key_new_alg_bad = NULL;
-  hs_service_authorized_client_t *client_good, *client_bad;
-  smartlist_t *list_v2, *list_good, *list_bad;
-  hs_service_ht *global_map;
-  rend_service_port_config_t *portcfg;
-  smartlist_t *portcfgs;
-  char *address_out_good, *address_out_bad;
+  control_connection_t conn;
+  char *args = NULL, *cp1 = NULL;
+  size_t sz;
 
   (void) arg;
 
   hs_init();
-  global_map = get_hs_service_map();
 
-  portcfg = rend_service_parse_port_config("8080", ",", NULL);
-  portcfgs = smartlist_new();
-  smartlist_add(portcfgs, portcfg);
+  memset(&conn, 0, sizeof(control_connection_t));
+  TO_CONN(&conn)->outbuf = buf_new();
+  conn.current_cmd = tor_strdup("ADD_ONION");
+  args = tor_strdup("ED25519-V3:KLMQ4CLKwlDCHuMPn8j3od33cU5LhnrLNoZh7CWChl3VkY"
+    "pNAkeP5dGW8xeKR9HxQBWQ/w7Kr12lA/U8Pd/oxw== "
+    "ClientAuthV3=dz4q5xqlb4ldnbs72iarrml4ephk3du4i7o2cgiva5lwr6wkquja "
+    "Flags=V3Auth Port=9735,127.0.0.1");
+  handle_control_command(&conn, (uint32_t) strlen(args), args);
+  cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz);
+  tt_str_op(cp1, OP_EQ,
+   "250-ServiceID=n35etu3yjxrqjpntmfziom5sjwspoydchmelc4xleoy4jk2u4lziz2yd\r\n"
+   "250-ClientAuthV3=dz4q5xqlb4ldnbs72iarrml4ephk3du4i7o2cgiva5lwr6wkquja\r\n"
+   "250 OK\r\n");
+  tor_free(args);
+  tor_free(cp1);
 
-  memset(&sk_good, 0, sizeof(sk_good));
-  memset(&sk_bad, 0, sizeof(sk_bad));
-
-  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_good,
-                         &key_new_blob_good, &sk_good, &hs_version_good, NULL);
-  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_bad,
-                         &key_new_blob_bad, &sk_bad, &hs_version_bad, NULL);
-
-  ed25519_public_key_generate(&pk_good, sk_good.v3);
-  ed25519_public_key_generate(&pk_bad, sk_bad.v3);
-
-  client_good = parse_authorized_client_key(
-                       "N2NU7BSRL6YODZCYPN4CREB54TYLKGIE2KYOQWLFYC23ZJVCE5DQ");
-  client_bad = parse_authorized_client_key("dummy");
-
-  list_v2 = smartlist_new();
-  list_good = smartlist_new();
-  smartlist_add(list_good, client_good);
-  list_bad = smartlist_new();
-  smartlist_add(list_bad, client_bad);
-
-  add_onion_helper_add_service(HS_VERSION_THREE, &sk_good, portcfgs, 1, 1,
-                          REND_V3_AUTH, list_v2, list_good, &address_out_good);
-  add_onion_helper_add_service(HS_VERSION_THREE, &sk_bad, portcfgs, 1, 1,
-                          REND_V3_AUTH, list_v2, list_bad, &address_out_bad);
-
-  hs_service_t *srv_good = find_service(global_map, &pk_good);
-  hs_service_t *srv_bad = find_service(global_map, &pk_bad);
-
-  tt_int_op(smartlist_len(srv_good->config.clients), OP_EQ, 1);
-  tt_int_op(smartlist_len(srv_bad->config.clients), OP_EQ, 0);
+  args = tor_strdup("ED25519-V3:iIU8EBi71qE7G6UTsROU1kWN0JMrRP/YukC0Xk5WLGyil3"
+    "gm4u3wEBXr+/TaCpXS+65Pcdqz+PG+4+oWHLN05A== "
+    "ClientAuthV3=dummy Flags=V3Auth Port=9735,127.0.0.1");
+  handle_control_command(&conn, (uint32_t) strlen(args), args);
+  cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz);
+  tt_str_op(cp1, OP_EQ, "512 Cannot decode v3 client auth key\r\n");
 
  done:
-  tor_free(key_new_blob_good);
-  tor_free(key_new_blob_bad);
-  tor_free(address_out_good);
-  tor_free(address_out_bad);
-
-  service_authorized_client_free(client_good);
-
-  smartlist_free(list_v2);
-  smartlist_free(list_good);
-  smartlist_free(list_bad);
+  tor_free(args);
+  tor_free(cp1);
+  tor_free(conn.current_cmd);
+  buf_free(TO_CONN(&conn)->outbuf);
+  SMARTLIST_FOREACH(conn.ephemeral_onion_services, char *,
+                    service, tor_free(service));
+  smartlist_free(conn.ephemeral_onion_services);
+  hs_client_free_all();
 }
 
 struct testcase_t hs_control_tests[] = {

From 7d54734900e3a11f007d9fdff1188be7cc2e7a01 Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Thu, 3 Dec 2020 17:40:55 -0800
Subject: [PATCH 6/9] More logic corrections

---
 src/feature/control/control_cmd.c |  5 +++--
 src/feature/control/control_cmd.h | 11 -----------
 src/feature/hs/hs_service.c       | 23 ++++++++---------------
 src/feature/hs/hs_service.h       |  2 +-
 4 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 8df9598c9f..4dccf2b249 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -1649,7 +1649,7 @@ handle_control_hspost(control_connection_t *conn,
  * On success (RSAE_OKAY), the address_out points to a newly allocated string
  * containing the onion address without the .onion part. On error, address_out
  * is untouched. */
-STATIC hs_service_add_ephemeral_status_t
+static hs_service_add_ephemeral_status_t
 add_onion_helper_add_service(int hs_version,
                              add_onion_secret_key_t *pk,
                              smartlist_t *port_cfgs, int max_streams,
@@ -1831,7 +1831,7 @@ handle_control_add_onion(control_connection_t *conn,
       }
     } else if (!strcasecmp(arg->key, "ClientAuthV3")) {
       hs_service_authorized_client_t *client_v3 =
-                                parse_authorized_client_key(arg->value, false);
+                             parse_authorized_client_key(arg->value, LOG_INFO);
       if (!client_v3) {
         control_write_endreply(conn, 512, "Cannot decode v3 client auth key");
         goto out;
@@ -1926,6 +1926,7 @@ handle_control_add_onion(control_connection_t *conn,
                                    auth_clients, auth_clients_v3, &service_id);
   port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */
   auth_clients = NULL; /* so is auth_clients */
+  auth_clients_v3 = NULL; /* so is auth_clients_v3 */
   switch (ret) {
   case RSAE_OKAY:
   {
diff --git a/src/feature/control/control_cmd.h b/src/feature/control/control_cmd.h
index 56ffb9f661..0ff0f0755f 100644
--- a/src/feature/control/control_cmd.h
+++ b/src/feature/control/control_cmd.h
@@ -103,17 +103,6 @@ STATIC control_cmd_args_t *control_cmd_parse_args(
                                    size_t body_len,
                                    const char *body,
                                    char **error_out);
-#ifdef TOR_UNIT_TESTS
-#include "feature/hs/hs_common.h"
-
-STATIC hs_service_add_ephemeral_status_t
-add_onion_helper_add_service(int hs_version,
-                             add_onion_secret_key_t *pk,
-                             smartlist_t *port_cfgs, int max_streams,
-                             int max_streams_close_circuit, int auth_type,
-                             smartlist_t *auth_clients,
-                             smartlist_t *auth_clients_v3, char **address_out);
-#endif /* defined(TOR_UNIT_TESTS) */
 
 #endif /* defined(CONTROL_CMD_PRIVATE) */
 
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index c173dbcbfe..aaf5833a87 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -1119,7 +1119,7 @@ client_filename_is_valid(const char *filename)
  *
  * Return the key on success, return NULL, otherwise. */
 hs_service_authorized_client_t *
-parse_authorized_client_key(const char *key_str, bool log)
+parse_authorized_client_key(const char *key_str, int severity)
 {
   hs_service_authorized_client_t *client = NULL;
 
@@ -1128,10 +1128,8 @@ parse_authorized_client_key(const char *key_str, bool log)
    * and end up in trouble when copying the decoded key into a fixed length
    * buffer. */
   if (strlen(key_str) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
-    if (log) {
-      log_warn(LD_REND, "Client authorization encoded base32 public key "
-                        "length is invalid: %s", key_str);
-    }
+    log_fn(severity, LD_REND, "Client authorization encoded base32 public key "
+                              "length is invalid: %s", key_str);
     goto err;
   }
 
@@ -1140,10 +1138,8 @@ parse_authorized_client_key(const char *key_str, bool log)
                     sizeof(client->client_pk.public_key),
                     key_str, strlen(key_str)) !=
       sizeof(client->client_pk.public_key)) {
-    if (log) {
-      log_warn(LD_REND, "Client authorization public key cannot be decoded: "
-               "%s", key_str);
-    }
+    log_fn(severity, LD_REND, "Client authorization public key cannot be "
+             "decoded: %s", key_str);
     goto err;
   }
 
@@ -1151,7 +1147,7 @@ parse_authorized_client_key(const char *key_str, bool log)
 
  err:
   if (client != NULL) {
-    tor_free(client);
+    service_authorized_client_free(client);
   }
   return NULL;
 }
@@ -1202,7 +1198,7 @@ parse_authorized_client(const char *client_key_str)
     goto err;
   }
 
-  if ((client = parse_authorized_client_key(pubkey_b32, true)) == NULL) {
+  if ((client = parse_authorized_client_key(pubkey_b32, LOG_WARN)) == NULL) {
     goto err;
   }
 
@@ -3759,11 +3755,8 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
 
   if (auth_clients_v3) {
     if (service->config.clients == NULL) {
-      service->config.clients = smartlist_new();
+      service->config.clients = auth_clients_v3;
     }
-    SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, {
-      smartlist_add(service->config.clients, c);
-    });
   }
 
   /* Build the onion address for logging purposes but also the control port
diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h
index 12698a483c..6a39dee037 100644
--- a/src/feature/hs/hs_service.h
+++ b/src/feature/hs/hs_service.h
@@ -390,7 +390,7 @@ void hs_service_dump_stats(int severity);
 void hs_service_circuit_cleanup_on_close(const circuit_t *circ);
 
 hs_service_authorized_client_t *
-parse_authorized_client_key(const char *key_str, bool log);
+parse_authorized_client_key(const char *key_str, int severity);
 
 void
 service_authorized_client_free_(hs_service_authorized_client_t *client);

From 65d60a16d96ad6e7f824225e3b9b109783575379 Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Tue, 8 Dec 2020 10:46:44 -0800
Subject: [PATCH 7/9] Remove unused NULL check in hs_service_add_ephemeral(),
 mention we take ownership of auth_clients_v3

---
 src/feature/hs/hs_service.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index aaf5833a87..eb4e3c2b78 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -3700,9 +3700,10 @@ hs_service_upload_desc_to_dir(const char *encoded_desc,
 /** Add the ephemeral service using the secret key sk and ports. Both max
  * streams parameter will be set in the newly created service.
  *
- * Ownership of sk and ports is passed to this routine.  Regardless of
- * success/failure, callers should not touch these values after calling this
- * routine, and may assume that correct cleanup has been done on failure.
+ * Ownership of sk, ports, and auth_clients_v3 is passed to this routine.
+ * Regardless of success/failure, callers should not touch these values
+ * after calling this routine, and may assume that correct cleanup has
+ * been done on failure.
  *
  * Return an appropriate hs_service_add_ephemeral_status_t. */
 hs_service_add_ephemeral_status_t
@@ -3754,9 +3755,7 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
   }
 
   if (auth_clients_v3) {
-    if (service->config.clients == NULL) {
-      service->config.clients = auth_clients_v3;
-    }
+    service->config.clients = auth_clients_v3;
   }
 
   /* Build the onion address for logging purposes but also the control port

From 8a2910461baffbf4c83905776ec2f0aa7abe23a3 Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Tue, 8 Dec 2020 11:24:27 -0800
Subject: [PATCH 8/9] Reinstate add_onion_helper_add_service() test, validate
 auth clients before adding them

---
 src/feature/control/control_cmd.c |  2 +-
 src/feature/control/control_cmd.h |  9 ++++
 src/feature/hs/hs_service.c       |  8 +++-
 src/test/test_hs_control.c        | 80 +++++++++++++++++++++++++++++--
 4 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 4dccf2b249..32c87c6daa 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -1649,7 +1649,7 @@ handle_control_hspost(control_connection_t *conn,
  * On success (RSAE_OKAY), the address_out points to a newly allocated string
  * containing the onion address without the .onion part. On error, address_out
  * is untouched. */
-static hs_service_add_ephemeral_status_t
+STATIC hs_service_add_ephemeral_status_t
 add_onion_helper_add_service(int hs_version,
                              add_onion_secret_key_t *pk,
                              smartlist_t *port_cfgs, int max_streams,
diff --git a/src/feature/control/control_cmd.h b/src/feature/control/control_cmd.h
index 0ff0f0755f..b3c1d5cb2f 100644
--- a/src/feature/control/control_cmd.h
+++ b/src/feature/control/control_cmd.h
@@ -75,6 +75,7 @@ typedef struct control_cmd_syntax_t {
 } control_cmd_syntax_t;
 
 #ifdef CONTROL_CMD_PRIVATE
+#include "feature/hs/hs_service.h"
 #include "lib/crypt_ops/crypto_ed25519.h"
 
 /* ADD_ONION secret key to create an ephemeral service. The command supports
@@ -94,6 +95,14 @@ STATIC int add_onion_helper_keyarg(const char *arg, int discard_pk,
                                    int *hs_version,
                                    control_connection_t *conn);
 
+STATIC hs_service_add_ephemeral_status_t add_onion_helper_add_service(
+                             int hs_version,
+                             add_onion_secret_key_t *pk,
+                             smartlist_t *port_cfgs, int max_streams,
+                             int max_streams_close_circuit, int auth_type,
+                             smartlist_t *auth_clients,
+                             smartlist_t *auth_clients_v3, char **address_out);
+
 STATIC rend_authorized_client_t *add_onion_helper_clientauth(const char *arg,
                                    int *created, control_connection_t *conn);
 
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index eb4e3c2b78..449870d3ba 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -3755,7 +3755,13 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
   }
 
   if (auth_clients_v3) {
-    service->config.clients = auth_clients_v3;
+    service->config.clients = smartlist_new();
+    SMARTLIST_FOREACH(auth_clients_v3, hs_service_authorized_client_t *, c, {
+      if (c != NULL) {
+        smartlist_add(service->config.clients, c);
+      }
+    });
+    smartlist_free(auth_clients_v3);
   }
 
   /* Build the onion address for logging purposes but also the control port
diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c
index e5401b4ce7..5788fa2a95 100644
--- a/src/test/test_hs_control.c
+++ b/src/test/test_hs_control.c
@@ -739,9 +739,9 @@ test_hs_control_add_onion_with_bad_pubkey(void *arg)
   tor_free(conn.current_cmd);
 }
 
-/** Test that add_onion_helper_add_service can add the service. */
+/** Test that we can add the service via the control port. */
 static void
-test_hs_add_onion_helper_add_service(void *arg)
+test_hs_control_add_auth_onion_service(void *arg)
 {
   control_connection_t conn;
   char *args = NULL, *cp1 = NULL;
@@ -785,6 +785,76 @@ test_hs_add_onion_helper_add_service(void *arg)
   hs_client_free_all();
 }
 
+/** Test that add_onion_helper_add_service can add the service. */
+static void
+test_hs_control_add_onion_helper_add_service(void *arg)
+{
+  int hs_version_good, hs_version_bad;
+  add_onion_secret_key_t sk_good, sk_bad;
+  ed25519_public_key_t pk_good, pk_bad;
+  char *key_new_blob_good = NULL, *key_new_blob_bad = NULL;
+  const char *key_new_alg_good = NULL, *key_new_alg_bad = NULL;
+  hs_service_authorized_client_t *client_good, *client_bad;
+  smartlist_t *list_v2, *list_good, *list_bad;
+  hs_service_ht *global_map;
+  rend_service_port_config_t *portcfg;
+  smartlist_t *portcfgs;
+  char *address_out_good, *address_out_bad;
+
+  (void) arg;
+
+  hs_init();
+  global_map = get_hs_service_map();
+
+  portcfg = rend_service_parse_port_config("8080", ",", NULL);
+  portcfgs = smartlist_new();
+  smartlist_add(portcfgs, portcfg);
+
+  memset(&sk_good, 0, sizeof(sk_good));
+  memset(&sk_bad, 0, sizeof(sk_bad));
+
+  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_good,
+                         &key_new_blob_good, &sk_good, &hs_version_good, NULL);
+  add_onion_helper_keyarg("NEW:ED25519-V3", 0, &key_new_alg_bad,
+                         &key_new_blob_bad, &sk_bad, &hs_version_bad, NULL);
+
+  ed25519_public_key_generate(&pk_good, sk_good.v3);
+  ed25519_public_key_generate(&pk_bad, sk_bad.v3);
+
+  client_good = parse_authorized_client_key(
+            "N2NU7BSRL6YODZCYPN4CREB54TYLKGIE2KYOQWLFYC23ZJVCE5DQ", LOG_INFO);
+  client_bad = parse_authorized_client_key("dummy", LOG_INFO);
+
+  list_v2 = smartlist_new();
+  list_good = smartlist_new();
+  smartlist_add(list_good, client_good);
+  list_bad = smartlist_new();
+  smartlist_add(list_bad, client_bad);
+
+  add_onion_helper_add_service(HS_VERSION_THREE, &sk_good, portcfgs, 1, 1,
+                          REND_V3_AUTH, list_v2, list_good, &address_out_good);
+  add_onion_helper_add_service(HS_VERSION_THREE, &sk_bad, portcfgs, 1, 1,
+                          REND_V3_AUTH, list_v2, list_bad, &address_out_bad);
+
+  hs_service_t *srv_good = find_service(global_map, &pk_good);
+  hs_service_t *srv_bad = find_service(global_map, &pk_bad);
+
+  tt_int_op(smartlist_len(srv_good->config.clients), OP_EQ, 1);
+  tt_int_op(smartlist_len(srv_bad->config.clients), OP_EQ, 0);
+
+ done:
+  tor_free(key_new_blob_good);
+  tor_free(key_new_blob_bad);
+  tor_free(address_out_good);
+  tor_free(address_out_bad);
+
+  service_authorized_client_free(client_good);
+
+  smartlist_free(list_v2);
+  smartlist_free(list_good);
+  smartlist_free(list_bad);
+}
+
 struct testcase_t hs_control_tests[] = {
   { "hs_desc_event", test_hs_desc_event, TT_FORK,
     NULL, NULL },
@@ -798,8 +868,10 @@ struct testcase_t hs_control_tests[] = {
     test_hs_control_store_permanent_creds, TT_FORK, NULL, NULL },
   { "hs_control_add_onion_with_bad_pubkey",
     test_hs_control_add_onion_with_bad_pubkey, TT_FORK, NULL, NULL },
-  { "hs_add_onion_helper_add_service",
-    test_hs_add_onion_helper_add_service, TT_FORK, NULL, NULL},
+  { "hs_control_add_auth_onion_service",
+    test_hs_control_add_auth_onion_service, TT_FORK, NULL, NULL},
+  { "hs_control_add_onion_helper_add_service",
+    test_hs_control_add_onion_helper_add_service, TT_FORK, NULL, NULL},
 
   END_OF_TESTCASES
 };

From 22f55fdb2a8455b344ae094ee2706710a2b4d941 Mon Sep 17 00:00:00 2001
From: Neel Chauhan <neel@neelc.org>
Date: Wed, 3 Feb 2021 10:25:46 -0800
Subject: [PATCH 9/9] Document REND_V3_AUTH flag

---
 src/core/or/or.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/core/or/or.h b/src/core/or/or.h
index 646dbf2c3a..182ebc48eb 100644
--- a/src/core/or/or.h
+++ b/src/core/or/or.h
@@ -404,7 +404,8 @@ typedef enum rend_auth_type_t {
   REND_NO_AUTH      = 0,
   REND_BASIC_AUTH   = 1,
   REND_STEALTH_AUTH = 2,
-  REND_V3_AUTH      = 3,
+  REND_V3_AUTH      = 3, /* Dummy flag to allow adding v3 services on the
+                          * control port */
 } rend_auth_type_t;
 
 /** Client-side configuration of authorization for a hidden service. */