Merge branch 'feature15588_squashed'

src/or/connection_edge.c

@@ -1691,7 +1691,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
     rend_service_authorization_t *client_auth =
       rend_client_lookup_service_authorization(socks->address);
 
-    const char *cookie = NULL;
+    const uint8_t *cookie = NULL;
     rend_auth_type_t auth_type = REND_NO_AUTH;
     if (client_auth) {
       log_info(LD_REND, "Using previously configured client authorization "
@@ -1703,7 +1703,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
     /* Fill in the rend_data field so we can start doing a connection to
      * a hidden service. */
     rend_data_t *rend_data = ENTRY_TO_EDGE_CONN(conn)->rend_data =
-      rend_data_client_create(socks->address, NULL, cookie, auth_type);
+      rend_data_client_create(socks->address, NULL, (char *) cookie, auth_type);
     if (rend_data == NULL) {
       return -1;
     }

src/or/control.c

@@ -3788,14 +3788,18 @@ handle_control_add_onion(control_connection_t *conn,
    * the other arguments are malformed.
    */
   smartlist_t *port_cfgs = smartlist_new();
+  smartlist_t *auth_clients = NULL;
+  smartlist_t *auth_created_clients = NULL;
   int discard_pk = 0;
   int detach = 0;
   int max_streams = 0;
   int max_streams_close_circuit = 0;
+  rend_auth_type_t auth_type = REND_NO_AUTH;
   for (size_t i = 1; i < arg_len; i++) {
     static const char *port_prefix = "Port=";
     static const char *flags_prefix = "Flags=";
     static const char *max_s_prefix = "MaxStreams=";
+    static const char *auth_prefix = "ClientAuth=";
 
     const char *arg = smartlist_get(args, i);
     if (!strcasecmpstart(arg, port_prefix)) {
@@ -3826,10 +3830,12 @@ handle_control_add_onion(control_connection_t *conn,
        *                connection.
        *   * 'MaxStreamsCloseCircuit' - Close the circuit if MaxStreams is
        *                                exceeded.
+       *   * 'BasicAuth' - Client authorization using the 'basic' method.
        */
       static const char *discard_flag = "DiscardPK";
       static const char *detach_flag = "Detach";
       static const char *max_s_close_flag = "MaxStreamsCloseCircuit";
+      static const char *basicauth_flag = "BasicAuth";
 
       smartlist_t *flags = smartlist_new();
       int bad = 0;
@@ -3848,6 +3854,8 @@ handle_control_add_onion(control_connection_t *conn,
           detach = 1;
         } else if (!strcasecmp(flag, max_s_close_flag)) {
           max_streams_close_circuit = 1;
+        } else if (!strcasecmp(flag, basicauth_flag)) {
+          auth_type = REND_BASIC_AUTH;
         } else {
           connection_printf_to_buf(conn,
                                    "512 Invalid 'Flags' argument: %s\r\n",
@@ -3860,6 +3868,42 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_free(flags);
       if (bad)
         goto out;
+    } else if (!strcasecmpstart(arg, auth_prefix)) {
+      char *err_msg = NULL;
+      int created = 0;
+      rend_authorized_client_t *client =
+        add_onion_helper_clientauth(arg + strlen(auth_prefix),
+                                    &created, &err_msg);
+      if (!client) {
+        if (err_msg) {
+          connection_write_str_to_buf(err_msg, conn);
+          tor_free(err_msg);
+        }
+        goto out;
+      }
+
+      if (auth_clients != NULL) {
+        int bad = 0;
+        SMARTLIST_FOREACH_BEGIN(auth_clients, rend_authorized_client_t *, ac) {
+          if (strcmp(ac->client_name, client->client_name) == 0) {
+            bad = 1;
+            break;
+          }
+        } SMARTLIST_FOREACH_END(ac);
+        if (bad) {
+          connection_printf_to_buf(conn,
+                                   "512 Duplicate name in ClientAuth\r\n");
+          rend_authorized_client_free(client);
+          goto out;
+        }
+      } else {
+        auth_clients = smartlist_new();
+        auth_created_clients = smartlist_new();
+      }
+      smartlist_add(auth_clients, client);
+      if (created) {
+        smartlist_add(auth_created_clients, client);
+      }
     } else {
       connection_printf_to_buf(conn, "513 Invalid argument\r\n");
       goto out;
@@ -3868,6 +3912,18 @@ handle_control_add_onion(control_connection_t *conn,
   if (smartlist_len(port_cfgs) == 0) {
     connection_printf_to_buf(conn, "512 Missing 'Port' argument\r\n");
     goto out;
+  } else if (auth_type == REND_NO_AUTH && auth_clients != NULL) {
+    connection_printf_to_buf(conn, "512 No auth type specified\r\n");
+    goto out;
+  } else if (auth_type != REND_NO_AUTH && auth_clients == NULL) {
+    connection_printf_to_buf(conn, "512 No auth clients specified\r\n");
+    goto out;
+  } else if ((auth_type == REND_BASIC_AUTH &&
+              smartlist_len(auth_clients) > 512) ||
+             (auth_type == REND_STEALTH_AUTH &&
+              smartlist_len(auth_clients) > 16)) {
+    connection_printf_to_buf(conn, "512 Too many auth clients\r\n");
+    goto out;
   }
 
   /* Parse the "keytype:keyblob" argument. */
@@ -3888,35 +3944,21 @@ handle_control_add_onion(control_connection_t *conn,
   }
   tor_assert(!err_msg);
 
-  /* Create the HS, using private key pk, and port config port_cfg.
+  /* Create the HS, using private key pk, client authentication auth_type,
+   * the list of auth_clients, 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, max_streams,
                                        max_streams_close_circuit,
+                                       auth_type, auth_clients,
                                        &service_id);
   port_cfgs = NULL; /* port_cfgs is now owned by the rendservice code. */
+  auth_clients = NULL; /* so is auth_clients */
   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();
@@ -3927,9 +3969,26 @@ handle_control_add_onion(control_connection_t *conn,
       smartlist_add(conn->ephemeral_onion_services, service_id);
     }
 
-    connection_write_str_to_buf(buf, conn);
+    tor_assert(service_id);
-    memwipe(buf, 0, strlen(buf));
+    connection_printf_to_buf(conn, "250-ServiceID=%s\r\n", service_id);
-    tor_free(buf);
+    if (key_new_alg) {
+      tor_assert(key_new_blob);
+      connection_printf_to_buf(conn, "250-PrivateKey=%s:%s\r\n",
+                               key_new_alg, key_new_blob);
+    }
+    if (auth_created_clients) {
+      SMARTLIST_FOREACH(auth_created_clients, rend_authorized_client_t *, ac, {
+        char *encoded = rend_auth_encode_cookie(ac->descriptor_cookie,
+                                                auth_type);
+        tor_assert(encoded);
+        connection_printf_to_buf(conn, "250-ClientAuth=%s:%s\r\n",
+                                 ac->client_name, encoded);
+        memwipe(encoded, 0, strlen(encoded));
+        tor_free(encoded);
+      });
+    }
+
+    connection_printf_to_buf(conn, "250 OK\r\n");
     break;
   }
   case RSAE_BADPRIVKEY:
@@ -3941,6 +4000,9 @@ handle_control_add_onion(control_connection_t *conn,
   case RSAE_BADVIRTPORT:
     connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n");
     break;
+  case RSAE_BADAUTH:
+    connection_printf_to_buf(conn, "512 Invalid client authorization\r\n");
+    break;
   case RSAE_INTERNAL: /* FALLSTHROUGH */
   default:
     connection_printf_to_buf(conn, "551 Failed to add Onion Service\r\n");
@@ -3957,6 +4019,16 @@ handle_control_add_onion(control_connection_t *conn,
     smartlist_free(port_cfgs);
   }
 
+  if (auth_clients) {
+    SMARTLIST_FOREACH(auth_clients, rend_authorized_client_t *, ac,
+                      rend_authorized_client_free(ac));
+    smartlist_free(auth_clients);
+  }
+  if (auth_created_clients) {
+    // Do not free entries; they are the same as auth_clients
+    smartlist_free(auth_created_clients);
+  }
+
   SMARTLIST_FOREACH(args, char *, cp, {
     memwipe(cp, 0, strlen(cp));
     tor_free(cp);
@@ -4065,6 +4137,65 @@ add_onion_helper_keyarg(const char *arg, int discard_pk,
   return pk;
 }
 
+/** Helper function to handle parsing a ClientAuth argument to the
+ * ADD_ONION command.  Return a new rend_authorized_client_t, or NULL
+ * and an optional control protocol error message on failure.  The
+ * caller is responsible for freeing the returned auth_client and err_msg.
+ *
+ * If 'created' is specified, it will be set to 1 when a new cookie has
+ * been generated.
+ */
+STATIC rend_authorized_client_t *
+add_onion_helper_clientauth(const char *arg, int *created, char **err_msg)
+{
+  int ok = 0;
+
+  tor_assert(arg);
+  tor_assert(created);
+  tor_assert(err_msg);
+  *err_msg = NULL;
+
+  smartlist_t *auth_args = smartlist_new();
+  rend_authorized_client_t *client =
+    tor_malloc_zero(sizeof(rend_authorized_client_t));
+  smartlist_split_string(auth_args, arg, ":", 0, 0);
+  if (smartlist_len(auth_args) < 1 || smartlist_len(auth_args) > 2) {
+    *err_msg = tor_strdup("512 Invalid ClientAuth syntax\r\n");
+    goto err;
+  }
+  client->client_name = tor_strdup(smartlist_get(auth_args, 0));
+  if (smartlist_len(auth_args) == 2) {
+    char *decode_err_msg = NULL;
+    if (rend_auth_decode_cookie(smartlist_get(auth_args, 1),
+                                client->descriptor_cookie,
+                                NULL, &decode_err_msg) < 0) {
+      tor_assert(decode_err_msg);
+      tor_asprintf(err_msg, "512 %s\r\n", decode_err_msg);
+      tor_free(decode_err_msg);
+      goto err;
+    }
+    *created = 0;
+  } else {
+    crypto_rand((char *) client->descriptor_cookie, REND_DESC_COOKIE_LEN);
+    *created = 1;
+  }
+
+  if (!rend_valid_client_name(client->client_name)) {
+    *err_msg = tor_strdup("512 Invalid name in ClientAuth\r\n");
+    goto err;
+  }
+
+  ok = 1;
+ err:
+  SMARTLIST_FOREACH(auth_args, char *, arg, tor_free(arg));
+  smartlist_free(auth_args);
+  if (!ok) {
+    rend_authorized_client_free(client);
+    client = NULL;
+  }
+  return client;
+}
+
 /** Called when we get a DEL_ONION command; parse the body, and remove
  * the existing ephemeral Onion Service. */
 static int

src/or/control.h

@@ -259,6 +259,8 @@ 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);
+STATIC rend_authorized_client_t *
+add_onion_helper_clientauth(const char *arg, int *created, char **err_msg_out);
 #endif
 
 #endif

src/or/or.h

@@ -784,7 +784,7 @@ typedef enum rend_auth_type_t {
 
 /** Client-side configuration of authorization for a hidden service. */
 typedef struct rend_service_authorization_t {
-  char descriptor_cookie[REND_DESC_COOKIE_LEN];
+  uint8_t descriptor_cookie[REND_DESC_COOKIE_LEN];
   char onion_address[REND_SERVICE_ADDRESS_LEN+1];
   rend_auth_type_t auth_type;
 } rend_service_authorization_t;
@@ -5039,7 +5039,7 @@ typedef enum {
 /** Hidden-service side configuration of client authorization. */
 typedef struct rend_authorized_client_t {
   char *client_name;
-  char descriptor_cookie[REND_DESC_COOKIE_LEN];
+  uint8_t descriptor_cookie[REND_DESC_COOKIE_LEN];
   crypto_pk_t *client_key;
 } rend_authorized_client_t;
 

src/or/rendclient.c

@@ -1466,12 +1466,10 @@ rend_parse_service_authorization(const or_options_t *options,
   strmap_t *parsed = strmap_new();
   smartlist_t *sl = smartlist_new();
   rend_service_authorization_t *auth = NULL;
-  char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2];
+  char *err_msg = NULL;
-  char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_BASE64+2+1];
 
   for (line = options->HidServAuth; line; line = line->next) {
     char *onion_address, *descriptor_cookie;
-    int auth_type_val = 0;
     auth = NULL;
     SMARTLIST_FOREACH(sl, char *, c, tor_free(c););
     smartlist_clear(sl);
@@ -1500,31 +1498,13 @@ rend_parse_service_authorization(const or_options_t *options,
     }
     /* Parse descriptor cookie. */
     descriptor_cookie = smartlist_get(sl, 1);
-    if (strlen(descriptor_cookie) != REND_DESC_COOKIE_LEN_BASE64) {
+    if (rend_auth_decode_cookie(descriptor_cookie, auth->descriptor_cookie,
-      log_warn(LD_CONFIG, "Authorization cookie has wrong length: '%s'",
+                                &auth->auth_type, &err_msg) < 0) {
-               descriptor_cookie);
+      tor_assert(err_msg);
+      log_warn(LD_CONFIG, "%s", err_msg);
+      tor_free(err_msg);
       goto err;
     }
-    /* Add trailing zero bytes (AA) to make base64-decoding happy. */
-    tor_snprintf(descriptor_cookie_base64ext,
-                 REND_DESC_COOKIE_LEN_BASE64+2+1,
-                 "%sAA", descriptor_cookie);
-    if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp),
-                      descriptor_cookie_base64ext,
-                      strlen(descriptor_cookie_base64ext)) < 0) {
-      log_warn(LD_CONFIG, "Decoding authorization cookie failed: '%s'",
-               descriptor_cookie);
-      goto err;
-    }
-    auth_type_val = (((uint8_t)descriptor_cookie_tmp[16]) >> 4) + 1;
-    if (auth_type_val < 1 || auth_type_val > 2) {
-      log_warn(LD_CONFIG, "Authorization cookie has unknown authorization "
-                          "type encoded.");
-      goto err;
-    }
-    auth->auth_type = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH;
-    memcpy(auth->descriptor_cookie, descriptor_cookie_tmp,
-           REND_DESC_COOKIE_LEN);
     if (strmap_get(parsed, auth->onion_address)) {
       log_warn(LD_CONFIG, "Duplicate authorization for the same hidden "
                           "service.");
@@ -1547,8 +1527,6 @@ rend_parse_service_authorization(const or_options_t *options,
   } else {
     strmap_free(parsed, rend_service_authorization_strmap_item_free);
   }
-  memwipe(descriptor_cookie_tmp, 0, sizeof(descriptor_cookie_tmp));
-  memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext));
   return res;
 }
 

src/or/rendcommon.c

@@ -720,6 +720,22 @@ rend_valid_descriptor_id(const char *query)
   return 0;
 }
 
+/** Return true iff <b>client_name</b> is a syntactically valid name
+ * for rendezvous client authentication. */
+int
+rend_valid_client_name(const char *client_name)
+{
+  size_t len = strlen(client_name);
+  if (len < 1 || len > REND_CLIENTNAME_MAX_LEN) {
+    return 0;
+  }
+  if (strspn(client_name, REND_LEGAL_CLIENTNAME_CHARACTERS) != len) {
+    return 0;
+  }
+
+  return 1;
+}
+
 /** Called when we get a rendezvous-related relay cell on circuit
  * <b>circ</b>.  Dispatch on rendezvous relay command. */
 void
@@ -941,3 +957,114 @@ hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
   return smartlist_len(responsible_dirs) ? 0 : -1;
 }
 
+/* Length of the 'extended' auth cookie used to encode auth type before
+ * base64 encoding. */
+#define REND_DESC_COOKIE_LEN_EXT (REND_DESC_COOKIE_LEN + 1)
+/* Length of the zero-padded auth cookie when base64 encoded. These two
+ * padding bytes always (A=) are stripped off of the returned cookie. */
+#define REND_DESC_COOKIE_LEN_EXT_BASE64 (REND_DESC_COOKIE_LEN_BASE64 + 2)
+
+/** Encode a client authorization descriptor cookie.
+ * The result of this function is suitable for use in the HidServAuth
+ * option.  The trailing padding characters are removed, and the
+ * auth type is encoded into the cookie.
+ *
+ * Returns a new base64-encoded cookie. This function cannot fail.
+ * The caller is responsible for freeing the returned value.
+ */
+char *
+rend_auth_encode_cookie(const uint8_t *cookie_in, rend_auth_type_t auth_type)
+{
+  uint8_t extended_cookie[REND_DESC_COOKIE_LEN_EXT];
+  char *cookie_out = tor_malloc_zero(REND_DESC_COOKIE_LEN_EXT_BASE64 + 1);
+  int re;
+
+  tor_assert(cookie_in);
+
+  memcpy(extended_cookie, cookie_in, REND_DESC_COOKIE_LEN);
+  extended_cookie[REND_DESC_COOKIE_LEN] = ((int)auth_type - 1) << 4;
+  re = base64_encode(cookie_out, REND_DESC_COOKIE_LEN_EXT_BASE64 + 1,
+                     (const char *) extended_cookie, REND_DESC_COOKIE_LEN_EXT,
+                     0);
+  tor_assert(re == REND_DESC_COOKIE_LEN_EXT_BASE64);
+
+  /* Remove the trailing 'A='.  Auth type is encoded in the high bits
+   * of the last byte, so the last base64 character will always be zero
+   * (A).  This is subtly different behavior from base64_encode_nopad. */
+  cookie_out[REND_DESC_COOKIE_LEN_BASE64] = '\0';
+  memwipe(extended_cookie, 0, sizeof(extended_cookie));
+  return cookie_out;
+}
+
+/** Decode a base64-encoded client authorization descriptor cookie.
+ * The descriptor_cookie can be truncated to REND_DESC_COOKIE_LEN_BASE64
+ * characters (as given to clients), or may include the two padding
+ * characters (as stored by the service).
+ *
+ * The result is stored in REND_DESC_COOKIE_LEN bytes of cookie_out.
+ * The rend_auth_type_t decoded from the cookie is stored in the
+ * optional auth_type_out parameter.
+ *
+ * Return 0 on success, or -1 on error.  The caller is responsible for
+ * freeing the returned err_msg.
+ */
+int
+rend_auth_decode_cookie(const char *cookie_in, uint8_t *cookie_out,
+                        rend_auth_type_t *auth_type_out, char **err_msg_out)
+{
+  uint8_t descriptor_cookie_decoded[REND_DESC_COOKIE_LEN_EXT + 1] = { 0 };
+  char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_EXT_BASE64 + 1];
+  const char *descriptor_cookie = cookie_in;
+  char *err_msg = NULL;
+  int auth_type_val = 0;
+  int res = -1;
+  int decoded_len;
+
+  size_t len = strlen(descriptor_cookie);
+  if (len == REND_DESC_COOKIE_LEN_BASE64) {
+    /* Add a trailing zero byte to make base64-decoding happy. */
+    tor_snprintf(descriptor_cookie_base64ext,
+                 sizeof(descriptor_cookie_base64ext),
+                 "%sA=", descriptor_cookie);
+    descriptor_cookie = descriptor_cookie_base64ext;
+  } else if (len != REND_DESC_COOKIE_LEN_EXT_BASE64) {
+    tor_asprintf(&err_msg, "Authorization cookie has wrong length: %s",
+                 escaped(cookie_in));
+    goto err;
+  }
+
+  decoded_len = base64_decode((char *) descriptor_cookie_decoded,
+                              sizeof(descriptor_cookie_decoded),
+                              descriptor_cookie,
+                              REND_DESC_COOKIE_LEN_EXT_BASE64);
+  if (decoded_len != REND_DESC_COOKIE_LEN &&
+      decoded_len != REND_DESC_COOKIE_LEN_EXT) {
+    tor_asprintf(&err_msg, "Authorization cookie has invalid characters: %s",
+                 escaped(cookie_in));
+    goto err;
+  }
+
+  if (auth_type_out) {
+    auth_type_val = (descriptor_cookie_decoded[REND_DESC_COOKIE_LEN] >> 4) + 1;
+    if (auth_type_val < 1 || auth_type_val > 2) {
+      tor_asprintf(&err_msg, "Authorization cookie type is unknown: %s",
+                   escaped(cookie_in));
+      goto err;
+    }
+    *auth_type_out = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH;
+  }
+
+  memcpy(cookie_out, descriptor_cookie_decoded, REND_DESC_COOKIE_LEN);
+  res = 0;
+ err:
+  if (err_msg_out) {
+    *err_msg_out = err_msg;
+  } else {
+    tor_free(err_msg);
+  }
+  memwipe(descriptor_cookie_decoded, 0, sizeof(descriptor_cookie_decoded));
+  memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext));
+  return res;
+}
+
+

src/or/rendcommon.h

@@ -45,6 +45,7 @@ void rend_intro_point_free(rend_intro_point_t *intro);
 
 int rend_valid_service_id(const char *query);
 int rend_valid_descriptor_id(const char *query);
+int rend_valid_client_name(const char *client_name);
 int rend_encode_v2_descriptors(smartlist_t *descs_out,
                                rend_service_descriptor_t *desc, time_t now,
                                uint8_t period, rend_auth_type_t auth_type,
@@ -68,5 +69,13 @@ rend_data_t *rend_data_service_create(const char *onion_address,
                                       const char *pk_digest,
                                       const uint8_t *cookie,
                                       rend_auth_type_t auth_type);
+
+char *rend_auth_encode_cookie(const uint8_t *cookie_in,
+                              rend_auth_type_t auth_type);
+int rend_auth_decode_cookie(const char *cookie_in,
+                            uint8_t *cookie_out,
+                            rend_auth_type_t *auth_type_out,
+                            char **err_msg_out);
+
 #endif
 

src/or/rendservice.c

@@ -183,14 +183,15 @@ num_rend_services(void)
 }
 
 /** Helper: free storage held by a single service authorized client entry. */
-static void
+void
 rend_authorized_client_free(rend_authorized_client_t *client)
 {
   if (!client)
     return;
   if (client->client_key)
     crypto_pk_free(client->client_key);
-  memwipe(client->client_name, 0, strlen(client->client_name));
+  if (client->client_name)
+    memwipe(client->client_name, 0, strlen(client->client_name));
   tor_free(client->client_name);
   memwipe(client->descriptor_cookie, 0, sizeof(client->descriptor_cookie));
   tor_free(client);
@@ -671,27 +672,17 @@ rend_config_services(const or_options_t *options, int validate_only)
       SMARTLIST_FOREACH_BEGIN(clients, const char *, client_name)
       {
         rend_authorized_client_t *client;
-        size_t len = strlen(client_name);
+        if (!rend_valid_client_name(client_name)) {
-        if (len < 1 || len > REND_CLIENTNAME_MAX_LEN) {
           log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains an "
-                              "illegal client name: '%s'. Length must be "
+                              "illegal client name: '%s'. Names must be "
-                              "between 1 and %d characters.",
+                              "between 1 and %d characters and contain "
+                              "only [A-Za-z0-9+_-].",
                    client_name, REND_CLIENTNAME_MAX_LEN);
           SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp));
           smartlist_free(clients);
           rend_service_free(service);
           return -1;
         }
-        if (strspn(client_name, REND_LEGAL_CLIENTNAME_CHARACTERS) != len) {
-          log_warn(LD_CONFIG, "HiddenServiceAuthorizeClient contains an "
-                              "illegal client name: '%s'. Valid "
-                              "characters are [A-Za-z0-9+_-].",
-                   client_name);
-          SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp));
-          smartlist_free(clients);
-          rend_service_free(service);
-          return -1;
-        }
         client = tor_malloc_zero(sizeof(rend_authorized_client_t));
         client->client_name = tor_strdup(client_name);
         smartlist_add(service->clients, client);
@@ -827,14 +818,17 @@ 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, with
+/** Add the ephemeral service <b>pk</b>/<b>ports</b> if possible, using
+ * client authorization <b>auth_type</b> and an optional list of
+ * rend_authorized_client_t in <b>auth_clients</b>, with
  * <b>max_streams_per_circuit</b> streams allowed per rendezvous circuit,
  * and circuit closure on max streams being exceeded set by
  * <b>max_streams_close_circuit</b>.
  *
- * Regardless of sucess/failure, callers should not touch pk/ports after
+ * Ownership of pk, ports, and auth_clients is passed to this routine.
- * calling this routine, and may assume that correct cleanup has been done
+ * Regardless of success/failure, callers should not touch these values
- * on failure.
+ * after calling this routine, and may assume that correct cleanup has
+ * been done on failure.
  *
  * Return an appropriate rend_service_add_ephemeral_status_t.
  */
@@ -843,6 +837,8 @@ rend_service_add_ephemeral(crypto_pk_t *pk,
                            smartlist_t *ports,
                            int max_streams_per_circuit,
                            int max_streams_close_circuit,
+                           rend_auth_type_t auth_type,
+                           smartlist_t *auth_clients,
                            char **service_id_out)
 {
   *service_id_out = NULL;
@@ -852,7 +848,8 @@ rend_service_add_ephemeral(crypto_pk_t *pk,
   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->auth_type = auth_type;
+  s->clients = auth_clients;
   s->ports = ports;
   s->intro_period_started = time(NULL);
   s->n_intro_points_wanted = NUM_INTRO_POINTS_DEFAULT;
@@ -868,6 +865,12 @@ rend_service_add_ephemeral(crypto_pk_t *pk,
     rend_service_free(s);
     return RSAE_BADVIRTPORT;
   }
+  if (s->auth_type != REND_NO_AUTH &&
+      (!s->clients || smartlist_len(s->clients) == 0)) {
+    log_warn(LD_CONFIG, "At least one authorized client must be specified.");
+    rend_service_free(s);
+    return RSAE_BADAUTH;
+  }
 
   /* Enforcing pk/id uniqueness should be done by rend_service_load_keys(), but
    * it's not, see #14828.
@@ -1156,7 +1159,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
   strmap_t *parsed_clients = strmap_new();
   FILE *cfile, *hfile;
   open_file_t *open_cfile = NULL, *open_hfile = NULL;
-  char extended_desc_cookie[REND_DESC_COOKIE_LEN+1];
   char desc_cook_out[3*REND_DESC_COOKIE_LEN_BASE64+1];
   char service_id[16+1];
   char buf[1500];
@@ -1208,10 +1210,12 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
       memcpy(client->descriptor_cookie, parsed->descriptor_cookie,
              REND_DESC_COOKIE_LEN);
     } else {
-      crypto_rand(client->descriptor_cookie, REND_DESC_COOKIE_LEN);
+      crypto_rand((char *) client->descriptor_cookie, REND_DESC_COOKIE_LEN);
     }
+    /* For compatibility with older tor clients, this does not
+     * truncate the padding characters, unlike rend_auth_encode_cookie.  */
     if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1,
-                      client->descriptor_cookie,
+                      (char *) client->descriptor_cookie,
                       REND_DESC_COOKIE_LEN, 0) < 0) {
       log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
       goto err;
@@ -1272,6 +1276,8 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
         log_warn(LD_BUG, "Could not write client entry.");
         goto err;
       }
+    } else {
+      strlcpy(service_id, s->service_id, sizeof(service_id));
     }
 
     if (fputs(buf, cfile) < 0) {
@@ -1280,27 +1286,18 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
       goto err;
     }
 
