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

Intercept requests and add skeleton for malicious site detection #5369

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import com.duckduckgo.app.browser.pageloadpixel.firstpaint.PagePaintedHandler
import com.duckduckgo.app.browser.print.PrintInjector
import com.duckduckgo.app.browser.trafficquality.AndroidFeaturesHeaderPlugin
import com.duckduckgo.app.browser.uriloaded.UriLoadedManager
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.pixels.remoteconfig.AndroidBrowserConfigFeature
import com.duckduckgo.app.statistics.pixels.Pixel
Expand Down Expand Up @@ -152,10 +153,11 @@ class BrowserWebViewClientTest {
private val mockUriLoadedManager: UriLoadedManager = mock()
private val mockAndroidBrowserConfigFeature: AndroidBrowserConfigFeature = mock()
private val mockAndroidFeaturesHeaderPlugin = AndroidFeaturesHeaderPlugin(mockDuckDuckGoUrlDetector, mockAndroidBrowserConfigFeature, mock())
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()

@UiThreadTest
@Before
fun setup() {
fun setup() = runTest {
webView = TestWebView(context)
whenever(mockDuckPlayer.observeShouldOpenInNewTab()).thenReturn(openInNewTabFlow)
testee = BrowserWebViewClient(
Expand Down Expand Up @@ -196,6 +198,7 @@ class BrowserWebViewClientTest {
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0)
whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1")
whenever(deviceInfo.appVersion).thenReturn("1")
whenever(mockMaliciousSiteProtection.shouldOverrideUrlLoading(any(), any(), any())).thenReturn(false)
}

@UiThreadTest
Expand Down Expand Up @@ -315,8 +318,8 @@ class BrowserWebViewClientTest {
@Test
fun whenOnReceivedHttpAuthRequestThenListenerNotified() {
val mockHandler = mock<HttpAuthHandler>()
val authenticationRequest = BasicAuthenticationRequest(mockHandler, EXAMPLE_URL, EXAMPLE_URL, EXAMPLE_URL)
testee.onReceivedHttpAuthRequest(webView, mockHandler, EXAMPLE_URL, EXAMPLE_URL)
val authenticationRequest = BasicAuthenticationRequest(mockHandler, "example.com", EXAMPLE_URL, EXAMPLE_URL)
testee.onReceivedHttpAuthRequest(webView, mockHandler, "example.com", EXAMPLE_URL)
verify(listener).requiresAuthentication(authenticationRequest)
}

Expand Down Expand Up @@ -765,6 +768,7 @@ class BrowserWebViewClientTest {
private fun getImmediatelyInvokedMockWebView(): WebView {
val mockWebView = mock<WebView>()
whenever(mockWebView.originalUrl).thenReturn(EXAMPLE_URL)
whenever(mockWebView.url).thenReturn(EXAMPLE_URL)
whenever(mockWebView.post(any())).thenAnswer { invocation ->
invocation.getArgument(0, Runnable::class.java).run()
null
Expand Down Expand Up @@ -1071,6 +1075,10 @@ class BrowserWebViewClientTest {
override fun getOriginalUrl(): String {
return EXAMPLE_URL
}

override fun getUrl(): String {
return EXAMPLE_URL
}
}

private class FakePluginPoint : PluginPoint<JsInjectorPlugin> {
Expand Down Expand Up @@ -1149,6 +1157,6 @@ class BrowserWebViewClientTest {
}

companion object {
const val EXAMPLE_URL = "example.com"
const val EXAMPLE_URL = "https://example.com"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import androidx.core.net.toUri
import androidx.test.annotation.UiThreadTest
import com.duckduckgo.adclick.api.AdClickManager
import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.fakes.FeatureToggleFake
import com.duckduckgo.app.fakes.UserAgentFake
import com.duckduckgo.app.fakes.UserAllowListRepositoryFake
Expand Down Expand Up @@ -97,6 +98,7 @@ class WebViewRequestInterceptorTest {
fakeToggle,
fakeUserAllowListRepository,
)
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()

private var webView: WebView = mock()

Expand All @@ -117,6 +119,7 @@ class WebViewRequestInterceptorTest {
cloakedCnameDetector = mockCloakedCnameDetector,
requestFilterer = mockRequestFilterer,
duckPlayer = mockDuckPlayer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import com.duckduckgo.app.browser.*
import com.duckduckgo.app.browser.certificates.rootstore.TrustedCertificateStore
import com.duckduckgo.app.browser.cookies.ThirdPartyCookieManager
import com.duckduckgo.app.browser.httpauth.WebViewHttpAuthStore
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.cookies.api.CookieManagerProvider
import kotlinx.coroutines.test.TestScope
Expand All @@ -50,6 +51,7 @@ class UrlExtractingWebViewClientTest {
private val thirdPartyCookieManager: ThirdPartyCookieManager = mock()
private val urlExtractor: DOMUrlExtractor = mock()
private val mockWebView: WebView = mock()
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()

@UiThreadTest
@Before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.adclick.api.AdClickManager
import com.duckduckgo.app.browser.WebViewRequestInterceptor
import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.fakes.FeatureToggleFake
import com.duckduckgo.app.fakes.UserAgentFake
import com.duckduckgo.app.fakes.UserAllowListRepositoryFake
Expand Down Expand Up @@ -119,6 +120,7 @@ class DomainsReferenceTest(private val testCase: TestCase) {
)
private val mockGpc: Gpc = mock()
private val mockAdClickManager: AdClickManager = mock()
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()

companion object {
private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build()
Expand Down Expand Up @@ -174,6 +176,7 @@ class DomainsReferenceTest(private val testCase: TestCase) {
adClickManager = mockAdClickManager,
cloakedCnameDetector = CloakedCnameDetectorImpl(tdsCnameEntityDao, mockTrackerAllowlist, mockUserAllowListRepository),
requestFilterer = mockRequestFilterer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
duckPlayer = mockDuckPlayer,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.adclick.api.AdClickManager
import com.duckduckgo.app.browser.WebViewRequestInterceptor
import com.duckduckgo.app.browser.useragent.provideUserAgentOverridePluginPoint
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.fakes.FeatureToggleFake
import com.duckduckgo.app.fakes.UserAgentFake
import com.duckduckgo.app.fakes.UserAllowListRepositoryFake
Expand Down Expand Up @@ -115,6 +116,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) {
private val mockGpc: Gpc = mock()
private val mockAdClickManager: AdClickManager = mock()
private val mockCloakedCnameDetector: CloakedCnameDetector = mock()
private val mockMaliciousSiteProtection: MaliciousSiteBlockerWebViewIntegration = mock()

companion object {
private val moshi = Moshi.Builder().add(ActionJsonAdapter()).build()
Expand Down Expand Up @@ -170,6 +172,7 @@ class SurrogatesReferenceTest(private val testCase: TestCase) {
adClickManager = mockAdClickManager,
cloakedCnameDetector = mockCloakedCnameDetector,
requestFilterer = mockRequestFilterer,
maliciousSiteBlockerWebViewIntegration = mockMaliciousSiteProtection,
duckPlayer = mockDuckPlayer,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ 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 @@ -158,6 +162,10 @@ class BrowserWebViewClient @Inject constructor(
try {
Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
webViewClientListener?.onShouldOverride()
if (requestInterceptor.shouldOverrideUrlLoading(url, isForMainFrame)) {
return true
}

if (isForMainFrame && dosDetector.isUrlGeneratingDos(url)) {
webView.loadUrl(ABOUT_BLANK)
webViewClientListener?.dosAttackDetected()
Expand Down Expand Up @@ -407,11 +415,11 @@ class BrowserWebViewClient @Inject constructor(
// See https://app.asana.com/0/0/1206159443951489/f (WebView limitations)
if (it != ABOUT_BLANK && start == null) {
start = currentTimeProvider.elapsedRealtime()
requestInterceptor.onPageStarted(url)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check if moving requestInterceptor.onPageStarted(url) here has any unintended side-effect. This is because onPageStarted can be called several times when a page is loaded

}
handleMediaPlayback(webView, it)
autoconsent.injectAutoconsent(webView, url)
adClickManager.detectAdDomain(url)
requestInterceptor.onPageStarted(url)
appCoroutineScope.launch(dispatcherProvider.io()) {
thirdPartyCookieManager.processUriForThirdPartyCookies(webView, url.toUri())
}
Expand Down Expand Up @@ -509,7 +517,12 @@ class BrowserWebViewClient @Inject constructor(
loginDetector.onEvent(WebNavigationEvent.ShouldInterceptRequest(webView, request))
}
Timber.v("Intercepting resource ${request.url} type:${request.method} on page $documentUrl")
requestInterceptor.shouldIntercept(request, webView, documentUrl?.toUri(), webViewClientListener)
requestInterceptor.shouldIntercept(
request,
webView,
documentUrl?.toUri(),
webViewClientListener,
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import android.webkit.WebResourceResponse
import android.webkit.WebView
import androidx.annotation.WorkerThread
import com.duckduckgo.adclick.api.AdClickManager
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.privacy.db.PrivacyProtectionCountDao
import com.duckduckgo.app.privacy.model.TrustedSites
import com.duckduckgo.app.surrogates.ResourceSurrogates
Expand Down Expand Up @@ -58,6 +59,12 @@ interface RequestInterceptor {
): WebResourceResponse?

fun onPageStarted(url: String)

@WorkerThread
fun shouldOverrideUrlLoading(
url: Uri,
isForMainFrame: Boolean,
): Boolean
}

class WebViewRequestInterceptor(
Expand All @@ -71,11 +78,13 @@ class WebViewRequestInterceptor(
private val cloakedCnameDetector: CloakedCnameDetector,
private val requestFilterer: RequestFilterer,
private val duckPlayer: DuckPlayer,
private val maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
private val dispatchers: DispatcherProvider = DefaultDispatcherProvider(),
) : RequestInterceptor {

override fun onPageStarted(url: String) {
requestFilterer.registerOnPageCreated(url)
maliciousSiteBlockerWebViewIntegration.onPageLoadStarted()
}

/**
Expand All @@ -96,6 +105,13 @@ class WebViewRequestInterceptor(
): WebResourceResponse? {
val url: Uri? = request.url

maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) {
handleSiteBlocked()
}?.let {
handleSiteBlocked()
return it
}

if (requestFilterer.shouldFilterOutRequest(request, documentUri.toString())) return WebResourceResponse(null, null, null)

adClickManager.detectAdClick(url?.toString(), request.isForMainFrame)
Expand Down Expand Up @@ -161,6 +177,24 @@ class WebViewRequestInterceptor(
return getWebResourceResponse(request, documentUrl, null)
}

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

private fun handleSiteBlocked() {
// TODO (cbarreiro): Handle site blocked
}

private fun getWebResourceResponse(
request: WebResourceRequest,
documentUrl: Uri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister
import com.duckduckgo.app.browser.urlextraction.DOMUrlExtractor
import com.duckduckgo.app.browser.urlextraction.JsUrlExtractor
import com.duckduckgo.app.browser.urlextraction.UrlExtractingWebViewClient
import com.duckduckgo.app.browser.webview.MaliciousSiteBlockerWebViewIntegration
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.app.fire.*
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository
Expand Down Expand Up @@ -201,6 +202,7 @@ class BrowserModule {
cloakedCnameDetector: CloakedCnameDetector,
requestFilterer: RequestFilterer,
duckPlayer: DuckPlayer,
maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
): RequestInterceptor =
WebViewRequestInterceptor(
resourceSurrogates,
Expand All @@ -213,6 +215,7 @@ class BrowserModule {
cloakedCnameDetector,
requestFilterer,
duckPlayer,
maliciousSiteBlockerWebViewIntegration,
)

@Provides
Expand Down
Loading
Loading