337484 - add the ability for users to report a phishing site

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, enhancement)

Description

[Tony Chang (Google)]

There needs to be a way for users to report phishing sites to their data provider.  The original extension code put a menu item in the tools menu.

The event handler should be "SB_executeCommandLocally('safebrowsing-submit-blacklist')".

Comment 1

[Sherman Dickman]

Some things to consider:
* Does selecting "Report Phishing Site" load the form in the active window, open a new window, or open a new tab?
* Where should the form and "submit success" pages be hosted?
* Where does the data get sent to?
* How is "Report a Phishing Page" handled if anti-phishing is turned off, in standard mode, or enhanced mode?

Some initial thoughts:
* Load the "Report Phishing Site" in a new tab in the active window
* Mozilla hosts both the submit and response form
* Modes:
     * Off - Report Phishing Site is not supported.  Display info/FAQ sheet with links to more information on anti-phishing
     * Standard Mode - Data gets posted directly to Google (since they are providing the default list)
     * Enhanced Mode - Data gets posted directly to selected provider

Thoughts?

Comment 2

[Sherman Dickman]

New thinking:
* Use chrome to solve the new tab vs existing tab question
* Leverage Reporter to support "Report Phishing Site" functionality since its really just another data type.
* Dispatch data asynchronously to partners using our existing mechanism, independant of whether or not anti phishing is in off, standard, or enhanced modes.

Comment 3

[Robert Accettura [:raccettura]]

My initial thought was this two part drama:

- When Phishy site is found, embed in UI ability to "report" the finding.  Which opens up the reporter wizard with a "problem type" of "report phishing/scam".  User can enter a description and the usual data is collected.

- Create interface to dump the data (mainly hostname, full url, description [if entered]) to file.  Perhaps even get them on ftp.  And let partners injest them into their systems as they desire.  I'd suggest csv, as the format for the dump, since it's pretty easy for anyone to work with (even excel handles it well)... but we could essentially do anything.  

I think that would essentially cover the goals, while keeping things pretty compact and clean... which is IMHO a good thing.


My concerns here are as follows:

- Privacy - we shouldn't collect *anything* without users express approval, since that's tin-foil-hat evil.  Not to mention I don't feel like patching reporter to use the NSA wiretapping API's.

- Data Accessibility - the data should be accessible for those who can use it (google, etc) in the fight against phishing.  How do they want it served?  Broiled, fried, BBQ? 

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

I just wanted to have a reference to bug 324658 in here somewhere, since these two are related, though likely not in a blocking or depending relationship. :)

Comment 5

[Robert Accettura [:raccettura]]

we need to hash out a plan on this (and that).

