Skip to content

Commit

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

Task/Issue URL: https://app.asana.com/0/488551667048375/1209389726956604

### Description
See attached task description

### Steps to test this PR
smoke test enabling VPN on a few devices and emulator
  • Loading branch information
karlenDimla authored Feb 13, 2025
1 parent bb82fb0 commit 28df088
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,26 @@
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.net.InetAddress
import java.security.KeyStore
import java.security.Security
import javax.inject.Named
import javax.inject.Qualifier
import logcat.logcat
import okhttp3.Dns
import okhttp3.Interceptor
import javax.net.ssl.SSLSocket
import javax.net.ssl.TrustManagerFactory
import javax.net.ssl.X509TrustManager
import okhttp3.OkHttpClient
import okhttp3.Response
import org.conscrypt.Conscrypt
import retrofit2.Retrofit
import retrofit2.http.Body
import retrofit2.http.GET
Expand All @@ -61,35 +57,37 @@ object WgVpnControllerServiceModule {
@SingleInstanceIn(VpnScope::class)
fun provideInternalCustomHttpClient(
@Named("api") okHttpClient: OkHttpClient,
context: Context,
appBuildConfig: AppBuildConfig,
delegatingSSLSocketFactory: DelegatingSSLSocketFactory,
): OkHttpClient {
val network = context.getNonVpnNetwork()
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

return okHttpClient.newBuilder()
.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)
}
}
},
)
}
}
.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
}
}
}

@Provides
@UnprotectedVpnControllerService
@SingleInstanceIn(VpnScope::class)
Expand All @@ -102,36 +100,10 @@ object WgVpnControllerServiceModule {
.callFactory { customClient.get().newCall(it) }
.build()

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
// insertProviderAt trick to avoid error during handshakes
Security.insertProviderAt(Conscrypt.newProvider(), 1)

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

return chain.proceed(request)
return customRetrofit.create(WgVpnControllerService::class.java)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ 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 @@ -68,7 +67,6 @@ 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 @@ -94,15 +92,11 @@ 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
logcat { "Handshake monitoring $lastHandshakeEpocSeconds" }
if (lastHandshakeEpocSeconds > 0 || attemptsWithZeroHandshakeEpoc.compareAndSet(MAX_ATTEMPTS_WITH_NO_HS_EPOC, 0)) {
attemptsWithZeroHandshakeEpoc.set(0) // reset in case lastHandshakeEpocSeconds > 0

if (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 @@ -124,8 +118,6 @@ class WireguardHandshakeMonitor @Inject constructor(
}
}
}
} else {
attemptsWithZeroHandshakeEpoc.incrementAndGet()
}
delay(1.minutes.inWholeMilliseconds)
}
Expand All @@ -136,9 +128,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ 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 @@ -153,7 +152,6 @@ 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 @@ -236,17 +234,7 @@ class NetPInternalSettingsActivity : DuckDuckGoActivity() {

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

binding.egressFailure.setClickListener {
lifecycleScope.launch {
modifyKeys()
sendBroadcast(Intent(DebugRekeyReceiver.ACTION_FORCE_REKEY))
}
}

Expand All @@ -260,23 +248,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,6 @@
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
<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 28df088

Please sign in to comment.