From ab932cd7bfb7e4cfe9c33416ca45e56448c57b58 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2016 20:04:16 -0400 Subject: [PATCH 1/2] Remove duplicate siging_key_cert fields. With the fix for #17150, I added a duplicate certificate here. Here I remove the original location in 0.2.8. (I wouldn't want to do that in 027, due to the amount of authority-voting-related code drift.) Closes 19073. --- src/or/dirserv.c | 16 ++++++------- src/or/dirvote.c | 7 +++--- src/or/or.h | 6 ----- src/or/router.c | 48 +++++++++++++++++++-------------------- src/or/routerlist.c | 4 +--- src/or/routerparse.c | 10 ++++---- src/test/test_dir.c | 9 ++++---- src/test/test_microdesc.c | 2 +- 8 files changed, 48 insertions(+), 54 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 754979581..c97e3bcbc 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -257,11 +257,11 @@ dirserv_router_get_status(const routerinfo_t *router, const char **msg, return FP_REJECT; } - if (router->signing_key_cert) { + if (router->cache_info.signing_key_cert) { /* This has an ed25519 identity key. */ if (KEYPIN_MISMATCH == keypin_check((const uint8_t*)router->cache_info.identity_digest, - router->signing_key_cert->signing_key.pubkey)) { + router->cache_info.signing_key_cert->signing_key.pubkey)) { log_fn(severity, LD_DIR, "Descriptor from router %s has an Ed25519 key, " "but the keys don't match what they were before.", @@ -629,10 +629,10 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source) /* Do keypinning again ... this time, to add the pin if appropriate */ int keypin_status; - if (ri->signing_key_cert) { + if (ri->cache_info.signing_key_cert) { keypin_status = keypin_check_and_add( (const uint8_t*)ri->cache_info.identity_digest, - ri->signing_key_cert->signing_key.pubkey, + ri->cache_info.signing_key_cert->signing_key.pubkey, ! key_pinning); } else { keypin_status = keypin_check_lone_rsa( @@ -2142,9 +2142,9 @@ routers_make_ed_keys_unique(smartlist_t *routers) SMARTLIST_FOREACH_BEGIN(routers, routerinfo_t *, ri) { ri->omit_from_vote = 0; - if (ri->signing_key_cert == NULL) + if (ri->cache_info.signing_key_cert == NULL) continue; /* No ed key */ - const uint8_t *pk = ri->signing_key_cert->signing_key.pubkey; + const uint8_t *pk = ri->cache_info.signing_key_cert->signing_key.pubkey; if ((ri2 = digest256map_get(by_ed_key, pk))) { /* Duplicate; must omit one. Set the omit_from_vote flag in whichever * one has the earlier published_on. */ @@ -2897,8 +2897,8 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, set_routerstatus_from_routerinfo(rs, node, ri, now, listbadexits); - if (ri->signing_key_cert) { - memcpy(vrs->ed25519_id, ri->signing_key_cert->signing_key.pubkey, + if (ri->cache_info.signing_key_cert) { + memcpy(vrs->ed25519_id, ri->cache_info.signing_key_cert->signing_key.pubkey, ED25519_PUBKEY_LEN); } diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 9854af7d7..62f85877f 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -3528,10 +3528,11 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) char idbuf[ED25519_BASE64_LEN+1]; const char *keytype; if (consensus_method >= MIN_METHOD_FOR_ED25519_ID_IN_MD && - ri->signing_key_cert && - ri->signing_key_cert->signing_key_included) { + ri->cache_info.signing_key_cert && + ri->cache_info.signing_key_cert->signing_key_included) { keytype = "ed25519"; - ed25519_public_to_base64(idbuf, &ri->signing_key_cert->signing_key); + ed25519_public_to_base64(idbuf, + &ri->cache_info.signing_key_cert->signing_key); } else { keytype = "rsa1024"; digest_to_base64(idbuf, ri->cache_info.identity_digest); diff --git a/src/or/or.h b/src/or/or.h index aa93c1c03..2252f3816 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2117,9 +2117,6 @@ typedef struct { crypto_pk_t *identity_pkey; /**< Public RSA key for signing. */ /** Public curve25519 key for onions */ curve25519_public_key_t *onion_curve25519_pkey; - /** Certificate for ed25519 signing key - * (XXXX duplicated in cache_info.) */ - struct tor_cert_st *signing_key_cert; /** What's the earliest expiration time on all the certs in this * routerinfo? */ time_t cert_expiration_time; @@ -2195,9 +2192,6 @@ typedef struct extrainfo_t { uint8_t digest256[DIGEST256_LEN]; /** The router's nickname. */ char nickname[MAX_NICKNAME_LEN+1]; - /** Certificate for ed25519 signing key - * (XXXX duplicated in cache_info.) */ - struct tor_cert_st *signing_key_cert; /** True iff we found the right key for this extra-info, verified the * signature, and found it to be bad. */ unsigned int bad_sig : 1; diff --git a/src/or/router.c b/src/or/router.c index cd8437d78..772a855d5 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2036,7 +2036,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) routerinfo_free(ri); return -1; } - ri->signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); ri->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); get_platform_str(platform, sizeof(platform)); @@ -2129,7 +2128,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ei->cache_info.is_extrainfo = 1; strlcpy(ei->nickname, get_options()->Nickname, sizeof(ei->nickname)); ei->cache_info.published_on = ri->cache_info.published_on; - ei->signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); ei->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest, @@ -2528,7 +2526,8 @@ router_dump_router_to_string(routerinfo_t *router, const or_options_t *options = get_options(); smartlist_t *chunks = NULL; char *output = NULL; - const int emit_ed_sigs = signing_keypair && router->signing_key_cert; + const int emit_ed_sigs = signing_keypair && + router->cache_info.signing_key_cert; char *ed_cert_line = NULL; char *rsa_tap_cc_line = NULL; char *ntor_cc_line = NULL; @@ -2540,12 +2539,12 @@ router_dump_router_to_string(routerinfo_t *router, goto err; } if (emit_ed_sigs) { - if (!router->signing_key_cert->signing_key_included || - !ed25519_pubkey_eq(&router->signing_key_cert->signed_key, + if (!router->cache_info.signing_key_cert->signing_key_included || + !ed25519_pubkey_eq(&router->cache_info.signing_key_cert->signed_key, &signing_keypair->pubkey)) { log_warn(LD_BUG, "Tried to sign a router descriptor with a mismatched " "ed25519 key chain %d", - router->signing_key_cert->signing_key_included); + router->cache_info.signing_key_cert->signing_key_included); goto err; } } @@ -2561,14 +2560,14 @@ router_dump_router_to_string(routerinfo_t *router, char ed_cert_base64[256]; char ed_fp_base64[ED25519_BASE64_LEN+1]; if (base64_encode(ed_cert_base64, sizeof(ed_cert_base64), - (const char*)router->signing_key_cert->encoded, - router->signing_key_cert->encoded_len, + (const char*)router->cache_info.signing_key_cert->encoded, + router->cache_info.signing_key_cert->encoded_len, BASE64_ENCODE_MULTILINE) < 0) { log_err(LD_BUG,"Couldn't base64-encode signing key certificate!"); goto err; } if (ed25519_public_to_base64(ed_fp_base64, - &router->signing_key_cert->signing_key)<0) { + &router->cache_info.signing_key_cert->signing_key)<0) { log_err(LD_BUG,"Couldn't base64-encode identity key\n"); goto err; } @@ -2595,13 +2594,13 @@ router_dump_router_to_string(routerinfo_t *router, } /* Cross-certify with RSA key */ - if (tap_key && router->signing_key_cert && - router->signing_key_cert->signing_key_included) { + if (tap_key && router->cache_info.signing_key_cert && + router->cache_info.signing_key_cert->signing_key_included) { char buf[256]; int tap_cc_len = 0; uint8_t *tap_cc = make_tap_onion_key_crosscert(tap_key, - &router->signing_key_cert->signing_key, + &router->cache_info.signing_key_cert->signing_key, router->identity_pkey, &tap_cc_len); if (!tap_cc) { @@ -2625,16 +2624,16 @@ router_dump_router_to_string(routerinfo_t *router, } /* Cross-certify with onion keys */ - if (ntor_keypair && router->signing_key_cert && - router->signing_key_cert->signing_key_included) { + if (ntor_keypair && router->cache_info.signing_key_cert && + router->cache_info.signing_key_cert->signing_key_included) { int sign = 0; char buf[256]; /* XXXX Base the expiration date on the actual onion key expiration time?*/ tor_cert_t *cert = make_ntor_onion_key_crosscert(ntor_keypair, - &router->signing_key_cert->signing_key, - router->cache_info.published_on, - MIN_ONION_KEY_LIFETIME, &sign); + &router->cache_info.signing_key_cert->signing_key, + router->cache_info.published_on, + MIN_ONION_KEY_LIFETIME, &sign); if (!cert) { log_warn(LD_BUG,"make_ntor_onion_key_crosscert failed!"); goto err; @@ -2981,7 +2980,8 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, time_t now = time(NULL); smartlist_t *chunks = smartlist_new(); extrainfo_t *ei_tmp = NULL; - const int emit_ed_sigs = signing_keypair && extrainfo->signing_key_cert; + const int emit_ed_sigs = signing_keypair && + extrainfo->cache_info.signing_key_cert; char *ed_cert_line = NULL; base16_encode(identity, sizeof(identity), @@ -2989,19 +2989,19 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, format_iso_time(published, extrainfo->cache_info.published_on); bandwidth_usage = rep_hist_get_bandwidth_lines(); if (emit_ed_sigs) { - if (!extrainfo->signing_key_cert->signing_key_included || - !ed25519_pubkey_eq(&extrainfo->signing_key_cert->signed_key, + if (!extrainfo->cache_info.signing_key_cert->signing_key_included || + !ed25519_pubkey_eq(&extrainfo->cache_info.signing_key_cert->signed_key, &signing_keypair->pubkey)) { log_warn(LD_BUG, "Tried to sign a extrainfo descriptor with a " "mismatched ed25519 key chain %d", - extrainfo->signing_key_cert->signing_key_included); + extrainfo->cache_info.signing_key_cert->signing_key_included); goto err; } char ed_cert_base64[256]; if (base64_encode(ed_cert_base64, sizeof(ed_cert_base64), - (const char*)extrainfo->signing_key_cert->encoded, - extrainfo->signing_key_cert->encoded_len, - BASE64_ENCODE_MULTILINE) < 0) { + (const char*)extrainfo->cache_info.signing_key_cert->encoded, + extrainfo->cache_info.signing_key_cert->encoded_len, + BASE64_ENCODE_MULTILINE) < 0) { log_err(LD_BUG,"Couldn't base64-encode signing key certificate!"); goto err; } diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 045d50c3d..2634e0d51 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2897,7 +2897,6 @@ routerinfo_free(routerinfo_t *router) tor_free(router->onion_curve25519_pkey); if (router->identity_pkey) crypto_pk_free(router->identity_pkey); - tor_cert_free(router->signing_key_cert); tor_cert_free(router->cache_info.signing_key_cert); if (router->declared_family) { SMARTLIST_FOREACH(router->declared_family, char *, s, tor_free(s)); @@ -2917,7 +2916,6 @@ extrainfo_free(extrainfo_t *extrainfo) { if (!extrainfo) return; - tor_cert_free(extrainfo->signing_key_cert); tor_cert_free(extrainfo->cache_info.signing_key_cert); tor_free(extrainfo->cache_info.signed_descriptor_body); tor_free(extrainfo->pending_sig); @@ -5217,7 +5215,7 @@ routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey, goto err; /* different servers */ } - if (! tor_cert_opt_eq(sd->signing_key_cert, ei->signing_key_cert)) { + if (! tor_cert_opt_eq(sd->signing_key_cert,ei->cache_info.signing_key_cert)) { if (msg) *msg = "Extrainfo signing key cert didn't match routerinfo"; goto err; /* different servers */ } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index b108dd71a..a7c29aa86 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1,4 +1,4 @@ - /* Copyright (c) 2001 Matej Pfajfar. +/* Copyright (c) 2001 Matej Pfajfar. * Copyright (c) 2001-2004, Roger Dingledine. * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. * Copyright (c) 2007-2016, The Tor Project, Inc. */ @@ -1405,8 +1405,7 @@ router_parse_entry_from_string(const char *s, const char *end, log_warn(LD_DIR, "Couldn't parse ed25519 cert"); goto err; } - router->signing_key_cert = cert; /* makes sure it gets freed. */ - router->cache_info.signing_key_cert = tor_cert_dup(cert); + router->cache_info.signing_key_cert = cert; /* makes sure it gets freed.*/ if (cert->cert_type != CERT_TYPE_ID_SIGNING || ! cert->signing_key_included) { @@ -1787,8 +1786,9 @@ extrainfo_parse_entry_from_string(const char *s, const char *end, log_warn(LD_DIR, "Couldn't parse ed25519 cert"); goto err; } - extrainfo->signing_key_cert = cert; /* makes sure it gets freed. */ - extrainfo->cache_info.signing_key_cert = tor_cert_dup(cert); + /* makes sure it gets freed. */ + extrainfo->cache_info.signing_key_cert = cert; + if (cert->cert_type != CERT_TYPE_ID_SIGNING || ! cert->signing_key_included) { log_warn(LD_DIR, "Invalid form for ed25519 cert"); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 401c7b299..2a3fa4a0d 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -160,15 +160,15 @@ test_dir_formats(void *arg) ed25519_secret_key_from_seed(&kp2.seckey, (const uint8_t*)"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"); ed25519_public_key_generate(&kp2.pubkey, &kp2.seckey); - r2->signing_key_cert = tor_cert_create(&kp1, + r2->cache_info.signing_key_cert = tor_cert_create(&kp1, CERT_TYPE_ID_SIGNING, &kp2.pubkey, now, 86400, CERT_FLAG_INCLUDE_SIGNING_KEY); char cert_buf[256]; base64_encode(cert_buf, sizeof(cert_buf), - (const char*)r2->signing_key_cert->encoded, - r2->signing_key_cert->encoded_len, + (const char*)r2->cache_info.signing_key_cert->encoded, + r2->cache_info.signing_key_cert->encoded_len, BASE64_ENCODE_MULTILINE); r2->platform = tor_strdup(platform); r2->cache_info.published_on = 5; @@ -279,7 +279,8 @@ test_dir_formats(void *arg) strlcat(buf2, "master-key-ed25519 ", sizeof(buf2)); { char k[ED25519_BASE64_LEN+1]; - tt_assert(ed25519_public_to_base64(k, &r2->signing_key_cert->signing_key) + tt_assert(ed25519_public_to_base64(k, + &r2->cache_info.signing_key_cert->signing_key) >= 0); strlcat(buf2, k, sizeof(buf2)); strlcat(buf2, "\n", sizeof(buf2)); diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index 7db819a62..581f58b45 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -483,7 +483,7 @@ test_md_generate(void *arg) md = dirvote_create_microdescriptor(ri, 21); tt_str_op(md->body, ==, test_md2_21); tt_assert(ed25519_pubkey_eq(md->ed25519_identity_pkey, - &ri->signing_key_cert->signing_key)); + &ri->cache_info.signing_key_cert->signing_key)); done: microdesc_free(md); From 2729f166cb715f0f4aff8779f0fadebafc56a70c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2016 20:08:03 -0400 Subject: [PATCH 2/2] whitespace fixes --- src/or/dirserv.c | 5 +++-- src/or/router.c | 15 ++++++++------- src/or/routerlist.c | 3 ++- src/or/routerparse.c | 7 ++++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index c97e3bcbc..dafaed8bf 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -261,7 +261,7 @@ dirserv_router_get_status(const routerinfo_t *router, const char **msg, /* This has an ed25519 identity key. */ if (KEYPIN_MISMATCH == keypin_check((const uint8_t*)router->cache_info.identity_digest, - router->cache_info.signing_key_cert->signing_key.pubkey)) { + router->cache_info.signing_key_cert->signing_key.pubkey)) { log_fn(severity, LD_DIR, "Descriptor from router %s has an Ed25519 key, " "but the keys don't match what they were before.", @@ -2898,7 +2898,8 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key, listbadexits); if (ri->cache_info.signing_key_cert) { - memcpy(vrs->ed25519_id, ri->cache_info.signing_key_cert->signing_key.pubkey, + memcpy(vrs->ed25519_id, + ri->cache_info.signing_key_cert->signing_key.pubkey, ED25519_PUBKEY_LEN); } diff --git a/src/or/router.c b/src/or/router.c index 772a855d5..aa4acf6f6 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2036,7 +2036,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) routerinfo_free(ri); return -1; } - ri->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); + ri->cache_info.signing_key_cert = + tor_cert_dup(get_master_signing_key_cert()); get_platform_str(platform, sizeof(platform)); ri->platform = tor_strdup(platform); @@ -2560,9 +2561,9 @@ router_dump_router_to_string(routerinfo_t *router, char ed_cert_base64[256]; char ed_fp_base64[ED25519_BASE64_LEN+1]; if (base64_encode(ed_cert_base64, sizeof(ed_cert_base64), - (const char*)router->cache_info.signing_key_cert->encoded, - router->cache_info.signing_key_cert->encoded_len, - BASE64_ENCODE_MULTILINE) < 0) { + (const char*)router->cache_info.signing_key_cert->encoded, + router->cache_info.signing_key_cert->encoded_len, + BASE64_ENCODE_MULTILINE) < 0) { log_err(LD_BUG,"Couldn't base64-encode signing key certificate!"); goto err; } @@ -2600,9 +2601,9 @@ router_dump_router_to_string(routerinfo_t *router, int tap_cc_len = 0; uint8_t *tap_cc = make_tap_onion_key_crosscert(tap_key, - &router->cache_info.signing_key_cert->signing_key, - router->identity_pkey, - &tap_cc_len); + &router->cache_info.signing_key_cert->signing_key, + router->identity_pkey, + &tap_cc_len); if (!tap_cc) { log_warn(LD_BUG,"make_tap_onion_key_crosscert failed!"); goto err; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 2634e0d51..f9247cba0 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5215,7 +5215,8 @@ routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey, goto err; /* different servers */ } - if (! tor_cert_opt_eq(sd->signing_key_cert,ei->cache_info.signing_key_cert)) { + if (! tor_cert_opt_eq(sd->signing_key_cert, + ei->cache_info.signing_key_cert)) { if (msg) *msg = "Extrainfo signing key cert didn't match routerinfo"; goto err; /* different servers */ } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index a7c29aa86..91025c156 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1405,7 +1405,8 @@ router_parse_entry_from_string(const char *s, const char *end, log_warn(LD_DIR, "Couldn't parse ed25519 cert"); goto err; } - router->cache_info.signing_key_cert = cert; /* makes sure it gets freed.*/ + /* makes sure it gets freed. */ + router->cache_info.signing_key_cert = cert; if (cert->cert_type != CERT_TYPE_ID_SIGNING || ! cert->signing_key_included) { @@ -1600,8 +1601,8 @@ router_parse_entry_from_string(const char *s, const char *end, } if (tok->n_args >= 2) { - if (digest256_from_base64(router->cache_info.extra_info_digest256, tok->args[1]) - < 0) { + if (digest256_from_base64(router->cache_info.extra_info_digest256, + tok->args[1]) < 0) { log_warn(LD_DIR, "Invalid extra info digest256 %s", escaped(tok->args[1])); }