Fix VPN rekeying #716
Fix VPN rekeying #716
Conversation
| @@ -238,9 +240,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { | |||
| } | |||
|
|
|||
| handle(clientError: error) | |||
|
|
|||
| let cachedServer = try cachedServer(registeredWith: keyPair) | |||
There was a problem hiding this comment.
This will need more cleanup as this code isn't used anymore, but not in this hotfix.
I'm removing this because:
- If the auth token is bad this code doesn't help.
- This code hides the actual backend issue from pixels.
There was a problem hiding this comment.
Good find, the try in the cachedServer call does indeed mask the real issue.
| @@ -595,7 +595,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { | |||
| serverSelectionMethod: currentServerSelectionMethod, | |||
| includedRoutes: includedRoutes ?? [], | |||
| excludedRoutes: settings.excludedRanges, | |||
| regenerateKey: false) | |||
| regenerateKey: true) | |||
There was a problem hiding this comment.
Always use a new key when starting up the tunnel. Why not?
There was a problem hiding this comment.
One downside here is that it'll make the NetP connection speed overall feel slower since it has to do extra work to get started.
There was a problem hiding this comment.
I'm not sure... after all we're not saving a call to register right now.
| // This way we respect the client-set expiration date, unless the server has set an earlier | ||
| // expiration for whatever reason (like if the subscription is known to expire). | ||
| // | ||
| if let newExpiration, newExpiration < keyPair.expirationDate { |
There was a problem hiding this comment.
Comments are self-explanatory, never extend the local expiration, but always shorten it if the server considers that important.
…h token expiration date
| if let existingKeyPair = keyStore.currentKeyPair(), | ||
| existingKeyPair.expirationDate > Date().addingTimeInterval(TimeInterval.day) { | ||
|
|
||
| keyPair = keyStore.newKeyPair() |
There was a problem hiding this comment.
If the expiration date is over a day from now, we create a new key pair. This is to fix users with long expiration dates... although this could be a good security measure in case users are manipulating expiration dates.
diegoreymendez
changed the title
Task/Issue URL: https://app.asana.com/0/1199230911884351/1206800069675113/f iOS: duckduckgo/iOS#2577 BSK: duckduckgo/BrowserServicesKit#716 ## Description Fixes the following: - Several users seem to be no longer rekeying in the latest releases. - Some users seem to have trouble connecting due to these rekeying issues.
Task/Issue URL: https://app.asana.com/0/1199230911884351/1206800069675113/f macOS: duckduckgo/macos-browser#2369 BSK: duckduckgo/BrowserServicesKit#716 ## Description Fixes the following: - Several users seem to be no longer rekeying in the latest releases. - Some users seem to have trouble connecting due to these rekeying issues.
Required
Task/Issue URL:
iOS PR: duckduckgo/iOS#2577
macOS PR: duckduckgo/macos-browser#2369
What kind of version bump will this require?: Patch
Description
Fixes the following:
Testing
See the platform-specific instructions for testing.
The pixels should show:
Internal references:
Software Engineering Expectations
Technical Design Template