diff options
4 files changed, 179 insertions, 24 deletions
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f1ea5d018490..552331ede99b 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5889,6 +5889,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void processLinkPropertiesFromAgent(NetworkAgentInfo nai, LinkProperties lp) { lp.ensureDirectlyConnectedRoutes(); + nai.clatd.setNat64PrefixFromRa(lp.getNat64Prefix()); } private void updateLinkProperties(NetworkAgentInfo networkAgent, LinkProperties newLp, diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index e6b2d2678f1d..8ad21966cb9f 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -81,15 +81,23 @@ public class Nat464Xlat extends BaseNetworkObserver { RUNNING, // start() called, and the stacked iface is known to be up. } - /** NAT64 prefix currently in use. Only valid in STARTING or RUNNING states. */ + /** + * NAT64 prefix currently in use. Only valid in STARTING or RUNNING states. + * Used, among other things, to avoid updates when switching from a prefix learned from one + * source (e.g., RA) to the same prefix learned from another source (e.g., RA). + */ private IpPrefix mNat64PrefixInUse; /** NAT64 prefix (if any) discovered from DNS via RFC 7050. */ private IpPrefix mNat64PrefixFromDns; + /** NAT64 prefix (if any) learned from the network via RA. */ + private IpPrefix mNat64PrefixFromRa; private String mBaseIface; private String mIface; private Inet6Address mIPv6Address; private State mState = State.IDLE; + private boolean mPrefixDiscoveryRunning; + public Nat464Xlat(NetworkAgentInfo nai, INetd netd, IDnsResolver dnsResolver, INetworkManagementService nmService) { mDnsResolver = dnsResolver; @@ -139,15 +147,6 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * @return true if we have started prefix discovery and not yet stopped it (regardless of - * whether it is still running or has succeeded). - * A true result corresponds to internal states DISCOVERING, STARTING and RUNNING. - */ - public boolean isPrefixDiscoveryStarted() { - return mState == State.DISCOVERING || isStarted(); - } - - /** * @return true if clatd has been started and has not yet stopped. * A true result corresponds to internal states STARTING and RUNNING. */ @@ -181,7 +180,7 @@ public class Nat464Xlat extends BaseNetworkObserver { return; } - mNat64PrefixInUse = getNat64Prefix(); + mNat64PrefixInUse = selectNat64Prefix(); String addrStr = null; try { addrStr = mNetd.clatdStart(baseIface, mNat64PrefixInUse.toString()); @@ -218,8 +217,14 @@ public class Nat464Xlat extends BaseNetworkObserver { mNat64PrefixInUse = null; mIface = null; mBaseIface = null; - if (requiresClat(mNetwork)) { - mState = State.DISCOVERING; + + if (isPrefixDiscoveryNeeded()) { + if (!mPrefixDiscoveryRunning) { + startPrefixDiscovery(); + } else { + // Prefix discovery is already running. Nothing to do. + mState = State.DISCOVERING; + } } else { stopPrefixDiscovery(); // Enters IDLE state. } @@ -283,6 +288,7 @@ public class Nat464Xlat extends BaseNetworkObserver { Slog.e(TAG, "Error starting prefix discovery on netId " + getNetId() + ": " + e); } mState = State.DISCOVERING; + mPrefixDiscoveryRunning = true; } private void stopPrefixDiscovery() { @@ -292,10 +298,18 @@ public class Nat464Xlat extends BaseNetworkObserver { Slog.e(TAG, "Error stopping prefix discovery on netId " + getNetId() + ": " + e); } mState = State.IDLE; + mPrefixDiscoveryRunning = false; + } + + private boolean isPrefixDiscoveryNeeded() { + // If there is no NAT64 prefix in the RA, prefix discovery is always needed. It cannot be + // stopped after it succeeds, because stopping it will cause netd to report that the prefix + // has been removed, and that will cause us to stop clatd. + return requiresClat(mNetwork) && mNat64PrefixFromRa == null; } private void maybeHandleNat64PrefixChange() { - final IpPrefix newPrefix = getNat64Prefix(); + final IpPrefix newPrefix = selectNat64Prefix(); if (!Objects.equals(mNat64PrefixInUse, newPrefix)) { Slog.d(TAG, "NAT64 prefix changed from " + mNat64PrefixInUse + " to " + newPrefix); @@ -314,13 +328,10 @@ public class Nat464Xlat extends BaseNetworkObserver { // TODO: turn this class into a proper StateMachine. http://b/126113090 switch (mState) { case IDLE: - if (requiresClat(mNetwork)) { - // Network is detected to be IPv6-only. - // TODO: consider going to STARTING directly if the NAT64 prefix is already - // known. This would however result in clatd running without prefix discovery - // running, which might be a surprising combination. + if (isPrefixDiscoveryNeeded()) { startPrefixDiscovery(); // Enters DISCOVERING state. - return; + } else if (requiresClat(mNetwork)) { + start(); // Enters STARTING state. } break; @@ -351,8 +362,20 @@ public class Nat464Xlat extends BaseNetworkObserver { } } - private IpPrefix getNat64Prefix() { - return mNat64PrefixFromDns; + /** + * Picks a NAT64 prefix to use. Always prefers the prefix from the RA if one is received from + * both RA and DNS, because the prefix in the RA has better security and updatability, and will + * almost always be received first anyway. + * + * Any network that supports legacy hosts will support discovering the DNS64 prefix via DNS as + * well. If the prefix from the RA is withdrawn, fall back to that for reliability purposes. + */ + private IpPrefix selectNat64Prefix() { + return mNat64PrefixFromRa != null ? mNat64PrefixFromRa : mNat64PrefixFromDns; + } + + public void setNat64PrefixFromRa(IpPrefix prefix) { + mNat64PrefixFromRa = prefix; } public void setNat64PrefixFromDns(IpPrefix prefix) { @@ -367,7 +390,7 @@ public class Nat464Xlat extends BaseNetworkObserver { public void fixupLinkProperties(@NonNull LinkProperties oldLp, @NonNull LinkProperties lp) { // This must be done even if clatd is not running, because otherwise shouldStartClat would // never return true. - lp.setNat64Prefix(getNat64Prefix()); + lp.setNat64Prefix(selectNat64Prefix()); if (!isRunning()) { return; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index dad0363a80ac..c1e51dd62ad4 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6179,6 +6179,95 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(networkCallback); } + private void expectNat64PrefixChange(TestableNetworkCallback callback, + TestNetworkAgentWrapper agent, IpPrefix prefix) { + callback.expectLinkPropertiesThat(agent, x -> Objects.equals(x.getNat64Prefix(), prefix)); + } + + @Test + public void testNat64PrefixMultipleSources() throws Exception { + final String iface = "wlan0"; + final String pref64FromRaStr = "64:ff9b::"; + final String pref64FromDnsStr = "2001:db8:64::"; + final IpPrefix pref64FromRa = new IpPrefix(InetAddress.getByName(pref64FromRaStr), 96); + final IpPrefix pref64FromDns = new IpPrefix(InetAddress.getByName(pref64FromDnsStr), 96); + final IpPrefix newPref64FromRa = new IpPrefix("2001:db8:64:64:64:64::/96"); + + final NetworkRequest request = new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_INTERNET) + .build(); + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerNetworkCallback(request, callback); + + final LinkProperties baseLp = new LinkProperties(); + baseLp.setInterfaceName(iface); + baseLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); + baseLp.addDnsServer(InetAddress.getByName("2001:4860:4860::6464")); + + reset(mMockNetd, mMockDnsResolver); + InOrder inOrder = inOrder(mMockNetd, mMockDnsResolver); + + // If a network already has a NAT64 prefix on connect, clatd is started immediately and + // prefix discovery is never started. + LinkProperties lp = new LinkProperties(baseLp); + lp.setNat64Prefix(pref64FromRa); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, lp); + mCellNetworkAgent.connect(false); + final Network network = mCellNetworkAgent.getNetwork(); + int netId = network.getNetId(); + callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + callback.assertNoCallback(); + assertEquals(pref64FromRa, mCm.getLinkProperties(network).getNat64Prefix()); + + // If the RA prefix is withdrawn, clatd is stopped and prefix discovery is started. + lp.setNat64Prefix(null); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, null); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockDnsResolver).startPrefix64Discovery(netId); + + mService.mNetdEventCallback.onNat64PrefixEvent(netId, true /* added */, + pref64FromDnsStr, 96); + expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromDns); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromDns.toString()); + + // If the RA prefix reappears, clatd is restarted and prefix discovery is stopped. + lp.setNat64Prefix(pref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, pref64FromRa); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockDnsResolver).stopPrefix64Discovery(netId); + inOrder.verify(mMockNetd).clatdStart(iface, pref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // If the RA prefix changes, clatd is restarted and prefix discovery is not started. + lp.setNat64Prefix(newPref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + expectNat64PrefixChange(callback, mCellNetworkAgent, newPref64FromRa); + inOrder.verify(mMockNetd).clatdStop(iface); + inOrder.verify(mMockNetd).clatdStart(iface, newPref64FromRa.toString()); + inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // If the RA prefix changes to the same value, nothing happens. + lp.setNat64Prefix(newPref64FromRa); + mCellNetworkAgent.sendLinkProperties(lp); + callback.assertNoCallback(); + assertEquals(newPref64FromRa, mCm.getLinkProperties(network).getNat64Prefix()); + inOrder.verify(mMockNetd, never()).clatdStop(iface); + inOrder.verify(mMockNetd, never()).clatdStart(eq(iface), anyString()); + inOrder.verify(mMockDnsResolver, never()).stopPrefix64Discovery(netId); + inOrder.verify(mMockDnsResolver, never()).startPrefix64Discovery(netId); + + // The transition between no prefix and DNS prefix is tested in testStackedLinkProperties. + + callback.assertNoCallback(); + mCellNetworkAgent.disconnect(); + mCm.unregisterNetworkCallback(callback); + } + @Test public void testDataActivityTracking() throws Exception { final TestNetworkCallback networkCallback = new TestNetworkCallback(); diff --git a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java index d0ebb5283f49..84dad981e0df 100644 --- a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java +++ b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java @@ -60,6 +60,7 @@ public class Nat464XlatTest { static final String STACKED_IFACE = "v4-test0"; static final LinkAddress ADDR = new LinkAddress("192.0.2.5/29"); static final String NAT64_PREFIX = "64:ff9b::/96"; + static final String OTHER_NAT64_PREFIX = "2001:db8:0:64::/96"; static final int NETID = 42; @Mock ConnectivityService mConnectivity; @@ -139,7 +140,7 @@ public class Nat464XlatTest { for (NetworkInfo.DetailedState state : supportedDetailedStates) { mNai.networkInfo.setDetailedState(state, "reason", "extraInfo"); - mNai.linkProperties.setNat64Prefix(new IpPrefix("2001:db8:0:64::/96")); + mNai.linkProperties.setNat64Prefix(new IpPrefix(OTHER_NAT64_PREFIX)); assertRequiresClat(false, mNai); assertShouldStartClat(false, mNai); @@ -398,6 +399,47 @@ public class Nat464XlatTest { verifyNoMoreInteractions(mNetd, mNms, mConnectivity); } + @Test + public void testNat64PrefixPreference() throws Exception { + final IpPrefix prefixFromDns = new IpPrefix(NAT64_PREFIX); + final IpPrefix prefixFromRa = new IpPrefix(OTHER_NAT64_PREFIX); + + Nat464Xlat nat = makeNat464Xlat(); + + final LinkProperties emptyLp = new LinkProperties(); + LinkProperties fixedupLp; + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromDns(prefixFromDns); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromDns, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(prefixFromRa); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromDns, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(prefixFromRa); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromDns(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(prefixFromRa, fixedupLp.getNat64Prefix()); + + fixedupLp = new LinkProperties(); + nat.setNat64PrefixFromRa(null); + nat.fixupLinkProperties(emptyLp, fixedupLp); + assertEquals(null, fixedupLp.getNat64Prefix()); + } + static void assertIdle(Nat464Xlat nat) { assertTrue("Nat464Xlat was not IDLE", !nat.isStarted()); } |