Project

General

Profile

Bug #16471

Drop time synchronization hacks that tor 0.3.5 and 0.4.x made obsolete (if any)

Added by hefee 3 months ago. Updated 3 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
Time synchronization
Target version:
Start date:
02/17/2019
Due date:
% Done:

0%

Estimated time:
8.00 h
QA Check:
Ready for QA
Feature Branch:
hefee/bugfix/16471-drop-time-synchronization-hacks+force-all-tests, https://salsa.debian.org/tails-team/tails/merge_requests/21
Type of work:
Research
Blueprint:
Starter:
Affected tool:

Description

And here's a list of little-t-tor changes that might be relevant, e.g. they may break or time sync hacks or instead make some of them obsolete:

Some of them were fixed in 0.3.5, some will be in 0.4.x but might be backported to 0.3.5 when we come back to this ticket. As part of this ticket, I expect we will:

  • look for breakage and fix it (subtasks of this ticket + report to Tor Network Team if the problem is on their side)
  • look for changes that make our code useless but not harmful and file new FT tickets for that (not blockers to close this ticket)
  • if it looks like 0.4.x will make a bigger difference, file a ticket about upgrading to 0.4.x and explain your findings there, so we don't do all the exact same work again once we're there

Screenshot_20190516_202417.png View - date -s 'now -60days' (50.8 KB) hefee, 05/16/2019 06:28 PM


Related issues

Related to Tails - Feature #16348: Upgrade to tor 0.3.5 Resolved 01/12/2019
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed 03/22/2019
Blocked by Tails - Feature #16687: Upgrade to tor 0.4.x Confirmed
Blocks Tails - Bug #9256: Don't restart Tor after setting the right clock Confirmed 04/17/2015

Associated revisions

Revision 44d0d25c (diff)
Added by Sandro Knauß about 1 month ago

