01/11/2012 11:35 PM, intrigeri:
>
> anonym wrote (11 Jan 2012 20:54:21 GMT) :
>
>> 2. Fetch configuration from email service provider: Disabled when
>> ssl_only == true.
>
>> It would be easy to make it try https first, and then fallback to http
>> only if ssl_only == false, but a comment in the code makes me unsure if
>> I want to do so:
>
>>> To support domain hosters, we cannot use SSL. That means we
>>> rely on insecure DNS and http, which means the results may be
>>> forged when under attack. The same is true for guessConfig(), >though.
>
>> Any one that understands the issue?
>
> I don't think there's anything more to understand than "they're aware
> than fetching over HTTP is deeply insecure, they even know some
> reasons why it's insecure, but they do it nevertheless because most
> crappy ISP's don't bother to serve their autoconfig file over HTTPS".
Ok... it just seems weird they didn't implement this from the beginning
(OTOH, that code was a complete mess... I refactored it to a sane state).
> I think the change you're hesitating to make would be consistent with
> the way ssl_only works.
Right, implemented (PATCH 4/6)
>> Issues I'd like input on:
>
>> * ssl_only really does two separate things:
>
>> 1. It forces SSL for *fetching* configs.
>> 2. It forces SSL or STARTTLS in the *resulting* configuration when
>> *guessing* the config.
>
>> Is this confusing?
>
> Anything else would look strongly inconsistent to me, at least in the
> threat model I think we're considering. So I don't find it confusing.
>
> OTOH, I don't understand what exactly "*resulting*" means above, so
> I may very well be entirely confused wrt. how ssl_only behaves in
> GuessConfig city.
With resulting I just meant the fetched config after it has been parsed.
>> * ssl_only does not check whether any fetched config (like in step
>> 1 and 3) uses plaintext smtp, pop or imap. Do we want this?
>> I wouldn't ask if it was a trivial change, but this code is
>> a complete mess and would need a complete reorganization for this
>> feature. It would make the above point less confusing. But can we
>> live without it?
>
> If things are kept as is, it seems to me ssl_only makes things more
> secure *only* for a given user if her email provider:
>
> - serves the email protocols over SSL;
> - does not serve an autoconfig file pointing her MUA to non-SSL.
>
> I'm not sure how we can possibly help other users: suddenly denying
> them access to their email does not look that appalling to me.
>
> OTOH, I would find it great if we could make it clear to them what
> risk they are taking. How hard would it be to implement this? (This
> would be a low-priority enhancement: I don't think our current Claws
> Mail setup has anything to say against SSL.)
Turned out to be easier than I initially thought (thanks, exceptions!).
> In any case, the current situation makes the "Only use secure
> protocols" checkbox a wrong statement, isn't it?
You're right. Implemented (PATCH 6/6).
Unless I've missed something my work here is done, see attached patches.
Well there is one more issue: if the email provider's http server that
serves the config is using a self-signed certificate (like my test
server), the user just gets an error prompt with no chance to manually
verify it and accept it. Hmm. We'll see if I care about looking into
this. Anyone think it's really important?
Since we have no icedove repo to push to I post my current work here in
case my primary hard drive + backup drive would fail :). Also, perhaps
someone with more javascript skills than me could spot some bug I've
missed (?). The patches are against commit aab48e6 in the Debian Mozilla
team's git repo:
http://anonscm.debian.org/gitweb/?p=pkg-mozilla/icedove.git
Cheers!
From 0651e1f6e2c4f76fc444969f7fc6600670b302da Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Wed, 4 Jan 2012 14:48:02 +0100
Subject: [PATCH 1/6] Optionally skip probing for plaintext protocols.
Setting mailnews.auto_config_ssl_only to True prevents detecting
plaintext protocols through autoconfiguration during account creation.
---
.../prefs/content/accountcreation/guessConfig.js | 68 +++++++++++++-------
1 files changed, 44 insertions(+), 24 deletions(-)
diff --git a/mailnews/base/prefs/content/accountcreation/guessConfig.js b/mailnews/base/prefs/content/accountcreation/guessConfig.js
index 02acf3c..a183ad3 100644
--- a/mailnews/base/prefs/content/accountcreation/guessConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/guessConfig.js
@@ -802,22 +802,32 @@ function getIncomingTryOrder(host, protocol, ssl, port)
else if (protocol == UNKNOWN && !lowerCaseHost.indexOf("imap."))
protocol = IMAP;
+ var prefs = Cc["@mozilla.org/preferences-service;1"]
+ .getService(Ci.nsIPrefBranch);
+ var ssl_only = prefs.getBoolPref("mailnews.auto_config_ssl_only");
+
if (protocol != UNKNOWN) {
- if (ssl == UNKNOWN)
- return [getHostEntry(protocol, TLS, port),
- getHostEntry(protocol, SSL, port),
- getHostEntry(protocol, NONE, port)];
- return [getHostEntry(protocol, ssl, port)];
- }
- if (ssl == UNKNOWN)
- return [getHostEntry(IMAP, TLS, port),
- getHostEntry(IMAP, SSL, port),
- getHostEntry(POP, TLS, port),
- getHostEntry(POP, SSL, port),
- getHostEntry(IMAP, NONE, port),
- getHostEntry(POP, NONE, port)];
- return [getHostEntry(IMAP, ssl, port),
- getHostEntry(POP, ssl, port)];
+ if (ssl == UNKNOWN) {
+ var order = [getHostEntry(protocol, TLS, port),
+ getHostEntry(protocol, SSL, port)];
+ if (!ssl_only)
+ order.push(getHostEntry(protocol, NONE, port));
+ return order;
+ } else {
+ return [getHostEntry(protocol, ssl, port)];
+ }
+ } else if (ssl == UNKNOWN) {
+ var order = [getHostEntry(IMAP, TLS, port),
+ getHostEntry(IMAP, SSL, port),
+ getHostEntry(POP, TLS, port),
+ getHostEntry(POP, SSL, port)];
+ if (!ssl_only)
+ order.push(getHostEntry(IMAP, NONE, port),
+ getHostEntry(POP, NONE, port));
+ return order;
+ } else
+ return [getHostEntry(IMAP, ssl, port),
+ getHostEntry(POP, ssl, port)];
};
/**
@@ -826,19 +836,29 @@ function getIncomingTryOrder(host, protocol, ssl, port)
function getOutgoingTryOrder(host, protocol, ssl, port)
{
assert(protocol == SMTP, "need SMTP as protocol for outgoing");
+ var prefs = Cc["@mozilla.org/preferences-service;1"]
+ .getService(Ci.nsIPrefBranch);
+ var ssl_only = prefs.getBoolPref("mailnews.auto_config_ssl_only");
+
if (ssl == UNKNOWN)
{
- if (port == UNKNOWN)
+ if (port == UNKNOWN) {
// neither SSL nor port known
- return [getHostEntry(SMTP, TLS, UNKNOWN),
- getHostEntry(SMTP, TLS, 25),
- getHostEntry(SMTP, SSL, UNKNOWN),
- getHostEntry(SMTP, NONE, UNKNOWN),
- getHostEntry(SMTP, NONE, 25)];
+ var order = [getHostEntry(SMTP, TLS, UNKNOWN),
+ getHostEntry(SMTP, TLS, 25),
+ getHostEntry(SMTP, SSL, UNKNOWN)];
+ if (!ssl_only)
+ order.push(getHostEntry(SMTP, NONE, UNKNOWN),
+ getHostEntry(SMTP, NONE, 25));
+ return order;
+ } else {
// port known, SSL not
- return [getHostEntry(SMTP, TLS, port),
- getHostEntry(SMTP, SSL, port),
- getHostEntry(SMTP, NONE, port)];
+ var order = [getHostEntry(SMTP, TLS, port),
+ getHostEntry(SMTP, SSL, port)];
+ if (!ssl_only)
+ order.push(getHostEntry(SMTP, NONE, port));
+ return order;
+ }
}
// SSL known, port not
if (port == UNKNOWN)
--
1.7.7.3
From 1de630482dadcef4ae5c751e152abdc05660f0a8 Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Wed, 4 Jan 2012 14:51:21 +0100
Subject: [PATCH 2/6] Optionally skip insecure database autoconfiguration
lookup.
Setting mailnews.auto_config_ssl_only to True makes autoconfiguration
skip database lookup during autoconfiguration if
mailnews.auto_config_url isn't HTTPS.
---
.../prefs/content/accountcreation/fetchConfig.js | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/mailnews/base/prefs/content/accountcreation/fetchConfig.js b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
index c4c43e9..5026892 100644
--- a/mailnews/base/prefs/content/accountcreation/fetchConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
@@ -142,6 +142,12 @@ function fetchConfigFromDB(domain, successCallback, errorCallback)
let pref = Cc["@mozilla.org/preferences-service;1"]
.getService(Ci.nsIPrefBranch);
let url = pref.getCharPref("mailnews.auto_config_url");
+ if (pref.getBoolPref("mailnews.auto_config_ssl_only") &&
+ url.indexOf("https://") != 0) {
+ errorCallback("Skipping insecure autoconfiguration method: " +
+ "non-SSL HTTP database lookup");
+ return;
+ }
domain = sanitize.hostname(domain);
// If we don't specify a place to put the domain, put it at the end.
--
1.7.7.3
From 00abc97618060972e29c31b4ffa4a75b8ef087e8 Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Wed, 4 Jan 2012 14:59:54 +0100
Subject: [PATCH 3/6] Optionally skip insecure DNS MX autoconfiguration
lookup.
Setting mailnews.auto_config_ssl_only to True makes autoconfiguration
skip DNS MX lookup during autoconfiguration.
---
.../prefs/content/accountcreation/fetchConfig.js | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/mailnews/base/prefs/content/accountcreation/fetchConfig.js b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
index 5026892..daa508b 100644
--- a/mailnews/base/prefs/content/accountcreation/fetchConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
@@ -194,6 +194,15 @@ function fetchConfigFromDB(domain, successCallback, errorCallback)
*/
function fetchConfigForMX(domain, successCallback, errorCallback)
{
+ var prefs = Cc["@mozilla.org/preferences-service;1"]
+ .getService(Ci.nsIPrefBranch);
+ if (prefs.getBoolPref("mailnews.auto_config_ssl_only")) {
+ // XXX We may not have to skip this method if we're using DNSSEC
+ errorCallback("Skipping insecure autoconfiguration method: " +
+ "DNS MX lookup");
+ return;
+ }
+
domain = sanitize.hostname(domain);
var sucAbortable = new SuccessiveAbortable();
--
1.7.7.3
From 8e8ae6cc51055edad7b104c309869e2d1a56389f Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Tue, 10 Jan 2012 20:49:20 +0100
Subject: [PATCH 4/6] Make ISP autoconfiguration lookup first try https, then
http.
Setting mailnews.auto_config_ssl_only to True makes autoconfiguration
use only https for ISP lookup.
---
.../prefs/content/accountcreation/fetchConfig.js | 74 ++++++++++---------
1 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/mailnews/base/prefs/content/accountcreation/fetchConfig.js b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
index daa508b..a064ef5 100644
--- a/mailnews/base/prefs/content/accountcreation/fetchConfig.js
+++ b/mailnews/base/prefs/content/accountcreation/fetchConfig.js
@@ -85,49 +85,53 @@ function fetchConfigFromDisk(domain, successCallback, errorCallback)
function fetchConfigFromISP(domain, emailAddress, successCallback,
errorCallback)
{
- let url1 = "http://autoconfig." + sanitize.hostname(domain) +
- "/mail/config-v1.1.xml";
+ let conf1 = "autoconfig." + sanitize.hostname(domain) +
+ "/mail/config-v1.1.xml";
// .well-known/ <http://tools.ietf.org/html/draft-nottingham-site-meta-04>
- let url2 = "http://" + sanitize.hostname(domain) +
- "/.well-known/autoconfig/mail/config-v1.1.xml";
+ let conf2 = sanitize.hostname(domain) +
+ "/.well-known/autoconfig/mail/config-v1.1.xml";
+ let url0 = "https://" + conf1;
+ let url1 = "https://" + conf2;
+ let url2 = "http://" + conf1;
+ let url3 = "http://" + conf2;
+ let prefs = Cc["@mozilla.org/preferences-service;1"]
+ .getService(Ci.nsIPrefBranch);
+ if (prefs.getBoolPref("mailnews.auto_config_ssl_only")) {
+ var urls = [url0, url1];
+ } else {
+ var urls = [url0, url1, url2, url3];
+ }
let sucAbortable = new SuccessiveAbortable();
- var time = Date.now();
- let fetch1 = new FetchHTTP(
- url1, { emailaddress: emailAddress }, false,
- function(result)
+ let time;
+
+ let success = function(result)
{
successCallback(readFromXML(result));
- },
- function(e1) // fetch1 failed
+ };
+
+ let error = function(i, e)
{
- ddump("fetchisp 1 <" + url1 + "> took " + (Date.now() - time) +
- "ms and failed with " + e1);
- time = Date.now();
- if (e1 instanceof CancelledException)
- {
- errorCallback(e1);
+ ddump("fetchisp " + i + " <" + urls[i] + "> took " +
+ (Date.now() - time) + "ms and failed with " + e);
+
+ if (i == urls.length - 1 || // implies all fetches failed
+ e instanceof CancelledException) {
+ errorCallback(e);
return;
}
+ let fetch = new FetchHTTP(urls[i + 1], { emailaddress: emailAddress },
+ false, success,
+ function(e) { error(i + 1, e) });
+ sucAbortable.current = fetch;
+ time = Date.now();
+ fetch.start();
+ };
- let fetch2 = new FetchHTTP(
- url2, { emailaddress: emailAddress }, false,
- function(result)
- {
- successCallback(readFromXML(result));
- },
- function(e2)
- {
- ddump("fetchisp 2 <" + url2 + "> took " + (Date.now() - time) +
- "ms and failed with " + e2);
- // return the error for the primary call,
- // unless the fetch was cancelled
- errorCallback(e2 instanceof CancelledException ? e2 : e1);
- });
- sucAbortable.current = fetch2;
- fetch2.start();
- });
- sucAbortable.current = fetch1;
- fetch1.start();
+ let fetch = new FetchHTTP(urls[0], { emailaddress: emailAddress }, false,
+ success, function(e) { error(0, e) });
+ sucAbortable.current = fetch;
+ time = Date.now();
+ fetch.start();
return sucAbortable;
}
--
1.7.7.3
From 8d0c3c29dbc2cbf4fbda8d2829737ada33fa6e70 Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Wed, 11 Jan 2012 15:09:57 +0100
Subject: [PATCH 5/6] Add checkbox for toggling mailnews.auto_config_ssl_only.
---
.../en-US/chrome/messenger/accountCreation.dtd | 2 ++
.../prefs/content/accountcreation/emailWizard.js | 14 ++++++++++++++
.../prefs/content/accountcreation/emailWizard.xul | 7 +++++++
3 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/mail/locales/en-US/chrome/messenger/accountCreation.dtd b/mail/locales/en-US/chrome/messenger/accountCreation.dtd
index 21693e5..a2849d8 100644
--- a/mail/locales/en-US/chrome/messenger/accountCreation.dtd
+++ b/mail/locales/en-US/chrome/messenger/accountCreation.dtd
@@ -12,6 +12,8 @@
<!ENTITY password.text "Optional, will only be used to validate the username">
<!ENTITY rememberPassword.label "Remember password">
<!ENTITY rememberPassword.accesskey "m">
+<!ENTITY secureProtocols.label "Only use secure protocols">
+<!ENTITY secureProtocols.accesskey "s">
<!ENTITY imapLong.label "IMAP (remote folders)">
<!ENTITY pop3Long.label "POP3 (keep mail on your computer)">
diff --git a/mailnews/base/prefs/content/accountcreation/emailWizard.js b/mailnews/base/prefs/content/accountcreation/emailWizard.js
index 55409d3..ae41c1e 100644
--- a/mailnews/base/prefs/content/accountcreation/emailWizard.js
+++ b/mailnews/base/prefs/content/accountcreation/emailWizard.js
@@ -240,6 +240,9 @@ EmailConfigWizard.prototype =
rememberPasswordE.disabled = true;
}
+ e("only_secure_protocols").checked =
+ Application.prefs.getValue("mailnews.auto_config_ssl_only", false);
+
// First, unhide the main window areas, and store the width,
// so that we don't resize wildly when we unhide areas.
// switchToMode() will then hide the unneeded parts again.
@@ -288,6 +291,7 @@ EmailConfigWizard.prototype =
//_show("initialSettings"); always visible
//_show("cancel_button"); always visible
if (modename == "start") {
+ _show("only_secure_protocols");
_hide("status_area");
_hide("result_area");
_hide("manual-edit_area");
@@ -325,6 +329,7 @@ EmailConfigWizard.prototype =
_show("manual-edit_button");
_hide("advanced-setup_button");
} else if (modename == "manual-edit") {
+ _hide("only_secure_protocols");
_show("status_area");
_hide("result_area");
_show("manual-edit_area");
@@ -339,6 +344,7 @@ EmailConfigWizard.prototype =
_show("advanced-setup_button");
_disable("advanced-setup_button");
} else if (modename == "manual-edit-have-hostname") {
+ _hide("only_secure_protocols");
_show("status_area");
_hide("result_area");
_show("manual-edit_area");
@@ -353,6 +359,7 @@ EmailConfigWizard.prototype =
_show("advanced-setup_button");
_disable("advanced-setup_button");
} else if (modename == "manual-edit-testing") {
+ _hide("only_secure_protocols");
_show("status_area");
_hide("result_area");
_show("manual-edit_area");
@@ -368,6 +375,7 @@ EmailConfigWizard.prototype =
_show("advanced-setup_button");
_disable("advanced-setup_button");
} else if (modename == "manual-edit-complete") {
+ _hide("only_secure_protocols");
_show("status_area");
_hide("result_area");
_show("manual-edit_area");
@@ -529,6 +537,12 @@ EmailConfigWizard.prototype =
e("password").type = "password";
},
+ toggleSecureProtocols : function()
+ {
+ Application.prefs.setValue("mailnews.auto_config_ssl_only",
+ e("only_secure_protocols").checked);
+ },
+
/**
* Check whether the user entered the minimum of information
* needed to leave the "start" mode (entering of name, email, pw)
diff --git a/mailnews/base/prefs/content/accountcreation/emailWizard.xul b/mailnews/base/prefs/content/accountcreation/emailWizard.xul
index 282edc5..8a618e0 100644
--- a/mailnews/base/prefs/content/accountcreation/emailWizard.xul
+++ b/mailnews/base/prefs/content/accountcreation/emailWizard.xul
@@ -214,6 +214,13 @@
accesskey="&rememberPassword.accesskey;"
checked="true"/>
</hbox>
+ <hbox align="center" pack="start">
+ <label class="autoconfigLabel"/>
+ <checkbox id="only_secure_protocols"
+ label="&secureProtocols.label;"
+ accesskey="&secureProtocols.accesskey;"
+ oncommand="gEmailConfigWizard.toggleSecureProtocols();"/>
+ </hbox>
</groupbox>
<spacer flex="1" />
--
1.7.7.3
From cdfe6a543094e172524c50f1766b8a08a22581df Mon Sep 17 00:00:00 2001
From: Tails developers <amnesia@???>
Date: Thu, 12 Jan 2012 15:38:55 +0100
Subject: [PATCH 6/6] Optionally skip fetched configs using plaintext
protocols.
Setting mailnews.auto_config_ssl_only to True completely discards
fetched configurations that are using plaintext protocols during
autoconfiguration.
---
.../prefs/content/accountcreation/readFromXML.js | 21 ++++++++++++++++++-
1 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/mailnews/base/prefs/content/accountcreation/readFromXML.js b/mailnews/base/prefs/content/accountcreation/readFromXML.js
index 5bcf09f..2ac6a3f 100644
--- a/mailnews/base/prefs/content/accountcreation/readFromXML.js
+++ b/mailnews/base/prefs/content/accountcreation/readFromXML.js
@@ -51,6 +51,9 @@
*/
function readFromXML(clientConfigXML)
{
+ var prefs = Cc["@mozilla.org/preferences-service;1"]
+ .getService(Ci.nsIPrefBranch);
+ var ssl_only = prefs.getBoolPref("mailnews.auto_config_ssl_only");
var exception;
if (typeof(clientConfigXML) != "xml" ||
!("emailProvider" in clientConfigXML))
@@ -96,13 +99,20 @@ function readFromXML(clientConfigXML)
try {
iO.socketType = sanitize.translate(iXsocketType,
{ plain : 1, SSL: 2, STARTTLS: 3 });
- break; // take first that we support
+
+ if (iO.socketType != 1) {
+ // pick first non-plaintext protocol, if available
+ break;
+ }
} catch (e) { exception = e; }
}
if (!iO.socketType)
throw exception ? exception : "need proper <socketType> in XML";
exception = null;
+ if (iO.socketType == 1 && ssl_only)
+ continue; // skip this configuration since we force ssl
+
for each (let iXauth in iX.authentication)
{
try {
@@ -170,13 +180,20 @@ function readFromXML(clientConfigXML)
try {
oO.socketType = sanitize.translate(oXsocketType,
{ plain : 1, SSL: 2, STARTTLS: 3 });
- break; // take first that we support
+
+ if (oO.socketType != 1) {
+ // pick first non-plaintext protocol, if available
+ break;
+ }
} catch (e) { exception = e; }
}
if (!oO.socketType)
throw exception ? exception : "need proper <socketType> in XML";
exception = null;
+ if (oO.socketType == 1 && ssl_only)
+ continue; // skip this configuration since we force ssl
+
for each (let oXauth in oX.authentication)
{
try {
--
1.7.7.3