btw... I was thinking of disabling the screenshot option if you come from safebrowsing (since that's unnecessary).  No need to hog bandwidth.

Comment 6

[Sherman Dickman]

With respect to privacy, we shouldn't be keeping anything that could uniquely identify a user (such as IP address) as there are country, governmental, state, and perhaps local data retention requirements.

On licenses, I believe that a user must click through a license/privacy policy when he/she turns on the Reporter feature (at least I did on FF on Mac OS X).  We may need to review and possibly update to support anti-phishing reporting requirements.

I've asked one service provider to state their requirements WRT how they want data transmitted to them.

Comment 7

[u88484]

Using the latest trunk with SB enabled (check google's test page) I went through my junk mail folder in TB today and went to every site that I know was a phishing attempt (which was about 40) and none of them were reported to me as such. If google doesn't know of the site it won't be reported to the user that its a phising attempt so I feel that this bug should block 2.0. No use in safebrowsing if we can't report a site to get added to the list because most new sites google won't know about right away to add to the list and the more users reporting such the faster it gets added and less users browsing unsafely.

btw some of the sites in my phishing trip were from more then 2 or 3 weeks ago...way too long to be out there and google not have them on the list.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #7)
> such. If google doesn't know of the site it won't be reported to the user that
> its a phising attempt so I feel that this bug should block 2.0. No use in

We're not actually using the full anti-phishing database that Google has access to due to licensing restrictions on some of the data in that database. They're working on relicensing that data, or creating a subset of the database with more content that we can use.

In the meantime, I'm hoping that sites our alpha and beta users report will get added to a public-licensed resource; at the very least, they'll get added to one which our users will have access to. 

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Proposal:

 - add this capability into reporter
 - depending on the anti-phishing provider selected, and based on that 
   provider's definition (see bug 339876), provide a consistent UI
   for reporting an anti-phishing site:

   .-------------------------------------------------.
   | Report a Web Forgery                            |
   |-------------------------------------------------|
   | Address of suspected forgery:                   |
   | [http://autofilled.address.com               ]  |
   |                                                 |
   | Suspected to be a forgery of:                   | <-- [1]
   | [Wells Fargo                                 ]  |
   |                                                 |
   | #########  Enter the word you see to the left   | <-- [2]
   | #########  [nospam          ]                   |
   |                                                 |
   |                               (Cancel) (Next >) | <-- [3]
   '-------------------------------------------------'

[1] based on the definition, we can capture optional information that a provider wants us to capture

[2] capacha to prevent spam at the endpoint

[3] after filling in this form, the user will see a summary of the information to be sent on to the provider & mozilla, and can cancel or finish the report.
   |
   |

Comment 10

[Robert Accettura [:raccettura]]

Somewhat out of order random thoughts (pay no attention to the numbers, they are just there for my amusement):

1.  We need to manage a privacy policy for each (and note if the user chooses another service, they need to accept that privacy policy as well).

2.  If we implement a CAPTCHA (good news, I just wrote a fair CAPTCHA script, though just image not audio this morning), we should leverage it for everything, not just phishing sites.

3.  Are we creating a new separate interface for reporting phishing sites?  We're capturing somewhat different data for evangelism reports.

4.  Who is the endpoint?  Are we creating 2 connections (one to provider, one to mozilla, will mozilla proxy?  will provider proxy?)

5.  Are the plans to leverage the existing reporter webapp?  Or a separate product?

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

I think we're going to go with a menuitem (in help) for Beta1. I believe what we should do (and yes, I know it's not ideal) is ..:

Help
----------------------------
Report >  Broken Web Site ...
          Phishing Web Site ...

With the Phishing Web Site opening a new tab using the entity used in safe browsing's code for reporting phishing web sites.

Roc, let me know if you need us to get someone else to work this up?

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

er, s/roc/robert/ oops

Comment 13

[Robert Accettura [:raccettura]]

I think I have an old patch for reporter (before the xul tool was created it was to be completely web based) that I can s/reporter/phishing/ for this task.  Does exactly that.

I presume it will use browser.safebrowsing.provider.[idnum].reportPhishURL for the url.

Comment 14

[Robert Accettura [:raccettura]]

Attachment: Work In Progress

This is a true work in progress (completely untested as I don't have a compiled tree and want to sleep).

- Son of Frankenstein, this is almost entirely code from other places (mainly reporter).

- Not sure how we would handle the nested menu, since while reporter isn't a true extension anymore, I believe it can still be disabled int he build.  Besides that it is shared between Firefox and SeaMonkey (who I don't believe is using safe browsing).  Not sure how to manage that.  As a result, the current patch is just a new menu under the current reporter menu option.

- Did I mention this is untested?

Not sure what I'll be doing this weekend, especially with it being a vacation weekend, so if someone desires to address the above, I won't mind ;-).

Comment 15

[Robert Accettura [:raccettura]]

Attachment: Work In Progress (2)

The other patch wasn't quite correct, looks like I had a stale tree.  This should be better.

Comment 16

[Robert Accettura [:raccettura]]

Attachment: Work In Progress (3) Broadcaster broken

This is a bit further along.  For some reason the broadcaster isn't working.  Haven't figured it out, likely just a typo...  Since I'm out of time for the evening posting what I have for anyone who wants to poke.

Comment 17

[Tony Chang (Google)]

(In reply to comment #16)
> Created an attachment (id=228084) [edit]
> Work In Progress (3) Broadcaster broken
> 
> This is a bit further along.  For some reason the broadcaster isn't working. 
> Haven't figured it out, likely just a typo...  Since I'm out of time for the
> evening posting what I have for anyone who wants to poke.

I'm looking at this now and will post a new patch.

Comment 18

[Tony Chang (Google)]

Attachment: v1: report phish menu item

Rather than using a web progress listener, just update the menu item using onpopupshowing.

Comment 19

[Tony Chang (Google)]

(In reply to comment #18)
> Created an attachment (id=228211) [edit]
> v1: report phish menu item

Oh, I should add that I put some google-style localization into application.js (i.e., appending &hl=<locale> to the url).  That's probably not the right way to do this, the url should be in a dtd file rather than a pref so we can change the param per locale.

Comment 20

[Tony Chang (Google)]

Comment on attachment 228211 [details] [diff] [review]
v1: report phish menu item

(adding monica as SR because I don't know a better way to have 2 reviewers)

Comment 21

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 228211 [details] [diff] [review]
v1: report phish menu item

The safe browsing stuff looks fine to me.

Comment 22

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 228211 [details] [diff] [review]
v1: report phish menu item

>Index: browser/locales/en-US/chrome/browser/safebrowsing/report-phishing.dtd

>+<!ENTITY reportPhishingMenu.title "Report Phishing Website">
>+<!ENTITY reportPhishingMenu.accesskey "P">
>\ No newline at end of file

add one?

>Index: browser/components/safebrowsing/content/sb-loader.js

>+  setReportPhishingMenu: function() {
>+    var uri = getBrowser().currentURI;
>+    if (!uri)
>+      return;

Is getBrowser defined for mac when there is no window open? I think you may need to compare window.location to getBrowserURL as in http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.657&mark=1628#1626 and deal with that appropriately (return early). Or does this code not run in that case? I don't know, I don't have a mac, or much knowledge of how mac menus work.

>Index: browser/components/safebrowsing/jar.mn

>+#ifdef MOZ_PHOENIX
>+% overlay chrome://browser/content/browser.xul         chrome://browser/content/safebrowsing/report-phishing-overlay.xul
>+#endif

Talked to Robert about this, there's no need for the ifdef. If SeaMonkey or others need to start building this, they can add the ifdefs then, but I don't think that it's likely that they would build components/safebrowsing anyways.

Comment 23

[Tony Chang (Google)]

Attachment: v2: report phish menu item

incorporates Gavin's feedback

Hasn't been tested on a mac (cvs updating my tree now . . .)

Comment 24

[Tony Chang (Google)]

(In reply to comment #23)
> Created an attachment (id=228250) [edit]
> v2: report phish menu item
> 
> incorporates Gavin's feedback
> 
> Hasn't been tested on a mac (cvs updating my tree now . . .)

Tested and works as expected on Mac (no menu item for non-browser windows).

Comment 25

[Tony Chang (Google)]

Comment on attachment 228250 [details] [diff] [review]
v2: report phish menu item

on trunk

Comment 26

[Tony Chang (Google)]

Attachment: v3: report phish menu item

Same as last patch without the mac getBrowserURL check (doesn't matter since we only overlay browser.xul).

Comment 27

[Axel Hecht [:Pike]]

(In reply to comment #19)
> (In reply to comment #18)
> > Created an attachment (id=228211) [edit]
> > v1: report phish menu item
> 
> Oh, I should add that I put some google-style localization into application.js
> (i.e., appending &hl=<locale> to the url).  That's probably not the right way
> to do this, the url should be in a dtd file rather than a pref so we can change
> the param per locale.

Or it should use a key to be replaced with the lang. I don't see where the URL is coming from, but maybe we could use {moz:locale} as in the search plugins and replace that.

Comment 28

[Tony Chang (Google)]

(In reply to comment #27)
> Or it should use a key to be replaced with the lang. I don't see where the URL
> is coming from, but maybe we could use {moz:locale} as in the search plugins
> and replace that.

The URL is coming from a pref value.  Other anti phishing data providers can be added by setting a few pref values, including one for the reportPhishURL.

I guess this means that data providers should try to provide localized pages to report phishing.  The data provider format (bug 339876) should allow a way to specify a localized page (maybe something like %LANG% getting replace by the locale).

Comment 29

[Tony Chang (Google)]

Err, I should read more carefully.  Yeah, I like the {moz:locale} suggestion.

Comment 30

[Marek Stępień [:marcoos, inactive]]

Attachment: Purge duplicate entities from reportPhishing.dtd

reportPhishing.dtd on trunk has it's content tripled (i.e. all the entities occur three times). 

This patch is a simple fix.

Comment 31

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 228267 [details] [diff] [review]
Purge duplicate entities from reportPhishing.dtd

How'd that happen? :)

Comment 32

[u60234]

The same thing (triple content) happened also to report-phishing-overlay.xul, causing the whole Help menu to disappear.

Comment 33

[Marek Stępień [:marcoos, inactive]]

Attachment: Purge duplicates from reportPhishing.dtd and report-phishing-overlay.xul

Indeed.

Updated my patch to clean up the overlay file as well.

Comment 34

[Tony Chang (Google)]

(In reply to comment #33)
> Created an attachment (id=228297) [edit]
>  Purge duplicates from reportPhishing.dtd and report-phishing-overlay.xul
> 
> Indeed.
> 
> Updated my patch to clean up the overlay file as well.

Sorry about that.  I must have patch'ed multiple times and gotten multiple content.

Comment 35

[Marek Stępień [:marcoos, inactive]]

Comment on attachment 228297 [details] [diff] [review]
 Purge duplicates from reportPhishing.dtd and report-phishing-overlay.xul

Can somebody please check this into trunk? L10n tinderboxen have been orange for the whole day because of this...

Comment 36

[Tony Chang (Google)]

(In reply to comment #35)
> (From update of attachment 228297 [details] [diff] [review] [edit])
> Can somebody please check this into trunk? L10n tinderboxen have been orange
> for the whole day because of this...

On trunk.

Comment 37

[Myk Melez [:myk] [@mykmelez]]

Landed attachment 228252 [details] [diff] [review] ("v3: report phish menu item") on the 1.8 branch.  Didn't land attachment 228297 [details] [diff] [review] ("Purge duplicates from reportPhishing.dtd and report-phishing-overlay.xul"), since that followup patch is specific to the way the v3 patch was landed on the trunk, not a problem with the v3 patch itself.

Comment 38

[Robert Accettura [:raccettura]]

Tony, Gavin: Thanks for taking care of this.