Tails

#5 Updated by intrigeri 2019-11-11 06:41:41

No time sync’ -related test suite failure on this branch so I think we’re good here.

#10 Updated by segfault 2019-11-13 19:26:45

I reviewed Bug #17214 and Bug #17143, so the branch can be merged once this issue is resolved.

#12 Updated by intrigeri 2019-11-14 16:45:15

Hi @hefee,

> But the rationale why we stop shipping the TorBrowser user agent may result in wired results, as a lot of websites use Cloudflare and they may block tor traffic completely for non TorBrowser user.

Very good point and thanks for the pointers!

> That’s why I would argue, that we have to add a test to check, that each domain in the HTP pools are reachable and the time is correct. The failure page of Cloudflare should also have correct headers and we don’t care, but I would like to make sure that we are still fine.

First, I wanted to at least know what the answer to these questions would currently be, so I did this:

. config/chroot_local-includes/etc/default/htpdate.pools  
for host in $(echo "$HTP_POOL_1 $HTP_POOL_2 $HTP_POOL_3" | tr ',' ' ' ) ; do
torsocks curl --silent --head "https://$host/" | grep -iE '^date: '
done

All servers replied and gave the correct date, except https://encrypted.google.com/ that had a Date header ~17 hours in the past. I’ve retried this one a few times with different circuits (torsocks --isolate), and got various different dates, that were all wrong. OTOH https://www.google.com/ always gives we the correct date so I’ve switched to this one on the topic branch.

But all this is just one data point i.e. mostly anecdotal evidence, so:

I agree we should have such tests; IMO they should run automatically and regularly (Bug #11562). The only thing I wonder about is whether we should block here on having these tests.

I agree that the changes brought by this ticket & branch may increase the chances that htpdate can’t connect to some webservers, in particular those managed by Cloudflare.

I’m not sure how likely that regression is: from the links you’ve shared, I understand that honoring alt-svc is a key part of Cloudflare’s special treatment of Tor Browser; and with or without the change this ticket & branch are about, our htpdate does not use alt-svc (https://tools.ietf.org/html/rfc7838) at all, so it already behaves differently than a real Tor Browser would. I don’t know which one among these is the most likely to be blocked by Cloudflare:

• a client that pretends being Tor Browser, connects over Tor, but does not behave like Tor Browser would (i.e. does not follow alt-svc) ← that’s the baseline set by our current implementation
• a client that honestly says it’s curl, connects over Tor, and does not behave like Tor Browser would (i.e. does not follow alt-svc) ← that’s what the proposed branch does

Keeping in mind that:

• the proposed branch saves the RM ~30 minutes during every release process, and we have good reasons to optimize this due to the upcoming 4-weeks release cycle;
• the data point / anecdotal evidence above suggests that there is no such regression at the moment

… at this point, I’m not sure whether the risk for such regressions is big enough to warrant blocking this change on adding tests (Bug #11562).

What do you think?

Personally, I’d prefer not blocking on Bug #11562. But I’m fine with blocking on it if you want, in which case I would re-prioritize Bug #11562, and merge a version of this branch without the htpdate-related commits. Your call!

> Btw. did we made sure, that only one pool has domains handled by Cloudflare?

Oh f, right, excellent question. I just did and as of now:

• no server is pool 1 is fronted by Cloudflare (rather expected as these folks tend to like self-hosting better than relying on big 3rd-party companies)
• 3 servers in pool 2 are fronted by Cloudflare
• no server is pool 3 is fronted by Cloudflare (rather expected as these companies are big enough to manage their own CDN and attack mitigation)

But I think we should check this regularly. Given this specific problem is not a regression brought by the changes this ticket & branch are about, I’ve filed Bug #17233 about it and I don’t think we should block on this here. But perhaps it’ll be cheap to add this check when we implement the other checks that your review suggests; we’ll see!

#13 Updated by hefee 2019-11-15 01:28:25

May thanks for this detailed answer. I learnt lot new stuff.

> Personally, I’d prefer not blocking on Bug #11562. But I’m fine with blocking on it if you want, in which case I would re-prioritize Bug #11562, and merge a version of this branch without the htpdate-related commits. Your call!

I think your short script is enough to tell, that it is not very likely to have a big regression. So I’m fine to accept this patch and not block on Bug #11562.

>
> > Btw. did we made sure, that only one pool has domains handled by Cloudflare?
>
> Oh f, right, excellent question. I just did and as of now:
>
> * no server is pool 1 is fronted by Cloudflare (rather expected as these folks tend to like self-hosting better than relying on big 3rd-party companies)
> * 3 servers in pool 2 are fronted by Cloudflare
> * no server is pool 3 is fronted by Cloudflare (rather expected as these companies are big enough to manage their own CDN and attack mitigation)
>
> But I think we should check this regularly. Given this specific problem is not a regression brought by the changes this ticket & branch are about, I’ve filed Bug #17233 about it and I don’t think we should block on this here. But perhaps it’ll be cheap to add this check when we implement the other checks that your review suggests; we’ll see!

ACK.

#14 Updated by hefee 2019-11-15 01:43:31

I reviewed 05db5aaa114245af4211a19438e4d442b058d617 and fc643144073f1f3975ab7d3f9f22264f74c32fee and both get accepted. But as they are on top of other patches, so I can’t merge.

#15 Updated by intrigeri 2019-11-15 09:33:01

Hi,

Thanks!

> I reviewed 05db5aaa114245af4211a19438e4d442b058d617 and fc643144073f1f3975ab7d3f9f22264f74c32fee and both get accepted. But as they are on top of other patches, so I can’t merge.

segfault already reviewed and ACK’ed these other commits on Bug #17143 and Bug #17214; the only blocker for merging was the discussion on this ticket. So I’m going to merge :)