-    /* Add line to hostname file. */
+    /* Add line to hostname file. This is not the same encoding as in
-    if (s->auth_type == REND_BASIC_AUTH) {
+     * client_keys. */
-      /* Remove == signs (newline has been removed above). */
+    char *encoded_cookie = rend_auth_encode_cookie(client->descriptor_cookie,
-      desc_cook_out[strlen(desc_cook_out)-2] = '\0';
+                                                   s->auth_type);
-      tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n",
+    if (!encoded_cookie) {
-                   s->service_id, desc_cook_out, client->client_name);
+      log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
-    } else {
+      goto err;
-      memcpy(extended_desc_cookie, client->descriptor_cookie,
-             REND_DESC_COOKIE_LEN);
-      extended_desc_cookie[REND_DESC_COOKIE_LEN] =
-        ((int)s->auth_type - 1) << 4;
-      if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1,
-                        extended_desc_cookie,
-                        REND_DESC_COOKIE_LEN+1, 0) < 0) {
-        log_warn(LD_BUG, "Could not base64-encode descriptor cookie.");
-        goto err;
-      }
-      desc_cook_out[strlen(desc_cook_out)-2] = '\0'; /* Remove A=. */
-      tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n",
-                   service_id, desc_cook_out, client->client_name);
     }
+    tor_snprintf(buf, sizeof(buf), "%s.onion %s # client: %s\n",
+                 service_id, encoded_cookie, client->client_name);
+    memwipe(encoded_cookie, 0, strlen(encoded_cookie));
+    tor_free(encoded_cookie);
 
     if (fputs(buf, hfile)<0) {
       log_warn(LD_FS, "Could not append host entry to file: %s",
@@ -1332,7 +1329,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname)
   memwipe(buf, 0, sizeof(buf));
   memwipe(desc_cook_out, 0, sizeof(desc_cook_out));
   memwipe(service_id, 0, sizeof(service_id));
-  memwipe(extended_desc_cookie, 0, sizeof(extended_desc_cookie));
 
   return r;
 }

