Refactor the offload permission check and add tests
- Rename the conntrack destination port check function - Use CollectionUtils.contains to check the denied ports - Add tests for the streams with ftp and pptp tcp port are not able to be offloaded Bug: 195914327 Test: atest TetheringCoverageTests Change-Id: I7e2591bea1f6db46271efb0c30970fb8d4efe1e4
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user