Opened 8 years ago

Last modified 4 months ago

#3652 needs_revision enhancement

Export clock skew opinion as getinfo command

Reported by: mikeperry Owned by:
Priority: High Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: small-feature, needs-proposal, tor-client, clock-skew, 032-unreached, tbb-needs
Cc: gk, adrelanos@…, arthuredelstein@…, brade, mcs, catalyst, dmr Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Torbutton could make use of a getinfo command to reduce the fingerprinting issues that a user has wrt their Date() object (see #2940).

However, the threading support in Firefox is not good enough for us to easily make a full tor controller that listens for events. It would be much easier if the current skew (ideally down to milliseconds if possible) was available as a getinfo command, so that Torbutton could just query it and update the browser's time offset.

I am making this a 'major' enhancement because clock skew is an important issue to address to reduce fingerprinting.

Child Tickets

Change History (34)

comment:1 Changed 8 years ago by nickm

Hm. So as the controller interface reports CLOCK_SKEW events now, it seems that there's no notion of "our best estimate of our skew so far". Instead, we only report sources that tell us we're skewed; we don't have a notion of "the current skew" internally.

We could get one, of course. We currently take skew from:

  • Any networkstatus or consensus that comes from the future.
  • Any time we connect to an authority (via a netinfo or a directory request) and it tells us the correct time.

comment:2 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final

comment:3 in reply to:  1 Changed 8 years ago by mikeperry

Replying to nickm:

Hm. So as the controller interface reports CLOCK_SKEW events now, it seems that there's no notion of "our best estimate of our skew so far". Instead, we only report sources that tell us we're skewed; we don't have a notion of "the current skew" internally.

We could get one, of course. We currently take skew from:

  • Any networkstatus or consensus that comes from the future.
  • Any time we connect to an authority (via a netinfo or a directory request) and it tells us the correct time.

Yeah, I'd need something like the latter. Roger mentioned some k of n authorities scheme you guys had briefly discussed to get an average estimate on the actual current time once drift is detected? Something like that would be ideal. The closer we can get our clock-drifted clients to the actual current time, the better. for fingerprinting reasons.

comment:4 Changed 8 years ago by nickm

Status: newneeds_review

I started doing some work here in a branch called "skew_estimate". Needs some attention and review for sanity.

comment:5 in reply to:  4 Changed 8 years ago by rransom

Replying to nickm:

I started doing some work here in a branch called "skew_estimate". Needs some attention and review for sanity.

Your clock-skew estimate will become invalid if the clock 'jumps'. As far as I can tell, the only thing we can reasonably do in that case is forget our current clock-skew information completely.

comment:6 in reply to:  4 ; Changed 8 years ago by mikeperry

Owner: set to nickm
Status: needs_reviewassigned

Replying to nickm:

I started doing some work here in a branch called "skew_estimate". Needs some attention and review for sanity.

In command_process_netinfo_cell(), why do you only set apparent_skew if conn->handshake_state->sent_versions_at was within 180 seconds? Is it possible that the dirauth just won't respond in any reasonable amount of time here?

In connection_dir_client_reached_eof(), you use conn->_base.timestamp_lastwritten. Is there any way to obtain the timestamp of the first byte back from the request? If you can get that, you can then do 'delta = lastwritten + (firstbyte - lastwritten)/2 - date_header' to adjust for network+dirauth processing latency...

In router_set_networkstatus_v2() and networkstatus_set_current_consensus(), you use the published/valid_after time. This is probably too low of a resolution to bother to record it to report for TBB. We care about precision down to the second (possibly even sub-second)... Recording this value will just cause all TBB users to all have weird, messed up clocks. If we want that property, we can get it other ways than through the control port command...

comment:7 Changed 8 years ago by mikeperry

WRT networkstatus_set_current_consensus(): I guess it is probably better than having a clock that is 10 years off (or whatever offset people who set their clocks to pirate time-limited demos will have).

Also, how often can we expect clients to actually talk to actual dirauths for command_process_netinfo_cell() and connection_dir_client_reached_eof() to record values from them?

comment:8 in reply to:  6 ; Changed 8 years ago by rransom

Replying to mikeperry:

Replying to nickm:

I started doing some work here in a branch called "skew_estimate". Needs some attention and review for sanity.

In command_process_netinfo_cell(), why do you only set apparent_skew if conn->handshake_state->sent_versions_at was within 180 seconds? Is it possible that the dirauth just won't respond in any reasonable amount of time here?

The SSL handshake itself is our only assurance that we are actually talking to the server we think we are talking to in something like 'real time'. After the handshake completes, an active attacker can delay the NETINFO and VERSIONS cells for arbitrarily long periods of time. We should probably decrease the 180-second window to around 30 seconds or less.

In router_set_networkstatus_v2() and networkstatus_set_current_consensus(), you use the published/valid_after time. This is probably too low of a resolution to bother to record it to report for TBB. We care about precision down to the second (possibly even sub-second)... Recording this value will just cause all TBB users to all have weird, messed up clocks. If we want that property, we can get it other ways than through the control port command...

The timestamp on a published consensus is only used as a lower bound on the current time. Tor reports this timestamp to its clock skew reporting system so that it can complain if the system clock is definitely skewed.

comment:9 in reply to:  8 Changed 8 years ago by mikeperry

Replying to rransom:

Replying to mikeperry:

Replying to nickm:

I started doing some work here in a branch called "skew_estimate". Needs some attention and review for sanity.

In command_process_netinfo_cell(), why do you only set apparent_skew if conn->handshake_state->sent_versions_at was within 180 seconds? Is it possible that the dirauth just won't respond in any reasonable amount of time here?

The SSL handshake itself is our only assurance that we are actually talking to the server we think we are talking to in something like 'real time'. After the handshake completes, an active attacker can delay the NETINFO and VERSIONS cells for arbitrarily long periods of time. We should probably decrease the 180-second window to around 30 seconds or less.

Hrmm, effectively fingerprinting the user. Very nice. I didn't see that. There should definitely be a comment on this line then. If you and nick had died before this merged, I might have removed that check. A future me might still try to remove it :).

I agree that it should be lowered, but I think we should use the circuit build timeout (perhaps div 3) to give us a floor for the value. 30s might be too low of a value on a satellite connection or protest-laden link.

In router_set_networkstatus_v2() and networkstatus_set_current_consensus(), you use the published/valid_after time. This is probably too low of a resolution to bother to record it to report for TBB. We care about precision down to the second (possibly even sub-second)... Recording this value will just cause all TBB users to all have weird, messed up clocks. If we want that property, we can get it other ways than through the control port command...

The timestamp on a published consensus is only used as a lower bound on the current time. Tor reports this timestamp to its clock skew reporting system so that it can complain if the system clock is definitely skewed.

I uh, failed to substract these two checks properly I guess ;). I didn't realize they were updating only in the case of the documents being from the future.. At the risk of asking another dumb question now, why 60s for the consensus but a whole day for the ns2 documents? Also, why 60s? Isn't any amount of time in the future evidence of drift?

