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:
Hungming Chen
2021-08-20 17:53:31 +08:00
parent 0084591e94
commit b344870ea0
2 changed files with 55 additions and 13 deletions

View File

@@ -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;

View File

@@ -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);
}
}
}