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

Network Protection #1211

Merged
merged 233 commits into from
May 26, 2023
Merged

Network Protection #1211

merged 233 commits into from
May 26, 2023

Conversation

samsymons
Copy link
Collaborator

@samsymons samsymons commented May 23, 2023

Task/Issue URL: https://app.asana.com/0/1199230911884351/1204654293315452/f
Tech Design URL:
CC: @ayoy @diegoreymendez @graeme @mallexxx

Description:

This PR adds Network Protection to the macOS browser.

Steps to test this PR:

To test basic connectivity:

  1. Test that building of all targets succeeds
  2. Test that you can archive a build of the app with NetP included, using the archive.sh script
  3. Once you have a build, move it to the /Applications directory and run it
  4. Visit the Settings -> About page, and click the version number label 7 times (until a prompt appears)
  5. Enter an invite code from here
  6. Connect to NetP - you should get a prompt asking you to allow the system extension
  7. Verify that you are now running and connected to a nearby server

Issues to fix:

  • Fix ShellCheck (done in a PR, not yet merged and pushed to this branch)
  • The WireGuard XCFramework is checked into the branch directly (done in a PR)
  • There is some config hardcoded into the project files

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

samsymons and others added 30 commits November 9, 2022 11:45
Task/Issue URL: https://app.asana.com/0/0/1203279827923530/f
Tech Design URL:
CC:

Description:

This PR sets the macOS repo up for NetP work in the following ways:

• Two new configurations have been added, NetworkProtection_Debug and NetworkProtection_Release, with their own bundle IDs
• A new app icon has been added to both of them, to differentiate these builds from others
• The product name has been set to DuckDuckGo Network Protection
• The Network Extension entitlement has been added to the new configurations
• Sparkle updates have been disabled. I am tracking this with a TODO (NetP): comment, but am open to a better way to keep track of changes that we need to undo before merging this into the upstream repo

Note, this change does not yet add a new target for the VPN extension. I plan to keep that in a separate PR to avoid this one getting too large.
Task/Issue URL: https://app.asana.com/0/0/1203340357678926/f
Tech Design URL:
CC:

Description:

This PR adds an empty packet tunnel provider to the macOS browser. It uses Network Protection Extension as the target name, but uses DuckDuckGo Network Protection as the product name, since that is what will be visible to users who are looking in Activity Monitor etc.
#if DEBUG
assertionFailure("\(message)")
func logOrAssertionFailure(_ message: String) {
#if DEBUG && !CI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should probably be reverted, and instead the cause of this failing on CI should be investigated fully

samsymons and others added 3 commits May 25, 2023 23:10
Task/Issue URL:
Tech Design URL:
CC:

Description:

This PR removes NetP from the App Store target.
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

I think that’s all I can do for this pull request. Configuration changes look good, the App Store app compiles and runs fine. I’ve also checked the review build again and this time, when installed to /Applications it worked great (notifications prompt + login item + agent). It’s good to go from my side, but let me know if you end up merging more fixes and you’d want me to have another look.

// This is temporarily set this back to its default value, as a part of merging Network Protection. There are a small number of warnings introduced in
// that feature, and more time is needed to address them. To avoid bothering other developers, this is being disabled and a task to fix it will be
// prioritized.
SWIFT_STRICT_CONCURRENCY = minimal;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, sorry for this - we tried to get these warnings resolved, but they're complicated. Alex has a PR up which addresses some of this, but it had other effects so we decided to hold off on merging it in favour of this approach.

@ayoy
Copy link
Collaborator

ayoy commented May 26, 2023

I forgot to congratulate all of you for the amazing work guys @samsymons @diegoreymendez @mallexxx @graeme. Hats off 🙇

# By Dominik Kapusta
# Via GitHub
* upstream_develop:
  Sync Engine with support for syncing bookmarks (#1203)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo/AppDelegate/AppDelegate.swift
This matches the convention already established in the browser.
* upstream_develop:
  Update BSK with autofill 7.1.0 (#1225)
  Autofill password generation support for iOS (#1212)
This should have no effect as Global.xcconfig specifies this, but I’d rather keep the xcconfig files the same as develop as possible.
@samsymons samsymons merged commit 75b6ee7 into develop May 26, 2023
@samsymons samsymons deleted the sam/network-protection branch May 26, 2023 19:02
samsymons added a commit that referenced this pull request May 27, 2023
* release/1.42.0: (26 commits)
  Bump version to 1.42.0 (28)
  Update embedded files
  Network Protection (#1211)
  Update BSK with autofill 7.1.0 (#1225)
  Autofill password generation support for iOS (#1212)
  Sync Engine with support for syncing bookmarks (#1203)
  Bump BSK for messaging updates (#1213)
  add newTab pixel, fire pixels only for new users first time (#1219)
  Update tds endpoint (#1218)
  prevent devices from being fetch when sync not visible (#1214)
  License and contributor guidelines for open-sourcing (#1189)
  Add a unique Bitwarden decryption failure pixel (#1197)
  Bump version to 1.41.0 (27)
  Update embedded files
  Check that the source and target frame security origins are equal (#1207)
  Add activation points pixels (#1206)
  Add Login export links to settings and Autofill 3 dot menu (#1183)
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants