Add support for client-side phishing detection for non-UMA users.

Add support for client-side phishing detection for non-UMA users.

In this mode, a sanitized pingback is sent that does not include the URL or any
tokens extracted from the URL or page content.  Currently, this feature is
behind a command-line flag.

BUG=none
TEST=ClientSideDetectionServiceTest,BrowserFeatureExtractorTest


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98168

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98383

[mattm, 2011-08-12 03:00:28]

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/browser_feature_extractor.h (right):

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/browser_feature_extractor.h:78: //
ClientPhishingRequest.suvfix_prefix_hash.  Public for testing.
suvfix

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/client_side_detection_service.cc:261: // Token and
term features are not included in the pingback.
Did you consider a white-listing approach to sanitizing as well?  Either way,
should add a comment to the features.h files mentioning that this function may
need to be updated if adding new features.

[Brian Ryner, 2011-08-12 04:52:24]

Please take another look.

Thanks,

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/browser_feature_extractor.h (right):

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/browser_feature_extractor.h:78: //
ClientPhishingRequest.suvfix_prefix_hash.  Public for testing.
On 2011/08/12 03:00:28, mattm wrote:
> suvfix

Done.

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

http://codereview.chromium.org/7635010/diff/1015/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/client_side_detection_service.cc:261: // Token and
term features are not included in the pingback.
On 2011/08/12 03:00:28, mattm wrote:
> Did you consider a white-listing approach to sanitizing as well?  Either way,
> should add a comment to the features.h files mentioning that this function may
> need to be updated if adding new features.

That's a good point; I think whitelisting is probably safer in terms of not
sending data unintentionally.  I've updated to use this approach, and also added
comments in features.h and  browser_features.h as you suggested.

[noelutz, 2011-08-12 21:27:03]

LGTM

Thanks,
noe.

http://codereview.chromium.org/7635010/diff/5001/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/browser_features.h (right):

http://codereview.chromium.org/7635010/diff/5001/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/browser_features.h:19: // TODO(noelutz): move
renderer/safe_browsing/features.h to common.
move remove this TODO?

[Brian Ryner, 2011-08-12 21:32:25]

http://codereview.chromium.org/7635010/diff/5001/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/browser_features.h (right):

http://codereview.chromium.org/7635010/diff/5001/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/browser_features.h:19: // TODO(noelutz): move
renderer/safe_browsing/features.h to common.
On 2011/08/12 21:27:03, noelutz wrote:
> move remove this TODO?

Removed.

[mattm, 2011-08-12 22:37:13]

LGTM. I think there will be some conflicts with
http://codereview.chromium.org/7583007/, to make merging easier could you wait
to submit until after that one has been committed?  Thanks

[Brian Ryner, 2011-08-23 22:11:37]

I merged with http://codereview.chromium.org/7583007/ , please take another look
before I submit.  This also fixes the logic so that
--disable-client-side-detection will consistently override
--enable-sanitized-client-side-detection if both flags are specified.

[noelutz, 2011-08-23 22:21:10]

LGTM

On 2011/08/23 22:11:37, Brian Ryner wrote:
> I merged with http://codereview.chromium.org/7583007/ , please take another
look
> before I submit.  This also fixes the logic so that
> --disable-client-side-detection will consistently override
> --enable-sanitized-client-side-detection if both flags are specified.

[noelutz, 2011-08-23 22:21:25]

LGTM

http://codereview.chromium.org/7635010/diff/14001/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/browser_feature_extractor.cc (right):

http://codereview.chromium.org/7635010/diff/14001/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/browser_feature_extractor.cc:447: void
BrowserFeatureExtractor::ComputeURLHashes(
nit: rename this function to ComputeURLHash?

[Brian Ryner, 2011-08-23 23:00:50]

http://codereview.chromium.org/7635010/diff/14001/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/browser_feature_extractor.cc (right):

http://codereview.chromium.org/7635010/diff/14001/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/browser_feature_extractor.cc:447: void
BrowserFeatureExtractor::ComputeURLHashes(
On 2011/08/23 22:21:25, noelutz wrote:
> nit: rename this function to ComputeURLHash?

I was leaving this generic in case we need to send multiple hashes in the
future, but I think you're right that it's clearer for now if it's singular. 
Changed.

[mattm, 2011-08-24 03:38:02]

lgtm

[commit-bot: I haz the power, 2011-08-25 02:14:02]

Change committed as 98168

[Brian Ryner, 2011-08-26 00:41:50]

I'll try to commit this again now that http://codereview.chromium.org/7748011/
is committed.

[commit-bot: I haz the power, 2011-08-26 02:29:05]

Change committed as 98383