commit 5f95a67eef006f9cef5369ff4dd334fd4efaa3b1
author Maciej Żenczykowski <maze@google.com> Mon Apr 17 09:10:14 2023 +0000
committer Maciej Żenczykowski <maze@google.com> Wed Apr 19 08:17:29 2023 +0000
tree cc03e9b98b09b2de46670004344689fd246f0aed
parent 2632888a107a15e7ef92208e080d0a9200175730

IpUtils - make checksum work with empty/zero buffer

also:
- document buffer endianness assumption
- simplify fold logic - it cannot overflow

Test: TreeHugger, atest NetworkStaticLibTests
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I521a7377d1310c0e9c46a5dff95dca451e5f889e

staticlibs/framework/com/android/net/module/util/IpUtils.java

diff --git a/staticlibs/framework/com/android/net/module/util/IpUtils.java b/staticlibs/framework/com/android/net/module/util/IpUtils.java
index 569733e..18d96f3 100644
--- a/staticlibs/framework/com/android/net/module/util/IpUtils.java
+++ b/staticlibs/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 @@
      * 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 @@
                 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(

staticlibs/tests/unit/src/com/android/net/module/util/IpUtilsTest.java

diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/IpUtilsTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
index 20555b3..d57023c 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/IpUtilsTest.java
@@ -74,6 +74,20 @@
     // 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,