Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  closed
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:  implemented
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => closed
 * resolution:   => implemented
 * milestone:  Tor: 0.2.9.x-final => Tor: 0.3.0.x-final


Comment:

 Given no objections, not backporting.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * keywords:  029-proposed, review-group-13 => 029-backport
 * milestone:  Tor: 0.3.0.x-final => Tor: 0.2.9.x-final


Comment:

 (d46c1b49a459f1249ef358b3751ef656d9e19038 would be the thing to cherry-
 pick, if we _do_ backport.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
---+---
 Reporter:  arma   |  Owner:
 Type:  enhancement| Status:  needs_review
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:  Tor:
   |  0.2.9.1-alpha
 Severity:  Normal | Resolution:
 Keywords:  029-proposed, review-group-13  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by nickm):

 LGTM.  I'm going to merge this in 0.3.0, and not plan to backport.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
---+---
 Reporter:  arma   |  Owner:
 Type:  enhancement| Status:  needs_review
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:  Tor:
   |  0.2.9.1-alpha
 Severity:  Normal | Resolution:
 Keywords:  029-proposed, review-group-13  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by arma):

 Code looks good! I haven't looked at the unit test part. I recommend
 putting it in an 0.3.0 for a while before we consider a backport to 0.2.9.
 (And I hope that we'll choose to not backport it, since clients use 3
 dirguards and we've survived this long with this bug in Tor.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
---+---
 Reporter:  arma   |  Owner:
 Type:  enhancement| Status:  needs_review
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:  Tor:
   |  0.2.9.1-alpha
 Severity:  Normal | Resolution:
 Keywords:  029-proposed, review-group-13  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by teor):

 The bug that serves stale consensuses is still happening in 0.2.9.5-alpha
 and later, see #20909.
 So we probably want this in 0.2.9.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  merge_ready => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by rubiate):

 > It might be what you wanted here.

 Yes! I did look if there was something to do this, and apparently I should
 have looked harder. Thanks.


 New patch:

 - Parens in define
 - Using rate_limit_log()
 - Log message changed
 - Removed unnecessary NULL assignment

 {{{
 diff --git a/changes/20511 b/changes/20511
 new file mode 100644
 index 000..d6e962e
 --- /dev/null
 +++ b/changes/20511
 @@ -0,0 +1,3 @@
 +  o Minor feature:
 +- Relays and bridges will now refuse to serve the consensus they have
 if
 +  they know it is too old for a client to use. Closes ticket 20511.
 diff --git a/src/or/directory.c b/src/or/directory.c
 index ba6d38c..b5c9d49 100644
 --- a/src/or/directory.c
 +++ b/src/or/directory.c
 @@ -2939,6 +2939,28 @@ handle_get_frontpage(dir_connection_t *conn, const
 get_handler_args_t *args)
return 0;
  }

 +/** Warn that the consensus v of type flavor is too old and
 will
 + * not be served to clients. Rate-limit the warning to avoid logging an
 entry
 + * on every request.
 + */
 +static void
 +warn_consensus_is_too_old(networkstatus_t *v, const char *flavor, time_t
 now)
 +{
 +#define TOO_OLD_WARNING_INTERVAL (60*60)
 +  static ratelim_t warned = RATELIM_INIT(TOO_OLD_WARNING_INTERVAL);
 +  char timestamp[ISO_TIME_LEN+1];
 +  char *dupes;
 +
 +  if ((dupes = rate_limit_log(, now))) {
 +format_local_iso_time(timestamp, v->valid_until);
 +log_warn(LD_DIRSERV, "Our %s%sconsensus is too old, so we will not "
 + "serve it to clients. It was valid until %s local time and
 we "
 + "continued to serve it for up to 24 hours after it
 expired.%s",
 + flavor ? flavor : "", flavor ? " " : "", timestamp, dupes);
 +tor_free(dupes);
 +  }
 +}
 +
  /** Helper function for GET /tor/status-vote/current/consensus
   */
  static int
 @@ -2983,6 +3005,15 @@ handle_get_current_consensus(dir_connection_t
 *conn,

v = networkstatus_get_latest_consensus_by_flavor(flav);

 +  if (v && !networkstatus_consensus_reasonably_live(v, now)) {
 +write_http_status_line(conn, 404, "Consensus is too old");
 +warn_consensus_is_too_old(v, flavor, now);
 +smartlist_free(dir_fps);
 +geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND);
 +tor_free(flavor);
 +goto done;
 +  }
 +
if (v && want_fps &&
!client_likes_consensus(v, want_fps)) {
  write_http_status_line(conn, 404, "Consensus not signed by
 sufficient "
 diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
 index ed888fb..fde0b18 100644
 --- a/src/or/networkstatus.c
 +++ b/src/or/networkstatus.c
 @@ -1342,6 +1342,24 @@ networkstatus_get_live_consensus,(time_t now))
  return NULL;
  }

 +/** Determine if consensus is valid or expired recently enough
 that
 + * we can still use it.
 + *
 + * Return 1 if the consensus is reasonably live, or 0 if it is too old.
 + */
 +int
 +networkstatus_consensus_reasonably_live(networkstatus_t *consensus,
 time_t now)
 +{
 +#define REASONABLY_LIVE_TIME (24*60*60)
 +  if (BUG(!consensus))
 +return 0;
 +
 +  if (now <= consensus->valid_until + REASONABLY_LIVE_TIME)
 +return 1;
 +
 +  return 0;
 +}
 +
  /*  remove this in favor of get_live_consensus. But actually,
   * leave something like it for bridge users, who need to not totally
   * lose if they spend a while fetching a new consensus. */
 @@ -1350,12 +1368,11 @@ networkstatus_get_live_consensus,(time_t now))
  networkstatus_t *
  networkstatus_get_reasonably_live_consensus(time_t now, int flavor)
  {
 -#define REASONABLY_LIVE_TIME (24*60*60)
networkstatus_t *consensus =
  networkstatus_get_latest_consensus_by_flavor(flavor);
if (consensus &&
consensus->valid_after <= now &&
 -  now <= consensus->valid_until+REASONABLY_LIVE_TIME)
 +  networkstatus_consensus_reasonably_live(consensus, now))
  return consensus;
else
  return NULL;
 diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
 index 71f36b6..172c0ea 100644
 --- a/src/or/networkstatus.h
 +++ b/src/or/networkstatus.h
 @@ -79,6 +79,8 @@ MOCK_DECL(networkstatus_t
 *,networkstatus_get_latest_consensus,(void));
  MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor,
(consensus_flavor_t f));
  MOCK_DECL(networkstatus_t *, 

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by arma):

 Replying to [comment:8 teor]:
 > I am not sure whether we should backport this to 0.2.9, but given that's
 where the trouble started, maybe it would be a good idea to do that as a
 precaution.

 I would turn that around and ask: is it likely that there's a bug in 0.2.9
 that *isn't* in 0.3.0? That angle makes me think 0.3.0 is fine.

 That said, I'm ok with either plan.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by arma):

 Replying to [comment:7 rubiate]:
 > +#define TOO_OLD_WARNING_TIMEOUT 60*60

 Putting parens around this value will make you a happier camper in the
 future. The above line is straight out of some arithmetic precedence
 example bug.

 Also, for your {{{static time_t warned}}}, did you know about the
 rate_limit_log() function? It might be what you wanted here.

 > +  format_local_iso_time(timestamp, v->valid_until);
 > +  log_fn(severity, LD_DIRSERV,
 > + "Our %s%sconsensus is too old, we will not serve it to
 clients. "
 > + "It was valid until %s and we continued to serve it for up to
 24 "
 > + "hours after it expired.",
 > + flavor ? flavor : "", flavor ? " " : "", timestamp);

 A) s/old, we/old, so we/

 B) I think format_local_iso_time() produces a time in the local time zone,
 right? So we should clarify in the log message by adding a phrase like
 "(in local time zone)". Otherwise we've left it ambiguous whether we mean
 the UTC time that consensuses use, or the local time that log lines
 sometimes use.

 > +  tor_free(header);
 > +  header = NULL;

 tor_free sets its argument to NULL already, right?

 Good to see unit tests!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:  Tor: 0.2.9.1-alpha
 Severity:  Normal| Resolution:
 Keywords:  029-proposed  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  needs_revision => merge_ready
 * keywords:   => 029-proposed
 * version:   => Tor: 0.2.9.1-alpha


