Merge remote-tracking branch 'teor/bug26627_033' into maint-0.3.3

changes/bug26627

@@ -0,0 +1,7 @@
+  o Minor bugfixes (v3 onion services):
+    - Stop sending ed25519 link specifiers in v3 onion service introduce
+      cells, when the rendezvous point doesn't support ed25519 link
+      authentication. Fixes bug 26627; bugfix on 0.3.2.4-alpha.
+    - Stop putting ed25519 link specifiers in v3 onion service descriptors,
+      when the intro point doesn't support ed25519 link authentication.
+      Fixes bug 26627; bugfix on 0.3.2.4-alpha.

src/or/hs_circuit.c

@@ -559,10 +559,14 @@ retry_service_rendezvous_point(const origin_circuit_t *circ)
   return;
 }
 
-/* Add all possible link specifiers in node to lspecs.
- * legacy ID is mandatory thus MUST be present in node. If the primary address
- * is not IPv4, log a BUG() warning, and return an empty smartlist.
- * Includes ed25519 id and IPv6 link specifiers if present in the node. */
+/* Add all possible link specifiers in node to lspecs:
+ *  - legacy ID is mandatory thus MUST be present in node;
+ *  - include ed25519 link specifier if present in the node, and the node
+ *    supports ed25519 link authentication, even if its link versions are not
+ *    compatible with us;
+ *  - include IPv4 link specifier, if the primary address is not IPv4, log a
+ *    BUG() warning, and return an empty smartlist;
+ *  - include IPv6 link specifier if present in the node. */
 static void
 get_lspecs_from_node(const node_t *node, smartlist_t *lspecs)
 {
@@ -600,8 +604,12 @@ get_lspecs_from_node(const node_t *node, smartlist_t *lspecs)
   link_specifier_set_ls_len(ls, link_specifier_getlen_un_legacy_id(ls));
   smartlist_add(lspecs, ls);
 
-  /* ed25519 ID is only included if the node has it. */
-  if (!ed25519_public_key_is_zero(&node->ed25519_id)) {
+  /* ed25519 ID is only included if the node has it, and the node declares a
+     protocol version that supports ed25519 link authentication, even if that
+     link version is not compatible with us. (We are sending the ed25519 key
+     to another tor, which may support different link versions.) */
+  if (!ed25519_public_key_is_zero(&node->ed25519_id) &&
+      node_supports_ed25519_link_authentication(node, 0)) {
     ls = link_specifier_new();
     link_specifier_set_ls_type(ls, LS_ED25519_ID);
     memcpy(link_specifier_getarray_un_ed25519_id(ls), &node->ed25519_id,

src/or/hs_service.c

@@ -376,17 +376,21 @@ service_intro_point_free_void(void *obj)
 }
 
 /* Return a newly allocated service intro point and fully initialized from the
- * given extend_info_t ei if non NULL. If is_legacy is true, we also generate
- * the legacy key. On error, NULL is returned.
+ * given extend_info_t ei if non NULL.
+ * If is_legacy is true, we also generate the legacy key.
+ * If supports_ed25519_link_handshake_any is true, we add the relay's ed25519
+ * key to the link specifiers.
  *
  * If ei is NULL, returns a hs_service_intro_point_t with an empty link
  * specifier list and no onion key. (This is used for testing.)
+ * On any other error, NULL is returned.
  *
  * ei must be an extend_info_t containing an IPv4 address. (We will add supoort
  * for IPv6 in a later release.) When calling extend_info_from_node(), pass
  * 0 in for_direct_connection to make sure ei always has an IPv4 address. */
 STATIC hs_service_intro_point_t *
-service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy)
+service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy,
+                        unsigned int supports_ed25519_link_handshake_any)
 {
   hs_desc_link_specifier_t *ls;
   hs_service_intro_point_t *ip;
@@ -453,10 +457,13 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy)
   }
   smartlist_add(ip->base.link_specifiers, ls);
 
