fix longname returned in HS_DESC control events

According to control spec, longname should not contain any spaces and is
consists only of identy_digest + nickname

added two functions:
* node_get_verbose_nickname_by_id()
* node_describe_longname_by_id()

7 files changed, 101 insertions, 40 deletions

diff --git a/src/or/control.c b/src/or/control.c
index d24b9e4ba..42db3613d 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -4991,6 +4991,21 @@ rend_auth_type_to_string(rend_auth_type_t auth_type)
   return str;
 }
 
+/** Return a longname the node whose identity is <b>id_digest</b>. If
+ * node_get_by_id() returns NULL, base 16 encoding of <b>id_digest</b> is
+ * returned instead.
+ *
+ * This function is not thread-safe.  Each call to this function invalidates
+ * previous values returned by this function.
+ */
+MOCK_IMPL(const char *,
+node_describe_longname_by_id,(const char *id_digest))
+{
+  static char longname[MAX_VERBOSE_NICKNAME_LEN+1];
+  node_get_verbose_nickname_by_id(id_digest, longname);
+  return longname;
+}
+
 /** send HS_DESC requested event.
  *
  * <b>rend_query</b> is used to fetch requested onion address and auth type.
@@ -4999,20 +5014,21 @@ rend_auth_type_to_string(rend_auth_type_t auth_type)
  */
 void
 control_event_hs_descriptor_requested(const rend_data_t *rend_query,
-                                      const char *hs_dir,
+                                      const char *id_digest,
                                       const char *desc_id_base32)
 {
-  if (!hs_dir || !rend_query || !desc_id_base32) {
+  if (!id_digest || !rend_query || !desc_id_base32) {
     log_warn(LD_BUG, "Called with rend_query==%p, "
-             "hs_dir==%p, desc_id_base32==%p",
-             rend_query, hs_dir, desc_id_base32);
+             "id_digest==%p, desc_id_base32==%p",
+             rend_query, id_digest, desc_id_base32);
     return;
   }
+
   send_control_event(EVENT_HS_DESC, ALL_FORMATS,
                      "650 HS_DESC REQUESTED %s %s %s %s\r\n",
                      rend_query->onion_address,
                      rend_auth_type_to_string(rend_query->auth_type),
-                     hs_dir,
+                     node_describe_longname_by_id(id_digest),
                      desc_id_base32);
 }
 
@@ -5027,19 +5043,20 @@ control_event_hs_descriptor_requested(const rend_data_t *rend_query,
 void
 control_event_hs_descriptor_receive_end(const char *action,
                                         const rend_data_t *rend_query,
-                                        const char *hs_dir)
+                                        const char *id_digest)
 {
-  if (!action || !rend_query || !hs_dir) {
+  if (!action || !rend_query || !id_digest) {
     log_warn(LD_BUG, "Called with action==%p, rend_query==%p, "
-             "hs_dir==%p", action, rend_query, hs_dir);
+             "id_digest==%p", action, rend_query, id_digest);
     return;
   }
+
   send_control_event(EVENT_HS_DESC, ALL_FORMATS,
                      "650 HS_DESC %s %s %s %s\r\n",
                      action,
                      rend_query->onion_address,
                      rend_auth_type_to_string(rend_query->auth_type),
-                     hs_dir);
+                     node_describe_longname_by_id(id_digest));
 }
 
 /** send HS_DESC RECEIVED event
@@ -5048,14 +5065,14 @@ control_event_hs_descriptor_receive_end(const char *action,
  */
 void
 control_event_hs_descriptor_received(const rend_data_t *rend_query,
-                                     const char *hs_dir)
+                                     const char *id_digest)
 {
-  if (!rend_query || !hs_dir) {
-    log_warn(LD_BUG, "Called with rend_query==%p, hs_dir==%p",
-             rend_query, hs_dir);
+  if (!rend_query || !id_digest) {
+    log_warn(LD_BUG, "Called with rend_query==%p, id_digest==%p",
+             rend_query, id_digest);
     return;
   }
-  control_event_hs_descriptor_receive_end("RECEIVED", rend_query, hs_dir);
+  control_event_hs_descriptor_receive_end("RECEIVED", rend_query, id_digest);
 }
 
 /** send HS_DESC FAILED event
@@ -5064,14 +5081,14 @@ control_event_hs_descriptor_received(const rend_data_t *rend_query,
  */
 void
 control_event_hs_descriptor_failed(const rend_data_t *rend_query,
-                                   const char *hs_dir)
+                                   const char *id_digest)
 {
-  if (!rend_query || !hs_dir) {
-    log_warn(LD_BUG, "Called with rend_query==%p, hs_dir==%p",
-             rend_query, hs_dir);
+  if (!rend_query || !id_digest) {
+    log_warn(LD_BUG, "Called with rend_query==%p, id_digest==%p",
+             rend_query, id_digest);
     return;
   }
-  control_event_hs_descriptor_receive_end("FAILED", rend_query, hs_dir);
+  control_event_hs_descriptor_receive_end("FAILED", rend_query, id_digest);
 }
 
 /** Free any leftover allocated memory of the control.c subsystem. */
diff --git a/src/or/control.h b/src/or/control.h
index be3cc2420..0466de17f 100644
--- a/src/or/control.h
+++ b/src/or/control.h
@@ -100,6 +100,7 @@ void control_event_transport_launched(const char *mode,
                                       const char *transport_name,
                                       tor_addr_t *addr, uint16_t port);
 const char *rend_auth_type_to_string(rend_auth_type_t auth_type);
+MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest));
 void control_event_hs_descriptor_requested(const rend_data_t *rend_query,
                                            const char *desc_id_base32,
                                            const char *hs_dir);