Comment:

 Looks ok to me. I am not sure whether we should backport this to 0.2.9,
 but given that's where the trouble started, maybe it would be a good idea
 to do that as a precaution.

 That said, I think it should be tested in master first, and not shoved
 straight into 0.2.9.5-alpha.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by rubiate):

 Here's an updated patch using a define for the warning timeout and a NULL
 consensus check:

 {{{
 diff --git a/changes/20511 b/changes/20511
 new file mode 100644
 index 000..d6e962e
 --- /dev/null
 +++ b/changes/20511
 @@ -0,0 +1,3 @@
 +  o Minor feature:
 +- Relays and bridges will now refuse to serve the consensus they have
 if
 +  they know it is too old for a client to use. Closes ticket 20511.
 diff --git a/src/or/directory.c b/src/or/directory.c
 index ba6d38c..24393eb 100644
 --- a/src/or/directory.c
 +++ b/src/or/directory.c
 @@ -2939,6 +2939,33 @@ handle_get_frontpage(dir_connection_t *conn, const
 get_handler_args_t *args)
return 0;
  }

 +/** Warn that the consensus v of type flavor is too old and
 will
 + * not be served. Log the message at a lower severity if we have already
 + * warned about this recently.
 + */
 +static void
 +warn_consensus_is_too_old(networkstatus_t *v, const char *flavor, time_t
 now)
 +{
 +  static time_t warned = 0;
 +  int severity;
 +  char timestamp[ISO_TIME_LEN+1];
 +
 +#define TOO_OLD_WARNING_TIMEOUT 60*60
 +  if (now >= warned + TOO_OLD_WARNING_TIMEOUT) {
 +severity = LOG_WARN;
 +warned = now;
 +  } else {
 +severity = LOG_DEBUG;
 +  }
 +
 +  format_local_iso_time(timestamp, v->valid_until);
 +  log_fn(severity, LD_DIRSERV,
 + "Our %s%sconsensus is too old, we will not serve it to clients.
 "
 + "It was valid until %s and we continued to serve it for up to 24
 "
 + "hours after it expired.",
 + flavor ? flavor : "", flavor ? " " : "", timestamp);
 +}
 +
  /** Helper function for GET /tor/status-vote/current/consensus
   */
  static int
 @@ -2983,6 +3010,15 @@ handle_get_current_consensus(dir_connection_t
 *conn,

v = networkstatus_get_latest_consensus_by_flavor(flav);

 +  if (v && !networkstatus_consensus_reasonably_live(v, now)) {
 +write_http_status_line(conn, 404, "Consensus is too old");
 +warn_consensus_is_too_old(v, flavor, now);
 +smartlist_free(dir_fps);
 +geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND);
 +tor_free(flavor);
 +goto done;
 +  }
 +
if (v && want_fps &&
!client_likes_consensus(v, want_fps)) {
  write_http_status_line(conn, 404, "Consensus not signed by
 sufficient "
 diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
 index ed888fb..fde0b18 100644
 --- a/src/or/networkstatus.c
 +++ b/src/or/networkstatus.c
 @@ -1342,6 +1342,24 @@ networkstatus_get_live_consensus,(time_t now))
  return NULL;
  }

 +/** Determine if consensus is valid or expired recently enough
 that
 + * we can still use it.
 + *
 + * Return 1 if the consensus is reasonably live, or 0 if it is too old.
 + */
 +int
 +networkstatus_consensus_reasonably_live(networkstatus_t *consensus,
 time_t now)
 +{
 +#define REASONABLY_LIVE_TIME (24*60*60)
 +  if (BUG(!consensus))
 +return 0;
 +
 +  if (now <= consensus->valid_until + REASONABLY_LIVE_TIME)
 +return 1;
 +
 +  return 0;
 +}
 +
  /*  remove this in favor of get_live_consensus. But actually,
   * leave something like it for bridge users, who need to not totally
   * lose if they spend a while fetching a new consensus. */
 @@ -1350,12 +1368,11 @@ networkstatus_get_live_consensus,(time_t now))
  networkstatus_t *
  networkstatus_get_reasonably_live_consensus(time_t now, int flavor)
  {
 -#define REASONABLY_LIVE_TIME (24*60*60)
networkstatus_t *consensus =
  networkstatus_get_latest_consensus_by_flavor(flavor);
if (consensus &&
consensus->valid_after <= now &&
 -  now <= consensus->valid_until+REASONABLY_LIVE_TIME)
 +  networkstatus_consensus_reasonably_live(consensus, now))
  return consensus;
else
  return NULL;
 diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
 index 71f36b6..172c0ea 100644
 --- a/src/or/networkstatus.h
 +++ b/src/or/networkstatus.h
 @@ -79,6 +79,8 @@ MOCK_DECL(networkstatus_t
 *,networkstatus_get_latest_consensus,(void));
  MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor,
(consensus_flavor_t f));
  MOCK_DECL(networkstatus_t *, networkstatus_get_live_consensus,(time_t
 now));
 +int networkstatus_consensus_reasonably_live(networkstatus_t *consensus,
 +time_t now);
  

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by teor):

 * status:  needs_review => needs_revision


