Project

General

Profile

Feature #6156

Upstream secure Thunderbird autoconfig wizard

Added by Tails about 5 years ago. Updated about 2 months ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
05/19/2016
Due date:
% Done:

100%

QA Check:
Feature Branch:
feature/6156-thunderbird-secure-auto-config
Type of work:
Communicate
Blueprint:
Starter:
No
Affected tool:
Email Client

Description

Try to get these patches merged upstream, preferably to Mozilla, but Debian would also be OK.

secure-account-creation.tar.gz (12.6 KB) anonym, 04/02/2018 01:21 PM


Subtasks

Bug #11450: Update the Icedove account setup wizard security patches according to what'll be decided upstreamResolvedu


Related issues

Related to Tails - Feature #6150: Help Tor people upstream the Torbirdy patches Resolved
Related to Tails - Feature #7064: Update our plans for securing Icedove's autoconfig wizard wrt. recent developments Resolved
Blocks Tails - Bug #11481: Alert TorBirdy devs about Icedove upstream patch prefs Confirmed 05/24/2016
Blocked by Tails - Bug #11536: Icedove autoconfiguration is broken for ISPs serving a OAuth config Resolved 06/17/2016
Blocked by Tails - Bug #12151: Get Thunderbird's test suite running and test our patches Resolved 01/17/2017

Associated revisions

Revision 8a9de848 (diff)
Added by anonym almost 2 years ago

Pin Icedove to be installed from our APT repo.

Debian's Icedove packages still do not have our secure Icedove
autoconfig wizard patches applied, so installing them would be a
serious security regression.

Refs: #6156
Will-fix: #11613

Revision 0d4b993c (diff)
Added by anonym 4 months ago

Enable the feature-6156-thunderbird-secure-auto-config APT overlay.

Will-fix: #6156

Revision 57b8798a (diff)
Added by anonym 4 months ago

Update Thunderbird prefs vs 52.7.0-1~deb9u1.0tails1+6156.

I.e. the first build with the rewritten secure auto config patch
series applied.

And as a preparation for the upstream work in TorBirdy that will
follow, let's set these prefs via a patch to TorBirdy.

Refs: #6156

History

#1 Updated by intrigeri about 5 years ago

  • Parent task set to #5663

#2 Updated by intrigeri about 5 years ago

  • Type of work changed from Wait to Code

#3 Updated by intrigeri almost 5 years ago

  • Type of work changed from Code to Upstream
  • Starter set to No

#4 Updated by intrigeri almost 5 years ago

  • Subject changed from upstream secure Icedove autoconfig wizard to Upstream secure Icedove autoconfig wizard

#5 Updated by intrigeri almost 4 years ago

  • Category set to 212

#6 Updated by intrigeri almost 4 years ago

  • Type of work changed from Upstream to Code

#7 Updated by BitingBird over 3 years ago

I think the type of work is communicate, since at least in Debian I see no big report, but I'm not sure so I let it as is for now.

#8 Updated by intrigeri over 3 years ago

  • Type of work changed from Code to Communicate

#9 Updated by intrigeri about 3 years ago

  • Assignee set to u
  • Target version set to 246

#11 Updated by intrigeri about 3 years ago

  • Related to Feature #6150: Help Tor people upstream the Torbirdy patches added

#12 Updated by sajolida over 2 years ago

  • Target version changed from 246 to Tails_2.0

#13 Updated by u over 2 years ago

  • Target version changed from Tails_2.0 to Tails_2.4

#14 Updated by u over 2 years ago

Setting realistic target version to 2.4, maximum latest delay would be 2.5.

#15 Updated by intrigeri over 2 years ago

Today we discussed that this should be started during the 2.2 cycle, span over the 2.3 and 2.4 ones. I would suggest setting a milestone that matches when you should start working on it, instead of one that matches when it should be finished, to prevent any risk that it's forgotten until 2.3 is out. Your call, of course :)

#16 Updated by u over 2 years ago

  • Related to Feature #7064: Update our plans for securing Icedove's autoconfig wizard wrt. recent developments added

