diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index de76e89c79..1df3e585f4 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -61,6 +61,7 @@ import androidx.annotation.Nullable; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.IndentingPrintWriter; import com.android.modules.utils.build.SdkLevel; +import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.NetworkStackConstants; import com.android.net.module.util.Struct; import com.android.net.module.util.netlink.ConntrackMessage; @@ -133,6 +134,7 @@ public class BpfCoordinator { // List of TCP port numbers which aren't offloaded because the packets require the netfilter // conntrack helper. See also TetherController::setForwardRules in netd. + @VisibleForTesting static final short [] NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS = new short [] { 21 /* ftp */, 1723 /* pptp */}; @@ -1561,17 +1563,14 @@ public class BpfCoordinator { 0 /* lastUsed, filled by bpf prog only */); } - private boolean requireOffload(ConntrackEvent e) { + private boolean allowOffload(ConntrackEvent e) { if (e.tupleOrig.protoNum != OsConstants.IPPROTO_TCP) return true; - - for (final short port : NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS) { - if (port == e.tupleOrig.dstPort) return false; - } - return true; + return !CollectionUtils.contains( + NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS, e.tupleOrig.dstPort); } public void accept(ConntrackEvent e) { - if (!requireOffload(e)) return; + if (!allowOffload(e)) return; final ClientInfo tetherClient = getClientInfo(e.tupleOrig.srcIp); if (tetherClient == null) return; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java index 6e9608581d..acc042b147 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -43,6 +43,7 @@ import static com.android.net.module.util.netlink.NetlinkConstants.IPCTNL_MSG_CT import static com.android.networkstack.tethering.BpfCoordinator.CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS; import static com.android.networkstack.tethering.BpfCoordinator.NF_CONNTRACK_TCP_TIMEOUT_ESTABLISHED; import static com.android.networkstack.tethering.BpfCoordinator.NF_CONNTRACK_UDP_TIMEOUT_STREAM; +import static com.android.networkstack.tethering.BpfCoordinator.NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS; import static com.android.networkstack.tethering.BpfCoordinator.StatsType; import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_IFACE; import static com.android.networkstack.tethering.BpfCoordinator.StatsType.STATS_PER_UID; @@ -51,6 +52,7 @@ import static com.android.networkstack.tethering.BpfUtils.UPSTREAM; import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -95,6 +97,7 @@ import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.dx.mockito.inline.extended.ExtendedMockito; +import com.android.net.module.util.CollectionUtils; import com.android.net.module.util.NetworkStackConstants; import com.android.net.module.util.Struct; import com.android.net.module.util.netlink.ConntrackMessage; @@ -1369,7 +1372,7 @@ public class BpfCoordinatorTest { } @NonNull - private ConntrackEvent makeTestConntrackEvent(short msgType, int proto) { + private ConntrackEvent makeTestConntrackEvent(short msgType, int proto, short remotePort) { if (msgType != IPCTNL_MSG_CT_NEW && msgType != IPCTNL_MSG_CT_DELETE) { fail("Not support message type " + msgType); } @@ -1383,13 +1386,18 @@ public class BpfCoordinatorTest { return new ConntrackEvent( (short) (NetlinkConstants.NFNL_SUBSYS_CTNETLINK << 8 | msgType), new Tuple(new TupleIpv4(PRIVATE_ADDR, REMOTE_ADDR), - new TupleProto((byte) proto, PRIVATE_PORT, REMOTE_PORT)), + new TupleProto((byte) proto, PRIVATE_PORT, remotePort)), new Tuple(new TupleIpv4(REMOTE_ADDR, PUBLIC_ADDR), - new TupleProto((byte) proto, REMOTE_PORT, PUBLIC_PORT)), + new TupleProto((byte) proto, remotePort, PUBLIC_PORT)), status, timeoutSec); } + @NonNull + private ConntrackEvent makeTestConntrackEvent(short msgType, int proto) { + return makeTestConntrackEvent(msgType, proto, REMOTE_PORT); + } + private void setUpstreamInformationTo(final BpfCoordinator coordinator) { final LinkProperties lp = new LinkProperties(); lp.setInterfaceName(UPSTREAM_IFACE); @@ -1563,14 +1571,14 @@ public class BpfCoordinatorTest { bpfMap.insertEntry(tcpKey, tcpValue); bpfMap.insertEntry(udpKey, udpValue); - // [1] Don't refresh contrack timeout. + // [1] Don't refresh conntrack timeout. setElapsedRealtimeNanos(expiredTime); mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS); waitForIdle(); ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class)); ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class)); - // [2] Refresh contrack timeout. + // [2] Refresh conntrack timeout. setElapsedRealtimeNanos(validTime); mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS); waitForIdle(); @@ -1587,7 +1595,7 @@ public class BpfCoordinatorTest { ExtendedMockito.verifyNoMoreInteractions(staticMockMarker(NetlinkSocket.class)); ExtendedMockito.clearInvocations(staticMockMarker(NetlinkSocket.class)); - // [3] Don't refresh contrack timeout if polling stopped. + // [3] Don't refresh conntrack timeout if polling stopped. coordinator.stopPolling(); mTestLooper.moveTimeForward(CONNTRACK_TIMEOUT_UPDATE_INTERVAL_MS); waitForIdle(); @@ -1629,4 +1637,39 @@ public class BpfCoordinatorTest { checkRefreshConntrackTimeout(bpfDownstream4Map, tcpKey, tcpValue, udpKey, udpValue); } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + public void testNotAllowOffloadByConntrackMessageDestinationPort() throws Exception { + final BpfCoordinator coordinator = makeBpfCoordinator(); + initBpfCoordinatorForRule4(coordinator); + + final short offloadedPort = 42; + assertFalse(CollectionUtils.contains(NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS, + offloadedPort)); + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_NEW, IPPROTO_TCP, offloadedPort)); + verify(mBpfUpstream4Map).insertEntry(any(), any()); + verify(mBpfDownstream4Map).insertEntry(any(), any()); + clearInvocations(mBpfUpstream4Map, mBpfDownstream4Map); + + for (final short port : NON_OFFLOADED_UPSTREAM_IPV4_TCP_PORTS) { + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_NEW, IPPROTO_TCP, port)); + verify(mBpfUpstream4Map, never()).insertEntry(any(), any()); + verify(mBpfDownstream4Map, never()).insertEntry(any(), any()); + + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_DELETE, IPPROTO_TCP, port)); + verify(mBpfUpstream4Map, never()).deleteEntry(any()); + verify(mBpfDownstream4Map, never()).deleteEntry(any()); + + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_NEW, IPPROTO_UDP, port)); + verify(mBpfUpstream4Map).insertEntry(any(), any()); + verify(mBpfDownstream4Map).insertEntry(any(), any()); + clearInvocations(mBpfUpstream4Map, mBpfDownstream4Map); + + mConsumer.accept(makeTestConntrackEvent(IPCTNL_MSG_CT_DELETE, IPPROTO_UDP, port)); + verify(mBpfUpstream4Map).deleteEntry(any()); + verify(mBpfDownstream4Map).deleteEntry(any()); + clearInvocations(mBpfUpstream4Map, mBpfDownstream4Map); + } + } }