Comment:

 This patch looks good.
 Since networkstatus_consensus_reasonably_live depends on consensus being
 non-NULL, I suggest you do something like:
 {{{
 if (BUG(!consensus)) {
   return 0;
 }
 }}}
 A nitpick, I would make the 60*60 log severity time in a #define.

 Another (separate) issue is whether clients should reject consensuses that
 are obviously too old. This was fixed in #20533: clients that receive a
 consensus after its valid_until time (or that try to download certificates
 for an expired consensus) will stop downloading certificates and consider
 the consensus a failure. I think we also reject old consensuses as soon as
 we parse them, but we should check this. See

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  new => needs_review


Comment:

 Thanks; putting this on the review queue!

 (Next time, if you use 'git format-patch' it produces something much
 easier to merge.)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by rubiate):

 Will this work? I'm not sure that this is all that's needed or if there's
 other places that it needs to be checked.

 I thought Tor should warn that this has happened, but probably shouldn't
 do it at a normal log-level on every dir request as that could get noisy.

 {{{
 diff --git a/src/or/directory.c b/src/or/directory.c
 index ba6d38c..16e33c5 100644
 --- a/src/or/directory.c
 +++ b/src/or/directory.c
 @@ -2939,6 +2939,32 @@ handle_get_frontpage(dir_connection_t *conn, const
 get_handler_args_t *args)
return 0;
  }

 +/** Warn that the consensus v of type flavor is too old and
 will
 + * not be served. Log the message at a lower severity if we have already
 + * warned about this recently.
 + */
 +static void
 +warn_consensus_is_too_old(networkstatus_t *v, const char *flavor, time_t
 now)
 +{
 +  static time_t warned = 0;
 +  int severity;
 +  char timestamp[ISO_TIME_LEN+1];
 +
 +  if (now >= warned + 60 * 60) {
 +severity = LOG_WARN;
 +warned = now;
 +  } else {
 +severity = LOG_DEBUG;
 +  }
 +
 +  format_local_iso_time(timestamp, v->valid_until);
 +  log_fn(severity, LD_DIRSERV,
 + "Our %s%sconsensus is too old, we will not serve it to clients.
 "
 + "It was valid until %s and we continued to serve it for up to 24
 "
 + "hours after it expired.",
 + flavor ? flavor : "", flavor ? " " : "", timestamp);
 +}
 +
  /** Helper function for GET /tor/status-vote/current/consensus
   */
  static int
 @@ -2983,6 +3009,15 @@ handle_get_current_consensus(dir_connection_t
 *conn,

v = networkstatus_get_latest_consensus_by_flavor(flav);

 +  if (v && !networkstatus_consensus_reasonably_live(v, now)) {
 +write_http_status_line(conn, 404, "Consensus is too old");
 +warn_consensus_is_too_old(v, flavor, now);
 +smartlist_free(dir_fps);
 +geoip_note_ns_response(GEOIP_REJECT_NOT_FOUND);
 +tor_free(flavor);
 +goto done;
 +  }
 +
if (v && want_fps &&
!client_likes_consensus(v, want_fps)) {
  write_http_status_line(conn, 404, "Consensus not signed by
 sufficient "
 diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
 index ed888fb..7f200b9 100644
 --- a/src/or/networkstatus.c
 +++ b/src/or/networkstatus.c
 @@ -1342,6 +1342,21 @@ networkstatus_get_live_consensus,(time_t now))
  return NULL;
  }

 +/** Determine if consensus is valid or expired recently enough
 that
 + * we can still use it.
 + *
 + * Return 1 if the consensus is reasonably live, or 0 if it is too old.
 + */
 +int
 +networkstatus_consensus_reasonably_live(networkstatus_t *consensus,
 time_t now)
 +{
 +#define REASONABLY_LIVE_TIME (24*60*60)
 +  if (now <= consensus->valid_until + REASONABLY_LIVE_TIME)
 +return 1;
 +
 +  return 0;
 +}
 +
  /*  remove this in favor of get_live_consensus. But actually,
   * leave something like it for bridge users, who need to not totally
   * lose if they spend a while fetching a new consensus. */
 @@ -1350,12 +1365,11 @@ networkstatus_get_live_consensus,(time_t now))
  networkstatus_t *
  networkstatus_get_reasonably_live_consensus(time_t now, int flavor)
  {
 -#define REASONABLY_LIVE_TIME (24*60*60)
networkstatus_t *consensus =
  networkstatus_get_latest_consensus_by_flavor(flavor);
if (consensus &&
consensus->valid_after <= now &&
 -  now <= consensus->valid_until+REASONABLY_LIVE_TIME)
 +  networkstatus_consensus_reasonably_live(consensus, now))
  return consensus;
else
  return NULL;
 diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
 index 71f36b6..172c0ea 100644
 --- a/src/or/networkstatus.h
 +++ b/src/or/networkstatus.h
 @@ -79,6 +79,8 @@ MOCK_DECL(networkstatus_t
 *,networkstatus_get_latest_consensus,(void));
  MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor,
(consensus_flavor_t f));
  MOCK_DECL(networkstatus_t *, networkstatus_get_live_consensus,(time_t
 now));
 +int networkstatus_consensus_reasonably_live(networkstatus_t *consensus,
 +time_t now);
  networkstatus_t *networkstatus_get_reasonably_live_consensus(time_t now,
   int flavor);
  MOCK_DECL(int, networkstatus_consensus_is_bootstrapping,(time_t now));
 }}}

 Tests and changes file:

 Note 

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by teor):

 Clock skew on relays becomes much more important after this fix, but the
 directory authorities already check for that (~3 hours).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * milestone:   => Tor: 0.3.0.x-final


Comment:

 404 seems fine to me.  You asked for a "current" consensus.  I don't have
 one of those.

 Tenatively assigning this to 0.3.0

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #20511 [Core Tor/Tor]: add a failsafe where if you're about to serve a consensus that you know is obsolete, don't do it

[Tor Bug Tracker & Wiki]

#20511: add a failsafe where if you're about to serve a consensus that you know 
is
obsolete, don't do it
--+-
 Reporter:  arma  |  Owner:
 Type:  enhancement   | Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+-

Comment (by rubiate):

 > Is there a consensus age past which the correct answer for a dir cache
 in that situation should be "I'm really sorry, but it's for your own good:
 404"?

 24 hours is how long past the valid-until time it can be before "We have
 no recent usable consensus" happens (per
 update_router_have_minimum_dir_info() and
 networkstatus_get_reasonably_live_consensus()).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs