From 721772d97eb9297bd098aa2d9243872e74a0fa97 Mon Sep 17 00:00:00 2001 From: Yan Yan Date: Wed, 21 Aug 2024 16:22:17 -0700 Subject: [PATCH 1/2] Add awaitEspPacket that requires sequence number match This is a preparation patch for improving testRequestIpSecTransformStateForTx This patch: - Adds another awaitEspPacket method that requires sequence number match - Defines ICMP header length in the PacketUtils Bug: 358324865 Test: atest IpSecManagerTunnelTest Flag: Exempt test code only Change-Id: I5365c4daf983cc98f8f869a30cee1799138fa2b3 --- .../net/src/android/net/cts/PacketUtils.java | 1 + .../cts/net/src/android/net/cts/TunUtils.java | 37 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/PacketUtils.java b/tests/cts/net/src/android/net/cts/PacketUtils.java index 4d924d1582..1588835aff 100644 --- a/tests/cts/net/src/android/net/cts/PacketUtils.java +++ b/tests/cts/net/src/android/net/cts/PacketUtils.java @@ -49,6 +49,7 @@ public class PacketUtils { static final int TCP_HDRLEN = 20; static final int TCP_HDRLEN_WITH_TIMESTAMP_OPT = TCP_HDRLEN + 12; static final int ESP_HDRLEN = 8; + static final int ICMP_HDRLEN = 8; static final int ESP_BLK_SIZE = 4; // ESP has to be 4-byte aligned static final int ESP_TRAILER_LEN = 2; diff --git a/tests/cts/net/src/android/net/cts/TunUtils.java b/tests/cts/net/src/android/net/cts/TunUtils.java index 268d8d2603..8bf4998c29 100644 --- a/tests/cts/net/src/android/net/cts/TunUtils.java +++ b/tests/cts/net/src/android/net/cts/TunUtils.java @@ -47,6 +47,8 @@ public class TunUtils { protected static final int IP4_PROTO_OFFSET = 9; protected static final int IP6_PROTO_OFFSET = 6; + private static final int SEQ_NUM_MATCH_NOT_REQUIRED = -1; + private static final int DATA_BUFFER_LEN = 4096; private static final int TIMEOUT = 2000; @@ -146,16 +148,30 @@ public class TunUtils { return espPkt; // We've found the packet we're looking for. } + /** Await the expected ESP packet */ public byte[] awaitEspPacket(int spi, boolean useEncap) throws Exception { - return awaitPacket((pkt) -> isEsp(pkt, spi, useEncap)); + return awaitEspPacket(spi, useEncap, SEQ_NUM_MATCH_NOT_REQUIRED); } - private static boolean isSpiEqual(byte[] pkt, int espOffset, int spi) { + /** Await the expected ESP packet with a matching sequence number */ + public byte[] awaitEspPacket(int spi, boolean useEncap, int seqNum) throws Exception { + return awaitPacket((pkt) -> isEsp(pkt, spi, seqNum, useEncap)); + } + + private static boolean isMatchingEspPacket(byte[] pkt, int espOffset, int spi, int seqNum) { ByteBuffer buffer = ByteBuffer.wrap(pkt); buffer.get(new byte[espOffset]); // Skip IP, UDP header int actualSpi = buffer.getInt(); + int actualSeqNum = buffer.getInt(); - return actualSpi == spi; + if (actualSeqNum < 0) { + throw new UnsupportedOperationException( + "actualSeqNum overflowed and needs to be converted to an unsigned integer"); + } + + boolean isSeqNumMatched = (seqNum == SEQ_NUM_MATCH_NOT_REQUIRED || seqNum == actualSeqNum); + + return actualSpi == spi && isSeqNumMatched; } /** @@ -173,29 +189,32 @@ public class TunUtils { fail("Banned plaintext packet found"); } - return isEsp(pkt, spi, encap); + return isEsp(pkt, spi, SEQ_NUM_MATCH_NOT_REQUIRED, encap); } - private static boolean isEsp(byte[] pkt, int spi, boolean encap) { + private static boolean isEsp(byte[] pkt, int spi, int seqNum, boolean encap) { if (isIpv6(pkt)) { if (encap) { return pkt[IP6_PROTO_OFFSET] == IPPROTO_UDP - && isSpiEqual(pkt, IP6_HDRLEN + UDP_HDRLEN, spi); + && isMatchingEspPacket(pkt, IP6_HDRLEN + UDP_HDRLEN, spi, seqNum); } else { - return pkt[IP6_PROTO_OFFSET] == IPPROTO_ESP && isSpiEqual(pkt, IP6_HDRLEN, spi); + return pkt[IP6_PROTO_OFFSET] == IPPROTO_ESP + && isMatchingEspPacket(pkt, IP6_HDRLEN, spi, seqNum); } } else { // Use default IPv4 header length (assuming no options) if (encap) { return pkt[IP4_PROTO_OFFSET] == IPPROTO_UDP - && isSpiEqual(pkt, IP4_HDRLEN + UDP_HDRLEN, spi); + && isMatchingEspPacket(pkt, IP4_HDRLEN + UDP_HDRLEN, spi, seqNum); } else { - return pkt[IP4_PROTO_OFFSET] == IPPROTO_ESP && isSpiEqual(pkt, IP4_HDRLEN, spi); + return pkt[IP4_PROTO_OFFSET] == IPPROTO_ESP + && isMatchingEspPacket(pkt, IP4_HDRLEN, spi, seqNum); } } } + public static boolean isIpv6(byte[] pkt) { // First nibble shows IP version. 0x60 for IPv6 return (pkt[0] & (byte) 0xF0) == (byte) 0x60; From 3479e57fd288fec7a735fad0cd2204bfca24cfdd Mon Sep 17 00:00:00 2001 From: Yan Yan Date: Tue, 20 Aug 2024 16:23:16 -0700 Subject: [PATCH 2/2] Handle ICMP in testRequestIpSecTransformStateForTx testRequestIpSecTransformStateForTs was flaky because sometimes ICMPv6(Router Solicitation) might be sent and failed the validation of packet/byte count. This CL provides two improvements - Change the validation to handle possible ICMP packets - In the await method, require sequence number match to ensure the expected packet has been sent out. Otherwise the await method will still return true on an old packet with the same SPI. Bug: 358324865 Test: atest IpSecManagerTunnelTest Flag: Exempt test code only Change-Id: I471014c11328bc0f32ee1f805accbaf7b29b8e7c --- .../src/android/net/cts/IpSecBaseTest.java | 16 ++++- .../net/cts/IpSecManagerTunnelTest.java | 70 ++++++++++++++----- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/IpSecBaseTest.java b/tests/cts/net/src/android/net/cts/IpSecBaseTest.java index 2a6c638243..c48013542f 100644 --- a/tests/cts/net/src/android/net/cts/IpSecBaseTest.java +++ b/tests/cts/net/src/android/net/cts/IpSecBaseTest.java @@ -23,10 +23,13 @@ import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA256; import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA384; import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA512; import static android.net.IpSecAlgorithm.CRYPT_AES_CBC; +import static android.net.cts.PacketUtils.ICMP_HDRLEN; +import static android.net.cts.PacketUtils.IP6_HDRLEN; import static android.system.OsConstants.FIONREAD; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import android.content.Context; import android.net.ConnectivityManager; @@ -152,10 +155,17 @@ public class IpSecBaseTest { final IpSecTransformState transformState = futureIpSecTransform.get(SOCK_TIMEOUT, TimeUnit.MILLISECONDS); - assertEquals(txHighestSeqNum, transformState.getTxHighestSequenceNumber()); + // There might be ICMPv6(Router Solicitation) packets. Thus we can only check the lower + // bound of the outgoing traffic. + final long icmpV6RsCnt = transformState.getTxHighestSequenceNumber() - txHighestSeqNum; + assertTrue(icmpV6RsCnt >= 0); + + final long adjustedPacketCnt = packetCnt + icmpV6RsCnt; + final long adjustedByteCnt = byteCnt + icmpV6RsCnt * (IP6_HDRLEN + ICMP_HDRLEN); + + assertEquals(adjustedPacketCnt, transformState.getPacketCount()); + assertEquals(adjustedByteCnt, transformState.getByteCount()); assertEquals(rxHighestSeqNum, transformState.getRxHighestSequenceNumber()); - assertEquals(packetCnt, transformState.getPacketCount()); - assertEquals(byteCnt, transformState.getByteCount()); assertArrayEquals(replayBitmap, transformState.getReplayBitmap()); } diff --git a/tests/cts/net/src/android/net/cts/IpSecManagerTunnelTest.java b/tests/cts/net/src/android/net/cts/IpSecManagerTunnelTest.java index 22a51d64a2..890c07134b 100644 --- a/tests/cts/net/src/android/net/cts/IpSecManagerTunnelTest.java +++ b/tests/cts/net/src/android/net/cts/IpSecManagerTunnelTest.java @@ -65,6 +65,7 @@ import android.platform.test.annotations.AppModeFull; import androidx.test.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; +import com.android.net.module.util.CollectionUtils; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; @@ -119,7 +120,7 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { private static final int TIMEOUT_MS = 500; - private static final int PACKET_COUNT = 5000; + private static final int PACKET_COUNT = 100; // Static state to reduce setup/teardown private static ConnectivityManager sCM; @@ -1088,6 +1089,27 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { UdpEncapsulationSocket encapSocket, IpSecTunnelTestRunnable test) throws Exception { + return buildTunnelNetworkAndRunTests( + localInner, + remoteInner, + localOuter, + remoteOuter, + spi, + encapSocket, + test, + true /* enableEncrypt */); + } + + private int buildTunnelNetworkAndRunTests( + InetAddress localInner, + InetAddress remoteInner, + InetAddress localOuter, + InetAddress remoteOuter, + int spi, + UdpEncapsulationSocket encapSocket, + IpSecTunnelTestRunnable test, + boolean enableEncrypt) + throws Exception { int innerPrefixLen = localInner instanceof Inet6Address ? IP6_PREFIX_LEN : IP4_PREFIX_LEN; TestNetworkCallback testNetworkCb = null; int innerSocketPort; @@ -1115,8 +1137,12 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { // Configure Transform parameters IpSecTransform.Builder transformBuilder = new IpSecTransform.Builder(sContext); - transformBuilder.setEncryption( - new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY)); + + if (enableEncrypt) { + transformBuilder.setEncryption( + new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY)); + } + transformBuilder.setAuthentication( new IpSecAlgorithm( IpSecAlgorithm.AUTH_HMAC_SHA256, AUTH_KEY, AUTH_KEY.length * 4)); @@ -1167,8 +1193,8 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { return innerSocketPort; } - private int buildTunnelNetworkAndRunTestsSimple(int spi, IpSecTunnelTestRunnable test) - throws Exception { + private int buildTunnelNetworkAndRunTestsSimple( + int spi, IpSecTunnelTestRunnable test, boolean enableEncrypt) throws Exception { return buildTunnelNetworkAndRunTests( LOCAL_INNER_6, REMOTE_INNER_6, @@ -1176,7 +1202,8 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { REMOTE_OUTER_6, spi, null /* encapSocket */, - test); + test, + enableEncrypt); } private static void receiveAndValidatePacket(JavaUdpSocket socket) throws Exception { @@ -1787,10 +1814,11 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { PACKET_COUNT, PACKET_COUNT, PACKET_COUNT * (long) innerPacketSize, - newReplayBitmap(REPLAY_BITMAP_LEN_BYTE * 8)); + newReplayBitmap(PACKET_COUNT)); return innerSocketPort; - }); + }, + true /* enableEncrypt */); } @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) @@ -1814,17 +1842,22 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { ipsecNetwork.bindSocket(outSocket.mSocket); int innerSocketPort = outSocket.getPort(); - int expectedPacketSize = - getPacketSize( - AF_INET6, - AF_INET6, - false /* useEncap */, - false /* transportInTunnelMode */); + int outSeqNum = 1; + int receivedTestDataEspCnt = 0; - for (int i = 0; i < PACKET_COUNT; i++) { + while (receivedTestDataEspCnt < PACKET_COUNT) { outSocket.sendTo(TEST_DATA, REMOTE_INNER_6, innerSocketPort); - tunUtils.awaitEspPacketNoPlaintext( - spi, TEST_DATA, false /* useEncap */, expectedPacketSize); + + byte[] pkt = null; + + // If it is an ESP that contains the TEST_DATA, move to the next + // loop. Otherwise, the ESP may contain an ICMPv6(Router Solicitation). + // In this case, just increase the expected sequence number and continue + // waiting for the ESP with TEST_DATA + do { + pkt = tunUtils.awaitEspPacket(spi, false /* useEncap */, outSeqNum++); + } while (CollectionUtils.indexOfSubArray(pkt, TEST_DATA) == -1); + receivedTestDataEspCnt++; } final int innerPacketSize = @@ -1838,6 +1871,7 @@ public class IpSecManagerTunnelTest extends IpSecBaseTest { newReplayBitmap(0)); return innerSocketPort; - }); + }, + false /* enableEncrypt */); } }