src/or/rendservice.h

@@ -106,8 +106,11 @@ rend_service_port_config_t *rend_service_parse_port_config(const char *string,
                                                            char **err_msg_out);
 void rend_service_port_config_free(rend_service_port_config_t *p);
 
+void rend_authorized_client_free(rend_authorized_client_t *client);
+
 /** Return value from rend_service_add_ephemeral. */
 typedef enum {
+  RSAE_BADAUTH = -5, /**< Invalid auth_type/auth_clients */
   RSAE_BADVIRTPORT = -4, /**< Invalid VIRTPORT/TARGET(s) */
   RSAE_ADDREXISTS = -3, /**< Onion address collision */
   RSAE_BADPRIVKEY = -2, /**< Invalid public key */
@@ -118,6 +121,8 @@ rend_service_add_ephemeral_status_t rend_service_add_ephemeral(crypto_pk_t *pk,
                                smartlist_t *ports,
                                int max_streams_per_circuit,
                                int max_streams_close_circuit,
+                               rend_auth_type_t auth_type,
+                               smartlist_t *auth_clients,
                                char **service_id_out);
 int rend_service_del_ephemeral(const char *service_id);
 

src/or/routerparse.c

@@ -5371,6 +5371,7 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
   directory_token_t *tok;
   const char *current_entry = NULL;
   memarea_t *area = NULL;
+  char *err_msg = NULL;
   if (!ckstr || strlen(ckstr) == 0)
     return -1;
   tokens = smartlist_new();
@@ -5380,8 +5381,6 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
   current_entry = eat_whitespace(ckstr);
   while (!strcmpstart(current_entry, "client-name ")) {
     rend_authorized_client_t *parsed_entry;
-    size_t len;
-    char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2];
     /* Determine end of string. */
     const char *eos = strstr(current_entry, "\nclient-name ");
     if (!eos)
@@ -5410,12 +5409,10 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
     tor_assert(tok == smartlist_get(tokens, 0));
     tor_assert(tok->n_args == 1);
 
-    len = strlen(tok->args[0]);
+    if (!rend_valid_client_name(tok->args[0])) {
-    if (len < 1 || len > 19 ||
-      strspn(tok->args[0], REND_LEGAL_CLIENTNAME_CHARACTERS) != len) {
       log_warn(LD_CONFIG, "Illegal client name: %s. (Length must be "
-               "between 1 and 19, and valid characters are "
+               "between 1 and %d, and valid characters are "
-               "[A-Za-z0-9+-_].)", tok->args[0]);
+               "[A-Za-z0-9+-_].)", tok->args[0], REND_CLIENTNAME_MAX_LEN);
       goto err;
     }
     /* Check if client name is duplicate. */
@@ -5437,23 +5434,13 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr)
     /* Parse descriptor cookie. */
     tok = find_by_keyword(tokens, C_DESCRIPTOR_COOKIE);
     tor_assert(tok->n_args == 1);