Remove time sychronization hacks (refs: #16471)

With new tor 0.3.5, tor get's more moderate in terms of connection to
tor network with a clock, that is out of sync. This makes it possible to
cleanup out time sync script.

Revision 3de159c9 (diff)
Added by Sandro Knauß 5 days ago

Remove maybe_set_time_from_tor_consensus completly. (refs: #16471)

History

#1 Updated by hefee 3 months ago

#2 Updated by hefee 3 months ago

#3 Updated by hefee 3 months ago

  • Feature Branch set to hefee-bugfix-16349-tor-0.3.5-force-all-tests

As I still have no test setup I can't really check, what will break if I remove time sync hacks.

Properly we can cleanup config/chroot_local-includes/etc/NetworkManager/dispatcher.d/20-time.sh as now Tor does not fail that often anymore. Maybe the function is_clock_way_off, and unverified-microdesc-consensus may also been not needed anymore. But form the tickets in the description it is not clea, if tor now never falls with such errors or if it just not fail that often into this error.

Jenkins fails for "Time syncing ǂ Clock is one day in the future in bridge mode".

https://jenkins.tails.boum.org/job/test_Tails_ISO_hefee-bugfix-16349-tor-0.3.5-force-all-tests/lastCompletedBuild/cucumberTestReport/time-syncing/clock-is-one-day-in-the-future-in-bridge-mode/

#4 Updated by intrigeri 3 months ago

  • Subject changed from Cleanup time sync hacks. to Drop time synchronization hacks that tor 0.3.5 made obsolete (if any)
  • Category set to Time synchronization
  • Status changed from New to Confirmed

#5 Updated by intrigeri 3 months ago

#6 Updated by intrigeri 3 months ago

#7 Updated by intrigeri 3 months ago

  • Assignee deleted (intrigeri)
  • Target version changed from Tails_3.13 to Tails_3.14
  • Feature Branch deleted (hefee-bugfix-16349-tor-0.3.5-force-all-tests)
  • Type of work changed from Code to Research

(There's very little chance I tackle this during the 3.13 dev cycle, hence why I had left #16348 unassigned until last FT meeting. And there's a chance that @anonym finds this interesting enough to take it ⇒ let's discuss this at the next FT meeting :)

#8 Updated by intrigeri about 2 months ago

  • Target version deleted (Tails_3.14)

#9 Updated by hefee about 1 month ago

  • Assignee set to hefee

#10 Updated by Anonymous about 1 month ago

  • Status changed from Confirmed to In Progress

#11 Updated by hefee about 1 month ago

  • Feature Branch set to hefee/bugfix/16471-drop-time-synchronization-hacks+force-all-tests

#12 Updated by intrigeri 24 days ago

  • Subject changed from Drop time synchronization hacks that tor 0.3.5 made obsolete (if any) to Drop time synchronization hacks that tor 0.3.5 and 0.4.x made obsolete (if any)

0.4.0.5 brings:

  o Minor bugfixes (client, clock skew):
    - Bootstrap successfully even when Tor's clock is behind the clocks
      on the authorities. Fixes bug 28591; bugfix on 0.2.0.9-alpha.
    - Select guards even if the consensus has expired, as long as the
      consensus is still reasonably live. Fixes bug 24661; bugfix
      on 0.3.0.1-alpha.

#13 Updated by hefee 19 days ago

  • Assignee deleted (hefee)
  • Target version set to Tails_3.14
  • QA Check set to Ready for QA
  • Feature Branch changed from hefee/bugfix/16471-drop-time-synchronization-hacks+force-all-tests to hefee/bugfix/16471-drop-time-synchronization-hacks+force-all-tests, https://salsa.debian.org/tails-team/tails/merge_requests/21

Jenkins status looks fine. We may can even strip more stuff with 0.4.0.5.

#14 Updated by intrigeri 19 days ago

  • Assignee set to intrigeri

#15 Updated by intrigeri 19 days ago

  • Assignee changed from intrigeri to hefee
  • QA Check changed from Ready for QA to Dev Needed

Reviewed on Salsa! Once we have test coverage for the cases that this branch affects, I'm happy to merge it.

But that won't fully resolve #16471 as we still need to take benefit, in the more common cases (hardware clock is correct but in local timezone instead of UTC) from:

To be clear, ideally we would be able to get rid of maybe_set_time_from_tor_consensus() entirely: that's the part of our time sync' system that is problematic from a security PoV). I suspect we'll need 0.4.x to do that.

In any case, even if we can't remove maybe_set_time_from_tor_consensus() yet, we should adjust 20-time.sh so that we only change the system clock when it's actually needed, i.e. expose our users to a dangerous situation less often: currently we do that all the time, as long as time_is_in_valid_tor_range returns false, which nowadays includes cases when tor will have actually judged the system clock was good enough, and accepted using the downloaded consensus, even if the system clock might not be in the expected range. So I guess that when time_is_in_valid_tor_range returns false, we should check (probably several times, waiting a little bit between each try) whether Tor has actually bootstrapped. If it has, we return without setting the system clock; if it hasn't, we set the system clock to the middle of the consensus validity range, just like we've been doing so far.

#16 Updated by hefee 10 days ago

intrigeri wrote:

Reviewed on Salsa! Once we have test coverage for the cases that this branch affects, I'm happy to merge it.

As I created tests, but those are failing also for released versions 3.13.2 and 3.12.1. After investigating the issue with a manual test. I see that 20-time.sh expects the timerage for a certificate to be 3h and not 5min.
@anonym where we can change the certificate time rage for our test tor network?

Btw. the manual test was successful, so it is "only" an issue of our test network.

But that won't fully resolve #16471 as we still need to take benefit, in the more common cases (hardware clock is correct but in local timezone instead of UTC) from:

To be clear, ideally we would be able to get rid of maybe_set_time_from_tor_consensus() entirely: that's the part of our time sync' system that is problematic from a security PoV). I suspect we'll need 0.4.x to do that.

My tests show, that we can't get rid of it completely atm. I removed the maybe_set_time_from_tor_consensus() and end up with the attached screenshot. I'm unsure if the mentioned tickets will help to get rid of it completely as they "only extend" the range of then a consensus is accepted. But if we have completely broken clock, that is reset to timestamp=0 we still need maybe_set_time_from_tor_consensus(). But we will se, if we have a working test for it, we can investigate further.

In any case, even if we can't remove maybe_set_time_from_tor_consensus() yet, we should adjust 20-time.sh so that we only change the system clock when it's actually needed, i.e. expose our users to a dangerous situation less often: currently we do that all the time, as long as time_is_in_valid_tor_range returns false, which nowadays includes cases when tor will have actually judged the system clock was good enough, and accepted using the downloaded consensus, even if the system clock might not be in the expected range. So I guess that when time_is_in_valid_tor_range returns false, we should check (probably several times, waiting a little bit between each try) whether Tor has actually bootstrapped. If it has, we return without setting the system clock; if it hasn't, we set the system clock to the middle of the consensus validity range, just like we've been doing so far.

That sounds like a very reasonable approach, but this would make the script even longer not shorter and this ticket is about removing code ;D

What I don't understand, why you think, that the date -s "{vmid}", is that dangerous. A user will need to wait till htpdate updated the clock anyways, before they are able to access the internet. Or is this about, that it makes Tails unique behavior visible withing the tor network? And that it makes everything more slow?

#17 Updated by hefee 10 days ago

@intrigeri - I spent 2h for this ticket - maybe another 2h to getting the tests working, some manual testing,...

#18 Updated by anonym 10 days ago

hefee wrote:

As I created tests, but those are failing also for released versions 3.13.2 and 3.12.1. After investigating the issue with a manual test. I see that 20-time.sh expects the timerage for a certificate to be 3h and not 5min.
@anonym where we can change the certificate time rage for our test tor network?

nickm?
You'll have to mess with the values in submodules/chutney/torrc_templates/authority.i. The tor man page is your friend. The little-t-tor people (perhaps Nick) can probably help.

#19 Updated by intrigeri 9 days ago

  • Assignee changed from intrigeri to hefee

anonym where we can change the certificate time rage for our test tor network?

You've got a reply so next step here is to use this info and make the tests pass.

Btw. the manual test was successful, so it is "only" an issue of our test network.

Great!

That sounds like a very reasonable approach, but this would make the script even longer not shorter and this ticket is about removing code ;D

Sorry I've been unclear: the goal here is to remove dangerous behavior. We can hope it allows us to remove code at the end of the day (once we have Tor 0.4.x) but it's OK if it actually adds code, if it protects our users better :)