#17 Updated by u over 2 years ago

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10

#18 Updated by u over 2 years ago

The current plan is to send our patches with a text to Torbirdy people for a first review with ETA Jan 24th 2016.

Then ideally we would be able to send them to the Thunderbird people and also update the corresponding TPO ticket. I'd also open a Debian bug with the same patches and link it to the upstream bug, so eventually the patches could be included in Debian directly - until upstream has them released.

#20 Updated by intrigeri over 2 years ago

I've posted the patches upstream today.

Great! A link would be useful next time we need to report about the progress of this task :)

#21 Updated by anonym about 2 years ago

  • Blocked by deleted (Feature #6154: Secure the Icedove autoconfig wizard)

#22 Updated by u about 2 years ago

  • Blocks Bug #11481: Alert TorBirdy devs about Icedove upstream patch prefs added

#23 Updated by u about 2 years ago

  • Target version changed from Tails_2.4 to Tails_2.5

Unlikely that this will happen in time for 2.4. Postponing.

#24 Updated by u about 2 years ago

  • % Done changed from 10 to 20

There has been quite some progress. Our patches have all been reviewed now and only code style improvements were asked for as a result of this review. This makes me think that we're nearly there! We'll work on a modified patchset taking upstream's remarks into account later this week.

#25 Updated by intrigeri about 2 years ago

There has been quite some progress. Our patches have all been reviewed now and only code style improvements were asked for as a result of this review. This makes me think that we're nearly there! We'll work on a modified patchset taking upstream's remarks into account later this week.

Excellent news, glad to read that!

#26 Updated by u about 2 years ago

We've resubmitted a new patchset today which was thouroughly tested by anonym. Waiting for a reply from upstream now. Let's hope for the best.

#27 Updated by u almost 2 years ago

  • Target version changed from Tails_2.5 to Tails_2.6

#28 Updated by u almost 2 years ago

  • Blocked by Bug #11536: Icedove autoconfiguration is broken for ISPs serving a OAuth config added

#29 Updated by intrigeri almost 2 years ago

  • Blocks Bug #11450: Update the Icedove account setup wizard security patches according to what'll be decided upstream added

#30 Updated by intrigeri almost 2 years ago

  • Blocks deleted (Bug #11450: Update the Icedove account setup wizard security patches according to what'll be decided upstream)

#31 Updated by u almost 2 years ago

I've submitted the latest patchset today. Waiting for upstream again now.

#32 Updated by anonym almost 2 years ago

  • Target version changed from Tails_2.6 to Tails_2.7

#33 Updated by u almost 2 years ago

Repinged upstream today.

I think that this ticket's deliverable has been accomplished "try hard to upstream the icedove autoconfig wizard". Thus, we'll continue to keep track of it, but not as part of the deliverable.

#34 Updated by u almost 2 years ago

  • Parent task deleted (#5663)

#35 Updated by intrigeri almost 2 years ago

I think that this ticket's deliverable has been accomplished "try hard to upstream the icedove autoconfig wizard". Thus, we'll continue to keep track of it, but not as part of the deliverable.

Woohoo! \o/

#36 Updated by bertagaz almost 2 years ago

intrigeri wrote:

Woohoo! \o/

+1 :)

#37 Updated by u over 1 year ago

  • Target version changed from Tails_2.7 to Tails_2.9.1

postponing

#38 Updated by anonym over 1 year ago

  • Target version changed from Tails_2.9.1 to Tails 2.10

#39 Updated by u over 1 year ago

  • Target version changed from Tails 2.10 to Tails_2.12

#40 Updated by intrigeri about 1 year ago

  • Target version changed from Tails_2.12 to Tails_3.1

I suspect that you folks will have other, more urgent things to do until 3.0 is out, so setting target version to 3.1.

#41 Updated by u 11 months ago

  • Target version changed from Tails_3.1 to Tails_3.3

#42 Updated by anonym 8 months ago

  • Target version changed from Tails_3.3 to Tails_3.5

#43 Updated by intrigeri 6 months ago

I see lots of discussion happening on the corresponding upstream bug in the past three months. Someone posted an updated patch and somewhat aggressively engaged with upstream. Upstream reacted (very understandably) somewhat defensively. If we still plan to get this merged some day, I think we need to intervene and help de-escalate the situation. anonym & u, did you follow this discussion, do you plan to get involved? If not, please let me/us know that you need help there: I don't think we can afford doing nothing about it on the long term, the risk of having to ship our patched Thunderbird package forever is just too great. Thanks in advance!

#44 Updated by u 6 months ago

See https://labs.riseup.net/code/issues/6156#note-35 for why I am unblocking #8668.

#46 Updated by u 6 months ago

I will take care of the upstream discussion.

However, our patches have not been merged because we were unable to provide a version that worked in Thunderbird's test suite (which we did not manage to run). This is #12151.

#47 Updated by u 6 months ago

  • Blocked by Bug #12151: Get Thunderbird's test suite running and test our patches added

#48 Updated by anonym 6 months ago

  • Target version changed from Tails_3.5 to Tails_3.6

#49 Updated by u 5 months ago

I replied on the bug - there was a lot of activity recently and it seems like people are still willing to have these patches reviewed and incorporated into Thunderbird. See https://bugzilla.mozilla.org/show_bug.cgi?id=971347

#50 Updated by intrigeri 5 months ago

it seems like people are still willing to have these patches reviewed and incorporated into Thunderbird.

I wonder who these people are and whether I missed something because my understanding of the situation is very different, and still the same as #6156#note-43 (nothing happened since then anyway until your own comment): among the two Thunderbird maintainers who commented, one nack-ed and the other one has strong doubts). I see someone else (alta88) arguing and insisting in favour of our patches but I'm not sure that will help much, quite the opposite IMO.

#51 Updated by u 5 months ago

intrigeri wrote:

it seems like people are still willing to have these patches reviewed and incorporated into Thunderbird.

I wonder who these people are and whether I missed something because my understanding of the situation is very different,

Looks like it.

and still the same as #6156#note-43 (nothing happened since then anyway until your own comment): among the two Thunderbird maintainers who commented, one nack-ed

I'm not sure what "nack'ing" is, may you please explain that?

and the other one has strong doubts).

This one says "Of course SSL/TLS connections would be preferred" and "I see the patch add proxy support in some of the paths, maybe at least that part could be accepted?". So the way you phrase it ("strong doubts") seems slightly pessimistic to me.

I see someone else (alta88) arguing and insisting in favour of our patches but I'm not sure that will help much, quite the opposite IMO.

Absolutely, but I sincerely hope Thunderbird people can make the distinction between this person, us, and the problem we're trying to solve.

#52 Updated by intrigeri 5 months ago

I'm not sure what "nack'ing" is, may you please explain that?

Sure, sorry! ACK = agreeing/approving != NACK = disagreeing/rejecting

and the other one has strong doubts).

