Intergration of the client-side phishing detection.

chrome/browser/safe_browsing/client_side_detection_host.cc

Index: chrome/browser/safe_browsing/client_side_detection_host.cc
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3d0a0a5cafccda514decac7d01585333be12f20a
--- /dev/null
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -0,0 +1,143 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/safe_browsing/client_side_detection_host.h"
+
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/task.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_thread.h"
+#include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/browser/renderer_host/render_view_host.h"
+#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/browser/safe_browsing/client_side_detection_service.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/tab_contents/navigation_controller.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/safebrowsing_messages.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/render_messages_params.h"
+#include "googleurl/src/gurl.h"
+#include "ipc/ipc_message.h"
+#include "ipc/ipc_message_macros.h"
+
+namespace safe_browsing {
+
+ClientSideDetectionHost::ClientSideDetectionHost(TabContents* tab)
+    : tab_(tab),
+      service_(g_browser_process->safe_browsing_detection_service()),
+      cb_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
+  DCHECK(tab);
+  // Note: service_ and sb_service_ might be NULL.
+  ResourceDispatcherHost* resource =
+      g_browser_process->resource_dispatcher_host();
+  if (resource) {
+    sb_service_ = resource->safe_browsing_service();
+  }
+}
+
+ClientSideDetectionHost::~ClientSideDetectionHost() {
+}
+
+bool ClientSideDetectionHost::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(ClientSideDetectionHost, message)
+    IPC_MESSAGE_HANDLER(SafeBrowsingDetectionHostMsg_DetectedPhishingSite,
+                        OnDetectedPhishingSite)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void ClientSideDetectionHost::DidNavigateMainFramePostCommit(
+    const NavigationController::LoadCommittedDetails& details,
+    const ViewHostMsg_FrameNavigate_Params& params) {
+  // TODO(noelutz): move this DCHECK to TabContents and fix all the unit tests
+  // that don't call this method on the UI thread.
+  // DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Brian Ryner, 2011/02/11 01:30:39]

Just so I understand, is the issue that the tests are actually calling this from
the wrong thread, or are they just not instantiating a BrowserThread for the UI
thread?

[noelutz, 2011/02/15 23:00:55]

Tests just don't create the UI browser thread object.

+
+  // If we navigate away and there currently is a pending phishing
+  // report request we have to cancel it to make sure we don't display
+  // an interstitial for the wrong page.  Note that this won't cancel
+  // the server ping back but only cancel the showing of the
+  // interstial.
+  if (cb_factory_.HasPendingCallbacks()) {

[Brian Ryner, 2011/02/11 01:30:39]

Is there a reason why you're wrapping this in HasPendingCallback, or could you
just always call RevokeAll?

[noelutz, 2011/02/15 23:00:55]

You're right.  It's not needed.

+    cb_factory_.RevokeAll();
+  }
+}
+
+void ClientSideDetectionHost::OnDetectedPhishingSite(const GURL& phishing_url,
+                                                     double phishing_score) {
+  // There is something seriously wrong if there is no service class but
+  // this method is called.  The renderer should not start phishing detection

[Brian Ryner, 2011/02/11 01:30:39]

Would this ever happen during shutdown? i.e. could the
ClientSideDetectionService class be destroyed while there's still a pending
DetectedPhishingSite IPC?

[noelutz, 2011/02/15 23:00:55]

Good question.  The ClientSideDetectionService class only gets destroyed once
the browser destructor is called.  I'm not sure if this object will still be
around at this point.

+  // if there isn't any service class in the browser.
+  DCHECK(service_);
+  if (service_ && tab_) {
+    // There shouldn't be any pending requests because we revoke them everytime
+    // we navigate away.
+    DCHECK(!cb_factory_.HasPendingCallbacks());
+    service_->SendClientReportPhishingRequest(
+        phishing_url,
+        phishing_score,
+        cb_factory_.NewCallback(
+            &ClientSideDetectionHost::MaybeShowPhishingWarning));
+  }
+}
+
+void ClientSideDetectionHost::MaybeShowPhishingWarning(GURL phishing_url,
+                                                       bool is_phishing) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (is_phishing &&
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableClientSidePhishingInterstitial)) {
+    DCHECK(tab_);
+    // TODO(noelutz): this is not perfect.  It's still possible that the
+    // user browses away before the interstitial is shown.  Maybe we should

[Brian Ryner, 2011/02/11 01:30:39]

It would be kind of nice if we could change
SafeBrowsingService::ShowBlockingPage to handle this for us.  For example, if we
could pass in a URL or a page id and it would verify that it's still showing
before it shows the interstitial.  This could be a future change though.

[noelutz, 2011/02/15 23:00:55]

Page id might work. We can't simply check the phishing url because it is
canonicalized.

+    // stop all pending navigations?
+    if (sb_service_) {
+      // TODO(noelutz): refactor this code so that we don't need to use
+      // the SafeBrowsing service class.

[lzheng, 2011/02/11 19:39:42]

I will be happy if the DisplayBlockingPage could be refactored so
malware/phishing/csd/download could share.

[noelutz, 2011/02/15 23:00:55]

what does download do right now?  We do share malware/phishing/csd but the csd
usage of it is a bit hacky.  See comment above.

[lzheng, 2011/02/16 19:09:32]

Download doesn't use the interstitial page(I should not include that in the
comments). What I meant is that we should avoid creating a new way of showing
warning pages for csd, we should share code with phishing/malware.
 
On 2011/02/15 23:00:55, noelutz wrote:

[noelutz, 2011/02/16 21:24:43]

I'm not sure I fully understand your comment.  This code does reuse the way we
show the phishing warning.  My point in the TODO is that the only reason we use
the SafeBrowsingService class and go through the IO thread is because the
service class keeps a whitelist around that contains sites where the client has
clicked through the warning.  I wouldn't want a phishing warning to show
followed by a csd warning.  Other than that we reuse all the code to show an
interstitial (we couldn't reuse more code I don't think).  

My comment basically suggests that we may want to move the whitelist logic that
is currently in the safebrowsing service class to the safebrowsing blocking page
class so that we don't need to use the service class here (and so that we don't
need to go through the IO thread).

Does that make sense?  I think this can be done in a separate CL though.

+      BrowserThread::PostTask(
+          BrowserThread::IO,
+          FROM_HERE,
+          NewRunnableMethod(sb_service_.get(),
+                            &SafeBrowsingService::DisplayBlockingPage,
+                            phishing_url, phishing_url,
+                            // We only classify the main frame URL.
+                            ResourceType::MAIN_FRAME,
+                            // TODO(noelutz): create a separate threat type
+                            // for client-side phishing detection.
+                            SafeBrowsingService::URL_PHISHING,
+                            new CsdClient() /* will delete itself */,
+                            tab_->GetRenderProcessHost()->id(),
+                            tab_->render_view_host()->routing_id()));
+    }
+  }
+}
+
+void ClientSideDetectionHost::set_client_side_detection_service(
+    ClientSideDetectionService* service) {
+  service_ = service;
+}
+
+void ClientSideDetectionHost::set_safe_browsing_service(
+    SafeBrowsingService* service) {
+  sb_service_ = service;
+}
+
+CsdClient::CsdClient() {}
+
+CsdClient::~CsdClient() {}
+
+void CsdClient::OnBlockingPageComplete(bool proceed) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // Delete this on the UI thread since it was created there.
+  BrowserThread::PostTask(BrowserThread::UI,
+                          FROM_HERE,
+                          new DeleteTask<CsdClient>(this));
+}
+
+}  // namespace safe_browsing