From d7be1595e714b21189f9b3cc80c571a7eba9bcc0 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 11 Feb 2025 08:30:02 -0500
Subject: [PATCH] Parse and validate family-certs in routerdescs

---
 src/feature/dirparse/parsecommon.h |   3 +-
 src/feature/dirparse/routerparse.c | 130 +++++++++++++++++++++++++
 src/feature/dirparse/routerparse.h |  10 ++
 src/feature/relay/router.c         |   3 +-
 src/test/test_dir.c                | 149 +++++++++++++++++++++++++++++
 5 files changed, 293 insertions(+), 2 deletions(-)

diff --git a/src/feature/dirparse/parsecommon.h b/src/feature/dirparse/parsecommon.h
index 2480460219..0b572632fa 100644
--- a/src/feature/dirparse/parsecommon.h
+++ b/src/feature/dirparse/parsecommon.h
@@ -45,7 +45,8 @@ typedef enum {
   K_UPTIME,
   K_DIR_SIGNING_KEY,
   K_FAMILY,
-  K_FAMILY_KEYS,
+  K_FAMILY_CERT,
+  K_FAMILY_IDS,
   K_FINGERPRINT,
   K_HIBERNATING,
   K_READ_HISTORY,
diff --git a/src/feature/dirparse/routerparse.c b/src/feature/dirparse/routerparse.c
index 6289b4c018..407aa03f1f 100644
--- a/src/feature/dirparse/routerparse.c
+++ b/src/feature/dirparse/routerparse.c
@@ -51,6 +51,7 @@
  **/
 
 #define ROUTERDESC_TOKEN_TABLE_PRIVATE
+#define ROUTERPARSE_PRIVATE
 
 #include "core/or/or.h"
 #include "app/config/config.h"
@@ -114,6 +115,7 @@ const token_rule_t routerdesc_token_table[] = {
   T01("allow-single-hop-exits",K_ALLOW_SINGLE_HOP_EXITS,    NO_ARGS, NO_OBJ ),
 
   T01("family",              K_FAMILY,              ARGS,    NO_OBJ ),
+  T0N("family-cert",         K_FAMILY_CERT,         ARGS,    NEED_OBJ ),
   T01("caches-extra-info",   K_CACHES_EXTRA_INFO,   NO_ARGS, NO_OBJ ),
   T0N("or-address",          K_OR_ADDRESS,          GE(1),   NO_OBJ ),
 
@@ -172,6 +174,10 @@ static token_rule_t extrainfo_token_table[] = {
 /* static function prototypes */
 static int router_add_exit_policy(routerinfo_t *router,directory_token_t *tok);
 static smartlist_t *find_all_exitpolicy(smartlist_t *s);
+static int check_family_certs(const smartlist_t *family_cert_tokens,
+                              const ed25519_public_key_t *identity_key,
+                              smartlist_t **family_ids_out,
+                              time_t *family_expiration_out);
 
 /** Set <b>digest</b> to the SHA-1 digest of the hash of the first router in
  * <b>s</b>. Return 0 on success, -1 on failure.
@@ -894,6 +900,21 @@ router_parse_entry_from_string(const char *s, const char *end,
     }
   }
 
+  {
+    smartlist_t *family_cert_toks = find_all_by_keyword(tokens, K_FAMILY_CERT);
+    time_t family_expiration = TIME_MAX;
+    int r = 0;
+    if (family_cert_toks)  {
+      r = check_family_certs(family_cert_toks,
+                             &router->cache_info.signing_key_cert->signing_key,
+                             &router->family_ids,
+                             &family_expiration);
+      smartlist_free(family_cert_toks);
+    }
+    if (r<0)
+      goto err;
+  }
+
   if (find_opt_by_keyword(tokens, K_CACHES_EXTRA_INFO))
     router->caches_extra_info = 1;
 
@@ -1250,6 +1271,115 @@ find_all_exitpolicy(smartlist_t *s)
   return out;
 }
 
+/**
+ * Parse and validate a single `FAMILY_CERT` token's object.
+ *
+ * Arguments are as for `check_family_certs()`.
+ */
+STATIC int
+check_one_family_cert(const uint8_t *cert_body,
+                      size_t cert_body_size,
+                      const ed25519_public_key_t *identity_key,
+                      char **family_id_out,
+                      time_t *family_expiration_out)
+{
+  tor_cert_t *cert = NULL;
+  int r = -1;
+
+  cert = tor_cert_parse(cert_body, cert_body_size);
+
+  if (! cert)
+    goto done;
+  if (cert->cert_type != CERT_TYPE_FAMILY_V_IDENTITY) {
+    log_warn(LD_DIR, "Wrong cert type in family certificate.");
+    goto done;
+  }
+  if (! cert->signing_key_included) {
+    log_warn(LD_DIR, "Missing family key in family certificate.");
+    goto done;
+  }
+  if (! ed25519_pubkey_eq(&cert->signed_key, identity_key)) {
+    log_warn(LD_DIR, "Key mismatch in family certificate.");
+    goto done;
+  }
+
+  time_t valid_until = cert->valid_until;
+
+  /* We're using NULL for the key, since the cert has the signing key included.
+   * We're using 0 for "now", since we're going to extract the expiration
+   * separately.
+   */
+  if (tor_cert_checksig(cert, NULL, 0) < 0) {
+    log_warn(LD_DIR, "Invalid signature in family certificate");
+    goto done;
+  }
+
+  /* At this point we know that the cert is valid.
+   * We extract the expiration time and the signing key. */
+  *family_expiration_out = valid_until;
+
+  char buf[ED25519_BASE64_LEN+1];
+  ed25519_public_to_base64(buf, &cert->signing_key);
+  tor_asprintf(family_id_out, "ed25519:%s", buf);
+
+  r = 0;
+ done:
+  tor_cert_free(cert);
+  return r;
+}
+
+/**
+ * Given a list of `FAMILY_CERT` tokens, and a relay's ed25519 `identity_key`,
+ * validate the family certificates in all the tokens, and convert them into
+ * family IDs in a newly allocated `family_ids_out` list.
+ * Set `family_expiration_out` to the earliest time at which any certificate
+ * in the list expires.
+ * Return 0 on success, and -1 on failure.
+ */
+static int
+check_family_certs(const smartlist_t *family_cert_tokens,
+                   const ed25519_public_key_t *identity_key,
+                   smartlist_t **family_ids_out,
+                   time_t *family_expiration_out)
+{
+  if (BUG(!identity_key) ||
+      BUG(!family_ids_out) ||
+      BUG(!family_expiration_out))
+    return -1;
+
+  *family_expiration_out = TIME_MAX;
+
+  if (family_cert_tokens == NULL || smartlist_len(family_cert_tokens) == 0) {
+    *family_ids_out = NULL;
+    return 0;
+  }
+
+  *family_ids_out = smartlist_new();
+  SMARTLIST_FOREACH_BEGIN(family_cert_tokens, directory_token_t *, tok) {
+    if (BUG(tok->object_body == NULL))
+      goto err;
+
+    char *this_id = NULL;
+    time_t this_expiration = TIME_MAX;
+    if (check_one_family_cert((const uint8_t*)tok->object_body,
+                              tok->object_size,
+                              identity_key,
+                              &this_id, &this_expiration) < 0)
+      goto err;
+    smartlist_add(*family_ids_out, this_id);
+    *family_expiration_out = MIN(*family_expiration_out, this_expiration);
+  } SMARTLIST_FOREACH_END(tok);
+
+  smartlist_sort_strings(*family_ids_out);
+  smartlist_uniq_strings(*family_ids_out);
+
+  return 0;
+ err:
+  SMARTLIST_FOREACH(*family_ids_out, char *, cp, tor_free(cp));
+  smartlist_free(*family_ids_out);
+  return -1;
+}
+
 /** Called on startup; right now we just handle scanning the unparseable
  * descriptor dumps, but hang anything else we might need to do in the
  * future here as well.
diff --git a/src/feature/dirparse/routerparse.h b/src/feature/dirparse/routerparse.h
index aeb9b72e52..5c6c6adee3 100644
--- a/src/feature/dirparse/routerparse.h
+++ b/src/feature/dirparse/routerparse.h
@@ -12,6 +12,8 @@
 #ifndef TOR_ROUTERPARSE_H
 #define TOR_ROUTERPARSE_H
 
+#include "lib/testsupport/testsupport.h"
+
 int router_get_router_hash(const char *s, size_t s_len, char *digest);
 int router_get_extrainfo_hash(const char *s, size_t s_len, char *digest);
 
@@ -47,4 +49,12 @@ extern const struct token_rule_t routerdesc_token_table[];
 
 #define ED_DESC_SIGNATURE_PREFIX "Tor router descriptor signature v1"
 
+#ifdef ROUTERPARSE_PRIVATE
+STATIC int check_one_family_cert(const uint8_t *cert_body,
+                      size_t cert_body_size,
+                      const struct ed25519_public_key_t *identity_key,
+                      char **family_id_out,
+                      time_t *family_expiration_out);
+#endif
+
 #endif /* !defined(TOR_ROUTERPARSE_H) */
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index cc317dfad3..9c199df74b 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -3042,7 +3042,8 @@ router_dump_router_to_string(routerinfo_t *router,
     tor_cert_t *family_cert = tor_cert_create_ed25519(
           k_family_id,
           CERT_TYPE_FAMILY_V_IDENTITY,
-          get_master_identity_key(),
+          // (this is the identity key "KP_relayid_ed")
+          &router->cache_info.signing_key_cert->signing_key,
           router->cache_info.published_on,
           FAMILY_CERT_LIFETIME, CERT_FLAG_INCLUDE_SIGNING_KEY);
     char family_cert_base64[256];
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index b34711dcad..ebb3975908 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -21,6 +21,7 @@
 #define RELAY_PRIVATE
 #define ROUTERLIST_PRIVATE
 #define ROUTER_PRIVATE
+#define ROUTERPARSE_PRIVATE
 #define UNPARSEABLE_PRIVATE
 #define VOTEFLAGS_PRIVATE
 
@@ -864,8 +865,21 @@ test_dir_formats_rsa_ed25519(void *arg)
   tt_str_op(buf, OP_EQ, buf2);
   tor_free(buf);
 
+  /* We make a couple of changes now before we make the desc that we're going
+   * to parse and check the signature on. */
   setup_mock_configured_ports(r2->ipv4_orport, 0);
 
+  ed25519_keypair_t family_1;
+  ed25519_keypair_t family_2;
+  ed25519_keypair_generate(&family_1, 0);
+  ed25519_keypair_generate(&family_2, 0);
+  {
+    smartlist_t *family_keys = smartlist_new();
+    smartlist_add(family_keys, tor_memdup(&family_1, sizeof(family_1)));
+    smartlist_add(family_keys, tor_memdup(&family_2, sizeof(family_2)));
+    set_mock_family_id_keys(family_keys); // takes ownership.
+  }
+
   buf = router_dump_router_to_string(r2, r2->identity_pkey,
                                      r2_onion_pkey,
                                      &r2_onion_keypair, &kp2);
@@ -883,6 +897,20 @@ test_dir_formats_rsa_ed25519(void *arg)
              r2->onion_curve25519_pkey->public_key,
              CURVE25519_PUBKEY_LEN);
 
+  // Check family ids.
+  tt_assert(rp2->family_ids != NULL);
+  tt_int_op(smartlist_len(rp2->family_ids), OP_EQ, 2);
+ {
+    char k[ED25519_BASE64_LEN+1];
+    char b[sizeof(k)+16];
+    ed25519_public_to_base64(k, &family_1.pubkey);
+    tor_snprintf(b, sizeof(b), "ed25519:%s", k);
+    tt_assert(smartlist_contains_string(rp2->family_ids, b));
+    ed25519_public_to_base64(k, &family_2.pubkey);
+    tor_snprintf(b, sizeof(b), "ed25519:%s", k);
+    tt_assert(smartlist_contains_string(rp2->family_ids, b));
+  }
+
   CHECK_PARSED_EXIT_POLICY(rp2);
 
   tor_free(buf);
@@ -7268,6 +7296,126 @@ test_dir_dirserv_add_own_fingerprint(void *arg)
   crypto_pk_free(pk);
 }
 
+static void
+test_dir_parse_family_cert(void *arg)
+{
+  (void)arg;
+  ed25519_keypair_t kp_family;
+  ed25519_keypair_t kp_id;
+  char family_b64[ED25519_BASE64_LEN+1];
+  tor_cert_t *cert = NULL;
+  int r;
+
+  time_t now = 1739288377;
+  time_t lifetime = 86400;
+  time_t got_expiration = -1;
+  char *got_family_id = NULL;
+  char *expect_family_id = NULL;
+
+  setup_capture_of_logs(LOG_WARN);
+
+  ed25519_keypair_generate(&kp_family, 0);
+  ed25519_keypair_generate(&kp_id, 0);
+  ed25519_public_to_base64(family_b64, &kp_family.pubkey);
+  tor_asprintf(&expect_family_id, "ed25519:%s", family_b64);
+
+  // Wrong type.
+  cert = tor_cert_create_ed25519(&kp_family,
+                                 CERT_TYPE_ID_SIGNING,
+                                 &kp_id.pubkey,
+                                 now, lifetime,
+                                 CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(cert);
+  r = check_one_family_cert(cert->encoded, cert->encoded_len,
+                            &kp_id.pubkey,
+                            &got_family_id,
+                            &got_expiration);
+  tt_ptr_op(got_family_id, OP_EQ, NULL);
+  tt_int_op(r, OP_EQ, -1);
+  expect_single_log_msg_containing("Wrong cert type");
+  mock_clean_saved_logs();
+  tor_cert_free(cert);
+
+  // Family key not included.
+  cert = tor_cert_create_ed25519(&kp_family,
+                                 CERT_TYPE_FAMILY_V_IDENTITY,
+                                 &kp_id.pubkey,
+                                 now, lifetime,
+                                 0);
+  tt_assert(cert);
+  r = check_one_family_cert(cert->encoded, cert->encoded_len,
+                            &kp_id.pubkey,
+                            &got_family_id,
+                            &got_expiration);
+  tt_ptr_op(got_family_id, OP_EQ, NULL);
+  tt_int_op(r, OP_EQ, -1);
+  expect_single_log_msg_containing("Missing family key");
+  mock_clean_saved_logs();
+  tor_cert_free(cert);
+
+  // Certified key isn't correct
+  cert = tor_cert_create_ed25519(&kp_family,
+                                 CERT_TYPE_FAMILY_V_IDENTITY,
+                                 &kp_family.pubkey,
+                                 now, lifetime,
+                                 CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(cert);
+  r = check_one_family_cert(cert->encoded, cert->encoded_len,
+                            &kp_id.pubkey,
+                            &got_family_id,
+                            &got_expiration);
+  tt_ptr_op(got_family_id, OP_EQ, NULL);
+  tt_int_op(r, OP_EQ, -1);
+  expect_single_log_msg_containing("Key mismatch");
+  mock_clean_saved_logs();
+  tor_cert_free(cert);
+
+  // Signature is bogus.
+  cert = tor_cert_create_ed25519(&kp_family,
+                                 CERT_TYPE_FAMILY_V_IDENTITY,
+                                 &kp_id.pubkey,
+                                 now, lifetime,
+                                 CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(cert);
+  cert->encoded[cert->encoded_len-1] ^= 0x77; // corrupt the signature
+  r = check_one_family_cert(cert->encoded, cert->encoded_len,
+                            &kp_id.pubkey,
+                            &got_family_id,
+                            &got_expiration);
+  tt_ptr_op(got_family_id, OP_EQ, NULL);
+  tt_int_op(r, OP_EQ, -1);
+  expect_single_log_msg_containing("Invalid signature");
+  mock_clean_saved_logs();
+  tor_cert_free(cert);
+
+  // Everything is okay!
+  cert = tor_cert_create_ed25519(&kp_family,
+                                 CERT_TYPE_FAMILY_V_IDENTITY,
+                                 &kp_id.pubkey,
+                                 now, lifetime,
+                                 CERT_FLAG_INCLUDE_SIGNING_KEY);
+  tt_assert(cert);
+  got_expiration = -1;
+  r = check_one_family_cert(cert->encoded, cert->encoded_len,
+                            &kp_id.pubkey,
+                            &got_family_id,
+                            &got_expiration);
+  expect_no_log_entry();
+  tt_int_op(r, OP_EQ, 0);
+  tt_int_op(got_expiration, OP_NE, -1);
+  // Cert expirations have 1-hour granularity
+  tt_int_op(got_expiration, OP_GE, now + lifetime);
+  tt_int_op(got_expiration, OP_LT, now + lifetime + 3601);
+  tt_str_op(got_family_id, OP_EQ, expect_family_id);
+  tt_assert(!strchr(got_family_id, '=')); // not family
+
+ done:
+  tor_cert_free(cert);
+  tor_free(got_family_id);
+  tor_free(expect_family_id);
+  teardown_capture_of_logs();
+}
+
 #ifndef COCCI
 #define DIR_LEGACY(name)                             \
   { #name, test_dir_ ## name , TT_FORK, NULL, NULL }
@@ -7354,5 +7502,6 @@ struct testcase_t dir_tests[] = {
   DIR(dirserv_router_get_status, TT_FORK),
   DIR(dirserv_would_reject_router, TT_FORK),
   DIR(dirserv_add_own_fingerprint, TT_FORK),
+  DIR(parse_family_cert, TT_FORK),
   END_OF_TESTCASES
 };
-- 
GitLab