diff --git a/src/or/directory.c b/src/or/directory.c
index 0cacf0661..ec1e776f0 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -2145,8 +2145,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
   if (conn->base_.purpose == DIR_PURPOSE_FETCH_RENDDESC_V2) {
     #define SEND_HS_DESC_FAILED_EVENT() ( \
       control_event_hs_descriptor_failed(conn->rend_data, \
-                                         node_describe_by_id( \
-                                             conn->identity_digest)) )
+                                         conn->identity_digest) )
     tor_assert(conn->rend_data);
     log_info(LD_REND,"Received rendezvous descriptor (size %d, status %d "
              "(%s))",
@@ -2173,8 +2172,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
             log_info(LD_REND, "Successfully fetched v2 rendezvous "
                      "descriptor.");
             control_event_hs_descriptor_received(conn->rend_data,
-                                                 node_describe_by_id(
-                                                     conn->identity_digest));
+                                                 conn->identity_digest);
             conn->base_.purpose = DIR_PURPOSE_HAS_FETCHED_RENDDESC;
             rend_client_desc_trynow(conn->rend_data->onion_address);
             break;
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index d1a226e4f..b1464422a 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -646,7 +646,7 @@ node_get_purpose(const node_t *node)
 
 /** Compute the verbose ("extended") nickname of <b>node</b> and store it
  * into the MAX_VERBOSE_NICKNAME_LEN+1 character buffer at
- * <b>verbose_nickname_out</b> */
+ * <b>verbose_name_out</b> */
 void
 node_get_verbose_nickname(const node_t *node,
                           char *verbose_name_out)
@@ -662,6 +662,25 @@ node_get_verbose_nickname(const node_t *node,
   strlcpy(verbose_name_out+1+HEX_DIGEST_LEN+1, nickname, MAX_NICKNAME_LEN+1);
 }
 
+/** Compute the verbose ("extended") nickname of node with
+ * given <b>id_digest</b> and store it into the MAX_VERBOSE_NICKNAME_LEN+1
+ * character buffer at <b>verbose_name_out</b>
+ *
+ * If node_get_by_id() returns NULL, base 16 encoding of
+ * <b>id_digest</b> is returned instead. */
+void
+node_get_verbose_nickname_by_id(const char *id_digest,
+                                char *verbose_name_out)
+{
+  const node_t *node = node_get_by_id(id_digest);
+  if (!node) {
+    verbose_name_out[0] = '$';
+    base16_encode(verbose_name_out+1, HEX_DIGEST_LEN+1, id_digest, DIGEST_LEN);
+  } else {
+    node_get_verbose_nickname(node, verbose_name_out);
+  }
+}
+
 /** Return true iff it seems that <b>node</b> allows circuits to exit
  * through it directlry from the client. */
 int
diff --git a/src/or/nodelist.h b/src/or/nodelist.h
index 8a4665a8b..565caa76c 100644
--- a/src/or/nodelist.h
+++ b/src/or/nodelist.h
@@ -33,6 +33,8 @@ void nodelist_assert_ok(void);
 const node_t *node_get_by_nickname(const char *nickname, int warn_if_unnamed);
 void node_get_verbose_nickname(const node_t *node,
                                char *verbose_name_out);
+void node_get_verbose_nickname_by_id(const char *id_digest,
+                                char *verbose_name_out);
 int node_is_named(const node_t *node);
 int node_is_dir(const node_t *node);
 int node_has_descriptor(const node_t *node);
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index de28bd1fc..4de010182 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -696,7 +696,7 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query)
             escaped_safe_str_client(descriptor_cookie_base64)),
            routerstatus_describe(hs_dir));
   control_event_hs_descriptor_requested(rend_query,
-                                        routerstatus_describe(hs_dir),
+                                        hs_dir->identity_digest,
                                         desc_id_base32);
   return 1;
 }
diff --git a/src/test/test_hs.c b/src/test/test_hs.c
index 75c9b6287..99ef7dd57 100644
--- a/src/test/test_hs.c
+++ b/src/test/test_hs.c
@@ -11,6 +11,17 @@
 #include "test.h"
 #include "control.h"
 
