From c0b0f616d0dc094666ac81f81ddda8913ff22ff0 Mon Sep 17 00:00:00 2001
From: Mark <markchien@google.com>
Date: Thu, 27 Jul 2023 08:55:20 +0000
Subject: [PATCH] Random select started prefix range

Currently, PrivateAddressCoordinator always selects the /24 prefix
from the 192.168.x.x/16 range first. If the upstream is also a /24
prefix in the 192.168.x.x/16 range, there's a 1/256 chance of a
conflict (192.168.0.x/24 ~ 192.168.255.x/24). Since 192.168.x.x/16
is a popular range for home and small business networks, we should
randomize the starting prefix range to reduce the chance of a
conflict. The /8, /12, and /16 range selection rates are 0.39%,
5.86%, and 93.7%, respectively (see the inline comments for how
the rates are calculated).

The chance of selecting a /24 prefix is:
- 10.0.0.0/8: 1/(256*256) * 93.7% = 0.001429%
- 172.16.0.0/12: 1/(16*256) * 5.86% = 0.00143%
- 192.168.0.0/16: 1/256 * 0.39% = 0.001523%

Note: This change is currently turned off by the
tether_force_random_prefix_base_selection flag. We'll run experiments
to make sure tethering stays reliable with this change enabled.

Bug: 312647670
Test: atest TetheringTests
Change-Id: Iea03fdcf0fccad95410e79ae87fcb046d75da457
---
 .../tethering/PrivateAddressCoordinator.java  | 27 ++++++++++++-
 .../tethering/TetheringConfiguration.java     | 12 ++++++
 .../PrivateAddressCoordinatorTest.java        | 39 +++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/Tethering/src/com/android/networkstack/tethering/PrivateAddressCoordinator.java b/Tethering/src/com/android/networkstack/tethering/PrivateAddressCoordinator.java
index 6c0ca822ac..54dee14b1b 100644
--- a/Tethering/src/com/android/networkstack/tethering/PrivateAddressCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/PrivateAddressCoordinator.java
@@ -187,7 +187,10 @@ public class PrivateAddressCoordinator {
             return cachedAddress;
         }
 
