Merge branch 'tor-gitlab/mr/173'

changes/ticket22668

+  o Minor features (logging):
+    - When describing a relay in th elogs, we now include its ed25519 identity.
+      Closes ticket 22668.

src/core/mainloop/connection.c

@@ -110,6 +110,7 @@
 #include "feature/stats/rephist.h"
 #include "feature/stats/bwhist.h"
 #include "lib/crypt_ops/crypto_util.h"
+#include "lib/crypt_ops/crypto_format.h"
 #include "lib/geoip/geoip.h"
 
 #include "lib/cc/ctassert.h"
@@ -440,11 +441,19 @@ connection_describe_peer_internal(const connection_t *conn,
       // This could be a client, so scrub it.  No identity to report.
       scrub = true;
     } else {
-      char id_buf[HEX_DIGEST_LEN+1];
-      base16_encode(id_buf, sizeof(id_buf),
+      const ed25519_public_key_t *ed_id =
+        connection_or_get_alleged_ed25519_id(or_conn);
+      char ed_id_buf[ED25519_BASE64_LEN+1];
+      char rsa_id_buf[HEX_DIGEST_LEN+1];
+      if (ed_id) {
+        ed25519_public_to_base64(ed_id_buf, ed_id);
+      } else {
+        strlcpy(ed_id_buf, "<none>", sizeof(ed_id_buf));
+      }
+      base16_encode(rsa_id_buf, sizeof(rsa_id_buf),
                     or_conn->identity_digest, DIGEST_LEN);
       tor_snprintf(extra_buf, sizeof(extra_buf),
-                   " ID=%s", id_buf);
+                   " ID=%s RSA_ID=%s", ed_id_buf, rsa_id_buf);
     }
     if (! scrub && (! tor_addr_eq(addr, &or_conn->canonical_orport.addr) ||
                     conn->port != or_conn->canonical_orport.port)) {

src/core/or/connection_or.c

@@ -207,6 +207,26 @@ connection_or_set_identity_digest(or_connection_t *conn,
     channel_set_identity_digest(chan, rsa_digest, ed_id);
 }
 
+/**
+ * Return the Ed25519 identity of the peer for this connection (if any).
+ *
+ * Note that this ID may not be the _actual_ identity for the peer if
+ * authentication is not complete.
+ **/
+const struct ed25519_public_key_t *
+connection_or_get_alleged_ed25519_id(const or_connection_t *conn)
+{
+  if (conn && conn->chan) {
+    const channel_t *chan = NULL;
+    chan = TLS_CHAN_TO_BASE(conn->chan);
+    if (!ed25519_public_key_is_zero(&chan->ed25519_identity)) {
+      return &chan->ed25519_identity;
+    }
+  }
+
+  return NULL;
+}
+
 /**************************************************************/
 
 /** Map from a string describing what a non-open OR connection was doing when

src/core/or/connection_or.h

@@ -73,6 +73,8 @@ void connection_or_init_conn_from_address(or_connection_t *conn,
 int connection_or_client_learned_peer_id(or_connection_t *conn,
                               const uint8_t *rsa_peer_id,
                               const struct ed25519_public_key_t *ed_peer_id);
+const struct ed25519_public_key_t *connection_or_get_alleged_ed25519_id(
+                              const or_connection_t *conn);
 time_t connection_or_client_used(or_connection_t *conn);
 MOCK_DECL(int, connection_or_get_num_circuits, (or_connection_t *conn));
 void or_handshake_state_free_(or_handshake_state_t *state);

src/feature/nodelist/describe.c

@@ -14,6 +14,10 @@
 #include "core/or/or.h"
 #include "core/or/extendinfo.h"
 #include "feature/nodelist/describe.h"
+#include "feature/nodelist/nodelist.h"
+#include "feature/nodelist/routerinfo.h"
+#include "lib/crypt_ops/crypto_ed25519.h"
+#include "lib/crypt_ops/crypto_format.h"
 
 #include "core/or/extend_info_st.h"
 #include "feature/nodelist/node_st.h"
@@ -34,7 +38,8 @@
  */
 STATIC const char *
 format_node_description(char *buf,
-                        const char *id_digest,
+                        const char *rsa_id_digest,
+                        const ed25519_public_key_t *ed25519_id,
                         const char *nickname,
                         const tor_addr_t *ipv4_addr,
                         const tor_addr_t *ipv6_addr)
@@ -48,7 +53,7 @@ format_node_description(char *buf,
 
   memset(buf, 0, NODE_DESC_BUF_LEN);
 
-  if (!id_digest) {
+  if (!rsa_id_digest) {
     /* strlcpy() returns the length of the source string it attempted to copy,
      * ignoring any required truncation due to the buffer length. */
     rv = strlcpy(buf, "<NULL ID DIGEST>", NODE_DESC_BUF_LEN);
@@ -66,7 +71,7 @@ format_node_description(char *buf,
     memset(hex_digest, 0, sizeof(hex_digest));
 
     base16_encode(hex_digest, sizeof(hex_digest),
-                  id_digest, DIGEST_LEN);
+                  rsa_id_digest, DIGEST_LEN);
     rv = strlcat(buf, hex_digest, NODE_DESC_BUF_LEN);
     tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
   }
@@ -77,6 +82,16 @@ format_node_description(char *buf,
     rv = strlcat(buf, nickname, NODE_DESC_BUF_LEN);
     tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
   }
+  if (ed25519_id) {
+    char ed_base64[ED25519_BASE64_LEN+1];
+    ed25519_public_to_base64(ed_base64, ed25519_id);
+    rv = strlcat(buf, " [", NODE_DESC_BUF_LEN);
+    tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
+    rv = strlcat(buf, ed_base64, NODE_DESC_BUF_LEN);
+    tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
+    rv = strlcat(buf, "]", NODE_DESC_BUF_LEN);
+    tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
+  }
   if (ipv4_addr || has_ipv6) {
     rv = strlcat(buf, " at ", NODE_DESC_BUF_LEN);
     tor_assert_nonfatal(rv < NODE_DESC_BUF_LEN);
@@ -126,8 +141,11 @@ router_describe(const routerinfo_t *ri)
   if (!ri)
     return "<null>";
 
+  const ed25519_public_key_t *ed25519_id = routerinfo_get_ed25519_id(ri);
+
   return format_node_description(buf,
                                  ri->cache_info.identity_digest,
+                                 ed25519_id,
                                  ri->nickname,
                                  &ri->ipv4_addr,
                                  &ri->ipv6_addr);
@@ -166,8 +184,11 @@ node_describe(const node_t *node)
     return "<null rs and ri>";
   }
 
+  const ed25519_public_key_t *ed25519_id = node_get_ed25519_id(node);
+
   return format_node_description(buf,
                                  node->identity,
+                                 ed25519_id,
                                  nickname,
                                  ipv4_addr,
                                  ipv6_addr);
@@ -188,6 +209,7 @@ routerstatus_describe(const routerstatus_t *rs)
 
   return format_node_description(buf,
                                  rs->identity_digest,
+                                 NULL,
                                  rs->nickname,
                                  &rs->ipv4_addr,
                                  &rs->ipv6_addr);
@@ -211,8 +233,13 @@ extend_info_describe(const extend_info_t *ei)
   const tor_addr_t *addr4 = ap4 ? &ap4->addr : NULL;
   const tor_addr_t *addr6 = ap6 ? &ap6->addr : NULL;
 
+  const ed25519_public_key_t *ed25519_id = &ei->ed_identity;
+  if (ed25519_public_key_is_zero(ed25519_id))
+    ed25519_id = NULL;
+
   return format_node_description(buf,
                                  ei->identity_digest,
+                                 ed25519_id,
                                  ei->nickname,
                                  addr4,
                                  addr6);

src/feature/nodelist/describe.h

@@ -35,22 +35,28 @@ void router_get_verbose_nickname(char *buf, const routerinfo_t *router);
 /**
  * Longest allowed output of format_node_description, plus 1 character for
  * NUL.  This allows space for:
- * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx at"
+ * "$FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF~xxxxxxxxxxxxxxxxxxx "
+ * "[+++++++++++++++++++++++++++++++++++++++++++] at"
  * " 255.255.255.255 and [ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255]"
  * plus a terminating NUL.
  */
 #define NODE_DESC_BUF_LEN \
-  (MAX_VERBOSE_NICKNAME_LEN+4+IPV4_BUF_LEN_NO_NUL+5+TOR_ADDR_BUF_LEN)
+  (MAX_VERBOSE_NICKNAME_LEN+4 \
+   + ED25519_BASE64_LEN+3 \
+   + IPV4_BUF_LEN_NO_NUL+5 \
+   + TOR_ADDR_BUF_LEN)
 
 #endif /* defined(DESCRIBE_PRIVATE) || defined(TOR_UNIT_TESTS) */
 
 #ifdef TOR_UNIT_TESTS
+struct ed25519_public_key_t;
 
 STATIC const char *format_node_description(char *buf,
-                                           const char *id_digest,
-                                           const char *nickname,
-                                           const tor_addr_t *ipv4_addr,
-                                           const tor_addr_t *ipv6_addr);
+                              const char *rsa_id_digest,
+                              const struct ed25519_public_key_t *ed25519_id,
+                              const char *nickname,
+                              const tor_addr_t *ipv4_addr,
+                              const tor_addr_t *ipv6_addr);
 
 #endif /* defined(TOR_UNIT_TESTS) */
 

src/feature/nodelist/routerinfo.c

@@ -13,6 +13,7 @@
 
 #include "feature/nodelist/nodelist.h"
 #include "feature/nodelist/routerinfo.h"
+#include "feature/nodelist/torcert.h"
 
 #include "feature/nodelist/node_st.h"
 #include "feature/nodelist/routerinfo_st.h"
@@ -75,6 +76,21 @@ router_get_all_orports(const routerinfo_t *ri)
   return node_get_all_orports(&fake_node);
 }
 
+/** Return the Ed25519 identity key for this routerinfo, or NULL if it
+ * doesn't have one. */
+const ed25519_public_key_t *
+routerinfo_get_ed25519_id(const routerinfo_t *ri)
+{
+  if (BUG(! ri))
+    return NULL;
+
+  const tor_cert_t *cert = ri->cache_info.signing_key_cert;
+  if (cert && ! ed25519_public_key_is_zero(&cert->signing_key))
+    return &cert->signing_key;
+  else
+    return NULL;
+}
+
 /** Given a router purpose, convert it to a string.  Don't call this on
  * ROUTER_PURPOSE_UNKNOWN: The whole point of that value is that we don't
  * know its string representation. */

src/feature/nodelist/routerinfo.h

@@ -18,6 +18,10 @@ int router_get_orport(const routerinfo_t *router,
 int router_has_orport(const routerinfo_t *router,
                       const tor_addr_port_t *orport);
 
+struct ed25519_public_key_t;
+const struct ed25519_public_key_t *routerinfo_get_ed25519_id(
+                      const routerinfo_t *ri);
+
 smartlist_t *router_get_all_orports(const routerinfo_t *ri);
 
 const char *router_purpose_to_string(uint8_t p);

src/test/test_connection.c

@@ -1049,20 +1049,20 @@ test_conn_describe(void *arg)
   options->SafeLogging_ = SAFELOG_SCRUB_RELAY; // back to safelogging.
   tt_str_op(connection_describe(conn), OP_EQ,
             "OR connection (open) with [ffff:3333:1111::2]:8080 "
-            "ID=0000000700000000000000000000000000000000");
+            "ID=<none> RSA_ID=0000000700000000000000000000000000000000");
   // Add a 'canonical address' that is the same as the one we have.
   tor_addr_parse(&TO_OR_CONN(conn)->canonical_orport.addr,
                  "[ffff:3333:1111::2]");
   TO_OR_CONN(conn)->canonical_orport.port = 8080;
   tt_str_op(connection_describe(conn), OP_EQ,
             "OR connection (open) with [ffff:3333:1111::2]:8080 "
-            "ID=0000000700000000000000000000000000000000");
+            "ID=<none> RSA_ID=0000000700000000000000000000000000000000");
   // Add a different 'canonical address'
   tor_addr_parse(&TO_OR_CONN(conn)->canonical_orport.addr,
                  "[ffff:3333:1111::8]");
   tt_str_op(connection_describe(conn), OP_EQ,
             "OR connection (open) with [ffff:3333:1111::2]:8080 "
-            "ID=0000000700000000000000000000000000000000 "
+            "ID=<none> RSA_ID=0000000700000000000000000000000000000000 "
             "canonical_addr=[ffff:3333:1111::8]:8080");
 
   // Clear identity_digest so that free_minimal won't complain.

