sarah
/
tor
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Bugfix: Regenerate more certificates when appropriate 

Previously we could sometimes change our signing key, but not
regenerate the certificates (signing->link and signing->auth) that
were signed with it.  Also, we would regularly replace our TLS x.509
link certificate (by rotating our TLS context) but not replace our
signing->link ed25519 certificate.  In both cases, the resulting
inconsistency would make other relays reject our link handshakes.

Fixes two cases of bug 22460; bugfix on 0.3.0.1-alpha.
This commit is contained in:
Nick Mathewson 2017-05-31 18:33:38 -04:00
parent 5b33d95a3d
commit a9be768959
7 changed files with 69 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
changes/bug22460_case1 Normal file
View File

@ -0,0 +1,10 @@
o Major bugfixes (relays, key management):
- Regenerate link and authentication certificates whenever the key that
signs them changes; also, regenerate link certificates whenever the
signed key changes. Previously, these processes were only weakly
coupled, and we relays could (for minutes to hours) wind up with an
inconsistent set of keys and certificates, which other relays
would not accept. Fixes two cases of bug 22460; bugfix on
0.3.0.1-alpha.

17
src/or/main.c
View File

@ -1506,8 +1506,9 @@ check_ed_keys_callback(time_t now, const or_options_t *options)
{
if (server_mode(options)) {
if (should_make_new_ed_keys(options, now)) {
if (load_ed_keys(options, now) < 0 ||
generate_ed_link_cert(options, now)) {
int new_signing_key = load_ed_keys(options, now);
if (new_signing_key < 0 ||
generate_ed_link_cert(options, now, new_signing_key > 0)) {
log_err(LD_OR, "Unable to update Ed25519 keys! Exiting.");
tor_cleanup();
exit(0);
@ -1559,6 +1560,11 @@ rotate_x509_certificate_callback(time_t now, const or_options_t *options)
log_err(LD_BUG, "Error reinitializing TLS context");
tor_assert_unreached();
}
if (generate_ed_link_cert(options, now, 1)) {
log_err(LD_OR, "Unable to update Ed25519->TLS link certificate for "
"new TLS context.");
tor_assert_unreached();
}
/* We also make sure to rotate the TLS connections themselves if they've
* been up for too long -- but that's done via is_bad_for_new_circs in
@ -2298,8 +2304,9 @@ do_hup(void)
/* Maybe we've been given a new ed25519 key or certificate?
*/
time_t now = approx_time();
if (load_ed_keys(options, now) < 0 ||
generate_ed_link_cert(options, now)) {
int new_signing_key = load_ed_keys(options, now);
if (new_signing_key < 0 ||
generate_ed_link_cert(options, now, new_signing_key > 0)) {
log_warn(LD_OR, "Problem reloading Ed25519 keys; still using old keys.");
}
@ -3627,7 +3634,7 @@ tor_main(int argc, char *argv[])
result = do_main_loop();
break;
case CMD_KEYGEN:
result = load_ed_keys(get_options(), time(NULL));
result = load_ed_keys(get_options(), time(NULL)) < 0;
break;
case CMD_LIST_FINGERPRINT:
result = do_list_fingerprint();

5
src/or/router.c
View File

@ -906,7 +906,8 @@ init_keys(void)
}
/* 1d. Load all ed25519 keys */
if (load_ed_keys(options,now) < 0)
const int new_signing_key = load_ed_keys(options,now);
if (new_signing_key < 0)
return -1;
/* 2. Read onion key. Make it if none is found. */
@ -976,7 +977,7 @@ init_keys(void)
/* 3b. Get an ed25519 link certificate. Note that we need to do this
* after we set up the TLS context */
if (generate_ed_link_cert(options, now) < 0) {
if (generate_ed_link_cert(options, now, new_signing_key > 0) < 0) {
log_err(LD_GENERAL,"Couldn't make link cert");
return -1;
}

37
src/or/routerkeys.c
View File

@ -672,6 +672,9 @@ static size_t rsa_ed_crosscert_len = 0;
/**
* Running as a server: load, reload, or refresh our ed25519 keys and
* certificates, creating and saving new ones as needed.
*
* Return -1 on failure; 0 on success if the signing key was not replaced;
* and 1 on success if the signing key was replaced.
*/
int
load_ed_keys(const or_options_t *options, time_t now)
@ -684,6 +687,7 @@ load_ed_keys(const or_options_t *options, time_t now)
const tor_cert_t *check_signing_cert = NULL;
tor_cert_t *sign_cert = NULL;
tor_cert_t *auth_cert = NULL;
int signing_key_changed = 0;
#define FAIL(msg) do { \
log_warn(LD_OR, (msg)); \
@ -719,7 +723,23 @@ load_ed_keys(const or_options_t *options, time_t now)
use_signing = sign;
}
if (use_signing) {
/* We loaded a signing key with its certificate. */
if (! master_signing_key) {
/* We didn't know one before! */
signing_key_changed = 1;
} else if (! ed25519_pubkey_eq(&use_signing->pubkey,
&master_signing_key->pubkey) ||
! tor_memeq(use_signing->seckey.seckey,
master_signing_key->seckey.seckey,
ED25519_SECKEY_LEN)) {
/* We loaded a different signing key than the one we knew before. */
signing_key_changed = 1;
}
}
if (!use_signing && master_signing_key) {
/* We couldn't load a signing key, but we already had one loaded */
check_signing_cert = signing_key_cert;
use_signing = master_signing_key;
}
@ -879,6 +899,7 @@ load_ed_keys(const or_options_t *options, time_t now)
if (!sign)
FAIL("Missing signing key");
use_signing = sign;
signing_key_changed = 1;
tor_assert(sign_cert->signing_key_included);
tor_assert(ed25519_pubkey_eq(&sign_cert->signing_key, &id->pubkey));
@ -910,6 +931,7 @@ load_ed_keys(const or_options_t *options, time_t now)
}
if (!current_auth_key ||
signing_key_changed ||
EXPIRES_SOON(auth_key_cert, options->TestingAuthKeySlop)) {
auth = ed_key_new(use_signing, INIT_ED_KEY_NEEDCERT,
now,
@ -937,7 +959,7 @@ load_ed_keys(const or_options_t *options, time_t now)
SET_CERT(auth_key_cert, auth_cert);
}
return 0;
return signing_key_changed;
err:
ed25519_keypair_free(id);
ed25519_keypair_free(sign);
@ -951,16 +973,18 @@ load_ed_keys(const or_options_t *options, time_t now)
* Retrieve our currently-in-use Ed25519 link certificate and id certificate,
* and, if they would expire soon (based on the time <b>now</b>, generate new
* certificates (without embedding the public part of the signing key inside).
* If <b>force</b> is true, always generate a new certificate.
*
* The signed_key from the expiring certificate will be used to sign the new
* key within newly generated X509 certificate.
* The signed_key from the current id->signing certificate will be used to
* sign the new key within newly generated X509 certificate.
*
* Returns -1 upon error. Otherwise, returns 0 upon success (either when the
* current certificate is still valid, or when a new certificate was
* successfully generated).
*/
int
generate_ed_link_cert(const or_options_t *options, time_t now)
generate_ed_link_cert(const or_options_t *options, time_t now,
int force)
{
const tor_x509_cert_t *link_ = NULL, *id = NULL;
tor_cert_t *link_cert = NULL;
@ -972,7 +996,8 @@ generate_ed_link_cert(const or_options_t *options, time_t now)
const common_digests_t *digests = tor_x509_cert_get_cert_digests(link_);
if (link_cert_cert &&
if (force == 0 &&
link_cert_cert &&
! EXPIRES_SOON(link_cert_cert, options->TestingLinkKeySlop) &&
fast_memeq(digests->d[DIGEST_SHA256], link_cert_cert->signed_key.pubkey,
DIGEST256_LEN)) {
@ -1073,7 +1098,7 @@ init_mock_ed_keys(const crypto_pk_t *rsa_identity_key)
MAKECERT(auth_key_cert,
master_signing_key, current_auth_key, CERT_TYPE_SIGNING_AUTH, 0);
if (generate_ed_link_cert(get_options(), time(NULL)) < 0) {
if (generate_ed_link_cert(get_options(), time(NULL), 0) < 0) {
log_warn(LD_BUG, "Couldn't make link certificate");
goto err;
}

2
src/or/routerkeys.h
View File

@ -66,7 +66,7 @@ MOCK_DECL(int, check_tap_onion_key_crosscert,(const uint8_t *crosscert,
int load_ed_keys(const or_options_t *options, time_t now);
int should_make_new_ed_keys(const or_options_t *options, const time_t now);
int generate_ed_link_cert(const or_options_t *options, time_t now);
int generate_ed_link_cert(const or_options_t *options, time_t now, int force);
int read_encrypted_secret_key(ed25519_secret_key_t *out,
const char *fname);

20
src/test/test_routerkeys.c
View File

@ -450,8 +450,8 @@ test_routerkeys_ed_keys_init_all(void *arg)
options->DataDirectory = dir;
tt_int_op(0, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now));
tt_int_op(1, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now, 0));
tt_assert(get_master_identity_key());
tt_assert(get_master_identity_key());
tt_assert(get_master_signing_keypair());
@ -466,7 +466,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
/* Call load_ed_keys again, but nothing has changed. */
tt_int_op(0, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now, 0));
tt_mem_op(&id, ==, get_master_identity_key(), sizeof(id));
tt_mem_op(&sign, ==, get_master_signing_keypair(), sizeof(sign));
tt_mem_op(&auth, ==, get_current_auth_keypair(), sizeof(auth));
@ -474,8 +474,8 @@ test_routerkeys_ed_keys_init_all(void *arg)
/* Force a reload: we make new link/auth keys. */
routerkeys_free_all();
tt_int_op(0, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now));
tt_int_op(1, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now, 0));
tt_mem_op(&id, ==, get_master_identity_key(), sizeof(id));
tt_mem_op(&sign, ==, get_master_signing_keypair(), sizeof(sign));
tt_assert(tor_cert_eq(link_cert, get_current_link_cert_cert()));
@ -489,7 +489,7 @@ test_routerkeys_ed_keys_init_all(void *arg)
/* Force a link/auth-key regeneration by advancing time. */
tt_int_op(0, ==, load_ed_keys(options, now+3*86400));
tt_int_op(0, ==, generate_ed_link_cert(options, now+3*86400));
tt_int_op(0, ==, generate_ed_link_cert(options, now+3*86400, 0));
tt_mem_op(&id, ==, get_master_identity_key(), sizeof(id));
tt_mem_op(&sign, ==, get_master_signing_keypair(), sizeof(sign));
tt_assert(! tor_cert_eq(link_cert, get_current_link_cert_cert()));
@ -502,8 +502,8 @@ test_routerkeys_ed_keys_init_all(void *arg)
memcpy(&auth, get_current_auth_keypair(), sizeof(auth));
/* Force a signing-key regeneration by advancing time. */
tt_int_op(0, ==, load_ed_keys(options, now+100*86400));
tt_int_op(0, ==, generate_ed_link_cert(options, now+100*86400));
tt_int_op(1, ==, load_ed_keys(options, now+100*86400));
tt_int_op(0, ==, generate_ed_link_cert(options, now+100*86400, 0));
tt_mem_op(&id, ==, get_master_identity_key(), sizeof(id));
tt_mem_op(&sign, !=, get_master_signing_keypair(), sizeof(sign));
tt_assert(! tor_cert_eq(link_cert, get_current_link_cert_cert()));
@ -520,8 +520,8 @@ test_routerkeys_ed_keys_init_all(void *arg)
routerkeys_free_all();
unlink(get_fname("test_ed_keys_init_all/keys/"
"ed25519_master_id_secret_key"));
tt_int_op(0, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now));
tt_int_op(1, ==, load_ed_keys(options, now));
tt_int_op(0, ==, generate_ed_link_cert(options, now, 0));
tt_mem_op(&id, ==, get_master_identity_key(), sizeof(id));
tt_mem_op(&sign, ==, get_master_signing_keypair(), sizeof(sign));
tt_assert(! tor_cert_eq(link_cert, get_current_link_cert_cert()));

4
src/test/test_shared_random.c
View File

@ -48,7 +48,7 @@ init_authority_state(void)
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
tt_assert(mock_cert);
options->AuthoritativeDir = 1;
tt_int_op(0, ==, load_ed_keys(options, time(NULL)));
tt_int_op(load_ed_keys(options, time(NULL)), OP_GE, 0);
sr_state_init(0, 0);
/* It's possible a commit has been generated in our state depending on
* the phase we are currently in which uses "now" as the starting
@ -286,7 +286,7 @@ test_sr_commit(void *arg)
tt_assert(auth_cert);
options->AuthoritativeDir = 1;
tt_int_op(0, ==, load_ed_keys(options, now));
tt_int_op(load_ed_keys(options, time(NULL)), OP_GE, 0);
}
/* Generate our commit object and validate it has the appropriate field