Ensure successful registration before key refresh #3736

aitorvs · 2023-10-21T09:31:52Z

Task/Issue URL: https://app.asana.com/0/0/1205631225739207/f

Description

Improve re-key so that:

it happens once a day
just re-key when the screen is off
avoids re-keying attempts when device is idle

Steps to test this PR

Test force re-key

build internal from this branch
use force rekey in the netp dev settings
new private key should be registered

Test rekey

apply the following patch

Index: network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt
--- a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt	(revision e44ddfce33069ae2ee2227bddffb24cc89049381)
+++ b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt	(date 1698157737720)
@@ -68,7 +68,7 @@
         val forceOrFalseInProductionBuilds = forceRekey.getAndResetValue()
 
         val millisSinceLastKeyUpdate = System.currentTimeMillis() - networkProtectionRepository.lastPrivateKeyUpdateTimeInMillis
-        if (!forceOrFalseInProductionBuilds && millisSinceLastKeyUpdate < TimeUnit.DAYS.toMillis(1)) {
+        if (!forceOrFalseInProductionBuilds && millisSinceLastKeyUpdate < TimeUnit.SECONDS.toMillis(5)) {
             logcat { "Less than 24h passed, skip re-keying" }
             return
         }
Index: network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt
--- a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt	(revision e44ddfce33069ae2ee2227bddffb24cc89049381)
+++ b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt	(date 1698157737723)
@@ -42,7 +42,7 @@
         job += coroutineScope.launch(dispatcherProvider.io()) {
             while (isActive && vpnFeaturesRegistry.isFeatureRegistered(NetPVpnFeature.NETP_VPN)) {
                 logcat { "Start periodic re-keying attempts" }
-                delay(TimeUnit.HOURS.toMillis(1)) // try to re-key every 1h
+                delay(TimeUnit.SECONDS.toMillis(10)) // try to re-key every 1h
 
                 netPRekeyer.doRekey()
             }

build internal build
launch app and enable NetP
with mobile device screen ON, see Device not locked, skip re-keying logcat appear every 10 seconds or so
with mobile device screen OFF, see Restarting VPN after clearing client keys logcat appear every 10 seconds or so

aitorvs · 2023-10-21T09:32:03Z

Current dependencies on/for this PR:

develop
- PR Ensure successful registration before key refresh #3736 👈

This comment was auto-generated by Graphite.

...l-compiler/src/main/java/com/duckduckgo/anvil/compiler/ContributesServiceApiCodeGenerator.kt

.../src/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt

...tection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt

aitorvs · 2023-10-21T17:04:35Z

...ork-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt

+    suspend fun forceRekey() {
+        if (appBuildConfig.isInternalBuild()) {
+            forceRekey.set(true)
+            doRekey()
+        } else {
+            logcat(LogPriority.ERROR) { "Force re-key not allowed in production builds" }
+        }
+    }


This is to keep the force rekey internal build functionality

aitorvs · 2023-10-21T17:05:19Z

...ion-impl/src/main/java/com/duckduckgo/networkprotection/impl/waitlist/NetPWaitlistManager.kt

@@ -52,7 +53,7 @@ interface NetPWaitlistManager {
 @SingleInstanceIn(AppScope::class)
 @ContributesBinding(AppScope::class)
 class RealNetPWaitlistManager @Inject constructor(
-    private val service: WgVpnControllerService,
+    @ProtectedVpnControllerService private val service: WgVpnControllerService,


Ditto, ie. this probably can also rather be @UnprotectedVpnControllerService at which point we delete the @ProtectedVpnControllerService altogether

aitorvs · 2023-10-21T17:06:01Z

.../main/java/com/duckduckgo/networkprotection/internal/feature/NetPInternalSettingsActivity.kt

@@ -215,7 +213,7 @@ class NetPInternalSettingsActivity : DuckDuckGoActivity() {

        binding.forceRekey.setClickListener {
            lifecycleScope.launch {
-                netPRekeyer.doRekey()
+                sendBroadcast(Intent(DebugRekeyReceiver.ACTION_FORCE_REKEY))


We use a broadcast as an IPC mechanism, as rekeying needs to be received/executed by the :vpn process

aitorvs · 2023-10-21T17:06:13Z

.../src/main/java/com/duckduckgo/networkprotection/internal/network/WgServerInternalProvider.kt

@@ -32,7 +33,7 @@ import javax.inject.Inject
 )
 class WgServerInternalProvider @Inject constructor(
    private val netPServerRepository: NetPServerRepository,
-    private val controllerService: WgVpnControllerService,
+    @ProtectedVpnControllerService private val controllerService: WgVpnControllerService,


aitorvs · 2023-10-21T17:06:39Z

...internal/src/main/java/com/duckduckgo/networkprotection/internal/rekey/DebugRekeyReceiver.kt

+class DebugRekeyReceiver @Inject constructor(
+    private val context: Context,
+    private val rekeyer: RealNetPRekeyer,
+    @ProcessName private val processName: String,


This is just for logging

.../src/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt

...ork-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt

Task/Issue URL: https://app.asana.com/0/0/1205631225739207/f ### Description Improve re-key so that: * it happens once a day * just re-key when the screen is off * avoids re-keying attempts when device is idle ### Steps to test this PR _Test force re-key_ - [x] build internal from this branch - [x] use force rekey in the netp dev settings - [x] new private key should be registered _Test rekey_ - apply the following patch ```diff Index: network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt --- a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt (revision e44ddfce33069ae2ee2227bddffb24cc89049381) +++ b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt (date 1698157737720) @@ -68,7 +68,7 @@ val forceOrFalseInProductionBuilds = forceRekey.getAndResetValue() val millisSinceLastKeyUpdate = System.currentTimeMillis() - networkProtectionRepository.lastPrivateKeyUpdateTimeInMillis - if (!forceOrFalseInProductionBuilds && millisSinceLastKeyUpdate < TimeUnit.DAYS.toMillis(1)) { + if (!forceOrFalseInProductionBuilds && millisSinceLastKeyUpdate < TimeUnit.SECONDS.toMillis(5)) { logcat { "Less than 24h passed, skip re-keying" } return } Index: network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt --- a/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt (revision e44ddfce33069ae2ee2227bddffb24cc89049381) +++ b/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt (date 1698157737723) @@ -42,7 +42,7 @@ job += coroutineScope.launch(dispatcherProvider.io()) { while (isActive && vpnFeaturesRegistry.isFeatureRegistered(NetPVpnFeature.NETP_VPN)) { logcat { "Start periodic re-keying attempts" } - delay(TimeUnit.HOURS.toMillis(1)) // try to re-key every 1h + delay(TimeUnit.SECONDS.toMillis(10)) // try to re-key every 1h netPRekeyer.doRekey() } ``` - build internal build - launch app and enable NetP - [x] with mobile device screen ON, see `Device not locked, skip re-keying` logcat appear every 10 seconds or so - [x] with mobile device screen OFF, see `Restarting VPN after clearing client keys` logcat appear every 10 seconds or so

aitorvs mentioned this pull request Oct 21, 2023

Improve rekey #3712

Closed

aitorvs commented Oct 21, 2023

View reviewed changes

...l-compiler/src/main/java/com/duckduckgo/anvil/compiler/ContributesServiceApiCodeGenerator.kt Show resolved Hide resolved

aitorvs marked this pull request as draft October 21, 2023 16:58

aitorvs force-pushed the feature/aitor/netp/rekey_ branch 3 times, most recently from 3c4ec73 to 4514bee Compare October 21, 2023 17:00

aitorvs commented Oct 21, 2023

View reviewed changes

.../src/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt Outdated Show resolved Hide resolved

aitorvs force-pushed the feature/aitor/netp/rekey_ branch 2 times, most recently from fb31b77 to 6967b32 Compare October 21, 2023 17:03

aitorvs commented Oct 21, 2023

View reviewed changes

...tection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyScheduler.kt Show resolved Hide resolved

aitorvs commented Oct 21, 2023

View reviewed changes

aitorvs force-pushed the feature/aitor/netp/rekey_ branch 3 times, most recently from 728ba37 to 1e99303 Compare October 21, 2023 18:39

aitorvs mentioned this pull request Oct 22, 2023

Bump to ng 1.6.6 #3738

Merged

aitorvs force-pushed the feature/aitor/netp/rekey branch from 0ed4fd1 to 57c48bd Compare October 23, 2023 12:31

aitorvs force-pushed the feature/aitor/netp/rekey_ branch from 1e99303 to 573f67e Compare October 23, 2023 12:31

karlenDimla reviewed Oct 23, 2023

View reviewed changes

.../src/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt Show resolved Hide resolved

karlenDimla reviewed Oct 23, 2023

View reviewed changes

...ork-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt Outdated Show resolved Hide resolved

karlenDimla reviewed Oct 23, 2023

View reviewed changes

...ork-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/rekey/NetPRekeyer.kt Outdated Show resolved Hide resolved

aitorvs changed the base branch from feature/aitor/netp/rekey to develop October 23, 2023 15:11

aitorvs force-pushed the feature/aitor/netp/rekey_ branch from 573f67e to 605400c Compare October 23, 2023 15:11

aitorvs marked this pull request as ready for review October 23, 2023 15:18

aitorvs force-pushed the feature/aitor/netp/rekey_ branch 3 times, most recently from c3ca5af to c897c06 Compare October 23, 2023 16:07

aitorvs force-pushed the feature/aitor/netp/rekey_ branch 3 times, most recently from a2ffe89 to cde8bfa Compare October 24, 2023 22:24

karlenDimla approved these changes Oct 26, 2023

View reviewed changes

aitorvs force-pushed the feature/aitor/netp/rekey_ branch from cde8bfa to 8027aaa Compare October 26, 2023 14:57

karlenDimla approved these changes Oct 26, 2023

View reviewed changes

aitorvs added 4 commits October 26, 2023 17:11

Improve rekey

5198f1d

Address code review comments

b93a3d4

Fix tests

1e51c7a

Avoid unnecessary key registering

771b666

aitorvs force-pushed the feature/aitor/netp/rekey_ branch from 8027aaa to 771b666 Compare October 26, 2023 16:12

Fix conflicts with geoswitching

f45e596

aitorvs merged commit 5038292 into develop Oct 26, 2023

aitorvs deleted the feature/aitor/netp/rekey_ branch October 26, 2023 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure successful registration before key refresh #3736

Ensure successful registration before key refresh #3736

aitorvs commented Oct 21, 2023 •

edited by karlenDimla

Loading

aitorvs commented Oct 21, 2023 •

edited

Loading

aitorvs Oct 21, 2023

aitorvs Oct 21, 2023

aitorvs Oct 21, 2023

aitorvs Oct 21, 2023

aitorvs Oct 21, 2023

Ensure successful registration before key refresh #3736

Ensure successful registration before key refresh #3736

Conversation

aitorvs commented Oct 21, 2023 • edited by karlenDimla Loading

Description

Steps to test this PR

aitorvs commented Oct 21, 2023 • edited Loading

aitorvs Oct 21, 2023

Choose a reason for hiding this comment

aitorvs Oct 21, 2023

Choose a reason for hiding this comment

aitorvs Oct 21, 2023

Choose a reason for hiding this comment

aitorvs Oct 21, 2023

Choose a reason for hiding this comment

aitorvs Oct 21, 2023

Choose a reason for hiding this comment

aitorvs commented Oct 21, 2023 •

edited by karlenDimla

Loading

aitorvs commented Oct 21, 2023 •

edited

Loading