Merge "Avoid creating a Network with INetd.LOCAL_NET_ID"
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 4c50bee..d2f6d6a 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -97,7 +97,13 @@
     },
     // Runs both NetHttpTests and CtsNetHttpTestCases
     {
-      "name": "NetHttpCoverageTests"
+      "name": "NetHttpCoverageTests",
+      "options": [
+        {
+          // These sometimes take longer than 1 min which is the presubmit timeout
+          "exclude-annotation": "androidx.test.filters.LargeTest"
+        }
+      ]
     }
   ],
   "postsubmit": [
@@ -211,7 +217,13 @@
       "name": "libnetworkstats_test[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]"
     },
     {
-      "name": "NetHttpCoverageTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]"
+      "name": "NetHttpCoverageTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]",
+      "options": [
+        {
+          // These sometimes take longer than 1 min which is the presubmit timeout
+          "exclude-annotation": "androidx.test.filters.LargeTest"
+        }
+      ]
     }
   ],
   "mainline-postsubmit": [
diff --git a/Tethering/tests/mts/src/android/tethering/mts/MtsEthernetTetheringTest.java b/Tethering/tests/mts/src/android/tethering/mts/MtsEthernetTetheringTest.java
index cb57d13..c2bc812 100644
--- a/Tethering/tests/mts/src/android/tethering/mts/MtsEthernetTetheringTest.java
+++ b/Tethering/tests/mts/src/android/tethering/mts/MtsEthernetTetheringTest.java
@@ -80,8 +80,8 @@
     // Per RX UDP packet size: iphdr (20) + udphdr (8) + payload (2) = 30 bytes.
     private static final int RX_UDP_PACKET_SIZE = 30;
     private static final int RX_UDP_PACKET_COUNT = 456;
-    // Per TX UDP packet size: ethhdr (14) + iphdr (20) + udphdr (8) + payload (2) = 44 bytes.
-    private static final int TX_UDP_PACKET_SIZE = 44;
+    // Per TX UDP packet size: iphdr (20) + udphdr (8) + payload (2) = 30 bytes.
+    private static final int TX_UDP_PACKET_SIZE = 30;
     private static final int TX_UDP_PACKET_COUNT = 123;
 
     private static final String DUMPSYS_TETHERING_RAWMAP_ARG = "bpfRawMap";
diff --git a/bpf_progs/offload.c b/bpf_progs/offload.c
index a8612df..56ace19 100644
--- a/bpf_progs/offload.c
+++ b/bpf_progs/offload.c
@@ -232,13 +232,13 @@
     // This would require a much newer kernel with newer ebpf accessors.
     // (This is also blindly assuming 12 bytes of tcp timestamp option in tcp header)
     uint64_t packets = 1;
-    uint64_t bytes = skb->len;
-    if (bytes > v->pmtu) {
-        const int tcp_overhead = sizeof(struct ipv6hdr) + sizeof(struct tcphdr) + 12;
-        const int mss = v->pmtu - tcp_overhead;
-        const uint64_t payload = bytes - tcp_overhead;
+    uint64_t L3_bytes = skb->len - l2_header_size;
+    if (L3_bytes > v->pmtu) {
+        const int tcp6_overhead = sizeof(struct ipv6hdr) + sizeof(struct tcphdr) + 12;
+        const int mss = v->pmtu - tcp6_overhead;
+        const uint64_t payload = L3_bytes - tcp6_overhead;
         packets = (payload + mss - 1) / mss;
-        bytes = tcp_overhead * packets + payload;
+        L3_bytes = tcp6_overhead * packets + payload;
     }
 
     // Are we past the limit?  If so, then abort...
@@ -247,7 +247,7 @@
     // a packet we let the core stack deal with things.
     // (The core stack needs to handle limits correctly anyway,
     // since we don't offload all traffic in both directions)
-    if (stat_v->rxBytes + stat_v->txBytes + bytes > *limit_v) TC_PUNT(LIMIT_REACHED);
+    if (stat_v->rxBytes + stat_v->txBytes + L3_bytes > *limit_v) TC_PUNT(LIMIT_REACHED);
 
     if (!is_ethernet) {
         // Try to inject an ethernet header, and simply return if we fail.
@@ -287,7 +287,7 @@
     bpf_csum_update(skb, 0xFFFF - ntohs(old_hl) + ntohs(new_hl));
 
     __sync_fetch_and_add(downstream ? &stat_v->rxPackets : &stat_v->txPackets, packets);
-    __sync_fetch_and_add(downstream ? &stat_v->rxBytes : &stat_v->txBytes, bytes);
+    __sync_fetch_and_add(downstream ? &stat_v->rxBytes : &stat_v->txBytes, L3_bytes);
 
     // Overwrite any mac header with the new one
     // For a rawip tx interface it will simply be a bunch of zeroes and later stripped.
@@ -449,13 +449,13 @@
     // This would require a much newer kernel with newer ebpf accessors.
     // (This is also blindly assuming 12 bytes of tcp timestamp option in tcp header)
     uint64_t packets = 1;
-    uint64_t bytes = skb->len;
-    if (bytes > v->pmtu) {
-        const int tcp_overhead = sizeof(struct iphdr) + sizeof(struct tcphdr) + 12;
-        const int mss = v->pmtu - tcp_overhead;
-        const uint64_t payload = bytes - tcp_overhead;
+    uint64_t L3_bytes = skb->len - l2_header_size;
+    if (L3_bytes > v->pmtu) {
+        const int tcp4_overhead = sizeof(struct iphdr) + sizeof(struct tcphdr) + 12;
+        const int mss = v->pmtu - tcp4_overhead;
+        const uint64_t payload = L3_bytes - tcp4_overhead;
         packets = (payload + mss - 1) / mss;
-        bytes = tcp_overhead * packets + payload;
+        L3_bytes = tcp4_overhead * packets + payload;
     }
 
     // Are we past the limit?  If so, then abort...
@@ -464,7 +464,7 @@
     // a packet we let the core stack deal with things.
     // (The core stack needs to handle limits correctly anyway,
     // since we don't offload all traffic in both directions)
-    if (stat_v->rxBytes + stat_v->txBytes + bytes > *limit_v) TC_PUNT(LIMIT_REACHED);
+    if (stat_v->rxBytes + stat_v->txBytes + L3_bytes > *limit_v) TC_PUNT(LIMIT_REACHED);
 
     if (!is_ethernet) {
         // Try to inject an ethernet header, and simply return if we fail.
@@ -540,7 +540,7 @@
     if (updatetime) v->last_used = bpf_ktime_get_boot_ns();
 
     __sync_fetch_and_add(downstream ? &stat_v->rxPackets : &stat_v->txPackets, packets);
-    __sync_fetch_and_add(downstream ? &stat_v->rxBytes : &stat_v->txBytes, bytes);
+    __sync_fetch_and_add(downstream ? &stat_v->rxBytes : &stat_v->txBytes, L3_bytes);
 
     // Redirect to forwarded interface.
     //
diff --git a/framework-t/src/android/net/NetworkTemplate.java b/framework-t/src/android/net/NetworkTemplate.java
index b3c70cf..9d0476e 100644
--- a/framework-t/src/android/net/NetworkTemplate.java
+++ b/framework-t/src/android/net/NetworkTemplate.java
@@ -61,6 +61,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -194,8 +195,22 @@
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.TIRAMISU,
             publicAlternatives = "Use {@code Builder} instead.")
     public static NetworkTemplate buildTemplateMobileAll(@NonNull String subscriberId) {
+        final Set<String> set;
+        // Prevent from crash for b/273963543, where the OEMs still call into this method
+        // with null subscriberId and get crashed.
+        final int firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT;
+        if (firstSdk > Build.VERSION_CODES.TIRAMISU && subscriberId == null) {
+            throw new IllegalArgumentException("buildTemplateMobileAll does not accept null"
+                    + " subscriberId on Android U devices or above");
+        }
+        if (subscriberId == null) {
+            set = new HashSet<>();
+            set.add(null);
+        } else {
+            set = Set.of(subscriberId);
+        }
         return new NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES)
-                .setSubscriberIds(Set.of(subscriberId)).build();
+                .setSubscriberIds(set).build();
     }
 
     /**
@@ -410,12 +425,20 @@
             // subscriber ID.
             case MATCH_CARRIER:
                 if (matchSubscriberIds.length == 0) {
-                    throw new IllegalArgumentException("checkValidMatchSubscriberIds with empty"
-                            + " list of ids for rule" + getMatchRuleName(matchRule));
+                    throw new IllegalArgumentException("matchSubscriberIds may not contain"
+                            + " null for rule " + getMatchRuleName(matchRule));
                 }
-                // fall through
-            case MATCH_MOBILE:
                 if (CollectionUtils.contains(matchSubscriberIds, null)) {
+                    throw new IllegalArgumentException("matchSubscriberIds may not contain"
+                            + " null for rule " + getMatchRuleName(matchRule));
+                }
+                break;
+            case MATCH_MOBILE:
+                // Prevent from crash for b/273963543, where the OEMs still call into unsupported
+                // buildTemplateMobileAll with null subscriberId and get crashed.
+                final int firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT;
+                if (firstSdk > Build.VERSION_CODES.TIRAMISU
+                        && CollectionUtils.contains(matchSubscriberIds, null)) {
                     throw new IllegalArgumentException("checkValidMatchSubscriberIds list of ids"
                             + " may not contain null for rule " + getMatchRuleName(matchRule));
                 }
diff --git a/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp b/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp
index 3d22aac..dc5732f 100644
--- a/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp
+++ b/service-t/native/libs/libnetworkstats/NetworkTraceHandler.cpp
@@ -136,10 +136,12 @@
 }
 
 void NetworkTraceHandler::OnStart(const StartArgs&) {
+  if (mIsTest) return;  // Don't touch non-hermetic bpf in test.
   mStarted = sPoller.Start(mPollMs);
 }
 
 void NetworkTraceHandler::OnStop(const StopArgs&) {
+  if (mIsTest) return;  // Don't touch non-hermetic bpf in test.
   if (mStarted) sPoller.Stop();
   mStarted = false;
 }
diff --git a/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp b/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp
index 27dce75..f2c1a86 100644
--- a/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp
+++ b/service-t/native/libs/libnetworkstats/NetworkTraceHandlerTest.cpp
@@ -33,14 +33,6 @@
 using ::perfetto::protos::TracePacket;
 using ::perfetto::protos::TrafficDirection;
 
-// This handler makes OnStart and OnStop a no-op so that tracing is not really
-// started on the device.
-class HandlerForTest : public NetworkTraceHandler {
- public:
-  void OnStart(const StartArgs&) override {}
-  void OnStop(const StopArgs&) override {}
-};
-
 class NetworkTraceHandlerTest : public testing::Test {
  protected:
   // Starts a tracing session with the handler under test.
@@ -52,7 +44,7 @@
 
     perfetto::DataSourceDescriptor dsd;
     dsd.set_name("test.network_packets");
-    HandlerForTest::Register(dsd);
+    NetworkTraceHandler::Register(dsd, /*isTest=*/true);
 
     perfetto::TraceConfig cfg;
     cfg.add_buffers()->set_size_kb(1024);
@@ -94,7 +86,7 @@
                            std::vector<TracePacket>* output,
                            NetworkPacketTraceConfig config = {}) {
     auto session = StartTracing(config);
-    HandlerForTest::Trace([&](HandlerForTest::TraceContext ctx) {
+    NetworkTraceHandler::Trace([&](NetworkTraceHandler::TraceContext ctx) {
       ctx.GetDataSourceLocked()->Write(input, ctx);
       ctx.Flush();
     });
@@ -357,7 +349,7 @@
 
   auto session = StartTracing(config);
 
-  HandlerForTest::Trace([&](HandlerForTest::TraceContext ctx) {
+  NetworkTraceHandler::Trace([&](NetworkTraceHandler::TraceContext ctx) {
     ctx.GetDataSourceLocked()->Write(inputs[0], ctx);
     ctx.GetDataSourceLocked()->Write(inputs[1], ctx);
     ctx.GetDataSourceLocked()->Write(inputs[2], ctx);
diff --git a/service-t/native/libs/libnetworkstats/NetworkTracePoller.cpp b/service-t/native/libs/libnetworkstats/NetworkTracePoller.cpp
index 3abb49a..5cf6262 100644
--- a/service-t/native/libs/libnetworkstats/NetworkTracePoller.cpp
+++ b/service-t/native/libs/libnetworkstats/NetworkTracePoller.cpp
@@ -99,6 +99,10 @@
     ALOGW("Failed to disable tracing: %s", res.error().message().c_str());
   }
 
+  // make sure everything in the system has actually seen the 'false' we just wrote
+  synchronizeKernelRCU();
+  // things should now be well and truly disabled
+
   mTaskRunner.reset();
   mRingBuffer.reset();
 
diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h
index 9bc777b..bc10e68 100644
--- a/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h
+++ b/service-t/native/libs/libnetworkstats/include/netdbpf/NetworkTraceHandler.h
@@ -70,6 +70,9 @@
   // Connects to the system Perfetto daemon and registers the trace handler.
   static void InitPerfettoTracing();
 
+  // When isTest is true, skip non-hermetic code.
+  NetworkTraceHandler(bool isTest = false) : mIsTest(isTest) {}
+
   // perfetto::DataSource overrides:
   void OnSetup(const SetupArgs& args) override;
   void OnStart(const StartArgs&) override;
@@ -92,6 +95,7 @@
 
   static internal::NetworkTracePoller sPoller;
   bool mStarted;
+  bool mIsTest;
 
   // Values from config, see proto for details.
   uint32_t mPollMs;
diff --git a/tests/common/java/android/net/netstats/NetworkTemplateTest.kt b/tests/common/java/android/net/netstats/NetworkTemplateTest.kt
index fb6759e..fd7bd74 100644
--- a/tests/common/java/android/net/netstats/NetworkTemplateTest.kt
+++ b/tests/common/java/android/net/netstats/NetworkTemplateTest.kt
@@ -30,16 +30,17 @@
 import android.net.NetworkTemplate.MATCH_WIFI
 import android.net.NetworkTemplate.NETWORK_TYPE_ALL
 import android.net.NetworkTemplate.OEM_MANAGED_ALL
+import android.os.Build
 import android.telephony.TelephonyManager
 import com.android.testutils.ConnectivityModuleTest
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.SC_V2
+import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.JUnit4
-import kotlin.test.assertEquals
-import kotlin.test.assertFailsWith
 
 private const val TEST_IMSI1 = "imsi"
 private const val TEST_WIFI_KEY1 = "wifiKey1"
@@ -96,9 +97,27 @@
         }
 
         // Verify carrier and mobile template cannot contain one of subscriber Id is null.
-        listOf(MATCH_MOBILE, MATCH_CARRIER).forEach {
+        assertFailsWith<IllegalArgumentException> {
+            NetworkTemplate.Builder(MATCH_CARRIER).setSubscriberIds(setOf(null)).build()
+        }
+        val firstSdk = Build.VERSION.DEVICE_INITIAL_SDK_INT
+        if (firstSdk > Build.VERSION_CODES.TIRAMISU) {
             assertFailsWith<IllegalArgumentException> {
-                NetworkTemplate.Builder(it).setSubscriberIds(setOf(null)).build()
+                NetworkTemplate.Builder(MATCH_MOBILE).setSubscriberIds(setOf(null)).build()
+            }
+        } else {
+            NetworkTemplate.Builder(MATCH_MOBILE).setSubscriberIds(setOf(null)).build().let {
+                val expectedTemplate = NetworkTemplate(
+                    MATCH_MOBILE,
+                    arrayOfNulls<String>(1) /*subscriberIds*/,
+                    emptyArray<String>() /*wifiNetworkKey*/,
+                    METERED_ALL,
+                    ROAMING_ALL,
+                    DEFAULT_NETWORK_ALL,
+                    NETWORK_TYPE_ALL,
+                    OEM_MANAGED_ALL
+                )
+                assertEquals(expectedTemplate, it)
             }
         }
 
diff --git a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
index b6902b5..c28ee64 100755
--- a/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
+++ b/tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java
@@ -1274,6 +1274,31 @@
     }
 
     @Test
+    public void testSocketClosed() throws Exception {
+        assumeTrue(supportedHardware());
+
+        final FileDescriptor localFd = openSocketFd(TEST_HOST, 80, TIMEOUT_MS);
+        final List<FileDescriptor> remoteFds = new ArrayList<>();
+
+        for (int i = 0; i < 30; i++) {
+            remoteFds.add(openSocketFdInOtherApp(TEST_HOST, 80, TIMEOUT_MS));
+        }
+
+        final String allowedApps = mRemoteSocketFactoryClient.getPackageName() + "," + mPackageName;
+        startVpn(new String[] {"192.0.2.2/32", "2001:db8:1:2::ffe/128"},
+                new String[] {"192.0.2.0/24", "2001:db8::/32"},
+                allowedApps, "", null, null /* underlyingNetworks */, false /* isAlwaysMetered */);
+
+        // Socket owned by VPN uid is not closed
+        assertSocketStillOpen(localFd, TEST_HOST);
+
+        // Sockets not owned by VPN uid are closed
+        for (final FileDescriptor remoteFd: remoteFds) {
+            assertSocketClosed(remoteFd, TEST_HOST);
+        }
+    }
+
+    @Test
     public void testExcludedRoutes() throws Exception {
         assumeTrue(supportedHardware());
         assumeTrue(SdkLevel.isAtLeastT());
diff --git a/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java b/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java
index 603779d..3ca4775 100644
--- a/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java
+++ b/tests/cts/hostside/src/com/android/cts/net/HostsideVpnTests.java
@@ -51,6 +51,10 @@
         runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testAppDisallowed");
     }
 
+    public void testSocketClosed() throws Exception {
+        runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testSocketClosed");
+    }
+
     public void testGetConnectionOwnerUidSecurity() throws Exception {
         runDeviceTests(TEST_PKG, TEST_PKG + ".VpnTest", "testGetConnectionOwnerUidSecurity");
     }
diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
index 774176f..e90e15a 100644
--- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
+++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
@@ -2409,6 +2409,7 @@
         }
     }
 
+    @AppModeFull(reason = "Cannot get WifiManager in instant app mode")
     @Test
     public void testBlockedStatusCallback() throws Exception {
         // Cannot use @IgnoreUpTo(Build.VERSION_CODES.R) because this test also requires API 31
diff --git a/tests/unit/java/android/net/NetworkTemplateTest.kt b/tests/unit/java/android/net/NetworkTemplateTest.kt
index fc25fd8..2f6c76b 100644
--- a/tests/unit/java/android/net/NetworkTemplateTest.kt
+++ b/tests/unit/java/android/net/NetworkTemplateTest.kt
@@ -50,16 +50,17 @@
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRunner
 import com.android.testutils.assertParcelSane
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mockito.mock
 import org.mockito.Mockito.`when`
 import org.mockito.MockitoAnnotations
-import kotlin.test.assertEquals
-import kotlin.test.assertFalse
-import kotlin.test.assertNotEquals
-import kotlin.test.assertTrue
 
 private const val TEST_IMSI1 = "imsi1"
 private const val TEST_IMSI2 = "imsi2"
@@ -70,6 +71,8 @@
 @RunWith(DevSdkIgnoreRunner::class)
 @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
 class NetworkTemplateTest {
+    @get:Rule
+    val ignoreRule = DevSdkIgnoreRule()
     private val mockContext = mock(Context::class.java)
     private val mockWifiInfo = mock(WifiInfo::class.java)
 
@@ -215,6 +218,18 @@
         templateNullWifiKey.assertDoesNotMatch(identWifiNullKey)
     }
 
+    @DevSdkIgnoreRule.IgnoreAfter(Build.VERSION_CODES.TIRAMISU)
+    @Test
+    fun testBuildTemplateMobileAll_nullSubscriberId() {
+        val templateMobileAllWithNullImsi = buildTemplateMobileAll(null)
+        val setWithNull = HashSet<String?>().apply {
+            add(null)
+        }
+        val templateFromBuilder = NetworkTemplate.Builder(MATCH_MOBILE).setMeteredness(METERED_YES)
+            .setSubscriberIds(setWithNull).build()
+        assertEquals(templateFromBuilder, templateMobileAllWithNullImsi)
+    }
+
     @Test
     fun testMobileMatches() {
         val templateMobileImsi1 = buildTemplateMobileAll(TEST_IMSI1)