mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-20 10:12:15 +01:00
Bugfixes on bug11243 fix for the not-added cases and tests
1. The test that adds things to the cache needs to set the clock back so that the descriptors it adds are valid. 2. We split ROUTER_NOT_NEW into ROUTER_TOO_OLD, so that we can distinguish "already had it" from "rejected because of old published date". 3. We make extrainfo_insert() return a was_router_added_t, and we make its caller use it correctly. This is probably redundant with the extrainfo_is_bogus flag.
This commit is contained in:
parent
39795e117f
commit
223d354e34
@ -4994,7 +4994,8 @@ typedef enum was_router_added_t {
|
||||
ROUTER_NOT_IN_CONSENSUS = -3,
|
||||
ROUTER_NOT_IN_CONSENSUS_OR_NETWORKSTATUS = -4,
|
||||
ROUTER_AUTHDIR_REJECTS = -5,
|
||||
ROUTER_WAS_NOT_WANTED = -6
|
||||
ROUTER_WAS_NOT_WANTED = -6,
|
||||
ROUTER_WAS_TOO_OLD = -7,
|
||||
} was_router_added_t;
|
||||
|
||||
/********************************* routerparse.c ************************/
|
||||
|
@ -2937,12 +2937,12 @@ routerlist_insert(routerlist_t *rl, routerinfo_t *ri)
|
||||
}
|
||||
|
||||
/** Adds the extrainfo_t <b>ei</b> to the routerlist <b>rl</b>, if there is a
|
||||
* corresponding router in rl-\>routers or rl-\>old_routers. Return true iff
|
||||
* we actually inserted <b>ei</b>. Free <b>ei</b> if it isn't inserted. */
|
||||
MOCK_IMPL(STATIC int,
|
||||
* corresponding router in rl-\>routers or rl-\>old_routers. Return the status
|
||||
* of inserting <b>ei</b>. Free <b>ei</b> if it isn't inserted. */
|
||||
MOCK_IMPL(STATIC was_router_added_t,
|
||||
extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei))
|
||||
{
|
||||
int r = 0;
|
||||
was_router_added_t r;
|
||||
routerinfo_t *ri = rimap_get(rl->identity_map,
|
||||
ei->cache_info.identity_digest);
|
||||
signed_descriptor_t *sd =
|
||||
@ -2956,9 +2956,11 @@ extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei))
|
||||
|
||||
if (!ri) {
|
||||
/* This router is unknown; we can't even verify the signature. Give up.*/
|
||||
r = ROUTER_NOT_IN_CONSENSUS;
|
||||
goto done;
|
||||
}
|
||||
if (routerinfo_incompatible_with_extrainfo(ri, ei, sd, NULL)) {
|
||||
r = (sd->extrainfo_is_bogus) ? ROUTER_BAD_EI : ROUTER_NOT_IN_CONSENSUS;
|
||||
goto done;
|
||||
}
|
||||
|
||||
@ -2968,7 +2970,7 @@ extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei))
|
||||
ei_tmp = eimap_set(rl->extra_info_map,
|
||||
ei->cache_info.signed_descriptor_digest,
|
||||
ei);
|
||||
r = 1;
|
||||
r = ROUTER_ADDED_SUCCESSFULLY;
|
||||
if (ei_tmp) {
|
||||
rl->extrainfo_store.bytes_dropped +=
|
||||
ei_tmp->cache_info.signed_descriptor_len;
|
||||
@ -2976,7 +2978,7 @@ extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei))
|
||||
}
|
||||
|
||||
done:
|
||||
if (r == 0)
|
||||
if (r != ROUTER_ADDED_SUCCESSFULLY)
|
||||
extrainfo_free(ei);
|
||||
|
||||
#ifdef DEBUG_ROUTERLIST
|
||||
@ -3302,7 +3304,7 @@ routerlist_reset_warnings(void)
|
||||
MOCK_IMPL(int,
|
||||
router_descriptor_is_older_than,(const routerinfo_t *router, int seconds))
|
||||
{
|
||||
return router->cache_info.published_on < time(NULL) - seconds;
|
||||
return router->cache_info.published_on < approx_time() - seconds;
|
||||
}
|
||||
|
||||
/** Add <b>router</b> to the routerlist, if we don't already have it. Replace
|
||||
@ -3477,7 +3479,7 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg,
|
||||
router_descriptor_is_older_than(router, OLD_ROUTER_DESC_MAX_AGE)) {
|
||||
*msg = "Router descriptor was really old.";
|
||||
routerinfo_free(router);
|
||||
return ROUTER_WAS_NOT_NEW;
|
||||
return ROUTER_WAS_TOO_OLD;
|
||||
}
|
||||
|
||||
/* We haven't seen a router with this identity before. Add it to the end of
|
||||
@ -3498,21 +3500,18 @@ was_router_added_t
|
||||
router_add_extrainfo_to_routerlist(extrainfo_t *ei, const char **msg,
|
||||
int from_cache, int from_fetch)
|
||||
{
|
||||
int inserted;
|
||||
was_router_added_t inserted;
|
||||
(void)from_fetch;
|
||||
if (msg) *msg = NULL;
|
||||
/*XXXX023 Do something with msg */
|
||||
|
||||
inserted = extrainfo_insert(router_get_routerlist(), ei);
|
||||
|
||||
if (inserted && !from_cache)
|
||||
if (WRA_WAS_ADDED(inserted) && !from_cache)
|
||||
signed_desc_append_to_journal(&ei->cache_info,
|
||||
&routerlist->extrainfo_store);
|
||||
|
||||
if (inserted)
|
||||
return ROUTER_ADDED_SUCCESSFULLY;
|
||||
else
|
||||
return ROUTER_BAD_EI;
|
||||
return inserted;
|
||||
}
|
||||
|
||||
/** Sorting helper: return <0, 0, or >0 depending on whether the
|
||||
@ -3912,7 +3911,7 @@ router_load_routers_from_string(const char *s, const char *eos,
|
||||
smartlist_add(changed, ri);
|
||||
routerlist_descriptors_added(changed, from_cache);
|
||||
smartlist_clear(changed);
|
||||
} else if (WRA_WAS_REJECTED(r)) {
|
||||
} else if (WRA_NEVER_DOWNLOADABLE(r)) {
|
||||
download_status_t *dl_status;
|
||||
dl_status = router_get_dl_status_by_descriptor_digest(d);
|
||||
if (dl_status) {
|
||||
@ -3977,6 +3976,8 @@ router_load_extrainfo_from_string(const char *s, const char *eos,
|
||||
SMARTLIST_FOREACH_BEGIN(extrainfo_list, extrainfo_t *, ei) {
|
||||
was_router_added_t added =
|
||||
router_add_extrainfo_to_routerlist(ei, &msg, from_cache, !from_cache);
|
||||
uint8_t d[DIGEST_LEN];
|
||||
memcpy(d, ei->cache_info.signed_descriptor_digest, DIGEST_LEN);
|
||||
if (WRA_WAS_ADDED(added) && requested_fingerprints) {
|
||||
char fp[HEX_DIGEST_LEN+1];
|
||||
base16_encode(fp, sizeof(fp), descriptor_digests ?
|
||||
@ -3988,6 +3989,14 @@ router_load_extrainfo_from_string(const char *s, const char *eos,
|
||||
* so long as we would have wanted them anyway. Since we always fetch
|
||||
* all the extrainfos we want, and we never actually act on them
|
||||
* inside Tor, this should be harmless. */
|
||||
} else if (WRA_NEVER_DOWNLOADABLE(added)) {
|
||||
signed_descriptor_t *sd = router_get_by_extrainfo_digest((char*)d);
|
||||
if (sd) {
|
||||
log_info(LD_GENERAL, "Marking extrainfo with descriptor %s as "
|
||||
"unparseable, and therefore undownloadable",
|
||||
hex_str((char*)d,DIGEST_LEN));
|
||||
download_status_mark_impossible(&sd->ei_dl_status);
|
||||
}
|
||||
}
|
||||
} SMARTLIST_FOREACH_END(ei);
|
||||
|
||||
|
@ -100,6 +100,7 @@ void routerlist_reset_warnings(void);
|
||||
static int WRA_WAS_ADDED(was_router_added_t s);
|
||||
static int WRA_WAS_OUTDATED(was_router_added_t s);
|
||||
static int WRA_WAS_REJECTED(was_router_added_t s);
|
||||
static int WRA_NEVER_DOWNLOADABLE(was_router_added_t s);
|
||||
/** Return true iff the outcome code in <b>s</b> indicates that the descriptor
|
||||
* was added. It might still be necessary to check whether the descriptor
|
||||
* generator should be notified.
|
||||
@ -116,7 +117,8 @@ WRA_WAS_ADDED(was_router_added_t s) {
|
||||
*/
|
||||
static INLINE int WRA_WAS_OUTDATED(was_router_added_t s)
|
||||
{
|
||||
return (s == ROUTER_WAS_NOT_NEW ||
|
||||
return (s == ROUTER_WAS_TOO_OLD ||
|
||||
s == ROUTER_WAS_NOT_NEW ||
|
||||
s == ROUTER_NOT_IN_CONSENSUS ||
|
||||
s == ROUTER_NOT_IN_CONSENSUS_OR_NETWORKSTATUS);
|
||||
}
|
||||
@ -126,6 +128,14 @@ static INLINE int WRA_WAS_REJECTED(was_router_added_t s)
|
||||
{
|
||||
return (s == ROUTER_AUTHDIR_REJECTS);
|
||||
}
|
||||
/** Return true iff the outcome code in <b>s</b> indicates that the descriptor
|
||||
* was flat-out rejected. */
|
||||
static INLINE int WRA_NEVER_DOWNLOADABLE(was_router_added_t s)
|
||||
{
|
||||
return (s == ROUTER_AUTHDIR_REJECTS ||
|
||||
s == ROUTER_BAD_EI ||
|
||||
s == ROUTER_WAS_TOO_OLD);
|
||||
}
|
||||
was_router_added_t router_add_to_routerlist(routerinfo_t *router,
|
||||
const char **msg,
|
||||
int from_cache,
|
||||
@ -216,7 +226,8 @@ STATIC void scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
|
||||
|
||||
MOCK_DECL(int, router_descriptor_is_older_than, (const routerinfo_t *router,
|
||||
int seconds));
|
||||
MOCK_DECL(STATIC int, extrainfo_insert,(routerlist_t *rl, extrainfo_t *ei));
|
||||
MOCK_DECL(STATIC was_router_added_t, extrainfo_insert,
|
||||
(routerlist_t *rl, extrainfo_t *ei));
|
||||
|
||||
#endif
|
||||
|
||||
|
@ -618,6 +618,8 @@ test_dir_load_routers(void *arg)
|
||||
|
||||
MOCK(router_get_dl_status_by_descriptor_digest, mock_router_get_dl_status);
|
||||
|
||||
update_approx_time(1412510400);
|
||||
|
||||
smartlist_add(chunks, tor_strdup(EX_RI_MINIMAL));
|
||||
smartlist_add(chunks, tor_strdup(EX_RI_BAD_FINGERPRINT));
|
||||
smartlist_add(chunks, tor_strdup(EX_RI_BAD_SIG2));
|
||||
@ -708,12 +710,12 @@ mock_get_by_ei_desc_digest(const char *d)
|
||||
}
|
||||
|
||||
static smartlist_t *mock_ei_insert_list = NULL;
|
||||
static int
|
||||
static was_router_added_t
|
||||
mock_ei_insert(routerlist_t *rl, extrainfo_t *ei)
|
||||
{
|
||||
(void) rl;
|
||||
smartlist_add(mock_ei_insert_list, ei);
|
||||
return 1;
|
||||
return ROUTER_ADDED_SUCCESSFULLY;
|
||||
}
|
||||
|
||||
static void
|
||||
|
Loading…
Reference in New Issue
Block a user