-    if (strlen(tok->args[0]) != REND_DESC_COOKIE_LEN_BASE64 + 2) {
+    if (rend_auth_decode_cookie(tok->args[0], parsed_entry->descriptor_cookie,
-      log_warn(LD_REND, "Descriptor cookie has illegal length: %s",
+                                NULL, &err_msg) < 0) {
-               escaped(tok->args[0]));
+      tor_assert(err_msg);
+      log_warn(LD_REND, "%s", err_msg);
+      tor_free(err_msg);
       goto err;
     }
-    /* The size of descriptor_cookie_tmp needs to be REND_DESC_COOKIE_LEN+2,
-     * because a base64 encoding of length 24 does not fit into 16 bytes in all
-     * cases. */
-    if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp),
-                      tok->args[0], strlen(tok->args[0]))
-        != REND_DESC_COOKIE_LEN) {
-      log_warn(LD_REND, "Descriptor cookie contains illegal characters: "
-               "%s", escaped(tok->args[0]));
-      goto err;
-    }
-    memcpy(parsed_entry->descriptor_cookie, descriptor_cookie_tmp,
-           REND_DESC_COOKIE_LEN);
   }
   result = strmap_size(parsed_clients);
   goto done;

src/test/test_controller.c

@@ -154,10 +154,61 @@ test_rend_service_parse_port_config(void *arg)
   tor_free(err_msg);
 }
 
