Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Malicious site protection navigation detection integration #3730

Closed
wants to merge 19 commits into from
Closed

Malicious site protection navigation detection integration #3730

wants to merge 19 commits into from

Conversation

alessandroboron
Copy link
Contributor

@alessandroboron alessandroboron commented Dec 16, 2024 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/72649045549333/1208758247571054/f
CC: @not-a-rootkit

Description:

Integrate BSK library for malicious threat detection.

Steps to test this PR:

Prerequisites: Return true in MaliciousSiteProtectionFeatureFlags.swift -> isMaliciousSiteProtectionEnabled and shouldDetectMaliciousThreat(forDomain domain: String?) -> Bool

Scenario 1 - Phishing

  1. Navigate to http://privacy-test-pages.site/security/badware/phishing.html
  2. Ensure special error pages is shown.

Scenario 2 - Malware

  1. Navigate to http://privacy-test-pages.site/security/badware/malware.html
  2. Ensure special error pages is shown.

Scenario 3 - Leave Site Creates an Empty Tab at same index of the tab closed

  1. Open multiple tabs and load some random websites
  2. Open a tab and load a malicious website.
  3. When the special error page is shown, tap the “Leave Site” Button.
  4. Ensure that when the Tab is closed a new empty one is created at the same index.

Definition of Done (Internal Only):

Copy Testing:

  • Use of correct apostrophes in new copy, ie ’ rather than '

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 15
  • iOS 16
  • iOS 17

Theme Testing:

  • Light theme
  • Dark theme
Internal references:

Software Engineering Expectations
Technical Design Template

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 16, 2024 •
edited
Loading

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 91622ac

@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-navigation-detection-integration branch 4 times, most recently from 2096a74 to 0838920 Compare December 16, 2024 16:07
alessandroboron
alessandroboron commented Dec 16, 2024
View reviewed changes
DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift
import Foundation
import MaliciousSiteProtection
import Networking
import PixelKit

final class MaliciousSiteProtectionManager: MaliciousSiteDetecting {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implementation from macOS https://github.com/duckduckgo/macos-browser/blob/main/DuckDuckGo/MaliciousSiteProtection/MaliciousSiteProtectionManager.swift

@alessandroboron alessandroboron changed the title Alessandro/malicious site protection navigation detection integration Malicious site protection navigation detection integration Dec 16, 2024
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from a41d843 to 99e6543 Compare December 19, 2024 12:48
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-navigation-detection-integration branch from ed4e32b to b5fe830 Compare December 19, 2024 12:49
@github-actions GitHub Actions
Copy link

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

@github-actions github-actions bot added the stale label Dec 27, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 4, 2025

This PR has been closed after 14 days of inactivity. Feel free to reopen it if you plan to continue working on it or have further discussions.

@github-actions github-actions bot closed this Jan 4, 2025
alessandroboron and others added 2 commits January 9, 2025 12:14
@alessandroboron
Malicious site protection - extract special error page navigation log…
21fac20 
…ic (#3624)

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208836682251611/f
Tech Design URL: https://app.asana.com/0/1206329551987282/1207273224076495/f

**Description**:
This PR extracts the navigation logic for special error pages in its own class.
This PR focuses on creating the the entites used to encapsulate the logic for SSL and Malicious site protection
@alessandroboron
Remove duplicated mock after merging
b60a018
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from 99e6543 to a76c78f Compare January 9, 2025 02:42
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-navigation-detection-integration branch from b5fe830 to 29703ba Compare January 9, 2025 02:49
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from a76c78f to 560ef30 Compare January 9, 2025 04:29
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-navigation-detection-integration branch 3 times, most recently from 966f6cc to 4d87dab Compare January 10, 2025 07:25
@alessandroboron
Malicious site protection navigation detection (#3707)
b6373fc 
Task/Issue URL: https://app.asana.com/0/1206329551987282/1207151848931030
Tech Design URL: https://app.asana.com/0/1206329551987282/1207273224076495/f

**Description**:
This PR adds the navigation logic for detecting a malicious site and navigating to a special error page if the site is malicious.
The original idea in the tech design was to intercept the Request in `decidePolicyForNavigationAction` and check whether a site is malicious cancelling the request accordingly.
We noticed that the above approach increases the page load time of websites due to the logic check.
I opted for an approach where in `decidePolicyForNavigationAction` we start the detection task in parallel without waiting and in `decidePolicyForNavigationResponse` we evaluate the task’s result.
Another approach I thought of was to perform the logic in the background in `didStartProvisionalNavigation`. The problem with this approach is that is called only for navigation that starts from the main frame so it
would not be possible to intercept malicious iFrame URLs.
@alessandroboron
Malicious site protection address bar and privacy dashboard changes (#…
cd64847 
…3718)

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208959082985728/f
Tech Design: https://app.asana.com/0/1206329551987282/1207273224076495/f

**Description**:
This PR addresses the following:
1. Updates the Privacy Icon to use the globe asset when visiting special
error pages (SSL error included).
2. Updates the Privacy icon to use the alert asset when the user accepts
the risk of visiting a malicious page.
3. Show an updated privacy dashboard for phishing and malware special
error pages.
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-feature-flags branch from 744d6aa to e791a06 Compare January 14, 2025 03:43
@alessandroboron
Malicious site protection feature flags (#3719)
3af5452 
Task/Issue URL: https://app.asana.com/0/72649045549333/1207944134334659/f
Tech Design URL: https://app.asana.com/0/1206329551987282/1207273224076495/f

**Description**:
Add Feature flag class as per tech design to check if malicious site protection feature is enabled and should check protection for domain based on the domain privacy preferences.
Base automatically changed from alessandro/malicious-site-protection-feature-flags to alessandro/malicious-site-protection January 14, 2025 03:53
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection-navigation-detection-integration branch from 4d87dab to 91622ac Compare January 14, 2025 07:49
@alessandroboron alessandroboron removed the request for review from jaceklyp January 15, 2025 00:48
@alessandroboron alessandroboron force-pushed the alessandro/malicious-site-protection branch from 3af5452 to beba8e0 Compare January 21, 2025 04:28
@alessandroboron
Copy link
Contributor Author

I close this PR because after the changes we did to fetch the initial datasets and background fetch it was easier to cherry pick the commit I needed rather than rebase the code. I will open a new PR.

@alessandroboron alessandroboron deleted the alessandro/malicious-site-protection-navigation-detection-integration branch February 17, 2025 04:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@alessandroboron