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

Display warning page for malicious sites #5416

Draft
wants to merge 28 commits into
base: feature/cris/malicious-site-protection/load-initial-dataset
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5d3fbbd
Intercept requests and add skeleton for malicious site detection
CrisBarreiro Dec 9, 2024
d3d228d
Check if request belongs to current page
CrisBarreiro Dec 9, 2024
6d081c6
Add tests
CrisBarreiro Dec 9, 2024
d64cb6f
Suppress DenyListedApi
CrisBarreiro Dec 9, 2024
ad2fe20
Make maliciousSiteProtection webView agnostic
CrisBarreiro Dec 10, 2024
58fce0d
Don't expose RC flag in MaliciousSiteProtection
CrisBarreiro Dec 10, 2024
030f7fa
Do not make MaliciousSiteBlockerWebViewIntegration public
CrisBarreiro Dec 10, 2024
7c55222
Modify MaliciousSiteProtection API
CrisBarreiro Dec 11, 2024
876a1a5
Do not compare webview URL for mainframe requests
CrisBarreiro Dec 16, 2024
f958840
Address PR comments
CrisBarreiro Dec 16, 2024
49534e9
Extract RC caching to a different class
CrisBarreiro Dec 17, 2024
bad45a7
Extract part of shouldOverrideUrlLoading to RequestInterceptor
CrisBarreiro Dec 19, 2024
20a8af2
Load and store embedded dataset
CrisBarreiro Dec 9, 2024
dcc6545
Reorganize classes
CrisBarreiro Dec 9, 2024
8950cd5
Convert revision to String
CrisBarreiro Dec 13, 2024
d925f34
Simplify DAO
CrisBarreiro Dec 17, 2024
d6c864d
Delete old filters and hashPrefixes
CrisBarreiro Dec 19, 2024
1f7ccf9
Fix delete query
CrisBarreiro Dec 20, 2024
bec30ce
Add blocking algorithm
CrisBarreiro Dec 10, 2024
6d382c3
Update logs
CrisBarreiro Dec 12, 2024
ad4cca2
Do not check for URL concidence in shouldOverride
CrisBarreiro Dec 12, 2024
cc07647
Fix pattern matching to accept "."
CrisBarreiro Dec 13, 2024
e8b886c
Only send 4 characters to the matches endpoint
CrisBarreiro Dec 16, 2024
f072051
Return safe if RC flag is disabled
CrisBarreiro Dec 17, 2024
67e60b4
Do not prettify JSON files
CrisBarreiro Dec 19, 2024
75f7e5d
Support duplicated hash filters
CrisBarreiro Dec 19, 2024
8cb9ae8
Remove unused import
CrisBarreiro Dec 20, 2024
97e340a
Display warning page for malicious sites
CrisBarreiro Dec 19, 2024
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 @@ -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 @@ -583,6 +583,9 @@ class BrowserTabFragment :
private val errorView
get() = binding.includeErrorView

private val warningView
get() = binding.maliciousSiteWarningLayout

private val sslErrorView
get() = binding.sslErrorWarningLayout

Expand Down Expand Up @@ -1371,6 +1374,26 @@ class BrowserTabFragment :
errorView.errorLayout.show()
}

private fun showWarning(url: Uri) {
webViewContainer.gone()
newBrowserTab.newTabLayout.gone()
newBrowserTab.newTabContainerLayout.gone()
sslErrorView.gone()
// TODO (cbarreiro) we should probably define a new view mode
omnibar.setViewMode(Omnibar.ViewMode.Error)
webView?.onPause()
webView?.hide()
warningView.bind(/*handler, errorResponse*/) /*{ action ->
viewModel.onSSLCertificateWarningAction(action, errorResponse.url)
}*/
warningView.show()
// warningView.leaveSiteButton.setOnClickListener {
// viewModel.closeCurrentTab()
// // TODO (cbarreiro) Fix, not working
// renderer.showNewTab()
// }
}

private fun showSSLWarning(
handler: SslErrorHandler,
errorResponse: SslErrorResponse,
Expand Down Expand Up @@ -1711,6 +1734,7 @@ class BrowserTabFragment :
)

is Command.WebViewError -> showError(it.errorType, it.url)
is Command.WebViewWarningMaliciousSite -> showWarning(it.url)
is Command.SendResponseToJs -> contentScopeScripts.onResponse(it.data)
is Command.SendResponseToDuckPlayer -> duckPlayerScripts.onResponse(it.data)
is Command.WebShareRequest -> webShareRequest.launch(it.data)
Expand Down Expand Up @@ -4006,7 +4030,7 @@ class BrowserTabFragment :
viewModel.onCtaShown()
}

