Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#4011 closed defect (fixed)

0.2.3.3 relay won't fetch microdesc consensus on startup if it already has a normal consensus

Reported by: arma Owned by:
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version: Tor: 0.2.3.3-alpha
Severity: Keywords: tor-relay
Cc: ln5 Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In networkstatus.c we have:

/** A time before which we shouldn't try to replace the current consensus:
 * this will be at some point after the next consensus becomes valid, but
 * before the current consensus becomes invalid. */
static time_t time_to_download_next_consensus = 0;

which we use in update_consensus_networkstatus_downloads() to decide whether to break out:

  if (time_to_download_next_consensus > now)
    return; /* Wait until the current consensus is older. */

The result is that we don't even try to fetch a microdesc consensus if the ns consensus we have is recent enough.

Child Tickets

Change History (11)

comment:1 Changed 8 years ago by arma

Probably the "right" answer here is to turn time_to_download_next_consensus into an array based on flavor. Then we can have a time for each flavor, and move the "if" clause down a few lines into the "for" loop in update_consensus_networkstatus_downloads().

I spent a while thinking about hackish ways too, e.g. time_to_download_next_consensus should be now if there's any consensus whose flavor we don't have. But I think they are all too hackish.

comment:2 Changed 8 years ago by arma

To be clear, this bug only hits us when we have an up-to-date-enough ns-flavor consensus but not microdesc-flavor consensus. This could happen on startup if we have a cached-consensus file but no cached-microdesc-consensus file. Or it could happen during regular operation if we try to fetch both and we successfully get the ns-flavor one but the mirror/authority we happen to pick fails to give us a microdesc-flavor one. Intended behavior is that we'd retry the microdesc-flavor one immediately, but the 'if' would cause us to bail instead.

comment:3 in reply to:  2 Changed 7 years ago by arma

Replying to arma:

To be clear, this bug only hits us when we have an up-to-date-enough ns-flavor consensus but not microdesc-flavor consensus. This could happen on startup if we have a cached-consensus file but no cached-microdesc-consensus file.

I suspect this bug hit dcf in https://trac.torproject.org/projects/tor/ticket/4013#comment:3

comment:4 Changed 7 years ago by arma

This was also reported as #3816.

Also, I think this bug makes our fix for #4013 not so good, since whenever you restart your client you won't try to get a microdesc consensus since you already have a normal one.

comment:5 Changed 7 years ago by ln5

Cc: ln5 added

comment:6 Changed 7 years ago by nickm

Status: newneeds_review

The "turn it into an array" approach seems correct. See branch "bug4011" in my public repository.

comment:7 Changed 7 years ago by arma

Patch looks fine to me. Yay.

That said, deciding we need a new consensus if "c->valid_after > now" freaks me out. That isn't new behavior from this patch (so it should not stop the patch from being merged). But is it wise behavior? If my clock is set 5 hours in the past and I get a consensus that's not valid yet, do I just keep fetching a new one? I guess the same question holds for if my clock is 5 hours in the future and I keeping getting consensuses that are no longer valid. Is this a solved issue (e.g. somewhere we notice that the new copy is the same as the one we already have), or is this worth opening a separate ticket about?

comment:8 Changed 7 years ago by nickm

worth opening a separate ticket about, I would say. Also, if we ever see that there is a consensus in the future, I believe we will warn about skew towards the end of networkstatus_set_current_consensus() and then.... Hm. I don't think we do more than warn there.

Merged the bug4011 branch. Please close this ticket when you open your new ticket about the other issue you thought of?

comment:9 Changed 7 years ago by arma

Resolution: fixed
Status: needs_reviewclosed

Opened #5506. Closing this one. Thanks!

comment:10 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:11 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.