src/test/test_nodelist.c

@@ -11,6 +11,7 @@
 
 #include "core/or/or.h"
 #include "lib/crypt_ops/crypto_rand.h"
+#include "lib/crypt_ops/crypto_format.h"
 #include "feature/nodelist/describe.h"
 #include "feature/nodelist/networkstatus.h"
 #include "feature/nodelist/nodefamily.h"
@@ -657,6 +658,7 @@ test_nodelist_format_node_description(void *arg)
   tor_addr_t mock_null_ip;
   tor_addr_t mock_ipv4;
   tor_addr_t mock_ipv6;
+  ed25519_public_key_t ed_id;
 
   char ndesc[NODE_DESC_BUF_LEN];
   const char *rv = NULL;
@@ -685,6 +687,7 @@ test_nodelist_format_node_description(void *arg)
                                mock_digest,
                                NULL,
                                NULL,
+                               NULL,
                                NULL);
   tt_ptr_op(rv, OP_EQ, ndesc);
   tt_str_op(ndesc, OP_EQ, "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA");
@@ -692,6 +695,7 @@ test_nodelist_format_node_description(void *arg)
   /* format node description should use ~ because named is deprecated */
   rv = format_node_description(ndesc,
                                mock_digest,
+                               NULL,
                                mock_nickname,
                                NULL,
                                NULL);
