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

RESOLVED FIXED in Thunderbird1.1

Status

Thunderbird
General
RESOLVED FIXED
13 years ago
5 years ago

People

(Reporter: Scott MacGregor, Assigned: Scott MacGregor)

Tracking

Trunk
Thunderbird1.1
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

13 years ago
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.
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird1.1
(Assignee)

Comment 1

13 years ago
Created attachment 189355 [details] [diff] [review]
work in progress

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

Comment 2

13 years ago
Created attachment 189698 [details] [diff] [review]
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.
Attachment #189355 - Attachment is obsolete: true
Attachment #189698 - Flags: superreview?(bienvenu)
Attachment #189698 - Flags: review?(dveditz)

Comment 3

13 years ago
Comment on attachment 189698 [details] [diff] [review]
updated fix

looks good to me, assuming Dan's r=
Attachment #189698 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 4

13 years ago
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.
(Assignee)

Comment 5

13 years ago
Created attachment 189723 [details] [diff] [review]
patch for the enterprise 1.0 branch
(Assignee)

Comment 6

13 years ago
Created attachment 189725 [details] [diff] [review]
updated trunk fix

carrying david's sr forward.
Attachment #189698 - Attachment is obsolete: true
Attachment #189725 - Flags: superreview+
Attachment #189725 - Flags: review?(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
Attachment #189725 - Flags: review?(dveditz) → review+
(Assignee)

Comment 8

13 years ago
Created attachment 189846 [details] [diff] [review]
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.
Attachment #189723 - Attachment is obsolete: true
Attachment #189725 - Attachment is obsolete: true
Attachment #189846 - Flags: superreview?(bienvenu)
Attachment #189846 - Flags: review?(dveditz)

Updated

13 years ago
Attachment #189846 - Flags: superreview?(bienvenu) → superreview+

Comment 9

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

Comment 10

13 years ago
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?
(Assignee)

Comment 11

13 years ago
we won't be adding any UI to end users for this. Thanks for the suggestion though.
(Assignee)

Comment 12

13 years ago
Created attachment 190437 [details] [diff] [review]
aviary 1.0 version of the patch

same patch but for the aviary 1.0 branch
(Assignee)

Comment 13

13 years ago
this is fixed on the trunk and the aviary 1.0 branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Attachment #189846 - Flags: review?(dveditz) → review+
Comment on attachment 189698 [details] [diff] [review]
updated fix

Clearing review request on obsolete patch
Attachment #189698 - Flags: review?(dveditz)
Summary: Add a pref for same host names which can bypass the remote image settings → Add a pref for host names which can bypass the remote image settings, phishing etc. (mail.trusteddomains)
You need to log in before you can comment on or make changes to this bug.