Update link properties early for VPN networks
When LinkProperties are updated after a private DNS configuration is set in updateNetworkInfo(), particularly when private DNS mode is set to opportunistic and the VPN server uses its own DNS servers, this can cause the private DNS to use the underlying network for TLS handshaking, potentially leading to a leak. To fix this, adjusting the order of operations—setting the private DNS configuration before updating LinkProperties should resolve the issue. This ensures that private DNS validation occurs after the VPN's network configuration is fully applied. Bug: 274867806 Test: atest ConnectivityCoverageTests Flag: EXEMPT legacy flag early_link_properties_update_for_vpn Change-Id: If684b51e6522a8284d5baee8db0d2f767405d359
This commit is contained in:
parent
74dcb4637c
commit
515efcf47d
6 changed files with 148 additions and 5 deletions
|
|
@ -549,6 +549,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
|
||||
// Flag to delay callbacks for frozen apps, suppressing duplicate and stale callbacks.
|
||||
private final boolean mQueueCallbacksForFrozenApps;
|
||||
// Flag for early link properties update
|
||||
private final boolean mSupportEarlyLinkPropertiesUpdateForVPN;
|
||||
|
||||
/**
|
||||
* Uids ConnectivityService tracks blocked status of to send blocked status callbacks.
|
||||
|
|
@ -1973,6 +1975,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
mQueueNetworkAgentEventsInSystemServer = mDeps.isAtLeastB()
|
||||
&& mDeps.isFeatureNotChickenedOut(context,
|
||||
ConnectivityFlags.QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER);
|
||||
mSupportEarlyLinkPropertiesUpdateForVPN = mDeps.isFeatureNotChickenedOut(context,
|
||||
ConnectivityFlags.EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN);
|
||||
// registerUidFrozenStateChangedCallback is only available on U+
|
||||
mQueueCallbacksForFrozenApps = mDeps.isAtLeastU()
|
||||
&& mDeps.isFeatureNotChickenedOut(context, QUEUE_CALLBACKS_FOR_FROZEN_APPS);
|
||||
|
|
@ -5532,6 +5536,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
&& !caps.hasTransport(TRANSPORT_THREAD));
|
||||
}
|
||||
|
||||
private boolean shouldUpdateLinkPropertiesEarlyForVPNNetwork(@NonNull NetworkAgentInfo nai) {
|
||||
return mSupportEarlyLinkPropertiesUpdateForVPN && nai.isVPN();
|
||||
}
|
||||
|
||||
private boolean shouldCreateNativeNetwork(@NonNull NetworkAgentInfo nai,
|
||||
@NonNull NetworkInfo.State state) {
|
||||
if (nai.isCreated()) return false;
|
||||
|
|
@ -12218,7 +12226,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
// interfaces and routing rules have been added, DNS servers programmed, etc.
|
||||
// For VPNs, this must be done before the capabilities are updated, because as soon as
|
||||
// that happens, UIDs are routed to the network.
|
||||
if (shouldCreateNetworksImmediately(networkAgent.getCapsNoCopy())) {
|
||||
if (shouldCreateNetworksImmediately(networkAgent.getCapsNoCopy())
|
||||
|| shouldUpdateLinkPropertiesEarlyForVPNNetwork(networkAgent)) {
|
||||
applyInitialLinkProperties(networkAgent);
|
||||
}
|
||||
|
||||
|
|
@ -12243,7 +12252,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
networkAgent.getAndSetNetworkCapabilities(networkAgent.networkCapabilities);
|
||||
|
||||
handlePerNetworkPrivateDnsConfig(networkAgent, mDnsManager.getPrivateDnsConfig());
|
||||
if (!shouldCreateNetworksImmediately(networkAgent.getCapsNoCopy())) {
|
||||
if (!(shouldCreateNetworksImmediately(networkAgent.getCapsNoCopy())
|
||||
|| shouldUpdateLinkPropertiesEarlyForVPNNetwork(networkAgent))) {
|
||||
applyInitialLinkProperties(networkAgent);
|
||||
} else {
|
||||
// The network was created when the agent registered, and the LinkProperties are
|
||||
|
|
|
|||
|
|
@ -70,6 +70,15 @@ public final class ConnectivityFlags {
|
|||
public static final String CONSTRAINED_DATA_SATELLITE_OPTIN =
|
||||
"constrained_data_satellite_optin";
|
||||
|
||||
/**
|
||||
* A feature flag to control whether the early link properties update for vpn should be enabled.
|
||||
*
|
||||
* Note: This feature is automatically enabled if the flag
|
||||
* QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER is enabled.
|
||||
*/
|
||||
public static final String EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN =
|
||||
"early_link_properties_update_for_vpn";
|
||||
|
||||
private boolean mNoRematchAllRequestsOnRegister;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -2209,6 +2209,7 @@ public class ConnectivityServiceTest {
|
|||
case ConnectivityFlags.QUEUE_CALLBACKS_FOR_FROZEN_APPS:
|
||||
case ConnectivityFlags.QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER:
|
||||
case ConnectivityFlags.CLOSE_QUIC_CONNECTION:
|
||||
case ConnectivityFlags.EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN:
|
||||
return true;
|
||||
default:
|
||||
throw new UnsupportedOperationException("Unknown flag " + name
|
||||
|
|
|
|||
|
|
@ -16,16 +16,27 @@
|
|||
|
||||
package com.android.server
|
||||
|
||||
import android.net.ConnectivitySettingsManager
|
||||
import android.net.InetAddresses
|
||||
import android.net.IpPrefix
|
||||
import android.net.LinkAddress
|
||||
import android.net.LinkProperties
|
||||
import android.net.NativeNetworkConfig
|
||||
import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET
|
||||
import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN
|
||||
import android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED
|
||||
import android.net.NetworkCapabilities.TRANSPORT_VPN
|
||||
import android.net.NetworkCapabilities.TRANSPORT_WIFI
|
||||
import android.net.ResolverParamsParcel
|
||||
import android.net.RouteInfo
|
||||
import android.net.VpnManager
|
||||
import android.net.VpnTransportInfo
|
||||
import android.net.netd.aidl.NativeUidRangeConfig
|
||||
import android.os.Build
|
||||
import android.os.Process
|
||||
import android.util.Range
|
||||
import com.android.server.connectivity.ConnectivityFlags.EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN
|
||||
import com.android.server.connectivity.ConnectivityFlags.QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER
|
||||
import com.android.testutils.ConnectivityModuleTest
|
||||
import com.android.testutils.DevSdkIgnoreRule
|
||||
import com.android.testutils.DevSdkIgnoreRunner
|
||||
|
|
@ -33,13 +44,20 @@ import org.junit.Test
|
|||
import org.junit.runner.RunWith
|
||||
import org.mockito.ArgumentMatchers.anyInt
|
||||
import org.mockito.ArgumentMatchers.argThat
|
||||
import org.mockito.ArgumentMatchers.eq
|
||||
import org.mockito.Mockito.inOrder
|
||||
import org.mockito.Mockito.never
|
||||
import org.mockito.Mockito.times
|
||||
|
||||
private const val DNS_ADDR = "8.8.8.8"
|
||||
private const val IPV4_ADDR = "192.168.2.1"
|
||||
private const val ROUTE_PREFIX = "0.0.0.0/0"
|
||||
|
||||
@DevSdkIgnoreRunner.MonitorThreadLeak
|
||||
@RunWith(DevSdkIgnoreRunner::class)
|
||||
@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S)
|
||||
class CSNetworkAgentTest : CSTest() {
|
||||
|
||||
@Test fun testVpnUidAgent() = testUidAgent(
|
||||
TRANSPORT_VPN,
|
||||
expectAddUidRanges = true
|
||||
|
|
@ -89,4 +107,105 @@ class CSNetworkAgentTest : CSTest() {
|
|||
// The old method should never be called in any case
|
||||
netdInOrder.verify(netd, never()).networkAddUidRanges(anyInt(), any())
|
||||
}
|
||||
|
||||
// Test the early link properties update for the VPN network when the flag
|
||||
// QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER is disabled. That flag is enabled by default in
|
||||
// the CSTest.
|
||||
@ConnectivityModuleTest
|
||||
@FeatureFlags(flags = [Flag(QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER, false)])
|
||||
@Test
|
||||
fun testEarlyLinkPropertiesUpdateForVPN() =
|
||||
testEarlyLinkPropertiesUpdate(TRANSPORT_VPN, expectEarlyLinkPropertiesUpdate = true)
|
||||
|
||||
// Test the early link properties update for the non-VPN network when the flag
|
||||
// QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER is disabled. That flag is enabled by default in
|
||||
// the CSTest.
|
||||
@ConnectivityModuleTest
|
||||
@FeatureFlags(flags = [Flag(QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER, false)])
|
||||
@Test
|
||||
fun testEarlyLinkPropertiesUpdateForNonVPN() =
|
||||
testEarlyLinkPropertiesUpdate(TRANSPORT_WIFI, expectEarlyLinkPropertiesUpdate = false)
|
||||
|
||||
// Test the early link properties update for the VPN network when both flags
|
||||
// QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER and EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN are
|
||||
// disabled. These flags are enabled by default in the CSTest.
|
||||
@ConnectivityModuleTest
|
||||
@FeatureFlags(
|
||||
flags = [Flag(QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER, false),
|
||||
Flag(EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN, false)]
|
||||
)
|
||||
@Test
|
||||
fun testEarlyLinkPropertiesUpdateDisabledForVPN() =
|
||||
testEarlyLinkPropertiesUpdate(TRANSPORT_VPN, expectEarlyLinkPropertiesUpdate = false)
|
||||
|
||||
private fun testEarlyLinkPropertiesUpdate(
|
||||
transport: Int,
|
||||
expectEarlyLinkPropertiesUpdate: Boolean
|
||||
) {
|
||||
val netdInOrder = inOrder(netd)
|
||||
val resolverInOrder = inOrder(dnsResolver)
|
||||
|
||||
val nc = defaultNc()
|
||||
.addTransportType(transport)
|
||||
.addCapability(NET_CAPABILITY_TRUSTED)
|
||||
.addCapability(NET_CAPABILITY_INTERNET)
|
||||
if (TRANSPORT_VPN == transport) {
|
||||
nc.removeCapability(NET_CAPABILITY_NOT_VPN)
|
||||
nc.setTransportInfo(
|
||||
VpnTransportInfo(
|
||||
VpnManager.TYPE_VPN_SERVICE,
|
||||
"MySession12345",
|
||||
true /* bypassable */,
|
||||
false /* longLivedTcpConnectionsExpensive */
|
||||
)
|
||||
)
|
||||
}
|
||||
val linkAddress = LinkAddress(InetAddresses.parseNumericAddress(IPV4_ADDR), 32)
|
||||
val lp = LinkProperties().apply {
|
||||
addLinkAddress(linkAddress)
|
||||
addRoute(RouteInfo(IpPrefix(ROUTE_PREFIX), null, null))
|
||||
addDnsServer(InetAddresses.parseNumericAddress(DNS_ADDR))
|
||||
}
|
||||
ConnectivitySettingsManager.setPrivateDnsMode(
|
||||
context,
|
||||
ConnectivitySettingsManager.PRIVATE_DNS_MODE_OPPORTUNISTIC
|
||||
)
|
||||
val agent = Agent(nc = nc, lp = lp)
|
||||
agent.connect()
|
||||
|
||||
netdInOrder.verify(netd).networkCreate(argThat { it.netId == agent.network.netId })
|
||||
netdInOrder.verify(netd).networkAddRouteParcel(
|
||||
eq(agent.network.netId),
|
||||
argThat { it.destination == ROUTE_PREFIX }
|
||||
)
|
||||
netdInOrder.verify(netd).networkAddRouteParcel(
|
||||
eq(agent.network.netId),
|
||||
argThat { it.destination == linkAddress.toString() }
|
||||
)
|
||||
|
||||
if (expectEarlyLinkPropertiesUpdate) {
|
||||
// Update the DNS without the TLS servers because the private DNS hasn't been set.
|
||||
resolverInOrder.verify(dnsResolver).setResolverConfiguration(
|
||||
argThat { it: ResolverParamsParcel ->
|
||||
it.netId == agent.network.netId &&
|
||||
it.servers.any { server -> server == DNS_ADDR } &&
|
||||
it.tlsServers.isEmpty() }
|
||||
)
|
||||
// The private DNS has been set. Update the DNS again with the TLS servers.
|
||||
resolverInOrder.verify(dnsResolver).setResolverConfiguration(
|
||||
argThat {
|
||||
it.netId == agent.network.netId &&
|
||||
it.servers.any { server -> server == DNS_ADDR } &&
|
||||
it.tlsServers.any { server -> server == DNS_ADDR }}
|
||||
)
|
||||
} else {
|
||||
// Update the DNS with the TLS servers because the private DNS has been set.
|
||||
resolverInOrder.verify(dnsResolver, times(2)).setResolverConfiguration(
|
||||
argThat {
|
||||
it.netId == agent.network.netId &&
|
||||
it.servers.any { server -> server == DNS_ADDR } &&
|
||||
it.tlsServers.any { server -> server == DNS_ADDR }}
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ import android.content.pm.PackageManager.PERMISSION_GRANTED
|
|||
import android.content.pm.UserInfo
|
||||
import android.content.res.Resources
|
||||
import android.net.ConnectivityManager
|
||||
import android.net.IDnsResolver
|
||||
import android.net.INetd
|
||||
import android.net.INetd.PERMISSION_INTERNET
|
||||
import android.net.InetAddresses
|
||||
|
|
@ -177,6 +178,7 @@ open class CSTest {
|
|||
it[ConnectivityFlags.QUEUE_CALLBACKS_FOR_FROZEN_APPS] = true
|
||||
it[ConnectivityFlags.QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER] = true
|
||||
it[ConnectivityFlags.CLOSE_QUIC_CONNECTION] = true
|
||||
it[ConnectivityFlags.EARLY_LINK_PROPERTIES_UPDATE_FOR_VPN] = true
|
||||
}
|
||||
fun setFeatureEnabled(flag: String, enabled: Boolean) = enabledFeatures.set(flag, enabled)
|
||||
|
||||
|
|
@ -227,6 +229,7 @@ open class CSTest {
|
|||
val satelliteAccessController = mock<SatelliteAccessController>()
|
||||
val quicConnectionCloser = mock<QuicConnectionCloser>()
|
||||
val destroySocketsWrapper = mock<DestroySocketsWrapper>()
|
||||
val dnsResolver = mock<IDnsResolver>()
|
||||
|
||||
val deps = CSDeps()
|
||||
val permDeps = PermDeps()
|
||||
|
|
@ -266,7 +269,7 @@ open class CSTest {
|
|||
|
||||
alarmHandlerThread = HandlerThread("TestAlarmManager").also { it.start() }
|
||||
alarmManager = makeMockAlarmManager(alarmHandlerThread)
|
||||
service = makeConnectivityService(context, netd, deps, permDeps).also {
|
||||
service = makeConnectivityService(context, netd, deps, permDeps, dnsResolver).also {
|
||||
it.systemReadyInternal()
|
||||
}
|
||||
cm = ConnectivityManager(context, service)
|
||||
|
|
|
|||
|
|
@ -223,11 +223,12 @@ internal fun makeConnectivityService(
|
|||
context: Context,
|
||||
netd: INetd,
|
||||
deps: Dependencies,
|
||||
mPermDeps: PermissionMonitor.Dependencies
|
||||
mPermDeps: PermissionMonitor.Dependencies,
|
||||
resolver: IDnsResolver
|
||||
) =
|
||||
ConnectivityService(
|
||||
context,
|
||||
mock<IDnsResolver>(),
|
||||
resolver,
|
||||
mock<IpConnectivityLog>(),
|
||||
netd,
|
||||
deps,
|
||||
|
|
|
|||
Loading…
Reference in a new issue