Scott MacGregor Assignee
	Description • 19 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.

	Scott MacGregor Assignee
	Comment 1 • 19 years ago

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

	Scott MacGregor Assignee
	Comment 2 • 19 years ago

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.

	David :Bienvenu
	Comment 3 • 19 years ago

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

looks good to me, assuming Dan's r=

	Scott MacGregor Assignee
	Comment 4 • 19 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.

	Scott MacGregor Assignee
	Comment 6 • 19 years ago

carrying david's sr forward.

	Daniel Veditz [:dveditz]
	Comment 7 • 19 years ago

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

	Scott MacGregor Assignee
	Comment 8 • 19 years ago

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.

	Alex Ihrig [:Thunderbird_Mail_DE]
	Comment 9 • 19 years ago

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

	Alex Ihrig [:Thunderbird_Mail_DE]
	Comment 10 • 19 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?

	Scott MacGregor Assignee
	Comment 11 • 19 years ago

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

	Scott MacGregor Assignee
	Comment 12 • 19 years ago

same patch but for the aviary 1.0 branch

	Scott MacGregor Assignee
	Comment 13 • 19 years ago

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

	Frank Wein [:mcsmurf]
	Comment 14 • 18 years ago

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

Clearing review request on obsolete patch