-        for (IpPrefix prefixRange : mTetheringPrefixes) {
+        final int prefixIndex = getStartedPrefixIndex();
+        for (int i = 0; i < mTetheringPrefixes.size(); i++) {
+            final IpPrefix prefixRange = mTetheringPrefixes.get(
+                    (prefixIndex + i) % mTetheringPrefixes.size());
             final LinkAddress newAddress = chooseDownstreamAddress(prefixRange);
             if (newAddress != null) {
                 mDownstreams.add(ipServer);
@@ -200,6 +203,28 @@ public class PrivateAddressCoordinator {
         return null;
     }
 
+    private int getStartedPrefixIndex() {
+        if (!mConfig.isRandomPrefixBaseEnabled()) return 0;
+
+        final int random = getRandomInt() & 0xffffff;
+        // This is to select the starting prefix range (/8, /12, or /16) instead of the actual
+        // LinkAddress. To avoid complex operations in the selection logic and make the selected
+        // rate approximate consistency with that /8 is around 2^4 times of /12 and /12 is around
+        // 2^4 times of /16, we simply define a map between the value and the prefix value like
+        // this:
+        //
+        // Value 0 ~ 0xffff (65536/16777216 = 0.39%) -> 192.168.0.0/16
+        // Value 0x10000 ~ 0xfffff (983040/16777216 = 5.86%) -> 172.16.0.0/12
+        // Value 0x100000 ~ 0xffffff (15728640/16777216 = 93.7%) -> 10.0.0.0/8
+        if (random > 0xfffff) {
+            return 2;
+        } else if (random > 0xffff) {
+            return 1;
+        } else {
+            return 0;
+        }
+    }
+
     private int getPrefixBaseAddress(final IpPrefix prefix) {
         return inet4AddressToIntHTH((Inet4Address) prefix.getAddress());
     }
diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
index 502fee816a..0678525efd 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringConfiguration.java
@@ -130,6 +130,8 @@ public class TetheringConfiguration {
     public static final String TETHER_ENABLE_WEAR_TETHERING =
             "tether_enable_wear_tethering";
 
+    public static final String TETHER_FORCE_RANDOM_PREFIX_BASE_SELECTION =
+            "tether_force_random_prefix_base_selection";
     /**
      * Default value that used to periodic polls tether offload stats from tethering offload HAL
      * to make the data warnings work.
@@ -171,6 +173,7 @@ public class TetheringConfiguration {
     private final int mP2pLeasesSubnetPrefixLength;
 
     private final boolean mEnableWearTethering;
+    private final boolean mRandomPrefixBase;
 
     private final int mUsbTetheringFunction;
     protected final ContentResolver mContentResolver;
@@ -288,6 +291,8 @@ public class TetheringConfiguration {
 
         mEnableWearTethering = shouldEnableWearTethering(ctx);
 
+        mRandomPrefixBase = mDeps.isFeatureEnabled(ctx, TETHER_FORCE_RANDOM_PREFIX_BASE_SELECTION);
+
         configLog.log(toString());
     }
 
@@ -376,6 +381,10 @@ public class TetheringConfiguration {
         return mEnableWearTethering;
     }
 
+    public boolean isRandomPrefixBaseEnabled() {
+        return mRandomPrefixBase;
+    }
+
     /** Does the dumping.*/
     public void dump(PrintWriter pw) {
         pw.print("activeDataSubId: ");
@@ -426,6 +435,9 @@ public class TetheringConfiguration {
 
         pw.print("mUsbTetheringFunction: ");
         pw.println(isUsingNcm() ? "NCM" : "RNDIS");
+
+        pw.print("mRandomPrefixBase: ");
+        pw.println(mRandomPrefixBase);
     }
 
     /** Returns the string representation of this object.*/
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java
index 6ebd6ae6b5..2298a1ad5f 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/PrivateAddressCoordinatorTest.java
@@ -30,6 +30,7 @@ import static com.android.networkstack.tethering.util.PrefixUtils.asIpPrefix;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
@@ -104,6 +105,7 @@ public final class PrivateAddressCoordinatorTest {
         when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(mConnectivityMgr);
         when(mConnectivityMgr.getAllNetworks()).thenReturn(mAllNetworks);
         when(mConfig.shouldEnableWifiP2pDedicatedIp()).thenReturn(false);
+        when(mConfig.isRandomPrefixBaseEnabled()).thenReturn(false);
         setUpIpServers();
         mPrivateAddressCoordinator = spy(new PrivateAddressCoordinator(mContext, mConfig));
     }
@@ -572,4 +574,41 @@ public final class PrivateAddressCoordinatorTest {
         assertEquals("Wrong local hotspot prefix: ", new LinkAddress("192.168.134.5/24"),
                 localHotspotAddress);
     }
+
+    @Test
+    public void testStartedPrefixRange() throws Exception {
+        when(mConfig.isRandomPrefixBaseEnabled()).thenReturn(true);
+
+        startedPrefixBaseTest("192.168.0.0/16", 0);
+
+        startedPrefixBaseTest("192.168.0.0/16", 1);
+
+        startedPrefixBaseTest("192.168.0.0/16", 0xffff);
+
+        startedPrefixBaseTest("172.16.0.0/12", 0x10000);
+
+        startedPrefixBaseTest("172.16.0.0/12", 0x11111);
+
+        startedPrefixBaseTest("172.16.0.0/12", 0xfffff);
+
+        startedPrefixBaseTest("10.0.0.0/8", 0x100000);
+
+        startedPrefixBaseTest("10.0.0.0/8", 0x1fffff);
+
+        startedPrefixBaseTest("10.0.0.0/8", 0xffffff);
+
+        startedPrefixBaseTest("192.168.0.0/16", 0x1000000);
+    }
+
+    private void startedPrefixBaseTest(final String expected, final int randomIntForPrefixBase)
+            throws Exception {
+        mPrivateAddressCoordinator = spy(new PrivateAddressCoordinator(mContext, mConfig));
+        when(mPrivateAddressCoordinator.getRandomInt()).thenReturn(randomIntForPrefixBase);
+        final LinkAddress address = requestDownstreamAddress(mHotspotIpServer,
+                CONNECTIVITY_SCOPE_GLOBAL, false /* useLastAddress */);
+        final IpPrefix prefixBase = new IpPrefix(expected);
+        assertTrue(address + " is not part of " + prefixBase,
+                prefixBase.containsPrefix(asIpPrefix(address)));
+
+    }
 }
-- 
GitLab