Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add blocking algorithm #5370

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
339df05
Load and store embedded dataset
CrisBarreiro Dec 9, 2024
d378e0d
Reorganize classes
CrisBarreiro Dec 9, 2024
c53814c
Convert revision to String
CrisBarreiro Dec 13, 2024
8c2c899
Simplify DAO
CrisBarreiro Dec 17, 2024
7bbe3f7
Delete old filters and hashPrefixes
CrisBarreiro Dec 19, 2024
b621b10
Fix delete query
CrisBarreiro Dec 20, 2024
7228a02
Update datasets
CrisBarreiro Jan 9, 2025
d878181
Add blocking algorithm
CrisBarreiro Dec 10, 2024
2d401fd
Update logs
CrisBarreiro Dec 12, 2024
7b9b5b5
Do not check for URL concidence in shouldOverride
CrisBarreiro Dec 12, 2024
0d4e109
Fix pattern matching to accept "."
CrisBarreiro Dec 13, 2024
04a1590
Only send 4 characters to the matches endpoint
CrisBarreiro Dec 16, 2024
9f15d1b
Return safe if RC flag is disabled
CrisBarreiro Dec 17, 2024
ddd2323
Support duplicated hash filters
CrisBarreiro Dec 19, 2024
0be137f
Remove unused import
CrisBarreiro Dec 20, 2024
be51ab5
Remove embedded dataset and add worker to download data
CrisBarreiro Jan 14, 2025
4359526
Only block site async if is malicious
CrisBarreiro Jan 15, 2025
8df62f8
Ignore network callback result if a new URL us processed
CrisBarreiro Jan 15, 2025
8eac330
Test callback is ignored if belongs to different request
CrisBarreiro Jan 20, 2025
6ea8379
Test MaliciousSiteDao
CrisBarreiro Jan 20, 2025
4e3f233
Test RealMaliciousSiteRepository
CrisBarreiro Jan 20, 2025
39bdd4a
Test workers
CrisBarreiro Jan 20, 2025
6e4a289
Clear processed URLs after a page is marked as malicious
CrisBarreiro Jan 23, 2025
8770d6e
Address PR comments
CrisBarreiro Jan 29, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,6 @@ class BrowserWebViewClient @Inject constructor(

private var shouldOpenDuckPlayerInNewTab: Boolean = true

private val confirmationCallback: (isMalicious: Boolean) -> Unit = {
// TODO (cbarreiro): Handle site blocked asynchronously
}

init {
appCoroutineScope.launch {
duckPlayer.observeShouldOpenInNewTab().collect {
Expand Down Expand Up @@ -165,7 +161,7 @@ class BrowserWebViewClient @Inject constructor(
try {
Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
webViewClientListener?.onShouldOverride()
if (requestInterceptor.shouldOverrideUrlLoading(url, isForMainFrame)) {
if (requestInterceptor.shouldOverrideUrlLoading(webView, url, isForMainFrame)) {
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import com.duckduckgo.httpsupgrade.api.HttpsUpgrader
import com.duckduckgo.privacy.config.api.Gpc
import com.duckduckgo.request.filterer.api.RequestFilterer
import com.duckduckgo.user.agent.api.UserAgentProvider
import com.google.android.material.snackbar.Snackbar
import kotlinx.coroutines.withContext
import timber.log.Timber

Expand All @@ -62,6 +63,7 @@ interface RequestInterceptor {

@WorkerThread
fun shouldOverrideUrlLoading(
webView: WebView,
url: Uri,
isForMainFrame: Boolean,
): Boolean
Expand Down Expand Up @@ -105,10 +107,12 @@ class WebViewRequestInterceptor(
): WebResourceResponse? {
val url: Uri? = request.url

maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) {
handleSiteBlocked()
maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) { isMalicious ->
if (isMalicious) {
handleSiteBlocked(webView)
}
}?.let {
handleSiteBlocked()
handleSiteBlocked(webView)
return it
}

Expand Down Expand Up @@ -177,22 +181,24 @@ class WebViewRequestInterceptor(
return getWebResourceResponse(request, documentUrl, null)
}

override fun shouldOverrideUrlLoading(url: Uri, isForMainFrame: Boolean): Boolean {
override fun shouldOverrideUrlLoading(webView: WebView, url: Uri, isForMainFrame: Boolean): Boolean {
if (maliciousSiteBlockerWebViewIntegration.shouldOverrideUrlLoading(
url,
isForMainFrame,
) {
handleSiteBlocked()
) { isMalicious ->
if (isMalicious) {
handleSiteBlocked(webView)
}
}
) {
handleSiteBlocked()
handleSiteBlocked(webView)
return true
}
return false
}

private fun handleSiteBlocked() {
// TODO (cbarreiro): Handle site blocked
private fun handleSiteBlocked(webView: WebView) {
Snackbar.make(webView, "Site blocked", Snackbar.LENGTH_SHORT).show()
}

private fun getWebResourceResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.duckduckgo.privacy.config.api.PrivacyConfigCallbackPlugin
import com.squareup.anvil.annotations.ContributesBinding
import com.squareup.anvil.annotations.ContributesMultibinding
import java.net.URLDecoder
import java.util.concurrent.atomic.AtomicInteger
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
Expand Down Expand Up @@ -68,6 +69,7 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
val processedUrls = mutableListOf<String>()
private var isFeatureEnabled = false
private var currentCheckId = AtomicInteger(0)

init {
if (isMainProcess) {
Expand Down Expand Up @@ -109,16 +111,13 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(
return null
}

if (request.isForMainFrame) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) {
return WebResourceResponse(null, null, null)
}
processedUrls.add(decodedUrl)
} else if (isForIframe(request) && documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) {
val belongsToCurrentPage = documentUri?.host == request.requestHeaders["Referer"]?.toUri()?.host
if (request.isForMainFrame || (isForIframe(request) && belongsToCurrentPage)) {
if (checkMaliciousUrl(decodedUrl, confirmationCallback)) {
return WebResourceResponse(null, null, null)
} else {
processedUrls.add(decodedUrl)
}
processedUrls.add(decodedUrl)
}
return null
}
Expand All @@ -142,15 +141,33 @@ class RealMaliciousSiteBlockerWebViewIntegration @Inject constructor(

// iframes always go through the shouldIntercept method, so we only need to check the main frame here
if (isForMainFrame) {
if (maliciousSiteProtection.isMalicious(decodedUrl.toUri(), confirmationCallback) == MALICIOUS) {
if (checkMaliciousUrl(decodedUrl, confirmationCallback)) {
return@runBlocking true
} else {
processedUrls.add(decodedUrl)
}
processedUrls.add(decodedUrl)
}
false
}
}

private suspend fun checkMaliciousUrl(
url: String,
confirmationCallback: (isMalicious: Boolean) -> Unit,
): Boolean {
val checkId = currentCheckId.incrementAndGet()
return maliciousSiteProtection.isMalicious(url.toUri()) {
// if another load has started, we should ignore the result
val isMalicious = if (checkId == currentCheckId.get()) {
it
} else {
false
}
processedUrls.clear()
confirmationCallback(isMalicious)
} == MALICIOUS
}

private fun isForIframe(request: WebResourceRequest) = request.requestHeaders["Sec-Fetch-Dest"] == "iframe" ||
request.url.path?.contains("/embed/") == true ||
request.url.path?.contains("/iframe/") == true ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ package com.duckduckgo.app.browser.webview

import android.webkit.WebResourceRequest
import androidx.core.net.toUri
import androidx.test.core.app.ActivityScenario.launch
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
import com.duckduckgo.feature.toggles.api.Toggle.State
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult.MALICIOUS
import com.duckduckgo.malicioussiteprotection.api.MaliciousSiteProtection.IsMaliciousResult.WAIT_FOR_CONFIRMATION
import junit.framework.TestCase.assertEquals
import junit.framework.TestCase.assertTrue
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
Expand Down Expand Up @@ -129,13 +135,101 @@ class RealMaliciousSiteBlockerWebViewIntegrationTest {
assertFalse(result)
}

@Test
fun `shouldIntercept returns null when feature is enabled, is malicious, and is mainframe but webView has different host`() = runTest {
whenever(maliciousSiteProtection.isMalicious(any(), any())).thenReturn(MALICIOUS)
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(false)

val result = testee.shouldIntercept(request, exampleUri) {}
assertNull(result)
}

@Test
fun `onPageLoadStarted clears processedUrls`() = runTest {
testee.processedUrls.add(exampleUri.toString())
testee.onPageLoadStarted()
assertTrue(testee.processedUrls.isEmpty())
}

@Test
fun `if a new page load triggering is malicious is started, isMalicious callback result should be ignored for the first page`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(true)

val callbackChannel = Channel<Unit>()
val firstCallbackDeferred = CompletableDeferred<Boolean>()
val secondCallbackDeferred = CompletableDeferred<Boolean>()

whenever(maliciousSiteProtection.isMalicious(any(), any())).thenAnswer { invocation ->
val callback = invocation.getArgument<(Boolean) -> Unit>(1)

launch {
callbackChannel.receive()
callback(true)
}
WAIT_FOR_CONFIRMATION
}

testee.shouldOverrideUrlLoading(maliciousUri, true) { isMalicious ->
firstCallbackDeferred.complete(isMalicious)
}

testee.shouldOverrideUrlLoading(exampleUri, true) { isMalicious ->
secondCallbackDeferred.complete(isMalicious)
}

callbackChannel.send(Unit)
callbackChannel.send(Unit)

val firstCallbackResult = firstCallbackDeferred.await()
val secondCallbackResult = secondCallbackDeferred.await()

assertEquals(false, firstCallbackResult)
assertEquals(true, secondCallbackResult)
}

@Test
fun `isMalicious callback result should be processed if no new page loads triggering isMalicious have started`() = runTest {
val request = mock(WebResourceRequest::class.java)
whenever(request.url).thenReturn(maliciousUri)
whenever(request.isForMainFrame).thenReturn(true)

val callbackChannel = Channel<Unit>()
val firstCallbackDeferred = CompletableDeferred<Boolean>()
val secondCallbackDeferred = CompletableDeferred<Boolean>()

whenever(maliciousSiteProtection.isMalicious(any(), any())).thenAnswer { invocation ->
val callback = invocation.getArgument<(Boolean) -> Unit>(1)

launch {
callbackChannel.receive()
callback(true)
}
WAIT_FOR_CONFIRMATION
}

testee.shouldOverrideUrlLoading(maliciousUri, true) { isMalicious ->
firstCallbackDeferred.complete(isMalicious)
}

callbackChannel.send(Unit)

testee.shouldOverrideUrlLoading(exampleUri, true) { isMalicious ->
secondCallbackDeferred.complete(isMalicious)
}

callbackChannel.send(Unit)

val firstCallbackResult = firstCallbackDeferred.await()
val secondCallbackResult = secondCallbackDeferred.await()

assertEquals(true, firstCallbackResult)
assertEquals(true, secondCallbackResult)
}

private fun updateFeatureEnabled(enabled: Boolean) {
fakeAndroidBrowserConfigFeature.enableMaliciousSiteProtection().setRawStoredState(State(enabled))
testee.onPrivacyConfigDownloaded()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ dependencies {
implementation project(path: ':anvil-annotations')
implementation project(path: ':di')
ksp AndroidX.room.compiler
implementation AndroidX.room.runtime
implementation AndroidX.room.ktx

implementation KotlinX.coroutines.android
implementation AndroidX.core.ktx
implementation AndroidX.work.runtimeKtx
implementation Google.dagger

implementation project(path: ':common-utils')
Expand All @@ -43,6 +46,7 @@ dependencies {
implementation Google.android.material

testImplementation AndroidX.test.ext.junit
testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.5.2'
testImplementation Testing.junit4
testImplementation "org.mockito.kotlin:mockito-kotlin:_"
testImplementation project(path: ':common-test')
Expand All @@ -54,6 +58,7 @@ dependencies {
// conflicts with mockito due to direct inclusion of byte buddy
exclude group: "org.jetbrains.kotlinx", module: "kotlinx-coroutines-debug"
}
testImplementation AndroidX.work.testing

coreLibraryDesugaring Android.tools.desugarJdkLibs
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.malicioussiteprotection.impl

import android.content.Context
import androidx.room.Room
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.malicioussiteprotection.impl.data.db.MaliciousSiteDao
import com.duckduckgo.malicioussiteprotection.impl.data.db.MaliciousSitesDatabase
import com.duckduckgo.malicioussiteprotection.impl.data.db.MaliciousSitesDatabase.Companion.ALL_MIGRATIONS
import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import dagger.SingleInstanceIn
import java.security.MessageDigest

@Module
@ContributesTo(AppScope::class)
class MaliciousSiteModule {

@Provides
@SingleInstanceIn(AppScope::class)
fun provideMaliciousSiteProtectionDatabase(context: Context): MaliciousSitesDatabase {
return Room.databaseBuilder(context, MaliciousSitesDatabase::class.java, "malicious_sites.db")
.addMigrations(*ALL_MIGRATIONS)
.fallbackToDestructiveMigration()
.build()
}

@Provides
@SingleInstanceIn(AppScope::class)
fun provideMaliciousSiteDao(database: MaliciousSitesDatabase): MaliciousSiteDao {
return database.maliciousSiteDao()
}

@Provides
@SingleInstanceIn(AppScope::class)
fun provideMessageDigest(): MessageDigest {
return MessageDigest.getInstance("SHA-256")
}
}
Loading
Loading