Closed Bug 669238 Opened 13 years ago Closed 5 years ago

[autoconfig] probing (guess config) does not use configured SOCKS proxy


(Thunderbird :: Account Manager, defect)

Not set


(Not tracked)

Thunderbird 68.0


(Reporter: tagnaq, Unassigned)



(Whiteboard: [fixed by bug 971347])


(1 obsolete file)

steps to reproduce:
1. set http + https + socks proxies
2. set network.proxy.socks_remote_dns = true
3. start your preferred network monitoring tool
4. Add Mail Account -> 'mail account setup' 
5. enter name (user) + email ( -> continue
6. first you will see ~170 packets (proxy), then you will see DNS requests and pop3, pop3s, imap, imaps, smtp, smtps connections bypassing the proxy 

note: the first part of the autoconfig process (, and ISPDB requests go through the configured proxies).
To reproduce this, it is important to choose a domain where no entry is found in the ISPDB and the provider does not provide autoconfig files
Ever confirmed: true
Summary: Proxy bypass during autoconfig probing → [autoconfig] probing (guess config) does not use configured SOCKS proxy
adding a reference here: (PDF file: section 3.6.5)
Also relevant for above document: Bug 599173 - autoconfig offline
This issue is a larger problem in 10.0.2, as there is no easy way to create accounts manually if you need to connect via a SOCKS proxy. The only way I could find to do this is to first work offline, then add correct account settings before working online again.
Still an issue in TB 17.
This looks like a serious issue, especially for Torbirdy users.
From what I understand it impacts mostly smaller/selfhosted domains - which worsens the issue imo, as people using this kind of domain probably look for more privacy then less.
Is there anybody who retested this recently? Is this bug still present in recent versions of Thunderbird?

this is a patch from Tails Developers to fix SOCKS proxy support for account autoconfiguration guessing. We are currently working on making Thunderbird more secure for Tails users and thus would kindly ask you to review and consider this patch.

I've tested this patch on upstream version 38.5.0. It applies cleanly.

for Tails Developers
Attachment #8724392 - Flags: review+
Attachment #8724392 - Flags: feedback+
Attachment #8724392 - Flags: review?(mkmelin+mozilla)
Hi Magnus,

I now understood that i need to ask for review, and as you are one of the suggested reviewers, I tentativly picked you from the list. I hope that that's the correct way to do it.

Yes it's the correct way. I'll try to get to reviewing the patch soon.
Assignee: nobody → u
We're doing about 50 probes simultaneously (really in parallel). You should probably cache the proxy, instead of getting one for each connection. (You could make an async helper function that gets the proxy - either returns the cached proxy, or gets it from the API, using an actual hostname.)
All probed hosts should be on the same network - if the API returns different proxies for any two of these probed hosts, then something is going wrong - so, it's actually more correct to cache than to query it each time.
Hi Ben, 

do you consider this a blocker?


this is a friendly ping, any progress on this?

Reassigning to Magnus Melin who said he'd review this patch.
Assignee: u → mkmelin+mozilla
Comment on attachment 8724392 [details] [diff] [review]

Review of attachment 8724392 [details] [diff] [review]:

::: mailnews/base/prefs/content/accountcreation/guessConfig.js
@@ +462,3 @@
>        if (i == 0) // showing 50 servers at once is pointless
>          this.mProgressCallback(thisTry);
> +      function proxyResolveCallback() { }

nicer with a capital P, as this is classish. Also add documentation that it's an nsIProtocolProxyCallback

@@ +463,5 @@
>          this.mProgressCallback(thisTry);
> +      function proxyResolveCallback() { }
> +      proxyResolveCallback.prototype = {
> +        onProxyAvailable : function(req, uri, proxyInfo, status) {
> +          // Anything but a SOCKS proxy will be unusable for the probes

nit: add period

@@ +465,5 @@
> +      proxyResolveCallback.prototype = {
> +        onProxyAvailable : function(req, uri, proxyInfo, status) {
> +          // Anything but a SOCKS proxy will be unusable for the probes
> +          if (proxyInfo.type != "socks" && proxyInfo.type != "socks4") {
> +            proxyInfo = null;

WHen there's no proxy available proxyInfo would be null. That doesn't seem to be handled?
Attachment #8724392 - Flags: review?(mkmelin+mozilla)
Attachment #8724392 - Attachment is obsolete: true

Magnus Melin 2016-06-07 05:06:16 PDT, comment 13
> WHen there's no proxy available proxyInfo would be null. That doesn't seem to be handled?

If you look at how this patch modifies SocketUtil() you'll see that it *always* used null before the patch, and that in the end this value is passed to transportService.createTransport(), where null means "do not use a proxy".

As for the rest of your review comments, they are addressed in the patch "Style and typo fixes after review". That and the rest of the patch can now be found on:
See Also: → 971347

If the patches in → bug 971347 get merged, this one can be closed.

Assignee: mkmelin+mozilla → nobody
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed by bug 971347]
Target Milestone: --- → Thunderbird 68.0
You need to log in before you can comment on or make changes to this bug.