commit b8ebe8b24f4402c332ae8127b30338f4e0517000
parent af6e4745ef34351dea11bcd40bd4afbc322bf8c9
Author: Nick Mathewson <nickm@torproject.org>
Date: Tue, 18 Feb 2025 09:53:59 -0500
Update interface for happy families
I'm hoping that this design will be a bit more ergonomic
than my first idea; the improvement here is that you have to list the family
IDs you expect in your torrc. This way, there's a cross-check between the
actual keys we use and your configuration for them.
Diffstat:
8 files changed, 162 insertions(+), 67 deletions(-)
diff --git a/doc/man/tor.1.txt b/doc/man/tor.1.txt
@@ -168,12 +168,11 @@ The following options in this section are only recognized on the
make sure that they are owned by the user actually running the Tor
daemon on your system.
-[[opt-keygen-family]] **`--keygen-family`** __filename__::
- Generate a new family ID key in `filename`.
+[[opt-keygen-family]] **`--keygen-family`** __basename__::
+ Generate a new family ID key in __basename__`.secret_family_key`.
To use this key, install it on every relay in your family.
- (Put it in the relay's `KeyDirectory`, with a filename like
- `secret_family_key`, `secret_family_key.1`, `secret_family_key.2`.)
- Then enable the UseFamilyKeys option on your relays.
+ (Put it in the relay's `KeyDirectory`.)
+ Then enable the corresponding FamilyID option on your relays.
See (XXXX INSERT URL HERE) for more information.
**`--passphrase-fd`** __FILEDES__::
@@ -2480,9 +2479,14 @@ is non-zero):
Note: do not use MyFamily when configuring your Tor instance as a
bridge.
-[[UseFamilyKeys]] **UseFamilyKeys** **0**|**1**::
- If 1, configure this relay to be part of a family identified by a shared
- secret family key. Family keys are generated with `--keygen-family`.
+[[FamilyId]] **FamilyId** __ident__::
+ Configure this relay to be part of a family
+ identified by a shared secret family key with the given key identity.
+ A corresponding family key must be stored in the relay's key directory.
+ This option can appear multiple times.
+ Family keys are generated with "--keygen-family";
+ this also generates the value you should use in the __ident__ field
+ in a file ending with ".public\_family\_id".
For information on generating and installing a family
key, see (XXXX INSERT URL HERE).
+
@@ -4060,7 +4064,7 @@ __KeyDirectory__/**`secret_onion_key_ntor`** and **`secret_onion_key_ntor.old`**
generated key, which the relay uses to handle any requests that were made
by clients that didn't have the new one.
-__KeyDirectory__/**`secret_family_key`**, **`secret_family_key.`**.__N__::
+__KeyDirectory__/__keyname__**`.secret_family_key`**::
A relay family's family identity key.
Used to prove membership in a relay family.
See (XXXX INSERT URL HERE) for more information.
diff --git a/src/app/config/config.c b/src/app/config/config.c
@@ -470,6 +470,7 @@ static const config_var_t option_vars_[] = {
V(UseDefaultFallbackDirs, BOOL, "1"),
OBSOLETE("FallbackNetworkstatusFile"),
+ VAR("FamilyId", LINELIST, FamilyId_lines, NULL),
V(FascistFirewall, BOOL, "0"),
V(FirewallPorts, CSV, ""),
OBSOLETE("FastFirstHopPK"),
@@ -572,7 +573,6 @@ static const config_var_t option_vars_[] = {
V(MetricsPortPolicy, LINELIST, NULL),
V(TestingMinTimeToReportBandwidth, INTERVAL, "1 day"),
VAR("MyFamily", LINELIST, MyFamily_lines, NULL),
- V(UseFamilyKeys, BOOL, "0"),
V(NewCircuitPeriod, INTERVAL, "30 seconds"),
OBSOLETE("NamingAuthoritativeDirectory"),
OBSOLETE("NATDListenAddress"),
@@ -1050,6 +1050,11 @@ options_clear_cb(const config_mgr_t *mgr, void *opts)
tor_free(options->command_arg);
tor_free(options->master_key_fname);
config_free_lines(options->MyFamily);
+ if (options->FamilyIds) {
+ SMARTLIST_FOREACH(options->FamilyIds,
+ ed25519_public_key_t *, k, tor_free(k));
+ smartlist_free(options->FamilyIds);
+ }
}
/** Release all memory allocated in options
diff --git a/src/app/config/or_options_st.h b/src/app/config/or_options_st.h
@@ -493,8 +493,10 @@ struct or_options_t {
struct config_line_t *MyFamily_lines; /**< Declared family for this OR. */
struct config_line_t *MyFamily; /**< Declared family for this OR,
normalized */
- int UseFamilyKeys; /**< If set, we use one or more family keys
+ struct config_line_t *FamilyId_lines; /**< If set, IDs for family keys to use
* to certify this OR's membership. */
+ struct smartlist_t *FamilyIds; /**< FamilyIds, parsed and converted
+ * to a list of ed25519_public_key_t */
struct config_line_t *NodeFamilies; /**< List of config lines for
* node families */
/** List of parsed NodeFamilies values. */
diff --git a/src/app/main/main.c b/src/app/main/main.c
@@ -830,6 +830,32 @@ do_dump_config(void)
return 0;
}
+/** Implement --keygen-family; create a family ID key and write it to a file.
+ */
+static int
+do_keygen_family(const char *fname_base)
+{
+ ed25519_public_key_t pk;
+ char *fname = NULL;
+ int r = -1;
+
+ if (BUG(!fname_base))
+ goto done;
+
+ tor_asprintf(&fname, "%s.secret_family_key", fname_base);
+
+ if (create_family_id_key(fname, &pk) < 0)
+ goto done;
+
+ printf("# Generated %s\n", fname);
+ printf("FamilyId %s\n", ed25519_fmt(&pk));
+ r = 0;
+
+ done:
+ tor_free(fname);
+ return r;
+}
+
static void
init_addrinfo(void)
{
@@ -1388,7 +1414,7 @@ tor_run_main(const tor_main_configuration_t *tor_cfg)
result = load_ed_keys(get_options(), time(NULL)) < 0;
break;
case CMD_KEYGEN_FAMILY:
- result = create_family_id_key(get_options()->command_arg);
+ result = do_keygen_family(get_options()->command_arg);
break;
case CMD_KEY_EXPIRATION:
init_keys();
diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c
@@ -21,6 +21,7 @@
#include "lib/meminfo/meminfo.h"
#include "lib/osinfo/uname.h"
#include "lib/process/setuid.h"
+#include "lib/crypt_ops/crypto_format.h"
/* Required for dirinfo_type_t in or_options_t */
#include "core/or/or.h"
@@ -1180,6 +1181,19 @@ options_validate_relay_mode(const or_options_t *old_options,
options->MyFamily_lines, "MyFamily", msg))
return -1;
+ if (options->FamilyId_lines) {
+ options->FamilyIds = smartlist_new();
+ config_line_t *line;
+ for (line = options->FamilyId_lines; line; line = line->next) {
+ ed25519_public_key_t pk;
+ if (ed25519_public_from_base64(&pk, line->value) < 0) {
+ tor_asprintf(msg, "Invalid FamilyId %s", line->value);
+ return -1;
+ }
+ smartlist_add(options->FamilyIds, tor_memdup(&pk, sizeof(pk)));
+ }
+ }
+
if (options->ConstrainedSockets) {
if (options->DirPort_set) {
/* Providing cached directory entries while system TCP buffers are scarce
@@ -1274,7 +1288,7 @@ options_transition_affects_descriptor(const or_options_t *old_options,
YES_IF_CHANGED_STRING(ContactInfo);
YES_IF_CHANGED_STRING(BridgeDistribution);
YES_IF_CHANGED_LINELIST(MyFamily);
- YES_IF_CHANGED_BOOL(UseFamilyKeys);
+ YES_IF_CHANGED_LINELIST(FamilyId_lines);
YES_IF_CHANGED_STRING(AccountingStart);
YES_IF_CHANGED_INT(AccountingMax);
YES_IF_CHANGED_INT(AccountingRule);
diff --git a/src/feature/relay/routerkeys.c b/src/feature/relay/routerkeys.c
@@ -27,6 +27,7 @@
#include "feature/dirauth/dirvote.h"
#include "lib/crypt_ops/crypto_util.h"
+#include "lib/crypt_ops/crypto_format.h"
#include "lib/tls/tortls.h"
#include "lib/tls/x509.h"
@@ -682,9 +683,9 @@ get_current_auth_key_cert(void)
}
/**
- * Prefix for the filename in which we expect to find a family ID key.
+ * Suffix for the filenames in which we expect to find a family ID key.
*/
-#define FAMILY_KEY_FNAME "secret_family_key"
+#define FAMILY_KEY_SUFFIX ".secret_family_key"
/**
* Return true if `fname` is a possible filename of a family ID key.
@@ -695,14 +696,32 @@ get_current_auth_key_cert(void)
STATIC bool
is_family_key_fname(const char *fname)
{
- if (0 == strcmp(fname, FAMILY_KEY_FNAME))
- return true;
+ return 0 == strcmpend(fname, FAMILY_KEY_SUFFIX);
+}
- unsigned num;
- char ch;
- if (tor_sscanf(fname, FAMILY_KEY_FNAME".%u%c", &num, &ch) == 1)
- return true;
+/** Return true if `id` is configured in `options`. */
+static bool
+family_key_id_is_expected(const or_options_t *options,
+ const ed25519_public_key_t *id)
+{
+ SMARTLIST_FOREACH(options->FamilyIds, const ed25519_public_key_t *, k, {
+ if (ed25519_pubkey_eq(k, id))
+ return true;
+ });
+ return false;
+}
+/** Return true if the key for `id` has been loaded. */
+static bool
+family_key_is_present(const ed25519_public_key_t *id)
+{
+ if (!family_id_keys)
+ return false;
+
+ SMARTLIST_FOREACH(family_id_keys, const ed25519_keypair_t *, kp, {
+ if (ed25519_pubkey_eq(&kp->pubkey, id))
+ return true;
+ });
return false;
}
@@ -716,9 +735,10 @@ is_family_key_fname(const char *fname)
* family_id_keys.
*/
STATIC int
-load_family_id_keys_impl(const char *keydir)
+load_family_id_keys_impl(const or_options_t *options,
+ const char *keydir)
{
- if (BUG(!keydir))
+ if (BUG(!options) || BUG(!keydir))
return -1;
smartlist_t *files = tor_listdir(keydir);
@@ -756,8 +776,14 @@ load_family_id_keys_impl(const char *keydir)
goto end;
}
- smartlist_add(new_keys, kp_tmp);
- kp_tmp = NULL; // prevent double-free
+ if (family_key_id_is_expected(options, &kp_tmp->pubkey)) {
+ smartlist_add(new_keys, kp_tmp);
+ kp_tmp = NULL; // prevent double-free
+ } else {
+ log_warn(LD_OR, "Found secret family key in %s "
+ "with unexpected FamilyID %s",
+ fn_tmp, ed25519_fmt(&kp_tmp->pubkey));
+ }
tor_free(fn_tmp);
tor_free(tag_tmp);
@@ -784,9 +810,11 @@ load_family_id_keys_impl(const char *keydir)
/**
* Create a new family ID key, and store it in `fname`.
+ *
+ * If pk_out is provided, set it to the generated public key.
**/
int
-create_family_id_key(const char *fname)
+create_family_id_key(const char *fname, ed25519_public_key_t *pk_out)
{
int r = -1;
ed25519_keypair_t *kp = tor_malloc_zero(sizeof(ed25519_keypair_t));
@@ -801,6 +829,10 @@ create_family_id_key(const char *fname)
goto done;
}
+ if (pk_out) {
+ ed25519_pubkey_copy(pk_out, &kp->pubkey);
+ }
+
r = 0;
done:
@@ -821,24 +853,24 @@ int
load_family_id_keys(const or_options_t *options,
const networkstatus_t *ns)
{
- if (options->UseFamilyKeys) {
- if (load_family_id_keys_impl(options->KeyDirectory) < 0)
+ if (options->FamilyIds) {
+ if (load_family_id_keys_impl(options, options->KeyDirectory) < 0)
return -1;
- // This warning is _here_ because we want to give it (or not)
- // every time keys are reloaded.
- if (!smartlist_len(get_current_family_id_keys())) {
- log_warn(LD_OR,
- "UseFamilyKeys was configured, but no family keys were found. "
- "Family keys need to be in %s, with names like "
- FAMILY_KEY_FNAME", "FAMILY_KEY_FNAME".1, "
- FAMILY_KEY_FNAME".2, etc. "
- "See (XXXX INSERT URL HERE) for instructions.",
- options->KeyDirectory);
- } else {
- log_info(LD_OR, "Found %d family ID keys",
- smartlist_len(get_current_family_id_keys()));
- }
+ bool any_missing = false;
+ SMARTLIST_FOREACH_BEGIN(options->FamilyIds,
+ const ed25519_public_key_t *, id) {
+ if (!family_key_is_present(id)) {
+ log_err(LD_OR, "No key was found for listed FamilyID %s",
+ ed25519_fmt(id));
+ any_missing = true;
+ }
+ } SMARTLIST_FOREACH_END(id);
+ if (any_missing)
+ return -1;
+
+ log_info(LD_OR, "Found %d family ID keys",
+ smartlist_len(get_current_family_id_keys()));
} else {
set_family_id_keys(NULL);
}
@@ -857,12 +889,12 @@ warn_about_family_id_config(const or_options_t *options,
static int have_warned_absent_myfamily = 0;
static int have_warned_absent_familykeys = 0;
- if (options->UseFamilyKeys) {
+ if (options->FamilyIds) {
if (!have_warned_absent_myfamily &&
!options->MyFamily && ns && should_publish_family_list(ns)) {
log_warn(LD_OR,
- "UseFamilyKeys was configured, but MyFamily was not. "
- "UseFamilyKeys is good, but the Tor network still requires "
+ "FamilyId was configured, but MyFamily was not. "
+ "FamilyId is good, but the Tor network still requires "
"MyFamily while clients are migrating to use family "
"keys instead.");
have_warned_absent_myfamily = 1;
@@ -872,7 +904,7 @@ warn_about_family_id_config(const or_options_t *options,
options->MyFamily &&
ns && ns->consensus_method >= MIN_METHOD_FOR_FAMILY_IDS) {
log_notice(LD_OR,
- "MyFamily was configured, but UseFamilyKeys was not. "
+ "MyFamily was configured, but FamilyId was not. "
"It's a good time to start migrating your relays "
"to use family keys. "
"See (XXXX INSERT URL HERE) for instructions.");
diff --git a/src/feature/relay/routerkeys.h b/src/feature/relay/routerkeys.h
@@ -43,7 +43,7 @@ int log_cert_expiration(void);
int load_ed_keys(const or_options_t *options, time_t now);
int load_family_id_keys(const or_options_t *options,
const networkstatus_t *ns);
-int create_family_id_key(const char *fname);
+int create_family_id_key(const char *fname, ed25519_public_key_t *pk_out);
void warn_about_family_id_config(const or_options_t *options,
const networkstatus_t *ns);
int should_make_new_ed_keys(const or_options_t *options, const time_t now);
@@ -133,7 +133,7 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key,
(puts("Not available: Tor has been compiled without relay support"), 0)
#define load_family_id_keys(x,y) \
(puts("Not available: Tor has been compiled without relay support"), 0)
-#define create_family_id_key(x) \
+#define create_family_id_key(x,y) \
(puts("Not available: Tor has been compiled without relay support"), -1)
#endif /* defined(HAVE_MODULE_RELAY) */
@@ -146,7 +146,8 @@ void init_mock_ed_keys(const crypto_pk_t *rsa_identity_key);
#ifdef ROUTERKEYS_PRIVATE
STATIC void set_family_id_keys(smartlist_t *keys);
STATIC bool is_family_key_fname(const char *fname);
-STATIC int load_family_id_keys_impl(const char *keydir);
+STATIC int load_family_id_keys_impl(const or_options_t *options,
+ const char *keydir);
#endif
#endif /* !defined(TOR_ROUTERKEYS_H) */
diff --git a/src/test/test_routerkeys.c b/src/test/test_routerkeys.c
@@ -741,15 +741,11 @@ test_routerkeys_family_key_fname(void *arg)
{
(void)arg;
- tt_assert(is_family_key_fname("secret_family_key"));
- tt_assert(is_family_key_fname("secret_family_key.1"));
- tt_assert(is_family_key_fname("secret_family_key.413"));
- tt_assert(! is_family_key_fname("secret_family_key.413x"));
- tt_assert(! is_family_key_fname("secret_family_key1"));
- tt_assert(! is_family_key_fname("secret_family_key.hello"));
- tt_assert(! is_family_key_fname("secret_family_key.-1"));
- tt_assert(! is_family_key_fname("fun_with_filenames.1"));
- tt_assert(! is_family_key_fname("fun_with_filenames"));
+ tt_assert(is_family_key_fname("hello.secret_family_key"));
+ tt_assert(is_family_key_fname("xyzzy.secret_family_key"));
+ tt_assert(is_family_key_fname("909.secret_family_key"));
+ tt_assert(! is_family_key_fname("zzz.secret_family_key~"));
+ tt_assert(! is_family_key_fname("secret_family_key"));
done:
;
@@ -761,6 +757,8 @@ test_routerkeys_load_family_keys(void *arg)
(void) arg;
char *dname = tor_strdup(get_fname_rnd("fkeys"));
char *fname = NULL;
+ or_options_t *options = get_options_mutable();
+ ed25519_public_key_t pubkey;
#ifdef _WIN32
tt_assert(0==mkdir(dname));
@@ -768,36 +766,49 @@ test_routerkeys_load_family_keys(void *arg)
tt_assert(0==mkdir(dname,0700));
#endif
+ options->FamilyIds = smartlist_new();
+
// Not a family key, will be ignored
tor_asprintf(&fname, "%s"PATH_SEPARATOR"junk.1", dname);
write_str_to_file(fname, "hello world", 0);
tor_free(fname);
- tt_int_op(0, OP_EQ, load_family_id_keys_impl(dname));
+ tt_int_op(0, OP_EQ, load_family_id_keys_impl(options, dname));
tt_int_op(0, OP_EQ, smartlist_len(get_current_family_id_keys()));
// Create a family key; make sure we can load it.
- tor_asprintf(&fname, "%s"PATH_SEPARATOR"secret_family_key.12", dname);
- tt_int_op(0, OP_EQ, create_family_id_key(fname));
+ tor_asprintf(&fname, "%s"PATH_SEPARATOR"cg.secret_family_key", dname);
+ tt_int_op(0, OP_EQ, create_family_id_key(fname, &pubkey));
tor_free(fname);
+ smartlist_add(options->FamilyIds, tor_memdup(&pubkey, sizeof(pubkey)));
- tt_int_op(0, OP_EQ, load_family_id_keys_impl(dname));
+ tt_int_op(0, OP_EQ, load_family_id_keys_impl(options, dname));
tt_int_op(1, OP_EQ, smartlist_len(get_current_family_id_keys()));
//Try a second key.
- tor_asprintf(&fname, "%s"PATH_SEPARATOR"secret_family_key.413", dname);
- tt_int_op(0, OP_EQ, create_family_id_key(fname));
+ tor_asprintf(&fname, "%s"PATH_SEPARATOR"eb.secret_family_key", dname);
+ tt_int_op(0, OP_EQ, create_family_id_key(fname, &pubkey));
+ smartlist_add(options->FamilyIds, tor_memdup(&pubkey, sizeof(pubkey)));
+ tor_free(fname);
+
+ tt_int_op(0, OP_EQ, load_family_id_keys_impl(options, dname));
+ tt_int_op(2, OP_EQ, smartlist_len(get_current_family_id_keys()));
+
+ // Try an unlisted key, make sure it isn't loaded.
+ tor_asprintf(&fname, "%s"PATH_SEPARATOR"gt.secret_family_key", dname);
+ tt_int_op(0, OP_EQ, create_family_id_key(fname, &pubkey));
+ // Do not add to FamilyIDs here; we're leaving it unlisted.
tor_free(fname);
- tt_int_op(0, OP_EQ, load_family_id_keys_impl(dname));
+ tt_int_op(0, OP_EQ, load_family_id_keys_impl(options, dname));
tt_int_op(2, OP_EQ, smartlist_len(get_current_family_id_keys()));
// Make a junk key, make sure it causes an error.
- tor_asprintf(&fname, "%s"PATH_SEPARATOR"secret_family_key.6", dname);
+ tor_asprintf(&fname, "%s"PATH_SEPARATOR"xyz.secret_family_key", dname);
write_str_to_file(fname, "hello world", 0);
tor_free(fname);
- tt_int_op(-1, OP_EQ, load_family_id_keys_impl(dname));
+ tt_int_op(-1, OP_EQ, load_family_id_keys_impl(options, dname));
// keys unchanged
tt_int_op(2, OP_EQ, smartlist_len(get_current_family_id_keys()));
