Opened 5 weeks ago

Closed 5 weeks ago

Last modified 5 weeks ago

#23506 closed defect (implemented)

clock_skew_warning should be a bootstrap event

Reported by: catalyst Owned by: catalyst
Priority: High Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Major Keywords: bootstrap clock-skew usability ux
Cc: brade, mcs Actual Points:
Parent ID: #23508 Points:
Reviewer: Sponsor:

Description

Currently clock_skew_warning() doesn't log anything easily visible to Tor Launcher. (It does a LOG_WARN but that's not enough to produce a failure dialog.)

When we get a trusted clock skew indication, we should make sure to report it as a bootstrap problem, so Tor Launcher's existing logic will detect it as a fatal error.

Child Tickets

Change History (14)

comment:1 Changed 5 weeks ago by arma

Really? Shouldn't the controller be listening for CLOCK_SKEW STATUS_GENERAL events, and reacting to them as a problem that it needs to bring to the user's attention?

comment:2 in reply to:  1 ; Changed 5 weeks ago by catalyst

Replying to arma:

Really? Shouldn't the controller be listening for CLOCK_SKEW STATUS_GENERAL events, and reacting to them as a problem that it needs to bring to the user's attention?

That's one possibility, but the Tor Launcher devs have rejected that before for various reasons, and this is the best way to get a substantial usability improvement with the existing Tor Launcher.

comment:3 in reply to:  2 ; Changed 5 weeks ago by mcs

Cc: brade mcs added

Replying to catalyst:

That's one possibility, but the Tor Launcher devs have rejected that before for various reasons, and this is the best way to get a substantial usability improvement with the existing Tor Launcher.

Hmm. I don't think we rejected that idea. When Kathy and I experimented with it, we found that in our testing it did not solve the problem. Specifically, a CLOCK_SKEW event was generated before Tor Launcher had established its control port connection, so Tor Launcher did not see it. But as I write this I wonder if our testing was flawed, because if we start tor with DisableNetwork=1, then connect to the control port, then monitor STATUS_GENERAL events, it seems like we would see the first CLOCK_SKEW event (but I know very little about tor internals).

Anyway, somewhere I have experimental code that monitors CLOCK_SKEW events... so if that is the Right Thing To Do we can add it to Tor Launcher.

comment:4 in reply to:  3 Changed 5 weeks ago by catalyst

Replying to mcs:

Specifically, a CLOCK_SKEW event was generated before Tor Launcher had established its control port connection, so Tor Launcher did not see it.

I think this happens sometimes when there's an existing cached consensus. Otherwise, I'm able to see the CLOCK_SKEW events when making a manual controller connection.

comment:5 Changed 5 weeks ago by catalyst

Parent ID: #23508

comment:6 Changed 5 weeks ago by catalyst

Milestone: Tor: 0.3.2.x-final

comment:7 Changed 5 weeks ago by catalyst

Status: assignedneeds_review

Proposed patches in https://oniongit.eu/catalyst/tor/merge_requests/2

This fixes several of the bootstrap stalling behaviors where the client's clock is behind the network's time. (I think these are mainly the stalls at 25% and 40%.)

comment:8 Changed 5 weeks ago by catalyst

Keywords: clock-skew added

comment:9 Changed 5 weeks ago by nickm

Status: needs_reviewneeds_revision

This looks fine to me, except for some minor function documentation issues.

comment:10 Changed 5 weeks ago by catalyst

Status: needs_revisionneeds_review

Updated the merge request with function documentation fixes, along with a changes file.

comment:11 Changed 5 weeks ago by catalyst

It would probably be good to backport this so it can get into deployed versions of Tor Browser.

comment:12 Changed 5 weeks ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged!

comment:13 Changed 5 weeks ago by nickm

I'm leaning towards no-backport; it's very close to a new feature in my book, and we should have 0.3.2.1-alpha out some time next week

comment:14 Changed 5 weeks ago by nickm

(but if you disagree, please reopen and put it in needs_review for 0.3.1?)

Note: See TracTickets for help on using tickets.