@@ -702,6 +706,7 @@ test_nodelist_format_node_description(void *arg)
   /* Try a null IP address, rather than NULL */
   rv = format_node_description(ndesc,
                                mock_digest,
+                               NULL,
                                mock_nickname,
                                NULL,
                                &mock_null_ip);
@@ -713,6 +718,7 @@ test_nodelist_format_node_description(void *arg)
   rv = format_node_description(ndesc,
                                mock_digest,
                                NULL,
+                               NULL,
                                &mock_ipv4,
                                NULL);
   tt_ptr_op(rv, OP_EQ, ndesc);
@@ -721,6 +727,7 @@ test_nodelist_format_node_description(void *arg)
 
   rv = format_node_description(ndesc,
                                mock_digest,
+                               NULL,
                                mock_nickname,
                                NULL,
                                &mock_ipv6);
@@ -731,6 +738,7 @@ test_nodelist_format_node_description(void *arg)
 
   rv = format_node_description(ndesc,
                                mock_digest,
+                               NULL,
                                mock_nickname,
                                &mock_ipv4,
                                &mock_ipv6);
@@ -739,11 +747,26 @@ test_nodelist_format_node_description(void *arg)
             "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 at "
             "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]");
 
+  /* Try some ed25519 keys. */
+  int n = ed25519_public_from_base64(&ed_id,
+              "+wBP6WVZzqKK+eTdwU7Hhb80xEm40FSZDBMNozTJpDE");
+  tt_int_op(n,OP_EQ,0);
+  rv = format_node_description(ndesc,
+                               mock_digest,
+                               &ed_id,
+                               mock_nickname,
+                               &mock_ipv4,
+                               &mock_ipv6);
+  tt_str_op(ndesc, OP_EQ,
+            "$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA~TestOR7890123456789 "
+            "[+wBP6WVZzqKK+eTdwU7Hhb80xEm40FSZDBMNozTJpDE] at "
+            "111.222.233.244 and [1111:2222:3333:4444:5555:6666:7777:8888]");
+
   /* test NULL handling */
-  rv = format_node_description(NULL, NULL, NULL, NULL, NULL);
+  rv = format_node_description(NULL, NULL, NULL, NULL, NULL, NULL);
   tt_str_op(rv, OP_EQ, "<NULL BUFFER>");
 
-  rv = format_node_description(ndesc, NULL, NULL, NULL, NULL);
+  rv = format_node_description(ndesc, NULL, NULL, NULL, NULL, NULL);
   tt_ptr_op(rv, OP_EQ, ndesc);
   tt_str_op(rv, OP_EQ, "<NULL ID DIGEST>");