From 84e3fb3afd39a3761df73444c48dd614bebdd15b Mon Sep 17 00:00:00 2001 From: KH Shi Date: Thu, 16 Nov 2023 13:32:36 +0000 Subject: [PATCH] Fix endian error when pass upstream prefixes to bpf TetherUpstream6Key#src64 should be stored in network order. Use a big-endian ByteBuffer to retrieve a value stored in network order is actually converts it into native order, which is little-endian in Android. In this CL, we changed it to use byte[] instead of long to store the top 64-bits of the source prefix. Test: atest TetheringTests manual test with IPv6-only tethering upstream, check BPF stats. Bug: 312072637 Change-Id: I79f9282d5eda28328aa6a764ea92b086d6285133 --- .../tethering/BpfCoordinator.java | 19 ++++--- .../tethering/TetherUpstream6Key.java | 6 +- .../tethering/BpfCoordinatorTest.java | 56 ++++++++++++++++--- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java index 4bd7e6af51..9f542f4aef 100644 --- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java +++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java @@ -90,7 +90,6 @@ import java.net.InetAddress; import java.net.NetworkInterface; import java.net.SocketException; import java.net.UnknownHostException; -import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -1258,12 +1257,18 @@ public class BpfCoordinator { pw.decreaseIndent(); } - private IpPrefix longToPrefix(long ip64) { - final ByteBuffer prefixBuffer = ByteBuffer.allocate(IPV6_ADDR_LEN); - prefixBuffer.putLong(ip64); + /** + * Returns a /64 IpPrefix corresponding to the passed in byte array + * + * @param ip64 byte array to convert format + * @return the converted IpPrefix + */ + @VisibleForTesting + public static IpPrefix bytesToPrefix(final byte[] ip64) { IpPrefix sourcePrefix; + byte[] prefixBytes = Arrays.copyOf(ip64, IPV6_ADDR_LEN); try { - sourcePrefix = new IpPrefix(InetAddress.getByAddress(prefixBuffer.array()), 64); + sourcePrefix = new IpPrefix(InetAddress.getByAddress(prefixBytes), 64); } catch (UnknownHostException e) { // Cannot happen. InetAddress.getByAddress can only throw an exception if the byte array // is the wrong length, but we allocate it with fixed length IPV6_ADDR_LEN. @@ -1274,7 +1279,7 @@ public class BpfCoordinator { private String ipv6UpstreamRuleToString(TetherUpstream6Key key, Tether6Value value) { return String.format("%d(%s) [%s] [%s] -> %d(%s) %04x [%s] [%s]", - key.iif, getIfName(key.iif), key.dstMac, longToPrefix(key.src64), value.oif, + key.iif, getIfName(key.iif), key.dstMac, bytesToPrefix(key.src64), value.oif, getIfName(value.oif), value.ethProto, value.ethSrcMac, value.ethDstMac); } @@ -1570,7 +1575,7 @@ public class BpfCoordinator { */ @NonNull public TetherUpstream6Key makeTetherUpstream6Key() { - long prefix64 = ByteBuffer.wrap(sourcePrefix.getRawAddress()).getLong(); + final byte[] prefix64 = Arrays.copyOf(sourcePrefix.getRawAddress(), 8); return new TetherUpstream6Key(downstreamIfindex, inDstMac, prefix64); } diff --git a/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java b/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java index 36a1c3c3ec..0cc3dd983a 100644 --- a/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java +++ b/Tethering/src/com/android/networkstack/tethering/TetherUpstream6Key.java @@ -32,10 +32,10 @@ public class TetherUpstream6Key extends Struct { @Field(order = 1, type = Type.EUI48, padding = 6) public final MacAddress dstMac; // Destination ethernet mac address (zeroed iff rawip ingress). - @Field(order = 2, type = Type.S64) - public final long src64; // The top 64-bits of the source ip. + @Field(order = 2, type = Type.ByteArray, arraysize = 8) + public final byte[] src64; // The top 64-bits of the source ip. - public TetherUpstream6Key(int iif, @NonNull final MacAddress dstMac, long src64) { + public TetherUpstream6Key(int iif, @NonNull final MacAddress dstMac, final byte[] src64) { Objects.requireNonNull(dstMac); this.iif = iif; 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 6c1721e9b9..01600b88bb 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/BpfCoordinatorTest.java @@ -64,6 +64,7 @@ import static com.android.networkstack.tethering.BpfUtils.UPSTREAM; import static com.android.networkstack.tethering.TetheringConfiguration.DEFAULT_TETHER_OFFLOAD_POLL_INTERVAL_MS; import static com.android.testutils.MiscAsserts.assertSameElements; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -166,7 +167,6 @@ import java.io.StringWriter; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; -import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -794,7 +794,7 @@ public class BpfCoordinatorTest { if (!mDeps.isAtLeastS()) return; ArrayMap expected = new ArrayMap<>(); for (IpPrefix upstreamPrefix : upstreamPrefixes) { - long prefix64 = prefixToLong(upstreamPrefix); + final byte[] prefix64 = prefixToIp64(upstreamPrefix); final TetherUpstream6Key key = new TetherUpstream6Key(DOWNSTREAM_IFACE_PARAMS.index, DOWNSTREAM_IFACE_PARAMS.macAddr, prefix64); final Tether6Value value = new Tether6Value(upstreamIfindex, @@ -822,7 +822,7 @@ public class BpfCoordinatorTest { if (!mDeps.isAtLeastS()) return; Set expected = new ArraySet<>(); for (IpPrefix upstreamPrefix : upstreamPrefixes) { - long prefix64 = prefixToLong(upstreamPrefix); + final byte[] prefix64 = prefixToIp64(upstreamPrefix); final TetherUpstream6Key key = new TetherUpstream6Key(DOWNSTREAM_IFACE_PARAMS.index, DOWNSTREAM_IFACE_PARAMS.macAddr, prefix64); expected.add(key); @@ -1275,7 +1275,7 @@ public class BpfCoordinatorTest { } @Test - public void testRuleMakeTetherDownstream6Key() throws Exception { + public void testIpv6DownstreamRuleMakeTetherDownstream6Key() throws Exception { final int mobileIfIndex = 100; final Ipv6DownstreamRule rule = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A); @@ -1288,7 +1288,7 @@ public class BpfCoordinatorTest { } @Test - public void testRuleMakeTether6Value() throws Exception { + public void testIpv6DownstreamRuleMakeTether6Value() throws Exception { final int mobileIfIndex = 100; final Ipv6DownstreamRule rule = buildTestDownstreamRule(mobileIfIndex, NEIGH_A, MAC_A); @@ -1302,6 +1302,46 @@ public class BpfCoordinatorTest { assertEquals(20, value.writeToBytes().length); } + @Test + public void testIpv6UpstreamRuleMakeTetherUpstream6Key() { + final byte[] bytes = new byte[]{(byte) 0x20, (byte) 0x01, (byte) 0x0d, (byte) 0xb8, + (byte) 0xab, (byte) 0xcd, (byte) 0xfe, (byte) 0x00}; + final IpPrefix prefix = new IpPrefix("2001:db8:abcd:fe00::/64"); + final Ipv6UpstreamRule rule = buildTestUpstreamRule(UPSTREAM_IFINDEX, + DOWNSTREAM_IFINDEX, prefix, DOWNSTREAM_MAC); + + final TetherUpstream6Key key = rule.makeTetherUpstream6Key(); + assertEquals(DOWNSTREAM_IFINDEX, key.iif); + assertEquals(DOWNSTREAM_MAC, key.dstMac); + assertArrayEquals(bytes, key.src64); + // iif (4) + dstMac (6) + padding (6) + src64 (8) = 24 + assertEquals(24, key.writeToBytes().length); + } + + @Test + public void testIpv6UpstreamRuleMakeTether6Value() { + final IpPrefix prefix = new IpPrefix("2001:db8:abcd:fe00::/64"); + final Ipv6UpstreamRule rule = buildTestUpstreamRule(UPSTREAM_IFINDEX, + DOWNSTREAM_IFINDEX, prefix, DOWNSTREAM_MAC); + + final Tether6Value value = rule.makeTether6Value(); + assertEquals(UPSTREAM_IFINDEX, value.oif); + assertEquals(MAC_NULL, value.ethDstMac); + assertEquals(MAC_NULL, value.ethSrcMac); + assertEquals(ETH_P_IPV6, value.ethProto); + assertEquals(NetworkStackConstants.ETHER_MTU, value.pmtu); + // oif (4) + ethDstMac (6) + ethSrcMac (6) + ethProto (2) + pmtu (2) = 20 + assertEquals(20, value.writeToBytes().length); + } + + @Test + public void testBytesToPrefix() { + final byte[] bytes = new byte[]{(byte) 0x20, (byte) 0x01, (byte) 0x0d, (byte) 0xb8, + (byte) 0x00, (byte) 0x00, (byte) 0x12, (byte) 0x34}; + final IpPrefix prefix = new IpPrefix("2001:db8:0:1234::/64"); + assertEquals(prefix, BpfCoordinator.bytesToPrefix(bytes)); + } + @Test public void testSetDataLimit() throws Exception { setupFunctioningNetdInterface(); @@ -2269,8 +2309,8 @@ public class BpfCoordinatorTest { 100 /* nonzero, CT_NEW */); } - private static long prefixToLong(IpPrefix prefix) { - return ByteBuffer.wrap(prefix.getRawAddress()).getLong(); + private static byte[] prefixToIp64(IpPrefix prefix) { + return Arrays.copyOf(prefix.getRawAddress(), 8); } void checkRule4ExistInUpstreamDownstreamMap() throws Exception { @@ -2515,7 +2555,7 @@ public class BpfCoordinatorTest { final Ipv6DownstreamRule rule = buildTestDownstreamRule(UPSTREAM_IFINDEX, NEIGH_A, MAC_A); mBpfDownstream6Map.insertEntry(rule.makeTetherDownstream6Key(), rule.makeTether6Value()); - final long prefix64 = prefixToLong(UPSTREAM_PREFIX); + final byte[] prefix64 = prefixToIp64(UPSTREAM_PREFIX); final TetherUpstream6Key upstream6Key = new TetherUpstream6Key(DOWNSTREAM_IFINDEX, DOWNSTREAM_MAC, prefix64); final Tether6Value upstream6Value = new Tether6Value(UPSTREAM_IFINDEX,