What I don't understand, why you think, that the date -s "{vmid}", is that dangerous. A user will need to wait till htpdate updated the clock anyways, before they are able to access the internet. Or is this about, that it makes Tails unique behavior visible withing the tor network? And that it makes everything more slow?

It essentially disables Tor's check for consensus freshness, which is there for a reason. Sure, htpdate might fix this after the fact, but htpdate can be run while tor is using a replayed (obsolete) consensus. The consequences are not well understood but:

  • At the very least, indeed that behavior is unique to Tails.
  • It's been pointed out by Tor developers that our security analysis is incorrect. I can't find the corresponding discussion anymore. We never took time to check this properly, because for years we've been operating under the assumption that #5774 would be solved soon.

#20 Updated by intrigeri 9 days ago

  • Related to Bug #9256: Don't restart Tor after setting the right clock added

#21 Updated by intrigeri 9 days ago

  • Assignee changed from hefee to intrigeri
  • Estimated time set to 8.00 h

#22 Updated by intrigeri 9 days ago

  • Assignee changed from intrigeri to hefee

#23 Updated by intrigeri 9 days ago

You'll have to mess with the values in submodules/chutney/torrc_templates/authority.i. The tor man page is your friend. The little-t-tor people (perhaps Nick) can probably help.

hefee, I'm happy to help decipher this before you ask Tor folks, if you have a hard time with it.