This one says "Of course SSL/TLS connections would be preferred" and "I see the patch add proxy support in some of the paths, maybe at least that part could be accepted?". So the way you phrase it ("strong doubts") seems slightly pessimistic to me.

I think we'll have to agree to disagree on this one :)

I see someone else (alta88) arguing and insisting in favour of our patches but I'm not sure that will help much, quite the opposite IMO.

Absolutely, but I sincerely hope Thunderbird people can make the distinction between this person, us, and the problem we're trying to solve.

+1

#53 Updated by u 5 months ago

  • Assignee changed from u to anonym

TB now implements a cool feature which display a big red warning to users when they use insecure configs. Great! So all we have to do now, and anonym will do that next week, is to reduce our patches to the "prefer SSL over plaintext" and proxy parts and submit them again for review. Yaaay!

#54 Updated by anonym 4 months ago

BTW, I've noticed that the Mozilla configuration database (https://live.mozillamessaging.com/autoconfig/v1.1/) often presents a CAPTCHA when the request originates from Tor => that method fails => #15387

#55 Updated by bertagaz 4 months ago

  • Target version changed from Tails_3.6 to Tails_3.7

#56 Updated by anonym 4 months ago

  • File secure-account-creation.tar.gz added
  • Subject changed from Upstream secure Icedove autoconfig wizard to Upstream secure Thunderbird autoconfig wizard
  • Assignee changed from anonym to u