+/* mock ID digest and longname for node that's in nodelist */
+#define HSDIR_EXIST_ID "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" \
+                       "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA"
+#define STR_HSDIR_EXIST_LONGNAME \
+                       "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=TestDir"
+/* mock ID digest and longname for node that's not in nodelist */
+#define HSDIR_NONE_EXIST_ID "\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB" \
+                            "\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB\xBB"
+#define STR_HSDIR_NONE_EXIST_LONGNAME \
+                       "$BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"
+
 /* Helper global variable for hidden service descriptor event test.
  * It's used as a pointer to dynamically created message buffer in
  * send_control_event_string_replacement function, which mocks
@@ -31,6 +42,19 @@ send_control_event_string_replacement(uint16_t event, event_format_t which,
   received_msg = tor_strdup(msg);
 }
 
+/** Mock function for node_describe_longname_by_id, it returns either
+ * STR_HSDIR_EXIST_LONGNAME or STR_HSDIR_NONE_EXIST_LONGNAME
+ */
+static const char *
+node_describe_longname_by_id_replacement(const char *id_digest)
+{
+  if (!strcmp(id_digest, HSDIR_EXIST_ID)) {
+    return STR_HSDIR_EXIST_LONGNAME;
+  } else {
+    return STR_HSDIR_NONE_EXIST_LONGNAME;
+  }
+}
+
 /** Make sure each hidden service descriptor async event generation
  *
  * function generates the message in expected format.
@@ -39,8 +63,6 @@ static void
 test_hs_desc_event(void *arg)
 {
   #define STR_HS_ADDR "ajhb7kljbiru65qo"
-  #define STR_HS_DIR_LONGNAME \
-      "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=TestDir at 1.2.3.4"
   #define STR_HS_ID "b3oeducbhjmbqmgw2i3jtz4fekkrinwj"
 
   rend_data_t rend_query;
@@ -49,6 +71,8 @@ test_hs_desc_event(void *arg)
   (void) arg;
   MOCK(send_control_event_string,
        send_control_event_string_replacement);
+  MOCK(node_describe_longname_by_id,
+       node_describe_longname_by_id_replacement);
 
   /* setup rend_query struct */
   strncpy(rend_query.onion_address, STR_HS_ADDR,
@@ -56,44 +80,44 @@ test_hs_desc_event(void *arg)
   rend_query.auth_type = 0;
 
   /* test request event */
-  control_event_hs_descriptor_requested(&rend_query, STR_HS_DIR_LONGNAME,
+  control_event_hs_descriptor_requested(&rend_query, HSDIR_EXIST_ID,
                                         STR_HS_ID);
-  expected_msg =
-    "650 HS_DESC REQUESTED "STR_HS_ADDR" NO_AUTH "STR_HS_DIR_LONGNAME\
-    " "STR_HS_ID"\r\n";
+  expected_msg = "650 HS_DESC REQUESTED "STR_HS_ADDR" NO_AUTH "\
+                  STR_HSDIR_EXIST_LONGNAME" "STR_HS_ID"\r\n";
   test_assert(received_msg);
   test_streq(received_msg, expected_msg);
   tor_free(received_msg);
 
   /* test received event */
   rend_query.auth_type = 1;
-  control_event_hs_descriptor_received(&rend_query, STR_HS_DIR_LONGNAME);
-  expected_msg =
-    "650 HS_DESC RECEIVED "STR_HS_ADDR" BASIC_AUTH "STR_HS_DIR_LONGNAME"\r\n";
+  control_event_hs_descriptor_received(&rend_query, HSDIR_EXIST_ID);
+  expected_msg = "650 HS_DESC RECEIVED "STR_HS_ADDR" BASIC_AUTH "\
+                  STR_HSDIR_EXIST_LONGNAME"\r\n";
   test_assert(received_msg);
   test_streq(received_msg, expected_msg);
   tor_free(received_msg);
 
   /* test failed event */
   rend_query.auth_type = 2;
-  control_event_hs_descriptor_failed(&rend_query, STR_HS_DIR_LONGNAME);
-  expected_msg =
-    "650 HS_DESC FAILED "STR_HS_ADDR" STEALTH_AUTH "STR_HS_DIR_LONGNAME"\r\n";
+  control_event_hs_descriptor_failed(&rend_query, HSDIR_NONE_EXIST_ID);
+  expected_msg = "650 HS_DESC FAILED "STR_HS_ADDR" STEALTH_AUTH "\
+                  STR_HSDIR_NONE_EXIST_LONGNAME"\r\n";
   test_assert(received_msg);
   test_streq(received_msg, expected_msg);
   tor_free(received_msg);
 
   /* test invalid auth type */
   rend_query.auth_type = 999;
-  control_event_hs_descriptor_failed(&rend_query, STR_HS_DIR_LONGNAME);
-  expected_msg =
-    "650 HS_DESC FAILED "STR_HS_ADDR" UNKNOWN "STR_HS_DIR_LONGNAME"\r\n";
+  control_event_hs_descriptor_failed(&rend_query, HSDIR_EXIST_ID);
+  expected_msg = "650 HS_DESC FAILED "STR_HS_ADDR" UNKNOWN "\
+                  STR_HSDIR_EXIST_LONGNAME"\r\n";
   test_assert(received_msg);
   test_streq(received_msg, expected_msg);
   tor_free(received_msg);
 
  done:
   UNMOCK(send_control_event_string);
+  UNMOCK(node_describe_longname_by_id);
   tor_free(received_msg);
 }