Opened 3 months ago

Last modified 10 days ago

#28591 merge_ready defect

Accept a future consensus for bootstrap

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: bootstrap, clock-skew, tor-guard, usability, ux, s8-errors, 035-roadmap-subtask, 035-triaged-in-20180711
Cc: iry, brade, mcs, intrigeri, torproject@… Actual Points: 0.5
Parent ID: #28018 Points:
Reviewer: catalyst Sponsor:

Description

#24661 allows tor to bootstrap when the client's clock is ahead of the network by up to 1 day.

But clients can't bootstrap when the client's clock is behind the network by more than a few hours:
https://trac.torproject.org/projects/tor/ticket/24661#comment:18

Child Tickets

TicketStatusOwnerSummaryComponent
#19460closedImprove consensus handling of clients with skewed clocksCore Tor/Tor
#28654closedAllow relays to serve future consensusesCore Tor/Tor

Change History (13)

comment:1 Changed 3 months ago by teor

Status: assigned → needs_review

Here's the pull request:
​https://github.com/torproject/tor/pull/551

The code is based on #24661 and maint-0.3.5. It also includes #28654. (I'm not sure if #28654 is a good idea, but it makes sense to be consistent.)

comment:2 Changed 3 months ago by dgoulet

Reviewer: → catalyst

comment:3 Changed 3 months ago by catalyst

Status: needs_review → needs_revision

Thanks! Mostly looks good, but I haven't tested it yet. I left a minor comment on the pull request about a build failure that I get. When you fix that, I can try to do some manual tests. I can also try to fix it up myself later if you don't have the time.

comment:4 Changed 3 months ago by teor

Sponsor: → Sponsor8-can

comment:5 in reply to:  3 Changed 2 months ago by teor

Status: needs_revision → needs_review

Replying to catalyst:

Thanks! Mostly looks good, but I haven't tested it yet. I left a minor comment on the pull request about a build failure that I get. When you fix that, I can try to do some manual tests. I can also try to fix it up myself later if you don't have the time.

I fixed the build failure by making the code simpler.

comment:6 Changed 7 weeks ago by catalyst

Status: needs_review → merge_ready

Thanks! Looks good. I did a little bit of manual testing with libfaketime.

comment:7 Changed 6 weeks ago by teor

Actual Points: → 0.5

comment:8 Changed 6 weeks ago by catalyst

Parent ID: #23605 → #28018

Reparent so we can close #23605.

comment:9 Changed 6 weeks ago by nickm

Milestone: Tor: 0.4.0.x-final → Tor: 0.3.4.x-final

I've squashed this as bug28591_035_squashed and merged it to master. We can consider for an 0.3.5 backport, but I am a little unsure about the complexity.

comment:10 Changed 6 weeks ago by nickm

Milestone: Tor: 0.3.4.x-final → Tor: 0.3.5.x-final

comment:11 in reply to:  9 Changed 6 weeks ago by teor

Replying to nickm:

I've squashed this as bug28591_035_squashed and merged it to master. We can consider for an 0.3.5 backport, but I am a little unsure about the complexity.

I would not backport #28564 / commit a9c766c , consensus expiry code on relays has historically caused us a lot of issues.

If we backport #28591, let's do it in two stages:

  • #24661 for entry guards from past consensuses: 805f751 and earlier
  • #28591 (excluding #28564) for bootstrap from future consensuses: 7a45bc7 and earlier

comment:12 Changed 5 weeks ago by gaba

Keywords: s8-bootstrap removed
Sponsor: Sponsor8-can

comment:13 Changed 10 days ago by hefee

Cc: torproject@… added
Note: See TracTickets for help on using tickets.