Skip to content

Commit

Permalink
[Android] Select the highest priority threat from SafetyNetClient.loo…
Browse files Browse the repository at this point in the history
…kupUri response

fixes brave/brave-browser#41581
  • Loading branch information
AlexeyBarabash committed Oct 24, 2024
1 parent e2ded45 commit 6e7daf6
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/* Copyright (c) 2023 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

package org.chromium.chrome.browser;

import androidx.test.filters.SmallTest;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.test.BaseJUnit4ClassRunner;
import org.chromium.base.test.util.Batch;
import org.chromium.components.safe_browsing.BraveSafeBrowsingUtils;
import org.chromium.components.safe_browsing.BraveSafeBrowsingUtils.SafetyNetJavaThreatType;

import java.util.ArrayList;
import java.util.Arrays;

@Batch(Batch.PER_CLASS)
@RunWith(BaseJUnit4ClassRunner.class)
public class BraveSafetyNetThreatsPrioritiesTest {

@Test
@SmallTest
public void testPriorities() throws Exception {
// Fail safe option for empty input, consider it is safe
Assert.assertEquals(
BraveSafeBrowsingUtils.getMostPriorityThreat(new ArrayList<>()),
SafetyNetJavaThreatType.MAX_VALUE);

// Single input must return the same value
Assert.assertEquals(
BraveSafeBrowsingUtils.getMostPriorityThreat(
Arrays.asList(new Integer[] {SafetyNetJavaThreatType.SOCIAL_ENGINEERING})),
SafetyNetJavaThreatType.SOCIAL_ENGINEERING);

// Several threats as input - return the most priority one
Assert.assertEquals(
BraveSafeBrowsingUtils.getMostPriorityThreat(
Arrays.asList(
new Integer[] {
SafetyNetJavaThreatType.UNWANTED_SOFTWARE,
SafetyNetJavaThreatType.SUBRESOURCE_FILTER,
SafetyNetJavaThreatType.SOCIAL_ENGINEERING,
SafetyNetJavaThreatType.BILLING,
SafetyNetJavaThreatType.POTENTIALLY_HARMFUL_APPLICATION,
})),
SafetyNetJavaThreatType.SOCIAL_ENGINEERING);

Assert.assertEquals(
BraveSafeBrowsingUtils.getMostPriorityThreat(
Arrays.asList(
new Integer[] {
SafetyNetJavaThreatType.SUBRESOURCE_FILTER,
SafetyNetJavaThreatType.CSD_ALLOWLIST,
SafetyNetJavaThreatType.POTENTIALLY_HARMFUL_APPLICATION,
SafetyNetJavaThreatType.UNWANTED_SOFTWARE,
})),
SafetyNetJavaThreatType.POTENTIALLY_HARMFUL_APPLICATION);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

import com.google.android.gms.common.api.ApiException;
import com.google.android.gms.common.api.CommonStatusCodes;
import com.google.android.gms.safetynet.SafeBrowsingThreat;
import com.google.android.gms.safetynet.SafetyNet;
import com.google.android.gms.safetynet.SafetyNetApi.SafeBrowsingResponse;
import com.google.android.gms.safetynet.SafetyNetStatusCodes;

import org.chromium.base.ContextUtils;
Expand All @@ -21,6 +21,9 @@
import org.chromium.components.safe_browsing.BraveSafeBrowsingUtils.SafeBrowsingJavaThreatType;
import org.chromium.components.safe_browsing.SafeBrowsingApiHandler.LookupResult;

import java.util.List;
import java.util.stream.Collectors;

/**
* Brave implementation of SafeBrowsingApiHandler for Safe Browsing Under the bonnet it still uses
* SafetyNet.
Expand Down Expand Up @@ -120,18 +123,18 @@ public void startUriLookup(long callbackId, String uri, int[] threatTypes, int p
DEFAULT_CHECK_DELTA);
return;
} else {
warnWhenMoreThanOneThreat(sbResponse);
List<Integer> threats =
sbResponse.getDetectedThreats().stream()
.map(SafeBrowsingThreat::getThreatType)
.collect(Collectors.toList());

// Response only with the first code
mObserver.onUrlCheckDone(
callbackId,
LookupResult.SUCCESS,
BraveSafeBrowsingUtils
.safetyNetToSafeBrowsingJavaThreatType(
sbResponse
.getDetectedThreats()
.get(0)
.getThreatType()),
BraveSafeBrowsingUtils
.getMostPriorityThreat(threats)),
THREAT_ATTRIBUTES_STUB,
SafeBrowsingJavaResponseStatus.SUCCESS_WITH_REAL_TIME,
DEFAULT_CHECK_DELTA);
Expand Down Expand Up @@ -201,18 +204,6 @@ public void startUriLookup(long callbackId, String uri, int[] threatTypes, int p
});
}

private void warnWhenMoreThanOneThreat(SafeBrowsingResponse sbResponse) {
if (sbResponse.getDetectedThreats().size() != 1) {
Log.d(TAG, "Unexpected threats count: " + sbResponse.getDetectedThreats().size());
String threats = "";
for (int i = 0; i < sbResponse.getDetectedThreats().size(); i++) {
threats += sbResponse.getDetectedThreats().get(i).getThreatType();
threats += " ";
}
Log.d(TAG, "Threats: [" + threats + "]");
}
}

public void initSafeBrowsing() {
SafetyNet.getClient(ContextUtils.getApplicationContext()).initSafeBrowsing();
mInitialized = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.chromium.components.safe_browsing;

import androidx.annotation.IntDef;
import androidx.annotation.VisibleForTesting;

import org.chromium.base.Log;

Expand Down Expand Up @@ -139,4 +140,35 @@ public static int safetyNetToSafeBrowsingJavaThreatType(int safetyNetThreatType)
return SafeBrowsingJavaThreatType.NO_THREAT;
}
}

// Priorities from most high to low
private static final int SAFETY_NET_THREAT_PRIORITIES[] = {
SafetyNetJavaThreatType.SOCIAL_ENGINEERING,
SafetyNetJavaThreatType.BILLING,
SafetyNetJavaThreatType.POTENTIALLY_HARMFUL_APPLICATION,
SafetyNetJavaThreatType.UNWANTED_SOFTWARE,
SafetyNetJavaThreatType.SUBRESOURCE_FILTER,
SafetyNetJavaThreatType.CSD_ALLOWLIST,
SafetyNetJavaThreatType.MAX_VALUE
};

@VisibleForTesting
public static int getMostPriorityThreat(List<Integer> threats) {
if (threats.size() == 0) {
return SafetyNetJavaThreatType.MAX_VALUE;
}

if (threats.size() == 1) {
return threats.get(0);
}

for (int i = 0; i < SAFETY_NET_THREAT_PRIORITIES.length; ++i) {
if (threats.contains(SAFETY_NET_THREAT_PRIORITIES[i])) {
return SAFETY_NET_THREAT_PRIORITIES[i];
}
}

assert false : "Unexpected threat";
return SafetyNetJavaThreatType.MAX_VALUE;
}
}
2 changes: 2 additions & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,7 @@ if (is_android) {
}

sources = [
"//brave/android/javatests/org/chromium/chrome/browser/BraveSafetyNetThreatsPrioritiesTest.java",
"//brave/android/javatests/org/chromium/chrome/browser/BraveSwipeRefreshHandlerTest.java",
"//brave/android/javatests/org/chromium/chrome/browser/BytecodeTest.java",
"//brave/android/javatests/org/chromium/chrome/browser/brave_wallet/BraveWalletUtilsTest.java",
Expand All @@ -1277,6 +1278,7 @@ if (is_android) {
"//base:base_java_test_support",
"//base:base_shared_preferences_java",
"//brave/components/brave_wallet/common:mojom_java",
"//brave/components/safe_browsing/android:brave_safe_browsing_java",
"//cc:cc_java",
"//chrome/android/features/keyboard_accessory/public:public_java",
"//chrome/browser/android/browserservices/intents:java",
Expand Down

0 comments on commit 6e7daf6

Please sign in to comment.