From 44da47d3c1e4809d5aaf91cd708c6f02b01a5396 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2016 12:53:12 -0400 Subject: Move extra_info_digest256 into signed_descriptor_t This patch includes no semantic changes; it's just a field movement. It's prerequisite for a fix to 19017/17150. --- src/or/router.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/or/router.c') diff --git a/src/or/router.c b/src/or/router.c index 841f6fde1b..3943643f9b 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2028,7 +2028,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) memcpy(ri->cache_info.extra_info_digest, ei->cache_info.signed_descriptor_digest, DIGEST_LEN); - memcpy(ri->extra_info_digest256, + memcpy(ri->cache_info.extra_info_digest256, ei->digest256, DIGEST256_LEN); } else { @@ -2543,9 +2543,9 @@ router_dump_router_to_string(routerinfo_t *router, char extra_info_digest[HEX_DIGEST_LEN+1]; base16_encode(extra_info_digest, sizeof(extra_info_digest), router->cache_info.extra_info_digest, DIGEST_LEN); - if (!tor_digest256_is_zero(router->extra_info_digest256)) { + if (!tor_digest256_is_zero(router->cache_info.extra_info_digest256)) { char d256_64[BASE64_DIGEST256_LEN+1]; - digest256_to_base64(d256_64, router->extra_info_digest256); + digest256_to_base64(d256_64, router->cache_info.extra_info_digest256); tor_asprintf(&extra_info_line, "extra-info-digest %s %s\n", extra_info_digest, d256_64); } else { -- cgit v1.2.3-54-g00ecf From 8acfac7375e7a0692193434704984d7eb507faf1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2016 13:14:04 -0400 Subject: Copy the signing_key_cert field into signed_descriptor_t We need this field to be in signed_descriptor_t so that routerinfo_incompatible_with_extrainfo can work correctly (#17150). But I don't want to move it completely in this patch, since a great deal of the code that messes with it has been in flux since 0.2.7, when this ticket was opened. I should open another ticket about removing the field from routerinfo_t and extrainfo_t later on. This patch fixes no actual behavior. --- src/or/or.h | 8 ++++++-- src/or/router.c | 3 +++ src/or/routerlist.c | 2 ++ src/or/routerparse.c | 2 ++ 4 files changed, 13 insertions(+), 2 deletions(-) (limited to 'src/or/router.c') diff --git a/src/or/or.h b/src/or/or.h index e0f2eb416a..ffff6d631a 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1995,6 +1995,8 @@ typedef struct signed_descriptor_t { char extra_info_digest[DIGEST_LEN]; /** For routerdescs only: A SHA256-digest of the extrainfo (if any) */ char extra_info_digest256[DIGEST256_LEN]; + /** Certificate for ed25519 signing key. */ + struct tor_cert_st *signing_key_cert; /** For routerdescs only: Status of downloading the corresponding * extrainfo. */ download_status_t ei_dl_status; @@ -2043,7 +2045,8 @@ 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 */ + /** 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? */ @@ -2115,7 +2118,8 @@ typedef struct extrainfo_t { uint8_t digest256[DIGEST256_LEN]; /** The router's nickname. */ char nickname[MAX_NICKNAME_LEN+1]; - /** Certificate for ed25519 signing key */ + /** 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. */ diff --git a/src/or/router.c b/src/or/router.c index 3943643f9b..b3523ec718 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1911,6 +1911,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) 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)); ri->platform = tor_strdup(platform); @@ -2003,6 +2004,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) 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, DIGEST_LEN); if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body, diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 140fe218c3..9b3b79444f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2670,6 +2670,7 @@ routerinfo_free(routerinfo_t *router) 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)); smartlist_free(router->declared_family); @@ -2689,6 +2690,7 @@ 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); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index bc6c35fe09..531a95d5b8 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -1403,6 +1403,7 @@ router_parse_entry_from_string(const char *s, const char *end, goto err; } router->signing_key_cert = cert; /* makes sure it gets freed. */ + router->cache_info.signing_key_cert = tor_cert_dup(cert); if (cert->cert_type != CERT_TYPE_ID_SIGNING || ! cert->signing_key_included) { @@ -1778,6 +1779,7 @@ extrainfo_parse_entry_from_string(const char *s, const char *end, goto err; } extrainfo->signing_key_cert = cert; /* makes sure it gets freed. */ + extrainfo->cache_info.signing_key_cert = tor_cert_dup(cert); if (cert->cert_type != CERT_TYPE_ID_SIGNING || ! cert->signing_key_included) { log_warn(LD_DIR, "Invalid form for ed25519 cert"); -- cgit v1.2.3-54-g00ecf From 00f74e0372a956f9db590e1cb2ddcfb265125023 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 May 2016 13:24:01 -0400 Subject: Improve API of routerinfo_incompatible_with_extrainfo() This API change makes it so that routerinfo_incompatible...() no longer takes a routerinfo_t, so that it's obvious that it should only look at fields from the signed_descriptor_t. This change should prevent a recurrence of #17150. --- src/or/dirserv.c | 9 ++++++--- src/or/router.c | 3 ++- src/or/routerlist.c | 21 ++++++++++----------- src/or/routerlist.h | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) (limited to 'src/or/router.c') diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 01b08ca41b..ab77021184 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -691,12 +691,14 @@ dirserv_add_descriptor(routerinfo_t *ri, const char **msg, const char *source) static was_router_added_t dirserv_add_extrainfo(extrainfo_t *ei, const char **msg) { - const routerinfo_t *ri; + routerinfo_t *ri; int r; tor_assert(msg); *msg = NULL; - ri = router_get_by_id_digest(ei->cache_info.identity_digest); + /* Needs to be mutable so routerinfo_incompatible_with_extrainfo + * can mess with some of the flags in ri->cache_info. */ + ri = router_get_mutable_by_digest(ei->cache_info.identity_digest); if (!ri) { *msg = "No corresponding router descriptor for extra-info descriptor"; extrainfo_free(ei); @@ -716,7 +718,8 @@ dirserv_add_extrainfo(extrainfo_t *ei, const char **msg) return ROUTER_BAD_EI; } - if ((r = routerinfo_incompatible_with_extrainfo(ri, ei, NULL, msg))) { + if ((r = routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, + &ri->cache_info, msg))) { extrainfo_free(ei); return r < 0 ? ROUTER_IS_ALREADY_KNOWN : ROUTER_BAD_EI; } diff --git a/src/or/router.c b/src/or/router.c index b3523ec718..37ce9e7b45 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2072,7 +2072,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ri->cache_info.signed_descriptor_digest); if (ei) { - tor_assert(! routerinfo_incompatible_with_extrainfo(ri, ei, NULL, NULL)); + tor_assert(! routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, + &ri->cache_info, NULL)); } *r = ri; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index f75ec11aca..0f1fac747f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2900,7 +2900,7 @@ extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei, int warn_if_incompatible)) "Mismatch in digest in extrainfo map."); goto done; } - if (routerinfo_incompatible_with_extrainfo(ri, ei, sd, + if (routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, sd, &compatibility_error_msg)) { char d1[HEX_DIGEST_LEN+1], d2[HEX_DIGEST_LEN+1]; r = (ri->cache_info.extrainfo_is_bogus) ? @@ -4901,9 +4901,9 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) /** Check whether sd describes a router descriptor compatible with the * extrainfo document ei. * - * ri (which must also be provided) is the full routerinfo corresponding - * to the same router -- but note that it might not refer to the same specific - * descriptor as sd. + * identity_pkey (which must also be provided) is RSA1024 identity key + * for the router. We use it to check the signature of the extrainfo document, + * if it has not already been checked. * * If no router is compatible with ei, ei should be * dropped. Return 0 for "compatible", return 1 for "reject, and inform @@ -4915,16 +4915,15 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) * but the extrainfo was nonetheless incompatible. **/ int -routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri, +routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey, extrainfo_t *ei, signed_descriptor_t *sd, const char **msg) { int digest_matches, digest256_matches, r=1; - tor_assert(ri); + tor_assert(identity_pkey); + tor_assert(sd); tor_assert(ei); - if (!sd) - sd = (signed_descriptor_t*)&ri->cache_info; if (ei->bad_sig) { if (msg) *msg = "Extrainfo signature was bad, or signed with wrong key."; @@ -4942,7 +4941,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri, /* The identity must match exactly to have been generated at the same time * by the same router. */ - if (tor_memneq(ri->cache_info.identity_digest, + if (tor_memneq(sd->identity_digest, ei->cache_info.identity_digest, DIGEST_LEN)) { if (msg) *msg = "Extrainfo nickname or identity did not match routerinfo"; @@ -4956,7 +4955,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri, if (ei->pending_sig) { char signed_digest[128]; - if (crypto_pk_public_checksig(ri->identity_pkey, + if (crypto_pk_public_checksig(identity_pkey, signed_digest, sizeof(signed_digest), ei->pending_sig, ei->pending_sig_len) != DIGEST_LEN || tor_memneq(signed_digest, ei->cache_info.signed_descriptor_digest, @@ -4967,7 +4966,7 @@ routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri, goto err; /* Bad signature, or no match. */ } - ei->cache_info.send_unencrypted = ri->cache_info.send_unencrypted; + ei->cache_info.send_unencrypted = sd->send_unencrypted; tor_free(ei->pending_sig); } diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 200533fe91..bbe99f8173 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -187,7 +187,7 @@ void update_extrainfo_downloads(time_t now); void router_reset_descriptor_download_failures(void); int router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2); -int routerinfo_incompatible_with_extrainfo(const routerinfo_t *ri, +int routerinfo_incompatible_with_extrainfo(const crypto_pk_t *ri, extrainfo_t *ei, signed_descriptor_t *sd, const char **msg); -- cgit v1.2.3-54-g00ecf