Skip to content

Commit

Permalink
Make sure UnprotectedVpnControllerService requests go outside the VPN…
Browse files Browse the repository at this point in the history 
… tunnel (#5635)

Task/Issue URL: https://app.asana.com/0/1198194956794324/1209378254127336

### Description
Make sure the `WgVpnControllerService` service marked as `UnprotectedVpnControllerService` makes calls outside the VPN tunnel.
This seems to fix rekey

### Steps to test this PR

_Test_
- [x] build the internal build
- [x] to go dev settings -> NetP dev settings and click on `Force rekey`
- [x] verify rekey should succeed all the time
- [x] verify the logcat contains `Request to: https://controller.netp.duckduckgo.com/register uses socket: <your local IP address>
  • Loading branch information
@aitorvs
aitorvs authored Feb 12, 2025
1 parent 63becef commit 8bfc982
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 37 deletions.
98 changes: 63 additions & 35 deletions ...c/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,30 @@
package com.duckduckgo.networkprotection.impl.configuration

import android.annotation.SuppressLint
import android.content.Context
import android.net.ConnectivityManager
import android.net.Network
import android.net.NetworkCapabilities
import com.duckduckgo.anvil.annotations.ContributesServiceApi
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.appbuildconfig.api.isInternalBuild
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.di.scopes.VpnScope
import com.duckduckgo.mobile.android.vpn.service.VpnSocketProtector
import com.duckduckgo.networkprotection.impl.di.ProtectedVpnControllerService
import com.duckduckgo.networkprotection.impl.di.UnprotectedVpnControllerService
import com.squareup.anvil.annotations.ContributesTo
import dagger.Lazy
import dagger.Module
import dagger.Provides
import dagger.SingleInstanceIn
import java.security.KeyStore
import java.security.Security
import java.net.InetAddress
import javax.inject.Named
import javax.inject.Qualifier
import javax.net.ssl.SSLSocket
import javax.net.ssl.TrustManagerFactory
import javax.net.ssl.X509TrustManager
import logcat.logcat
import okhttp3.Dns
import okhttp3.Interceptor
import okhttp3.OkHttpClient
import org.conscrypt.Conscrypt
import okhttp3.Response
import retrofit2.Retrofit
import retrofit2.http.Body
import retrofit2.http.GET
Expand All @@ -57,35 +61,33 @@ object WgVpnControllerServiceModule {
@SingleInstanceIn(VpnScope::class)
fun provideInternalCustomHttpClient(
@Named("api") okHttpClient: OkHttpClient,
delegatingSSLSocketFactory: DelegatingSSLSocketFactory,
context: Context,
appBuildConfig: AppBuildConfig,
): OkHttpClient {
val trustManagerFactory = TrustManagerFactory.getInstance(
TrustManagerFactory.getDefaultAlgorithm(),
)
trustManagerFactory.init(null as KeyStore?)
val trustManagers = trustManagerFactory.trustManagers
check(!(trustManagers.size != 1 || trustManagers[0] !is X509TrustManager)) {
("Unexpected default trust managers: ${trustManagers.contentToString()}")
}
val trustManager = trustManagers[0] as X509TrustManager
val network = context.getNonVpnNetwork()

return okHttpClient.newBuilder()
.sslSocketFactory(delegatingSSLSocketFactory, trustManager)
.build()
}

@Provides
@SingleInstanceIn(VpnScope::class)
fun provideDelegatingSSLSocketFactory(
socketProtector: Lazy<VpnSocketProtector>,
@Named("api") okHttpClient: Lazy<OkHttpClient>,
): DelegatingSSLSocketFactory {
return object : DelegatingSSLSocketFactory(okHttpClient.get().sslSocketFactory) {
override fun configureSocket(sslSocket: SSLSocket): SSLSocket {
socketProtector.get().protect(sslSocket)
return sslSocket
.apply {
if (appBuildConfig.isInternalBuild()) {
addNetworkInterceptor(LoggingNetworkInterceptor())
}
network?.socketFactory?.let {
socketFactory(it)

dns(
object : Dns {
override fun lookup(hostname: String): List<InetAddress> {
return try {
network.getAllByName(hostname).toList()
} catch (t: Throwable) {
Dns.SYSTEM.lookup(hostname)
}
}
},
)
}
}
}
.build()
}

@Provides
Expand All @@ -100,11 +102,37 @@ object WgVpnControllerServiceModule {
.callFactory { customClient.get().newCall(it) }
.build()

// insertProviderAt trick to avoid error during handshakes
Security.insertProviderAt(Conscrypt.newProvider(), 1)

return customRetrofit.create(WgVpnControllerService::class.java)
}

private fun Context.getNonVpnNetwork(): Network? {
val connectivityManager = this.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager?
connectivityManager?.let { cm ->
cm.allNetworks.firstOrNull()?.let { network ->
cm.getNetworkCapabilities(network)?.let { capabilities ->
if (!capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN)) {
return network
}
}
}
}

return null
}
}

private class LoggingNetworkInterceptor : Interceptor {
override fun intercept(chain: Interceptor.Chain): Response {
val request = chain.request()
val connection = chain.connection()

// Get the IP of the socket used
val socketAddress = connection?.socket()?.localAddress?.hostAddress

logcat { "Request to: ${request.url} uses socket: $socketAddress" }

return chain.proceed(request)
}
}

@ContributesServiceApi(AppScope::class)
Expand Down
13 changes: 12 additions & 1 deletion ...l/src/main/java/com/duckduckgo/networkprotection/impl/pixels/WireguardHandshakeMonitor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.duckduckgo.networkprotection.impl.pixels.WireguardHandshakeMonitor.Li
import com.squareup.anvil.annotations.ContributesMultibinding
import java.time.Instant
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicInteger
import javax.inject.Inject
import kotlin.time.Duration.Companion.minutes
import kotlin.time.Duration.Companion.seconds
Expand Down Expand Up @@ -67,6 +68,7 @@ class WireguardHandshakeMonitor @Inject constructor(

private val job = ConflatedJob()
private val failureReported = AtomicBoolean(false)
private val attemptsWithZeroHandshakeEpoc = AtomicInteger(0)

override fun onVpnStarted(coroutineScope: CoroutineScope) {
job += coroutineScope.launch {
Expand All @@ -92,11 +94,15 @@ class WireguardHandshakeMonitor @Inject constructor(
if (networkProtectionState.isEnabled()) {
failureReported.set(false)
currentNetworkState.start()
attemptsWithZeroHandshakeEpoc.set(0)

while (isActive && networkProtectionState.isEnabled()) {
val nowSeconds = Instant.now().epochSecond
val lastHandshakeEpocSeconds = wgProtocol.getStatistics().lastHandshakeEpochSeconds
if (lastHandshakeEpocSeconds > 0) {
logcat { "Handshake monitoring $lastHandshakeEpocSeconds" }
if (lastHandshakeEpocSeconds > 0 || attemptsWithZeroHandshakeEpoc.compareAndSet(MAX_ATTEMPTS_WITH_NO_HS_EPOC, 0)) {
attemptsWithZeroHandshakeEpoc.set(0) // reset in case lastHandshakeEpocSeconds > 0

val diff = nowSeconds - lastHandshakeEpocSeconds
if (diff.seconds.inWholeMinutes > REPORT_TUNNEL_FAILURE_IN_THRESHOLD_MINUTES && currentNetworkState.isConnected()) {
logcat(WARN) { "Last handshake was more than 5 minutes ago" }
Expand All @@ -118,6 +124,8 @@ class WireguardHandshakeMonitor @Inject constructor(
}
}
}
} else {
attemptsWithZeroHandshakeEpoc.incrementAndGet()
}
delay(1.minutes.inWholeMilliseconds)
}
Expand All @@ -128,6 +136,9 @@ class WireguardHandshakeMonitor @Inject constructor(
// WG handshakes happen every 2min, this means we'd miss 2+ handshakes
private const val REPORT_TUNNEL_FAILURE_IN_THRESHOLD_MINUTES = 5

// WG handshakes happen every 2min, this mean 5 handshakes
private const val MAX_ATTEMPTS_WITH_NO_HS_EPOC = 10

// WG handshakes happen every 2min
private const val REPORT_TUNNEL_FAILURE_RECOVERY_THRESHOLD_MINUTES = 2
}
Expand Down
31 changes: 30 additions & 1 deletion ...in/java/com/duckduckgo/networkprotection/internal/feature/NetPInternalSettingsActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import com.duckduckgo.networkprotection.store.NetPGeoswitchingRepository
import com.duckduckgo.networkprotection.store.NetPGeoswitchingRepository.UserPreferredLocation
import com.duckduckgo.networkprotection.store.remote_config.NetPServerRepository
import com.google.android.material.snackbar.Snackbar
import com.wireguard.crypto.KeyPair
import java.io.FileInputStream
import java.text.SimpleDateFormat
import java.util.*
Expand Down Expand Up @@ -152,6 +153,7 @@ class NetPInternalSettingsActivity : DuckDuckGoActivity() {
binding.overrideServerBackendSelector.isEnabled = isEnabled
binding.overrideServerBackendSelector.setSecondaryText(serverRepository.getSelectedServer()?.name ?: AUTOMATIC)
binding.forceRekey.isEnabled = isEnabled
binding.egressFailure.isEnabled = isEnabled
if (isEnabled) {
val wgConfig = wgTunnelConfig.getWgConfig()
wgConfig?.`interface`?.addresses?.joinToString(", ") { it.toString() }?.let {
Expand Down Expand Up @@ -234,7 +236,17 @@ class NetPInternalSettingsActivity : DuckDuckGoActivity() {

binding.forceRekey.setClickListener {
lifecycleScope.launch {
sendBroadcast(Intent(DebugRekeyReceiver.ACTION_FORCE_REKEY))
Intent(DebugRekeyReceiver.ACTION_FORCE_REKEY).apply {
setPackage(this@NetPInternalSettingsActivity.packageName)
}.also {
sendBroadcast(it)
}
}
}

binding.egressFailure.setClickListener {
lifecycleScope.launch {
modifyKeys()
}
}

Expand All @@ -248,6 +260,23 @@ class NetPInternalSettingsActivity : DuckDuckGoActivity() {
}
}

private suspend fun modifyKeys() = withContext(dispatcherProvider.io()) {
val oldConfig = wgTunnelConfig.getWgConfig()
val newConfig = oldConfig?.builder?.let { config ->
val interfaceBuilder = config.interfaze?.builder?.apply {
this.setKeyPair(KeyPair())
}

config.setInterface(interfaceBuilder?.build())
}

if (newConfig != null) {
val config = newConfig.build()
wgTunnelConfig.setWgConfig(config)
networkProtectionState.restart()
}
}

private fun handleStagingInput(isOverrideEnabled: Boolean) {
if (isOverrideEnabled) {
binding.stagingEnvironment.show()
Expand Down
9 changes: 9 additions & 0 deletions ...ction/network-protection-internal/src/main/res/layout/activity_netp_internal_settings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@
app:primaryText="@string/netpForceRekey"
app:showSwitch="false"
/>

<com.duckduckgo.common.ui.view.listitem.TwoLineListItem
android:id="@+id/egressFailure"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:primaryText="@string/netpEgressFailure"
app:showSwitch="false"
/>

<com.duckduckgo.common.ui.view.listitem.TwoLineListItem
android:id="@+id/changeEnvironment"
android:layout_width="match_parent"
Expand Down
1 change: 1 addition & 0 deletions network-protection/network-protection-internal/src/main/res/values/donottranslate.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<string name="netpDevSettingConnectionQuality">Connection Quality</string>
<string name="netpPcapRecording">Enable PCAP recording</string>
<string name="netpForceRekey">Force Rekey</string>
<string name="netpEgressFailure">Simulate egress failure</string>
<string name="netpUnsafeWifiDetectionPrimary">Unsafe Wi-Fi detection</string>
<string name="netpUnsafeWifiDetectionByline">Get notified when connected to unsafe Wi-Fi without a VPN.</string>
<string name="netpBlockMalwarePrimary">Block Malware</string>
Expand Down

0 comments on commit 8bfc982

Please sign in to comment.