Synchronous implementation of CSD allowlist check on Android

Before allowlist is available from GmsCore, we plan to have it
hardcoded in our code. This call is synchronous for now,
since it gets result directly instead of a IPC call.

I will have a follow up CL for Async implementation.

Bug: 999344
Change-Id: Ide3b7de83f0e32c5be8ac657369ce9f95a848f7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776601
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694876}

android_webview/javatests/src/org/chromium/android_webview/test/SafeBrowsingTest.java

@@ -51,7 +51,6 @@ import org.chromium.base.test.util.FlakyTest;
 import org.chromium.base.test.util.InMemorySharedPreferences;
 import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
 import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
-import org.chromium.components.safe_browsing.SafeBrowsingApiHandler.Observer;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.browser.test.util.Criteria;
@@ -194,6 +193,11 @@ public class SafeBrowsingTest {
                         callbackId, SafeBrowsingResult.SUCCESS, metadata, CHECK_DELTA_US));
             // clang-format on
         }
+
+        @Override
+        public boolean startAllowlistLookup(final String uri, int[] threatsOfInterest) {
+            return false;
+        }
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/MockSafeBrowsingApiHandler.java

@@ -11,7 +11,6 @@ import org.json.JSONObject;
 import org.chromium.base.annotations.UsedByReflection;
 import org.chromium.base.task.PostTask;
 import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
-import org.chromium.components.safe_browsing.SafeBrowsingApiHandler.Observer;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 
 import java.util.HashMap;
@@ -62,6 +61,11 @@ public class MockSafeBrowsingApiHandler implements SafeBrowsingApiHandler {
         // clang-format on
     }
 
