Bug #17277

Make the Thunderbird autoconfig wizard not trust the result of DNS requests

Added by intrigeri 2019-11-30 19:20:46 . Updated 2020-05-15 11:26:00 .

Status:
In Progress
Priority:
Normal
Assignee:
anonym
Category:
Target version:
Tails_4.7
Start date:
Due date:
% Done:

50%

Feature Branch:
bugfix/17277-disable-mx-autoconfig
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Email Client
Deliverable for:

Description

Our design doc claims that Thunderbird trusts the result of DNS requests and we fix that, but I can’t find how we do this.

I think something might have got lost along the way here: we used to have a patch for this, e.g. 3b437622ec44784fafb764071c34988213af2977 in our icedove.git.
We still had it in d9e6ba7978a685ea1a87cbc43e5a6279c6586e22 on the tails/stretch branch in our icedove.git.

We did not have it anymore in 2b0b9e0fd74b3de6841b3ef21a6f89c66f7a1c47, when I imported a patch series refreshed for v60. I think that’s why I removed the corresponding pref in commit:5f2020524066458003022e77a23c8d9af05eb8b6 (Feature #15091): the pref was not used anymore.
I’m wondering if back then, we’ve imported a patch series that was meant solely to generate patches that upstream might be fine with, but not meant for Tails. This dates back from Feature #15091#note-24 where I had to guess what branch we should import, and I may have guessed wrong.

anonym, could you please check if we still need a patch (and lost it along the way)?

Subtasks

Related issues

Related to Tails - Feature #17259: Update Thunderbird design doc Resolved
Related to Tails - Feature #15091: Upgrade to Thunderbird 60 Resolved 2018-05-09
Related to Tails - Feature #6156: Upstream secure Thunderbird autoconfig wizard In Progress 2016-05-19
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed
Blocks Tails - Feature #17654: Disable unsafe MS Exchange autoconfig method in Thunderbird In Progress

History

#1 Updated by intrigeri 2019-11-30 19:21:08

#2 Updated by intrigeri 2019-11-30 19:21:26

#3 Updated by intrigeri 2019-11-30 19:21:32

#4 Updated by intrigeri 2019-11-30 19:35:38

  • related to Feature #6156: Upstream secure Thunderbird autoconfig wizard added

#5 Updated by intrigeri 2020-01-06 07:58:29

  • Target version changed from Tails_4.2 to Tails_4.3

#6 Updated by anonym 2020-02-09 12:14:00

  • Status changed from Confirmed to In Progress

Applied in changeset commit:tails|d94317d9f0e15dd3e4272599ea98badc1fc0f782.

#7 Updated by anonym 2020-02-09 12:18:50

  • % Done changed from 0 to 20
  • Feature Branch set to bugfix/17277-disable-exchange-mx-autoconfig
  • Type of work changed from Research to Code

Indeed! That patch was dropped since I noticed that one could just set mailnews.mx_service_url to the empty string and it would have the same effect, without introducing another pref. So we need to do that.

Related, we have not disabled Exchange server lookup, which I added a patch for, so we also need to set mailnews.auto_config.fetchFromExchange.enabled to false. Whoops! :S

Not sure if this will make it into 4.3 at this late hour though..

#8 Updated by anonym 2020-02-11 15:26:20

  • Target version changed from Tails_4.3 to Tails_4.4

#9 Updated by anonym 2020-02-27 14:57:07

This branch is failing on Jenkins. Apparently setting fetchFromExchange.enabled to false breaks the autoconfig wizard (after entering the email address, pressing “Continue” does nothing). I can reproduce this in Debian with 1:68.5.0-1~deb10u1 so I’m pretty sure something with the patch I upstreamed has broken. Looking at the local “test suite” I did for this work, this used to work, so I’m not sure what’s going on.

Note to self: the problem origins here:

function fetchConfigFromExchange(domain, emailAddress, username, password,
                                 successCallback, errorCallback) {
  assert(typeof(successCallback) == "function");
  assert(typeof(errorCallback) == "function");
  if (!Services.prefs.getBoolPref(
      "mailnews.auto_config.fetchFromExchange.enabled", true)) {
    errorCallback("Exchange AutoDiscover disabled per user preference");
    return new Abortable();
  }

The errorCallback() is:

        (e, allErrors) => {
          // Must call error callback in any case to stop the discover mode.
          call.errorCallback()(e); // ()(e) is correct
          if (e.code == 401 || allErrors && allErrors.find(e => e.code == 401)) { // Auth failed
            // Ask user for username.
            _show("usernameRow");
            this.switchToMode("start");
          }

which seems to indeed not match how it’s called above. Will look into this deeper at some point.

Note that the DNS pref works, so we could fix that in this ticket/branch, and move the Exchange stuff to a new ticket/branch.

#10 Updated by CyrilBrulebois 2020-03-12 09:56:00

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

#11 Updated by CyrilBrulebois 2020-04-07 17:05:18

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

#12 Updated by anonym 2020-04-24 12:46:41

  • Status changed from In Progress to Needs Validation
  • % Done changed from 20 to 50
  • Feature Branch changed from bugfix/17277-disable-exchange-mx-autoconfig to bugfix/17277-disable-mx-autoconfig

anonym wrote:
> Note that the DNS pref works, so we could fix that in this ticket/branch, and move the Exchange stuff to a new ticket/branch.

Done! I opened Feature #17654 to track this other issue.

Let’s see if Jenkins is happy with this now.

#13 Updated by anonym 2020-04-24 12:51:11

  • Status changed from Needs Validation to In Progress

Applied in changeset commit:tails|3e79ef04cf56164d519bb6de627e8efa7094c5fa.

#14 Updated by CyrilBrulebois 2020-05-06 04:28:57

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

#15 Updated by anonym 2020-05-14 10:28:30

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (anonym)

Tests LGTM! Please review’n’merge! You might as well want do Feature #17654 at the same time since its branch is based on this branch.

#16 Updated by intrigeri 2020-05-14 19:07:25

  • Assignee set to intrigeri

#17 Updated by intrigeri 2020-05-15 10:11:13

  • blocks Feature #17654: Disable unsafe MS Exchange autoconfig method in Thunderbird added

#18 Updated by intrigeri 2020-05-15 10:28:35

  • Subject changed from Check if the Thunderbird autoconfig wizard trusts the result of DNS requests to Make the Thunderbird autoconfig wizard not trust the result of DNS requests
  • Status changed from Needs Validation to In Progress
  • Assignee changed from intrigeri to anonym

Hi @anonym,

I’m not sure I understand what threat we’re trying to protect against here.

Initially I thought this was about distrusting unauthenticated DNS resolution done by exit nodes for us (be it because we don’t trust exit nodes, or because we don’t trust their own DNS resolution setup). But then I understood the MX resolution is done over HTTPS to https://live.thunderbird.net/dns/mx/, so that’s not it.

The comment you added reads “Disable automatic configuration methods that could be serving configurations without authentication”. This suggests the problem is not about the security of the connection to live.thunderbird.net, nor about whether we trust that MX resolution service, but rather about the fact it may give us answers that point to “configurations without authentication”. Correct?
But now, I don’t know what sort of authentication this is about, so I’m a little confused.

I could find nothing about this in our design doc.

Could you please clarify this at least in the comment, and ideally in our design doc?

Thanks in advance!

#19 Updated by intrigeri 2020-05-15 10:36:27

Also, mailnews.mx_service_url was removed in Thunderbird 68.3.1, so I doubt this branch does what we hope it does — whatever that is.

#20 Updated by anonym 2020-05-15 11:15:32

intrigeri wrote:
> Hi @anonym,
>
> I’m not sure I understand what threat we’re trying to protect against here.
>
> Initially I thought this was about distrusting unauthenticated DNS resolution done by exit nodes for us (be it because we don’t trust exit nodes, or because we don’t trust their own DNS resolution setup). But then I understood the MX resolution is done over HTTPS to https://live.thunderbird.net/dns/mx/, so that’s not it.
>
> The comment you added reads “Disable automatic configuration methods that could be serving configurations without authentication”. This suggests the problem is not about the security of the connection to live.thunderbird.net, nor about whether we trust that MX resolution service, but rather about the fact it may give us answers that point to “configurations without authentication”. Correct?
> But now, I don’t know what sort of authentication this is about, so I’m a little confused.
>
> I could find nothing about this in our design doc.
>
> Could you please clarify this at least in the comment, and ideally in our design doc?

The way I have been thinking about this is that, sure, whatever we fetch from https://live.thunderbird.net/dns/mx/ is authenticated, but that service cannot be trusted since it in turn fetches configs without authenticating the results (i.e. over plain DNS, not over Tor, but still). So somewhere in the chain the config we fetched wasn’t authenticated.

However, it turns out we don’t need to have this discussion…

> Also, mailnews.mx_service_url was removed in Thunderbird 68.3.1, so I doubt this branch does what we hope it does — whatever that is.

Wow, I’m not so happy about myself not noticing this. Yeah, this branch does nothing. Note that Feature #17654 actually works as (I) intended, due to the other patch I add there which still looks for that removed pref.

This option was removed as part of upstream bug 1349337, which actually switches from the web service to using the native system resolver. So now we’re in the situation you talked about above with rogue exit nodes being a problem. So I’ll have to let upstream know that they broke functionality of our patch that they upstreamed, i.e. it’s no longer possible to disable MX. I guess I’ll have to write another patch. :)

Back to the drawing board!

#21 Updated by anonym 2020-05-15 11:26:00

@intrigeri: ↑