+static void
+test_add_onion_helper_clientauth(void *arg)
+{
+  rend_authorized_client_t *client = NULL;
+  char *err_msg = NULL;
+  int created = 0;
+
+  (void)arg;
+
+  /* Test "ClientName" only. */
+  client = add_onion_helper_clientauth("alice", &created, &err_msg);
+  tt_assert(client);
+  tt_assert(created);
+  tt_assert(!err_msg);
+  rend_authorized_client_free(client);
+
+  /* Test "ClientName:Blob" */
+  client = add_onion_helper_clientauth("alice:475hGBHPlq7Mc0cRZitK/B",
+                                       &created, &err_msg);
+  tt_assert(client);
+  tt_assert(!created);
+  tt_assert(!err_msg);
+  rend_authorized_client_free(client);
+
+  /* Test invalid client names */
+  client = add_onion_helper_clientauth("no*asterisks*allowed", &created,
+                                       &err_msg);
+  tt_assert(!client);
+  tt_assert(err_msg);
+  tor_free(err_msg);
+
+  /* Test invalid auth cookie */
+  client = add_onion_helper_clientauth("alice:12345", &created, &err_msg);
+  tt_assert(!client);
+  tt_assert(err_msg);
+  tor_free(err_msg);
+
+  /* Test invalid syntax */
+  client = add_onion_helper_clientauth(":475hGBHPlq7Mc0cRZitK/B", &created,
+                                       &err_msg);
+  tt_assert(!client);
+  tt_assert(err_msg);
+  tor_free(err_msg);
+
+ done:
+  rend_authorized_client_free(client);
+  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 },
+  { "add_onion_helper_clientauth", test_add_onion_helper_clientauth, 0, NULL,
+    NULL },
   END_OF_TESTCASES
 };
 

