Fix two parsing bugs in new DHCP client.

1. We don't parse PAD options properly, leading in failure to
   parse packets sent by DHCP servers that put the end of options
   marker after pad options and at an odd offset.
2. We get the DhcpResults vendorInfo from the wrong option type
   (60 instead of 43).

Fix these and add unit tests for the offer packets sent by a few
different DHCP servers.

Bug: 21955617
Bug: 22281295
Change-Id: I5d13f1a6a3ff0b53112f18f3db8792fa32ad2da3

1 files changed, 141 insertions, 9 deletions

diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
index e0e3fcf..ba1231f 100644
--- a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
+++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
@@ -25,22 +25,27 @@ import junit.framework.TestCase;
 
 import java.net.Inet4Address;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+
+import libcore.util.HexEncoding;
 
 import static android.net.dhcp.DhcpPacket.*;
 
 
 public class DhcpPacketTest extends TestCase {
 
-    private static Inet4Address SERVER_ADDR =
-            (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");
-    private static Inet4Address CLIENT_ADDR =
-            (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.234");
+    private static Inet4Address SERVER_ADDR = v4Address("192.0.2.1");
+    private static Inet4Address CLIENT_ADDR = v4Address("192.0.2.234");
     // Use our own empty address instead of Inet4Address.ANY or INADDR_ANY to ensure that the code
     // doesn't use == instead of equals when comparing addresses.
-    private static Inet4Address ANY = (Inet4Address) NetworkUtils.numericToInetAddress("0.0.0.0");
+    private static Inet4Address ANY = (Inet4Address) v4Address("0.0.0.0");
 
     private static byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
 
+    private static final Inet4Address v4Address(String addrString) throws IllegalArgumentException {
+        return (Inet4Address) NetworkUtils.numericToInetAddress(addrString);
+    }
+
     class TestDhcpPacket extends DhcpPacket {
         private byte mType;
         // TODO: Make this a map of option numbers to bytes instead.
@@ -89,7 +94,7 @@ public class DhcpPacketTest extends TestCase {
                 addTlv(buffer, DHCP_DOMAIN_NAME, mDomainBytes);
             }
             if (mVendorInfoBytes != null) {
-                addTlv(buffer, DHCP_VENDOR_CLASS_ID, mVendorInfoBytes);
+                addTlv(buffer, DHCP_VENDOR_INFO, mVendorInfoBytes);
             }
             if (mLeaseTimeBytes != null) {
                 addTlv(buffer, DHCP_LEASE_TIME, mLeaseTimeBytes);
@@ -118,7 +123,7 @@ public class DhcpPacketTest extends TestCase {
                 .build();
         DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
         assertEquals(expectedDomain, offerPacket.mDomainName);
-        assertEquals(expectedVendorInfo, offerPacket.mVendorId);
+        assertEquals(expectedVendorInfo, offerPacket.mVendorInfo);
     }
 
     @SmallTest
@@ -221,8 +226,8 @@ public class DhcpPacketTest extends TestCase {
         byte[] slash11Netmask = new byte[] { (byte) 0xff, (byte) 0xe0, 0x00, 0x00 };
         byte[] slash24Netmask = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x00 };
         byte[] invalidNetmask = new byte[] { (byte) 0xff, (byte) 0xfb, (byte) 0xff, 0x00 };
-        Inet4Address example1 = (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");
-        Inet4Address example2 = (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.43");
+        Inet4Address example1 = v4Address("192.0.2.1");
+        Inet4Address example2 = v4Address("192.0.2.43");
 
         // A packet without any addresses is not valid.
         checkIpAddress(null, ANY, ANY, slash24Netmask);
@@ -238,4 +243,131 @@ public class DhcpPacketTest extends TestCase {
         // If there is no netmask, implicit netmasks are used.
         checkIpAddress("192.0.2.43/24", ANY, example2, null);
     }
+
+    private void assertDhcpResults(String ipAddress, String gateway, String dnsServersString,
+            String domains, String serverAddress, String vendorInfo, int leaseDuration,
+            boolean hasMeteredHint, DhcpResults dhcpResults) throws Exception {
+        assertEquals(new LinkAddress(ipAddress), dhcpResults.ipAddress);
+        assertEquals(v4Address(gateway), dhcpResults.gateway);
+
+        String[] dnsServerStrings = dnsServersString.split(",");
+        ArrayList dnsServers = new ArrayList();
+        for (String dnsServerString : dnsServerStrings) {
+            dnsServers.add(v4Address(dnsServerString));
+        }
+        assertEquals(dnsServers, dhcpResults.dnsServers);
+
+        assertEquals(domains, dhcpResults.domains);
+        assertEquals(v4Address(serverAddress), dhcpResults.serverAddress);
+        assertEquals(vendorInfo, dhcpResults.vendorInfo);
+        assertEquals(leaseDuration, dhcpResults.leaseDuration);
+        assertEquals(hasMeteredHint, dhcpResults.hasMeteredHint());
+    }
+
+    @SmallTest
+    public void testOffer1() throws Exception {
+        // TODO: Turn all of these into golden files. This will probably require modifying
+        // Android.mk appropriately, making this into an AndroidTestCase, and adding code to read
+        // the golden files from the test APK's assets via mContext.getAssets().
+        final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
+            // IP header.
+            "451001480000000080118849c0a89003c0a89ff7" +
+            // UDP header.
+            "004300440134dcfa" +
+            // BOOTP header.
+            "02010600c997a63b0000000000000000c0a89ff70000000000000000" +
+            // MAC address.
+            "30766ff2a90c00000000000000000000" +
+            // Server name.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // File.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // Options
+            "638253633501023604c0a89003330400001c200104fffff0000304c0a89ffe06080808080808080404" +
+            "3a0400000e103b040000189cff00000000000000000000"
+        ).toCharArray(), false));
+
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
+        assertTrue(offerPacket instanceof DhcpOfferPacket);  // Implicitly checks it's non-null.
+        DhcpResults dhcpResults = offerPacket.toDhcpResults();
+        assertDhcpResults("192.168.159.247/20", "192.168.159.254", "8.8.8.8,8.8.4.4",
+                null, "192.168.144.3", null, 7200, false, dhcpResults);
+    }
+
+    @SmallTest
+    public void testOffer2() throws Exception {
+        final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
+            // IP header.
+            "450001518d0600004011144dc0a82b01c0a82bf7" +
+            // UDP header.
+            "00430044013d9ac7" +
+            // BOOTP header.
+            "02010600dfc23d1f0002000000000000c0a82bf7c0a82b0100000000" +
+            // MAC address.
+            "30766ff2a90c00000000000000000000" +
+            // Server name.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // File.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // Options
+            "638253633501023604c0a82b01330400000e103a04000007083b0400000c4e0104ffffff00" +
+            "1c04c0a82bff0304c0a82b010604c0a82b012b0f414e44524f49445f4d455445524544ff"
+        ).toCharArray(), false));
+
+        assertEquals(337, packet.limit());
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
+        assertTrue(offerPacket instanceof DhcpOfferPacket);  // Implicitly checks it's non-null.
+        DhcpResults dhcpResults = offerPacket.toDhcpResults();
+        assertDhcpResults("192.168.43.247/24", "192.168.43.1", "192.168.43.1",
+                null, "192.168.43.1", "ANDROID_METERED", 3600, true, dhcpResults);
+        assertTrue(dhcpResults.hasMeteredHint());
+    }
+
+    @SmallTest
+    public void testPadAndOverloadedOptionsOffer() throws Exception {
+        // A packet observed in the real world that is interesting for two reasons:
+        //
+        // 1. It uses pad bytes, which we previously didn't support correctly.
+        // 2. It uses DHCP option overloading, which we don't currently support (but it doesn't
+        //    store any information in the overloaded fields).
+        //
+        // For now, we just check that it parses correctly.
+        final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
+            // Ethernet header.
+            "b4cef6000000e80462236e300800" +
+            // IP header.
+            "4500014c00000000ff11741701010101ac119876" +
+            // UDP header. TODO: fix invalid checksum (due to MAC address obfuscation).
+            "004300440138ae5a" +
+            // BOOTP header.
+            "020106000fa0059f0000000000000000ac1198760000000000000000" +
+            // MAC address.
+            "b4cef600000000000000000000000000" +
+            // Server name.
+            "ff00000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // File.
+            "ff00000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // Options
+            "638253633501023604010101010104ffff000033040000a8c03401030304ac1101010604ac110101" +
+            "0000000000000000000000000000000000000000000000ff000000"
+        ).toCharArray(), false));
+
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L2);
+        assertTrue(offerPacket instanceof DhcpOfferPacket);
+        DhcpResults dhcpResults = offerPacket.toDhcpResults();
+        assertDhcpResults("172.17.152.118/16", "172.17.1.1", "172.17.1.1",
+                null, "1.1.1.1", null, 43200, false, dhcpResults);
+    }
 }