private fun showNewTab() {
fun showNewTab() {
newTabPageProvider.provideNewTabPageVersion().onEach { newTabPage ->
if (newBrowserTab.newTabContainerLayout.childCount == 0) {
newBrowserTab.newTabContainerLayout.addView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ import com.duckduckgo.app.browser.commands.Command.ShowWebPageTitle
import com.duckduckgo.app.browser.commands.Command.ToggleReportFeedback
import com.duckduckgo.app.browser.commands.Command.WebShareRequest
import com.duckduckgo.app.browser.commands.Command.WebViewError
import com.duckduckgo.app.browser.commands.Command.WebViewWarningMaliciousSite
import com.duckduckgo.app.browser.commands.NavigationCommand
import com.duckduckgo.app.browser.customtabs.CustomTabPixelNames
import com.duckduckgo.app.browser.duckplayer.DUCK_PLAYER_FEATURE_NAME
Expand Down Expand Up @@ -3187,6 +3188,11 @@ class BrowserTabViewModel @Inject constructor(
command.postValue(WebViewError(errorType, url))
}

override fun onReceivedMaliciousSiteWarning(url: Uri) {
// TODO (cbarreiro): Fire pixel
command.postValue(WebViewWarningMaliciousSite(url))
}

override fun recordErrorCode(
error: String,
url: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ class BrowserWebViewClient @Inject constructor(
try {
Timber.v("shouldOverride webViewUrl: ${webView.url} URL: $url")
webViewClientListener?.onShouldOverride()
if (requestInterceptor.shouldOverrideUrlLoading(webViewClientListener, url, isForMainFrame)) {
return true
}

if (isForMainFrame && dosDetector.isUrlGeneratingDos(url)) {
webView.loadUrl(ABOUT_BLANK)
webViewClientListener?.dosAttackDetected()
Expand Down Expand Up @@ -407,11 +411,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)
}
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 +513,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 @@ -95,6 +95,7 @@ interface WebViewClientListener {
fun linkOpenedInNewTab(): Boolean
fun isActiveTab(): Boolean
fun onReceivedError(errorType: WebViewErrorResponse, url: String)
fun onReceivedMaliciousSiteWarning(url: Uri)
fun recordErrorCode(error: String, url: String)
fun recordHttpErrorCode(statusCode: Int, url: String)

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,13 @@ interface RequestInterceptor {
): WebResourceResponse?

fun onPageStarted(url: String)

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

class WebViewRequestInterceptor(
Expand All @@ -71,11 +79,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 +106,13 @@ class WebViewRequestInterceptor(
): WebResourceResponse? {
val url: Uri? = request.url

maliciousSiteBlockerWebViewIntegration.shouldIntercept(request, documentUri) {
handleSiteBlocked(webViewClientListener, url)
}?.let {
handleSiteBlocked(webViewClientListener, url)
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 +178,24 @@ class WebViewRequestInterceptor(
return getWebResourceResponse(request, documentUrl, null)
}

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

private fun handleSiteBlocked(webViewClientListener: WebViewClientListener?, url: Uri?) {
url?.let { webViewClientListener?.onReceivedMaliciousSiteWarning(it) }
}

private fun getWebResourceResponse(
request: WebResourceRequest,
documentUrl: Uri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ sealed class Command {
val url: String,
) : Command()

data class WebViewWarningMaliciousSite(
val url: Uri,
) : Command()

// TODO (cbarreiro) Rename to SendResponseToCSS
data class SendResponseToJs(val data: JsCallbackData) : Command()
data class SendResponseToDuckPlayer(val data: JsCallbackData) : Command()
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 @@ -213,6 +214,7 @@ class BrowserModule {
cloakedCnameDetector: CloakedCnameDetector,
requestFilterer: RequestFilterer,
duckPlayer: DuckPlayer,
maliciousSiteBlockerWebViewIntegration: MaliciousSiteBlockerWebViewIntegration,
): RequestInterceptor =
WebViewRequestInterceptor(
resourceSurrogates,
Expand All @@ -225,6 +227,7 @@ class BrowserModule {
cloakedCnameDetector,
requestFilterer,
duckPlayer,
maliciousSiteBlockerWebViewIntegration,
)

@Provides
Expand Down
Loading
Loading