-  /* ed25519 identity key is optional for intro points */
-  ls = hs_desc_link_specifier_new(ei, LS_ED25519_ID);
-  if (ls) {
-    smartlist_add(ip->base.link_specifiers, ls);
+  /* ed25519 identity key is optional for intro points. If the node supports
+   * ed25519 link authentication, we include it. */
+  if (supports_ed25519_link_handshake_any) {
+    ls = hs_desc_link_specifier_new(ei, LS_ED25519_ID);
+    if (ls) {
+      smartlist_add(ip->base.link_specifiers, ls);
+    }
   }
 
   /* IPv6 is not supported in this release. */
@@ -1586,8 +1593,12 @@ pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes)
     tor_assert_nonfatal(!ed25519_public_key_is_zero(&info->ed_identity));
   }
 
-  /* Create our objects and populate them with the node information. */
-  ip = service_intro_point_new(info, !node_supports_ed25519_hs_intro(node));
+  /* Create our objects and populate them with the node information.
+   * We don't care if the intro's link auth is compatible with us, because
+   * we are sending the ed25519 key to a remote client via the descriptor. */
+  ip = service_intro_point_new(info, !node_supports_ed25519_hs_intro(node),
+                               node_supports_ed25519_link_authentication(node,
+                                                                         0));
   if (ip == NULL) {
     goto err;
   }

src/or/hs_service.h

@@ -307,8 +307,9 @@ STATIC void remove_service(hs_service_ht *map, hs_service_t *service);
 STATIC int register_service(hs_service_ht *map, hs_service_t *service);
 /* Service introduction point functions. */
 STATIC hs_service_intro_point_t *service_intro_point_new(
-                                         const extend_info_t *ei,
-                                         unsigned int is_legacy);
+                            const extend_info_t *ei,
+                            unsigned int is_legacy,
+                            unsigned int supports_ed25519_link_handshake_any);
 STATIC void service_intro_point_free_(hs_service_intro_point_t *ip);
 #define service_intro_point_free(ip)                            \
   FREE_AND_NULL(hs_service_intro_point_t,             \

src/test/test_hs_cell.c

@@ -38,7 +38,7 @@ test_gen_establish_intro_cell(void *arg)
      attempt to parse it. */
   {
     /* We only need the auth key pair here. */
-    hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0);
+    hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0);
     /* Auth key pair is generated in the constructor so we are all set for
      * using this IP object. */
     ret = hs_cell_build_establish_intro(circ_nonce, ip, buf);
@@ -106,7 +106,7 @@ test_gen_establish_intro_cell_bad(void *arg)
      ed25519_sign_prefixed() function and make it fail. */
   cell = trn_cell_establish_intro_new();
   tt_assert(cell);
-  ip = service_intro_point_new(NULL, 0);
+  ip = service_intro_point_new(NULL, 0, 0);
   cell_len = hs_cell_build_establish_intro(circ_nonce, ip, NULL);
   service_intro_point_free(ip);
   expect_log_msg_containing("Unable to make signature for "

src/test/test_hs_intropoint.c

@@ -49,7 +49,7 @@ new_establish_intro_cell(const char *circ_nonce,
 
   /* Auth key pair is generated in the constructor so we are all set for
    * using this IP object. */
-  ip = service_intro_point_new(NULL, 0);
+  ip = service_intro_point_new(NULL, 0, 0);
   tt_assert(ip);
   cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf);
   tt_i64_op(cell_len, OP_GT, 0);
@@ -75,7 +75,7 @@ new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out)
 
   /* Auth key pair is generated in the constructor so we are all set for
    * using this IP object. */
-  ip = service_intro_point_new(NULL, 0);
+  ip = service_intro_point_new(NULL, 0, 0);
   tt_assert(ip);
   cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out);
   tt_i64_op(cell_len, OP_GT, 0);

src/test/test_hs_service.c

@@ -241,7 +241,7 @@ static hs_service_intro_point_t *
 helper_create_service_ip(void)
 {
   hs_desc_link_specifier_t *ls;
-  hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0);
+  hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0);
   tor_assert(ip);
   /* Add a first unused link specifier. */
   ls = tor_malloc_zero(sizeof(*ls));