comment:10 Changed 8 years ago by nickm

Status: assignedneeds_review

Throwing this back into needs_review, which is the closest we have to a needs_revision. Actually, it needs revision; cant' tell how much from the above, though.

comment:11 Changed 8 years ago by gk

Cc: g.koppen@… added

comment:12 Changed 8 years ago by nickm

Keywords: small-feature added

I should take a quick look in the next day or two to see how hard this would be to clean up for merge in 0.2.3.x. If it's too hard for the time I have, and mike is busy with other stuff, it'll have to wait for the next Tor series.

comment:13 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

On review, I'm going to kick this into 0.2.4.x, and solicit ideas for how to make it better. As it stands, it isn't too wrong, but it isn't actually useful: we only venture an opinion as to our skew if we have talked to >3 directory authorities. But that will practically never happen for clients; only for caches or relays, which generally don't do torbutton and don't have a great use for this info.

IOW, this exposes the info we have, but we don't typically have enough info here to be useful for clients.

comment:14 Changed 8 years ago by nickm

Status: needs_reviewneeds_revision

comment:15 Changed 7 years ago by proper

Cc: adrelanos@… added
Parent ID: #3059

comment:16 Changed 7 years ago by nickm

Keywords: needs-proposal added

comment:17 Changed 7 years ago by nickm

Keywords: tor-client added

comment:18 Changed 7 years ago by nickm

Component: Tor ClientTor

comment:19 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: unspecified

comment:20 Changed 5 years ago by arthuredelstein

Cc: arthuredelstein@… added

comment:21 Changed 5 years ago by mikeperry

I called #6894 a duplicate of this bug.

comment:22 Changed 5 years ago by mikeperry

Parent ID: #3059

comment:23 Changed 3 years ago by femme

Severity: Normal

Has there been any further development in this area? #17739 was merged and so the clock skew code should be somewhat consistent but it still uses the date header and the related ticket #17728 was removed from the milestone.

comment:24 Changed 3 years ago by mcs

Cc: brade. mcs added

comment:25 Changed 3 years ago by teor

Keywords: tbb-wants added

This could be used to implement #9675 (clock skew warning) in Tor Browser.

comment:26 in reply to:  23 Changed 3 years ago by teor

Replying to femme:

Has there been any further development in this area? #17739 was merged and so the clock skew code should be somewhat consistent but it still uses the date header

No, Tor uses both the date header and NETINFO cells to trigger CLOCK_SKEW events.

and the related ticket #17728 was removed from the milestone.

Early aborts on authority connections (to check the time, but avoid downloading directory documents from them) are irrelevant to how Tor exports the clock skew to applications.

comment:27 Changed 3 years ago by teor

Milestone: Tor: unspecifiedTor: 0.3.2.x-final

Aspirationally putting this in 0.3.2 because multiple applications would benefit.

comment:28 Changed 3 years ago by gk

Cc: gk brade added; g.koppen@… brade. removed

comment:29 Changed 2 years ago by catalyst

Cc: catalyst added
Keywords: clock-skew added

comment:30 Changed 2 years ago by nickm

Keywords: 032-unreached added
Milestone: Tor: 0.3.2.x-finalTor: unspecified

Mark a large number of tickets that I do not think we will do for 0.3.2.

comment:31 Changed 2 years ago by gk

Keywords: tbb-needs added; tbb-wants removed

Let's just use tbb-needs and use priority there to indicate how urgent we'd like to have those tickets fixed.

comment:32 Changed 14 months ago by dmr

Cc: dmr added

comment:33 Changed 4 months ago by nickm

Owner: nickm deleted
Status: needs_revisionassigned

comment:34 Changed 4 months ago by nickm

Status: assignedneeds_revision

None of these revisions are in my near-term plans.

Note: See TracTickets for help on using tickets.