#24 Updated by intrigeri 9 days ago

Also, note that we have a feature/tor-nightly-master branch that can be used as a basis to try stuff based on a more recent tor. If it turns out that tor master allows us to get rid of maybe_set_time_from_tor_consensus() entirely, we can skip the intermediary solution I suggested earlier here.

#25 Updated by intrigeri 8 days ago

Taking another step back, given this likely won't make it into 3.14, and there's a good chance 3.15 (or worst case 3.16) ships with tor 0.4.x, I'm starting to think our time would be better spend doing this directly with 0.4.x, using https://deb.torproject.org/torproject.org/dists/tor-experimental-0.4.0.x-stretch/ for now. A branch for #16687 could upgrade to that version of tor (that will already teach us something) and the branch for this ticket would be based on it. In other words: I recommend you forget about 0.3.5 and focus on 0.4.x. What do you think?

#26 Updated by anonym 6 days ago

intrigeri wrote:

You'll have to mess with the values in submodules/chutney/torrc_templates/authority.i. The tor man page is your friend. The little-t-tor people (perhaps Nick) can probably help.

hefee, I'm happy to help decipher this before you ask Tor folks, if you have a hard time with it.

Me too!

#27 Updated by hefee 5 days ago

intrigeri wrote:

You'll have to mess with the values in submodules/chutney/torrc_templates/authority.i. The tor man page is your friend. The little-t-tor people (perhaps Nick) can probably help.

hefee, I'm happy to help decipher this before you ask Tor folks, if you have a hard time with it.

I found the solution to make the validity check in 20time.sh pass, also for 3.13.2. But I'm unsure how I can propose an update, as it is in chutney submodule and I do not have write permissions.

@intrigeri: should I extract the additional tests + chutney updates for stable branch? As those tests should pass also for current branch?

#28 Updated by hefee 5 days ago

#29 Updated by hefee 5 days ago

  • Related to deleted (Bug #9256: Don't restart Tor after setting the right clock)

#30 Updated by hefee 5 days ago

  • Blocks Bug #9256: Don't restart Tor after setting the right clock added

#31 Updated by hefee 5 days ago

intrigeri wrote:

Taking another step back, given this likely won't make it into 3.14, and there's a good chance 3.15 (or worst case 3.16) ships with tor 0.4.x, I'm starting to think our time would be better spend doing this directly with 0.4.x, using https://deb.torproject.org/torproject.org/dists/tor-experimental-0.4.0.x-stretch/ for now. A branch for #16687 could upgrade to that version of tor (that will already teach us something) and the branch for this ticket would be based on it. In other words: I recommend you forget about 0.3.5 and focus on 0.4.x. What do you think?

I use now feature/tor-nightly-master and whoo, we can get rid of maybe_set_time_from_tor_consensus() entirely. As we have now tests to set the system clock +40days and -10 years and they pass. I updated blocks/blocked by relations. To make it visible, that this is blocked by tor 4.x in tails.

#32 Updated by hefee 5 days ago

  • Assignee changed from hefee to intrigeri

#33 Updated by intrigeri 4 days ago

  • Target version changed from Tails_3.14 to Tails_3.15

#34 Updated by intrigeri 3 days ago

  • QA Check changed from Dev Needed to Ready for QA

@hefee, I assume you meant to set this "Ready for QA"; otherwise, please clarify what dev you expect me to do :)

#35 Updated by hefee 3 days ago

intrigeri wrote:

@hefee, I assume you meant to set this "Ready for QA"; otherwise, please clarify what dev you expect me to do :)

yes "ready for QA" is the correct status, forgotten to update the status.

Also available in: Atom PDF