300834 - Add a pref for host names which can bypass the remote image settings, phishing etc. (mail.trusteddomains)

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Scott MacGregor]

We should allow enterprise customers to set a hidden pref which lists a set of
hostnames which should allow a remote image that originates from that host to load.

This will allow them to use the remote image blocker, without their users seeing
the blocked remote images bar for company mail where the images reside on their
intranet.

Comment 1

[Scott MacGregor]

Attachment: work in progress

our network is down except for reaching bugzilla so I haven't been able to test
this yet.

Comment 2

[Scott MacGregor]

Attachment: updated fix

David, I updated the patch to make the trusted host prefs generic and not
specific to remote image blocking. This means we could later use these same
prefs to bypass the phishing detector and other things.

I'm asking Dan to review this as well just so he's aware of what we are doing
in case he sees a big no no. Dan, we're adding some generic prefs that business
customers could set. The idea is that a business could list a set of servers on
their intranet which they wish to trust. This means no remote image blocking if
the image resides on a intranet server.

Comment 3

[David :Bienvenu]

Comment on attachment 189698 [details] [diff] [review]
updated fix

looks good to me, assuming Dan's r=

Comment 4

[Scott MacGregor]

This patch had two extraneous lines in nsMsgContentPolicy::IsTrustedHost:

+  nsCOMPtr<nsIPrefBranch2> prefInternal =
do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);

which I removed. I'm not going to bother posting another patch with that change.

Comment 5

[Scott MacGregor]

Attachment: patch for the enterprise 1.0 branch

Comment 6

[Scott MacGregor]

Attachment: updated trunk fix

carrying david's sr forward.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 189725 [details] [diff] [review]
updated trunk fix

A plea: using the cvs diff -p option really helps reviewers.

I don't have a problem in principle with this. I do have a certain knee-jerk
rejection of inventing yet another site-based permission mechanism, but then I
remember the PITA of prepopulating the permission-manager-based xpinstall
whitelist. So OK! A permission-manager approach would have made it far easier
for an enterprise to whitelist all of myco.com -- with this pref-based approach
they'll have to explicitly add each host that might ever need to pass. Or
you'll need to change the IsTrustedHost() algorithm to chop off hosts in a
loop.

I am slightly worried people might complain the image blocking feature is
"broken" because it mysteriously lets certain images through and there is no UI
for the pref. Not even about:config in Thunderbird unless you know about the
extension. But that's just a personal UE concern, not a security issue.

>+// Just append the host name after the root pref name,
>+// mailnews.trustedhost., and set the pref value to true. For example:
>+// pref("mail.trustedhost.intranet.mozilla.org", true);

www.mozilla.org or some other known host might be a better example,
just in case someone thinks "intranet" is a keyword.

>+nsresult nsMsgContentPolicy::IsTrustedHost

You don't check this return value, why not just make it void?

r=dveditz

Comment 8

[Scott MacGregor]

Attachment: new approach that is domain instead of host based

Dan suggested this feature would be more useful if it were domain based instead
of host based and I agree. So companies can now white list any server under
say, the mozilla.org domain instead of listing each host individually.

The pref is now a comma delimited list of domains instead of a pref for each
host name. This complicated the comparison logic in IsTrustedDomain. I snagged
most of the string magic for that comparison from some code Darin wrote in the
DNS service.

Comment 9

[Alex Ihrig [:Thunderbird_Mail_DE]]

What about adding a button "Add to whitelist" in the remote image / phishing /
junk bars?

Comment 10

[Alex Ihrig [:Thunderbird_Mail_DE]]

If we would be adding the "Add to whitelist" buttons, it would be necessary to
have a GUI to manage the whitelists. The simplest would be to use addressbooks
as whitelists - so we could use the addressbook GUI to manage these lists.

Maybe we could use the Firefox extension managers whitelist GUI, to add a
whitelist manager in Thunderbird?

Comment 11

[Scott MacGregor]

we won't be adding any UI to end users for this. Thanks for the suggestion though.

Comment 12

[Scott MacGregor]

Attachment: aviary 1.0 version of the patch

same patch but for the aviary 1.0 branch

Comment 13

[Scott MacGregor]

this is fixed on the trunk and the aviary 1.0 branch.

Comment 14

[Frank Wein [:mcsmurf]]

Comment on attachment 189698 [details] [diff] [review]
updated fix

Clearing review request on obsolete patch