+    @Override
+    public boolean startAllowlistLookup(final String uri, int[] threatsOfInterest) {
+        return false;
+    }
+
     private String getMetadata(String uri, int[] threatsOfInterest) {
         if (mResponseMap.containsKey(uri)) {
             try {

components/safe_browsing/android/java/src/org/chromium/components/safe_browsing/SafeBrowsingApiBridge.java

@@ -81,6 +81,18 @@ public final class SafeBrowsingApiBridge {
         }
     }
 
+    /**
+     * TODO(crbug.com/995926): Make this call async
+     * Starts a Safe Browsing Allowlist check.
+     *
+     * If the uri is in the allowlist, return true. Otherwise, return false.
+     */
+    @CalledByNative
+    private static boolean startAllowlistLookup(
+            SafeBrowsingApiHandler handler, String uri, int[] threatsOfInterest) {
+        return handler.startAllowlistLookup(uri, threatsOfInterest);
+    }
+
     private static native boolean nativeAreLocalBlacklistsEnabled();
     private static native void nativeOnUrlCheckDone(
             long callbackId, int resultStatus, String metadata, long checkDelta);

components/safe_browsing/android/java/src/org/chromium/components/safe_browsing/SafeBrowsingApiHandler.java

@@ -71,4 +71,20 @@ public interface SafeBrowsingApiHandler {
      * This is called on every URL resource Chrome loads, on the same sequence as |init|.
      */
     public void startUriLookup(long callbackId, String uri, int[] threatsOfInterest);
+
+    /**
+     * Start a check to determine if a uri is in a set of allowlists. If true, password protection
+     * service will assume the url to be safe and skip it. This feature is not applicable to Android
+     * Webview, because there is no saved password or GAIA password in AW.
+     *
+     * @param uri The uri from a password protection event(user focuses on password form
+     *      * or user reuses their password)
+     * @param threatsOfInterest determines the type of allowlists that the uri will be matched to.
+     *
+     * @return true if the uri is found in the set of allowlists. Otherwise, false.
+     */
+    // TODO(xinghuilu@): remove default once downstream implementation is patched
+    default boolean startAllowlistLookup(String uri, int[] threatsOfInterest) {
+        return false;
+    }
 }

components/safe_browsing/android/remote_database_manager.cc

@@ -271,8 +271,17 @@ bool RemoteSafeBrowsingDatabaseManager::CheckUrlForSubresourceFilter(
 AsyncMatch RemoteSafeBrowsingDatabaseManager::CheckCsdWhitelistUrl(
     const GURL& url,
     Client* client) {
-  // TODO(crbug.com/995926): Enable CSD allowlist on Android
-  return AsyncMatch::NO_MATCH;
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // If this URL's scheme isn't supported, call is safe.
+  if (!CanCheckUrl(url)) {
+    return AsyncMatch::MATCH;
+  }
+
+  // TODO(crbug.com/995926): Make this call async
+  SafeBrowsingApiHandler* api_handler = SafeBrowsingApiHandler::GetInstance();
+  bool is_match = api_handler->StartCSDAllowlistCheck(url);
+  return is_match ? AsyncMatch::MATCH : AsyncMatch::NO_MATCH;
 }
 
 bool RemoteSafeBrowsingDatabaseManager::MatchDownloadWhitelistString(

components/safe_browsing/android/remote_database_manager_unittest.cc

@@ -27,6 +27,7 @@ class TestSafeBrowsingApiHandler : public SafeBrowsingApiHandler {
   void StartURLCheck(std::unique_ptr<URLCheckCallbackMeta> callback,
                      const GURL& url,
                      const SBThreatTypeSet& threat_types) override {}
+  bool StartCSDAllowlistCheck(const GURL& url) override { return false; }
 };
 
 }  // namespace

components/safe_browsing/android/safe_browsing_api_handler.h

@@ -36,6 +36,8 @@ class SafeBrowsingApiHandler {
                              const GURL& url,
                              const SBThreatTypeSet& threat_types) = 0;
 
+  virtual bool StartCSDAllowlistCheck(const GURL& url) = 0;
+
   virtual ~SafeBrowsingApiHandler() {}
 
  private:

components/safe_browsing/android/safe_browsing_api_handler_bridge.cc

@@ -63,6 +63,8 @@ int SBThreatTypeToJavaThreatType(const SBThreatType& sb_threat_type) {
       return safe_browsing::JAVA_THREAT_TYPE_POTENTIALLY_HARMFUL_APPLICATION;
     case SB_THREAT_TYPE_URL_UNWANTED:
       return safe_browsing::JAVA_THREAT_TYPE_UNWANTED_SOFTWARE;
+    case SB_THREAT_TYPE_CSD_WHITELIST:
+      return safe_browsing::JAVA_THREAT_TYPE_CSD_ALLOWLIST;
     default:
       NOTREACHED();
       return 0;
@@ -118,8 +120,6 @@ void OnUrlCheckDoneOnIOThread(jlong callback_id,
                               jint result_status,
                               const std::string metadata) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DVLOG(1) << __FUNCTION__ << ": check: " << callback_id
-           << " status: " << result_status << " metadata: [" << metadata << "]";
 
   PendingCallbacksMap* pending_callbacks = GetPendingCallbacksMapOnIOThread();
   bool found = base::Contains(*pending_callbacks, callback_id);
@@ -137,7 +137,6 @@ void OnUrlCheckDoneOnIOThread(jlong callback_id,
       CHECK(!callback->is_null());  // Remove after fixing crbug.com/889972
 
       ReportUmaResult(UMA_STATUS_TIMEOUT);
-      DVLOG(1) << "Safe browsing API call timed-out";
     } else {
       CHECK(!callback->is_null());  // Remove after fixing crbug.com/889972
 
@@ -162,9 +161,6 @@ void OnUrlCheckDoneOnIOThread(jlong callback_id,
     ThreatMetadata threat_metadata;
     ReportUmaResult(
         ParseJsonFromGMSCore(metadata, &worst_threat, &threat_metadata));
-    if (worst_threat != SB_THREAT_TYPE_SAFE) {
-      DVLOG(1) << "Check " << callback_id << " was a MATCH";
-    }
 
     std::move(*callback).Run(worst_threat, threat_metadata);
   }
@@ -195,10 +191,6 @@ void JNI_SafeBrowsingApiBridge_OnUrlCheckDone(
   TRACE_EVENT1("safe_browsing", "SafeBrowsingApiHandlerBridge::OnUrlCheckDone",
                "metadata", metadata_str);
 
-  DVLOG(1) << "OnURLCheckDone invoked for check " << callback_id
-           << " with status=" << result_status << " and metadata=["
-           << metadata_str << "]";
-
   base::PostTask(FROM_HERE, {BrowserThread::IO},
                  base::BindOnce(&OnUrlCheckDoneOnIOThread, callback_id,
                                 result_status, metadata_str));
@@ -215,7 +207,6 @@ SafeBrowsingApiHandlerBridge::~SafeBrowsingApiHandlerBridge() {}
 bool SafeBrowsingApiHandlerBridge::CheckApiIsSupported() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!checked_api_support_) {
-    DVLOG(1) << "Checking API support.";
     j_api_handler_ = base::android::ScopedJavaGlobalRef<jobject>(
         Java_SafeBrowsingApiBridge_create(AttachCurrentThread()));
     checked_api_support_ = true;
@@ -235,7 +226,6 @@ std::string SafeBrowsingApiHandlerBridge::GetSafetyNetId() {
         Java_SafeBrowsingApiBridge_getSafetyNetId(env, j_api_handler_);
     safety_net_id =
         jsafety_net_id ? ConvertJavaStringToUTF8(env, jsafety_net_id) : "";
-    DVLOG(1) << __FUNCTION__ << ": safety_net_id: " << safety_net_id;
   }
 
   return safety_net_id;
@@ -258,7 +248,6 @@ void SafeBrowsingApiHandlerBridge::StartURLCheck(
   jlong callback_id = next_callback_id_++;
   GetPendingCallbacksMapOnIOThread()->insert(
       {callback_id, std::move(callback)});
-  DVLOG(1) << "Starting check " << callback_id << " for URL " << url;
 
   DCHECK(!threat_types.empty());
 
@@ -271,4 +260,22 @@ void SafeBrowsingApiHandlerBridge::StartURLCheck(
                                             j_url, j_threat_types);
 }
 
+bool SafeBrowsingApiHandlerBridge::StartCSDAllowlistCheck(const GURL& url) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!CheckApiIsSupported()) {
+    return false;
+  }
+
+  SBThreatTypeSet threat_types(
+      CreateSBThreatTypeSet({safe_browsing::SB_THREAT_TYPE_CSD_WHITELIST}));
+
+  // TODO(crbug.com/999344): Add UMA metrics
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> j_url = ConvertUTF8ToJavaString(env, url.spec());
+  ScopedJavaLocalRef<jintArray> j_threat_types =
+      SBThreatTypeSetToJavaArray(env, threat_types);
+  return Java_SafeBrowsingApiBridge_startAllowlistLookup(env, j_api_handler_,
+                                                         j_url, j_threat_types);
+}
+
 }  // namespace safe_browsing

components/safe_browsing/android/safe_browsing_api_handler_bridge.h

@@ -32,6 +32,8 @@ class SafeBrowsingApiHandlerBridge : public SafeBrowsingApiHandler {
                      const GURL& url,
                      const SBThreatTypeSet& threat_types) override;
 
+  bool StartCSDAllowlistCheck(const GURL& url) override;
+
  private:
   // Creates the |j_api_handler_| if it hasn't been already.  If the API is not
   // supported, this will return false and j_api_handler_ will remain nullptr.

components/safe_browsing/android/safe_browsing_api_handler_util.cc

@@ -163,6 +163,8 @@ int GetThreatSeverity(JavaThreatTypes threat_type) {
       return 3;
     case JAVA_THREAT_TYPE_BILLING:
       return 4;
+    case JAVA_THREAT_TYPE_CSD_ALLOWLIST:
+      return 5;
     case JAVA_THREAT_TYPE_MAX_VALUE:
       return std::numeric_limits<int>::max();
   }

components/safe_browsing/android/safe_browsing_api_handler_util.h

@@ -28,6 +28,9 @@ enum JavaThreatTypes {
   JAVA_THREAT_TYPE_SOCIAL_ENGINEERING = 5,
   JAVA_THREAT_TYPE_SUBRESOURCE_FILTER = 13,
   JAVA_THREAT_TYPE_BILLING = 15,
+  // TODO(crbug.com/999344): temp magic number, update once GMSCore is
+  // available.
+  JAVA_THREAT_TYPE_CSD_ALLOWLIST = 16,
   JAVA_THREAT_TYPE_MAX_VALUE
 };