Merge "IpUtils - make checksum work with empty/zero buffer" am: efdaea7829

Original change: https://android-review.googlesource.com/c/platform/frameworks/libs/net/+/2540110

Change-Id: Ie507dccc471a641c16c26c6db99f2a3a58491563
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 23 insertions, 7 deletions

diff --git a/common/framework/com/android/net/module/util/IpUtils.java b/common/framework/com/android/net/module/util/IpUtils.java
index 569733ed..18d96f30 100644
--- a/common/framework/com/android/net/module/util/IpUtils.java
+++ b/common/framework/com/android/net/module/util/IpUtils.java
@@ -16,6 +16,8 @@
 
 package com.android.net.module.util;
 
+import com.android.internal.annotations.VisibleForTesting;
+
 import static android.system.OsConstants.IPPROTO_ICMPV6;
 import static android.system.OsConstants.IPPROTO_TCP;
 import static android.system.OsConstants.IPPROTO_UDP;
@@ -42,8 +44,9 @@ public class IpUtils {
      * payload) or ICMP checksum on the specified portion of a ByteBuffer.  The seed
      * allows the checksum to commence with a specified value.
      */
-    private static int checksum(ByteBuffer buf, int seed, int start, int end) {
-        int sum = seed;
+    @VisibleForTesting
+    public static int checksum(ByteBuffer buf, int seed, int start, int end) {
+        int sum = seed + 0xFFFF;  // to make things work with empty / zero-filled buffer
         final int bufPosition = buf.position();
 
         // set position of original ByteBuffer, so that the ShortBuffer
@@ -69,13 +72,12 @@ public class IpUtils {
                 b += 256;
             }
 
-            sum += b * 256;
+            sum += b * 256;  // assumes bytebuffer is network order (ie. big endian)
         }
 
-        sum = ((sum >> 16) & 0xFFFF) + (sum & 0xFFFF);
-        sum = ((sum + ((sum >> 16) & 0xFFFF)) & 0xFFFF);
-        int negated = ~sum;
-        return intAbs((short) negated);
+        sum = ((sum >> 16) & 0xFFFF) + (sum & 0xFFFF);  // max sum is 0x1FFFE
+        sum = ((sum >> 16) & 0xFFFF) + (sum & 0xFFFF);  // max sum is 0xFFFF
+        return sum ^ 0xFFFF;  // u16 bitwise negation
     }
 
     private static int pseudoChecksumIPv4(
diff --git a/common/tests/unit/src/com/android/net/module/util/IpUtilsTest.java b/common/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
index 20555b35..d57023ca 100644
--- a/common/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
+++ b/common/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
@@ -74,6 +74,20 @@ public class IpUtilsTest {
     // print JavaPacketDefinition(str(packet))
 
     @Test
+    public void testEmptyAndZeroBufferChecksum() throws Exception {
+        ByteBuffer packet = ByteBuffer.wrap(new byte[] { (byte) 0x00, (byte) 0x00, });
+        // the following should *not* return 0xFFFF
+        assertEquals(0, IpUtils.checksum(packet, 0, 0, 0));
+        assertEquals(0, IpUtils.checksum(packet, 0, 0, 1));
+        assertEquals(0, IpUtils.checksum(packet, 0, 0, 2));
+        assertEquals(0, IpUtils.checksum(packet, 0, 1, 2));
+        assertEquals(0, IpUtils.checksum(packet, 0xFFFF, 0, 0));
+        assertEquals(0, IpUtils.checksum(packet, 0xFFFF, 0, 1));
+        assertEquals(0, IpUtils.checksum(packet, 0xFFFF, 0, 2));
+        assertEquals(0, IpUtils.checksum(packet, 0xFFFF, 1, 2));
+    }
+
+    @Test
     public void testIpv6TcpChecksum() throws Exception {
         // packet = (scapy.IPv6(src="2001:db8::1", dst="2001:db8::2", tc=0x80) /
         //           scapy.TCP(sport=12345, dport=7,