Kill switch #438
Kill switch #438
Conversation
| ipv4Settings.includedRoutes = [.default()] // 0.0.0.0/0 | ||
| ipv4Settings.excludedRoutes = excludedRoutes.ipv4 |
There was a problem hiding this comment.
Is this the configuration just for when the kill-switch is enabled? or it's now the configuration always
There was a problem hiding this comment.
Always, dns server added to Exclusion list (as it was before) if the kill switch is off
There was a problem hiding this comment.
Is not that a change in behavior?
When the kill-switch is OFF we should comply with this
There was a problem hiding this comment.
It seems smart enough to not route the local network requests through the tunnel, in fact the excludeLocalNetworks flag didn't work for me, but I'm yet to validate this next week
There was a problem hiding this comment.
The exclusions are now provided from the main app, something to handle in the follow up project
There was a problem hiding this comment.
I agree the routing table shouldn't be affected in this PR as we agreed to the kill switch feature not affecting existing features.
Let's re-enable the routing table we had when the kill switch is off.
There was a problem hiding this comment.
since we have an exclusion list planned and hardcoded items added in Debug menu, this is now set from the NetworkConnectionController
There was a problem hiding this comment.
Nice work @mallexxx !
Submitted a first round of reviews with some questions and comments. Let me know if there's anything!
| ipv4Settings.includedRoutes = [.default()] // 0.0.0.0/0 | ||
| ipv4Settings.excludedRoutes = excludedRoutes.ipv4 |
There was a problem hiding this comment.
I agree the routing table shouldn't be affected in this PR as we agreed to the kill switch feature not affecting existing features.
Let's re-enable the routing table we had when the kill switch is off.
| @@ -18,17 +18,199 @@ | |||
|
|
|||
| import Foundation | |||
|
|
|||
| public enum ExtensionMessage: UInt8 { | |||
| case resetAllState = 0 | |||
| public enum ExtensionMessage: RawRepresentable { | |||
There was a problem hiding this comment.
refactored the Message encoding/decoding in sake of encapsulation and adding new messages
| ipv4Settings.includedRoutes = ipv4IncludedRoutes | ||
| ipv4Settings.excludedRoutes = Self.ipv4ExcludedRoutes | ||
| let ipv4Settings = NEIPv4Settings(addresses: addresses.ipv4.map { $0.destinationAddress }, subnetMasks: addresses.ipv4.map { $0.destinationSubnetMask }) | ||
| ipv4Settings.includedRoutes = includedRoutes.ipv4 |
There was a problem hiding this comment.
these are coming from NetworkConnectionController now
There was a problem hiding this comment.
@mallexxx - I approved this and then realized there's no iOS PR for it. Could you please create the iOS PR that integrates these changes?
Thanks.
|
@diegoreymendez I’ve approved the iOS PR. Any issues are now fixed. |
Task/Issue URL: https://app.asana.com/0/72649045549333/1204311881295998/f BSK PR: duckduckgo/BrowserServicesKit#438 macOS PR: duckduckgo/macos-browser#1408
Task/Issue URL: https://app.asana.com/0/72649045549333/1204311881295998/f Tech Design URL: BSK PR: duckduckgo/BrowserServicesKit#438
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/72649045549333/1204311881295998/f
iOS PR: duckduckgo/iOS#1891
macOS PR: duckduckgo/macos-browser#1408
What kind of version bump will this require?: Major
Optional:
Description:
Steps to test this PR: