Project

General

Profile

Feature #6156

Upstream secure Thunderbird autoconfig wizard

Added by Tails almost 6 years ago. Updated 5 days 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

Get these patches merged in Thunderbird upstream.

Upstream merge request: https://bugzilla.mozilla.org/show_bug.cgi?id=971347

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

Bug #15788: Rethink our patches with Thunderbird 60Resolved

Bug #15790: Thunderbird autoconfig does not respect the ISP's hintsRejected

Feature #16147: Re-import secure-account-creation patch series for thunderbirdRejected


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
Related to Tails - Bug #15387: The Mozilla auto_config database requires an unusable CAPTCHA for Torified requests Rejected 03/07/2018
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
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed 03/22/2019

Associated revisions

Revision 8a9de848 (diff)
Added by anonym over 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 about 1 year ago

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

Will-fix: #6156

Revision 57b8798a (diff)
Added by anonym about 1 year 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 almost 6 years ago

  • Parent task set to #5663

#2 Updated by intrigeri almost 6 years ago

  • Type of work changed from Wait to Code

#3 Updated by intrigeri over 5 years ago

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

#4 Updated by intrigeri over 5 years ago

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

#5 Updated by intrigeri over 4 years ago

  • Category set to 212

#6 Updated by intrigeri over 4 years ago

  • Type of work changed from Upstream to Code

#7 Updated by BitingBird over 4 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 4 years ago

  • Type of work changed from Code to Communicate

#9 Updated by intrigeri almost 4 years ago

  • Assignee set to u
  • Target version set to 246

#11 Updated by intrigeri almost 4 years ago

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

#12 Updated by sajolida over 3 years ago

  • Target version changed from 246 to Tails_2.0

#13 Updated by u over 3 years ago

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

#14 Updated by u over 3 years ago

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

#15 Updated by intrigeri over 3 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 3 years ago

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

#17 Updated by u over 3 years ago

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

#18 Updated by u over 3 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 about 3 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 almost 3 years ago

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

#22 Updated by u almost 3 years ago

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

#23 Updated by u almost 3 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 almost 3 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 almost 3 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 almost 3 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 over 2 years ago

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

#28 Updated by u over 2 years ago

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

#29 Updated by intrigeri over 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 over 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 over 2 years ago

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

#32 Updated by anonym over 2 years ago

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

#33 Updated by u over 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 over 2 years ago

  • Parent task deleted (#5663)

#35 Updated by intrigeri over 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 over 2 years ago

intrigeri wrote:

Woohoo! \o/

+1 :)

#37 Updated by u over 2 years ago

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

postponing

#38 Updated by anonym over 2 years ago

  • Target version changed from Tails_2.9.1 to Tails 2.10

#39 Updated by u over 2 years ago

  • Target version changed from Tails 2.10 to Tails_2.12

#40 Updated by intrigeri about 2 years 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 over 1 year ago

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

#42 Updated by anonym over 1 year ago

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

#43 Updated by intrigeri over 1 year 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 over 1 year ago

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

#46 Updated by u over 1 year 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 over 1 year ago

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

#48 Updated by anonym about 1 year ago

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

#49 Updated by u about 1 year 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 about 1 year 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 about 1 year 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 about 1 year 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 about 1 year 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 about 1 year 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 about 1 year ago

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

#56 Updated by anonym about 1 year 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 about 1 year ago

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

#58 Updated by u about 1 year ago

I added those to the upstream bug today.

#59 Updated by u about 1 year 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 12 months ago

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

#61 Updated by intrigeri 11 months ago

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

#62 Updated by intrigeri 8 months ago

I think #15788 should be done first thing before any more work is put into this project :)

#63 Updated by intrigeri 8 months ago

  • Description updated (diff)

#64 Updated by intrigeri 8 months ago

  • Assignee changed from anonym to u
  • QA Check set to Info Needed

Ulrike, one question for you at the bottom :)

The recurring cost of maintaining & building custom Thunderbird packages a few times a year keeps piling up and it does not help make the FT sustainable. So at the summit I'll nominate this ticket for the roadmap + FT work, on the ground that it's become a serious maintainability hinderance and we've been waiting too long for the last mile to be walked based on good will and free time.

So let's sum up what we have. First, it looks like the patches are in good way to be upstreamed. We're pretty close! \o/

Second, things got more interesting recently and we now have three different patchsets:

  • anonym's rebase of his original patchset, done in March 2018, on top of the upstream/60.0_b2 from Debian's Vcs-Git, that lives in the secure_account_creation-60.0_b2 branch of our Git repo; assuming kibi's refresh (see below) is OK, I think this one is merely of historical interest at this point
  • kibi's refresh of the aforementioned patchset so that it applies on 60.0b10; that's what we currently have as a quilt patch series on our tails/stretch packaging branch and that was used to build debian/1%60.0_b10-1_deb9u1.0tails1 which whe'll ship in Tails 3.9~rc1; a more Git-friendly version lives on the secure_account_creation-60.0_b10 branch, based on the upstream/60.0_b10 tag from Debian's Vcs-Git
  • anonym's improved patchset, that takes into account the last review done upstream in April 2018:
    • it was mistakenly based on Thunderbird 52.7 so probably needs a serious refresh
    • it is probably better (as in, closer to something that can be upstreamed eventually) than the other patchsets
    • AFAICT it was never published, let alone submitted upstream, so far; 4 people have access to it with the attached comments that explain things: anonym, Ulrike, kibi and myself (the 2 latter in <335bfd60-7be2-b0e3-9fbd-8a58db17e10d@451f.org>). Apparently anonym wanted the patches and the associated comments to go on the upstream bug report, so perhaps I could attach all this here to start with, so at least the material to submit is not kept secret, but there may be a reason why this did not happen yet. Ulrike?

#65 Updated by u 8 months ago

  • AFAICT it was never published, let alone submitted upstream, so far; 4 people have access to it with the attached comments that explain things: anonym, Ulrike, kibi and myself (the 2 latter in <335bfd60-7be2-b0e3-9fbd-8a58db17e10d@451f.org>). Apparently anonym wanted the patches and the associated comments to go on the upstream bug report, so perhaps I could attach all this here to start with, so at least the material to submit is not kept secret, but there may be a reason why this did not happen yet. Ulrike?

It has not happened because it's not based on the current development version of TB, so these patches are very likely to be rejected - something I'd like to avoid, after all the complicated discussions we've gone through with upstream until now. And I was still expecting anonym to rebase the patches on a newer version of TB…

But yes, indeed, you can attach the patches here, and we can improve upon them from this point on. Please not the comments on the upstream bug: some patches have already been reviewed and are considered mergeably by them. If possible I'd like to add this in a comment somewhere, so that we don't redo work that has already been done.

#66 Updated by u 8 months ago

  • Assignee changed from u to intrigeri
  • QA Check deleted (Info Needed)

#67 Updated by u 8 months ago

I would love us to rework on these patches and resubmit them upstream for good. Let me know if you need my help for this or if FT will handle it from this point on.

#68 Updated by u 8 months ago

  • Related to Bug #15387: The Mozilla auto_config database requires an unusable CAPTCHA for Torified requests added

#69 Updated by intrigeri 8 months ago

Let me know if you need my help

Anyone moving this forward is warmly welcome :)

for this or if FT will handle it from this point on.

We'll see at the summit. In the current state of things I doubt the FT has the budget to do that.

#70 Updated by intrigeri 7 months ago

u wrote:

But yes, indeed, you can attach the patches here, and we can improve upon them from this point on.

Note to myself: once this is done, set target version to 2019 and reassign to lamby, as per summit 2018.

#71 Updated by lamby 7 months ago

Again, Did we discuss this? I don't recall doing so, thus just wondering if it's a mistake to assign it over to me specifically? (If it's "to be discussed" feel free to assign back - I note the "2019" target.)

#72 Updated by intrigeri 7 months ago

Again, Did we discuss this?

Yes (but very briefly!) during the FT meeting when we were brainstorming the tasks we want to add to our roadmap for the next years. Here's what I remember: IIRC kibi expressed interest because he's already worked on our Thunderbird patched package, then I asked "how is your JavaScript", his answer suggested it would be better to assign the task to someone else, and after a show of hands you mentioned your JavaScript was OK. I probably jumped too fast to conclusions: the fact your JS is OK does not necessarily imply that you want to work on this specific task. Sorry! Either way, I think there's quite some info I need to share with you before you can tell whether you want to take it or not, so let's take this assignment with a grain of salt. If you prefer, feel free to de-assign yourself until we've had this conversation :)

#73 Updated by intrigeri 7 months ago

  • Assignee changed from intrigeri to lamby

#74 Updated by intrigeri 6 months ago

  • Assignee changed from lamby to anonym
  • Target version deleted (Tails_3.10.1)

#75 Updated by intrigeri 2 months ago

#76 Updated by intrigeri 2 months ago

#77 Updated by intrigeri 2 months ago

#78 Updated by u about 2 months ago

anonym has rebased our patches - once again - on the current code base of Thunderbird. I've posted them all to the bug at https://bugzilla.mozilla.org/show_bug.cgi?id=971347 (also see https://bugzilla.mozilla.org/show_bug.cgi?id=971347#c197 for explanations).

Let's hope that's the last time we need to do that.

#79 Updated by intrigeri about 2 months ago

  • Target version set to Tails_3.13

This probably won't be completed in time for 3.13 but let's make it clear that this should be on top of your list of non-urgent tasks.

#80 Updated by intrigeri about 1 month ago

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

#81 Updated by intrigeri about 1 month ago

#82 Updated by intrigeri about 1 month ago

#83 Updated by intrigeri about 1 month ago

  • Blocks deleted (Bug #11481: Alert TorBirdy devs about Thunderbird upstream patch prefs)

#85 Updated by u 27 days ago

Thundebird people are organizing a bugday.
They have a document where they collect bugs to focus on: https://docs.google.com/document/d/1jQfOIuu_9KYPw_hfrPjwGrhQWKv-bzBDr214_tstMbc/edit#
I've added "our" bug there.

#86 Updated by intrigeri 18 days ago

  • Target version changed from Tails_3.14 to 2019

#87 Updated by intrigeri 11 days ago

  • Description updated (diff)

Also available in: Atom PDF