Sorry for the delay, but this turned out quite a bit more work than expected. In the end, I actually god Thunderbird's own automated test suite to work and now it passes with all patches applied!'

Below you'll find comments I'd like to prove to the reviewer. However, I'm unsure of what to do about patch 0018 now. The "red warning" is enough, IMHO, and if we and/or TorBirdy use the pref it adds, then most email over .onion-space will be impossible since they rely on end-to-end encryption/authentication provided by the .onion and do not provide SSL for the email protocols. If you agree, just remove patch 0018 and its comment below.


The previous approach of "one big patch" clearly failed, so let's try an incremental approach with one patch per commit (besides now there are several bugfixes that IMHO deserves their own commits). Please review and merge this series up to the patch you are happy with; each commit is atomic and at most depends on earlier patches, so this is safe and makes sense, and we can return to the remaining patches later. In fact, in most cases if you dislike a particular patch you could just skip that one and at worst get a simple conflict in mailnews/mailnews.js when importing a later patch.

Here I provide some extra patch comments for the reviewer:

0001-Fix-the-prefilled-hostnames-for-manual-edit.patch
0002-Invalidate-config-when-restarting-autoconfiguration.patch
0003-Remove-buggy-debug-code.patch
0004-Fix-issue-when-mailnews.auto_config_url-is-empty.patch
0005-Document-that-mailnews.-auto_config-mx_service-_url-.patch
0006-Comment-pref.patch

These are bugfixes and minor improvements which I suspect you'll want to merge right away.

0007-Prefer-fetched-configurations-using-SSL-over-plainte.patch

There's quite some effort made to prefer SSL in other places, so this is arguably a bugfix as well.

0008-Add-SOCKS-proxy-support-for-account-autoconfiguratio.patch

Fixes: https://bugzilla.mozilla.org/show_bug.cgi?id=669238

0009-Generalize.patch
0010-Fix-outdated-error-handling.patch
0011-Improve-logging-for-fetchConfigFromISP-during-autoco.patch

These are nice on their own, but builds up to the next patch:

0012-Fetch-ISP-configuration-using-SSL-if-possible.patch

This opportunistic attempt to prevent MitM can easily be worked around (attacker blocks https, thunderbird downgrades to http and the attacker wins) but is better than nothing, and builds up to:

0013-Add-pref-to-force-SSL-for-ISP-fetch-during-autoconfi.patch

This adds an opt-in pref that fixes the above MitM completely.

0014-Add-pref-for-whether-we-accept-OAuth2-during-autocon.patch
0015-Use-overridden-user-agent-for-HTTP-s-during-autoconf.patch
0016-Add-pref-for-each-guess-timeout-during-autoconfigura.patch

These allows TorBirdy to greatly improve the autoconfiguration UX when using Tor.

0017-Improve-logging-of-guess-instances-during-autoconfig.patch

This one helped me debugging 0015 and 0016.

0018-Add-pref-for-whether-we-accept-plaintext-protocols-d.patch

Given the "red warning" that appears whenever a user tries to accept a plaintext protocol (see comment 127), this patch is not critical for us any more.

FWIW, I've seen the test-mail-account-setup-wizard.js test pass with all these patches applied.

#57 Updated by anonym 4 months ago

  • Feature Branch set to feature/6156-thunderbird-secure-auto-config

#58 Updated by u 4 months ago

I added those to the upstream bug today.

#59 Updated by u 4 months ago

  • Assignee changed from u to anonym

Forwarded Ben Buksch's review to anonym. Reassigning this ticket to you - feel free to reassign it to me later on.

#60 Updated by bertagaz 2 months ago

  • Target version changed from Tails_3.7 to Tails_3.8

#61 Updated by intrigeri about 2 months ago

  • Target version changed from Tails_3.8 to Tails_3.10

Also available in: Atom PDF