src/test/test_hs.c

@@ -435,6 +435,67 @@ test_hs_rend_data(void *arg)
   rend_data_free(client_dup);
 }
 
+/* Test encoding and decoding service authorization cookies */
+static void
+test_hs_auth_cookies(void *arg)
+{
+#define TEST_COOKIE_RAW ((const uint8_t *) "abcdefghijklmnop")
+#define TEST_COOKIE_ENCODED "YWJjZGVmZ2hpamtsbW5vcA"
+#define TEST_COOKIE_ENCODED_STEALTH "YWJjZGVmZ2hpamtsbW5vcB"
+#define TEST_COOKIE_ENCODED_INVALID "YWJjZGVmZ2hpamtsbW5vcD"
+
+  char *encoded_cookie;
+  uint8_t raw_cookie[REND_DESC_COOKIE_LEN];
+  rend_auth_type_t auth_type;
+  char *err_msg;
+  int re;
+
+  (void)arg;
+
+  /* Test that encoding gives the expected result */
+  encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_BASIC_AUTH);
+  tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED);
+  tor_free(encoded_cookie);
+
+  encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_STEALTH_AUTH);
+  tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED_STEALTH);
+  tor_free(encoded_cookie);
+
+  /* Decoding should give the original value */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED, raw_cookie, &auth_type,
+                               &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+  tt_int_op(auth_type, OP_EQ, REND_BASIC_AUTH);
+  memset(raw_cookie, 0, sizeof(raw_cookie));
+
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_STEALTH, raw_cookie,
+                               &auth_type, &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+  tt_int_op(auth_type, OP_EQ, REND_STEALTH_AUTH);
+  memset(raw_cookie, 0, sizeof(raw_cookie));
+
+  /* Decoding with padding characters should also work */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED "==", raw_cookie, NULL,
+                               &err_msg);
+  tt_assert(!re);
+  tt_assert(!err_msg);
+  tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN);
+
+  /* Decoding with an unknown type should fail */
+  re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_INVALID, raw_cookie,
+                               &auth_type, &err_msg);
+  tt_int_op(re, OP_LT, 0);
+  tt_assert(err_msg);
+  tor_free(err_msg);
+
+ done:
+  return;
+}
+
 struct testcase_t hs_tests[] = {
   { "hs_rend_data", test_hs_rend_data, TT_FORK,
     NULL, NULL },
@@ -445,6 +506,8 @@ struct testcase_t hs_tests[] = {
   { "pick_bad_tor2web_rendezvous_node",
     test_pick_bad_tor2web_rendezvous_node, TT_FORK,
     NULL, NULL },
+  { "hs_auth_cookies", test_hs_auth_cookies, TT_FORK,
+    NULL, NULL },
   END_OF_TESTCASES
 };