Merge changes Id6a0b0de,I5f03b8b2,I62464b92 into mnc-dr-dev

* changes:
  Support DHCP replies with multiple default gateways.
  Accept DHCP responses from non-67 server source ports
  Improve logging of DHCP parse errors using exceptions.

5 files changed, 164 insertions, 48 deletions

diff --git a/services/net/java/android/net/dhcp/DhcpAckPacket.java b/services/net/java/android/net/dhcp/DhcpAckPacket.java
index 334f708..df44b11 100644
--- a/services/net/java/android/net/dhcp/DhcpAckPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpAckPacket.java
@@ -46,7 +46,7 @@ class DhcpAckPacket extends DhcpPacket {
 
         return s + " ACK: your new IP " + mYourIp +
                 ", netmask " + mSubnetMask +
-                ", gateway " + mGateway + dnsServers +
+                ", gateways " + mGateways + dnsServers +
                 ", lease time " + mLeaseTime;
     }
 
@@ -79,7 +79,7 @@ class DhcpAckPacket extends DhcpPacket {
         }
 
         addTlv(buffer, DHCP_SUBNET_MASK, mSubnetMask);
-        addTlv(buffer, DHCP_ROUTER, mGateway);
+        addTlv(buffer, DHCP_ROUTER, mGateways);
         addTlv(buffer, DHCP_DOMAIN_NAME, mDomainName);
         addTlv(buffer, DHCP_BROADCAST_ADDRESS, mBroadcastAddress);
         addTlv(buffer, DHCP_DNS_SERVER, mDnsServers);
diff --git a/services/net/java/android/net/dhcp/DhcpClient.java b/services/net/java/android/net/dhcp/DhcpClient.java
index e0d2ac1..3bf117f 100644
--- a/services/net/java/android/net/dhcp/DhcpClient.java
+++ b/services/net/java/android/net/dhcp/DhcpClient.java
@@ -345,21 +345,22 @@ public class DhcpClient extends BaseDhcpStateMachine {
         public void run() {
             maybeLog("Receive thread started");
             while (!stopped) {
+                int length = 0;  // Or compiler can't tell it's initialized if a parse error occurs.
                 try {
-                    int length = Os.read(mPacketSock, mPacket, 0, mPacket.length);
+                    length = Os.read(mPacketSock, mPacket, 0, mPacket.length);
                     DhcpPacket packet = null;
                     packet = DhcpPacket.decodeFullPacket(mPacket, length, DhcpPacket.ENCAP_L2);
-                    if (packet != null) {
-                        maybeLog("Received packet: " + packet);
-                        sendMessage(CMD_RECEIVED_PACKET, packet);
-                    } else if (PACKET_DBG) {
-                        Log.d(TAG,
-                                "Can't parse packet" + HexDump.dumpHexString(mPacket, 0, length));
-                    }
+                    maybeLog("Received packet: " + packet);
+                    sendMessage(CMD_RECEIVED_PACKET, packet);
                 } catch (IOException|ErrnoException e) {
                     if (!stopped) {
                         Log.e(TAG, "Read error", e);
                     }
+                } catch (DhcpPacket.ParseException e) {
+                    Log.e(TAG, "Can't parse packet: " + e.getMessage());
+                    if (PACKET_DBG) {
+                        Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length));
+                    }
                 }
             }
             maybeLog("Receive thread stopped");
diff --git a/services/net/java/android/net/dhcp/DhcpOfferPacket.java b/services/net/java/android/net/dhcp/DhcpOfferPacket.java
index 7ca7100..99154ef 100644
--- a/services/net/java/android/net/dhcp/DhcpOfferPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpOfferPacket.java
@@ -48,7 +48,7 @@ class DhcpOfferPacket extends DhcpPacket {
         }
 
         return s + " OFFER, ip " + mYourIp + ", mask " + mSubnetMask +
-                dnsServers + ", gateway " + mGateway +
+                dnsServers + ", gateways " + mGateways +
                 " lease time " + mLeaseTime + ", domain " + mDomainName;
     }
 
@@ -81,7 +81,7 @@ class DhcpOfferPacket extends DhcpPacket {
         }
 
         addTlv(buffer, DHCP_SUBNET_MASK, mSubnetMask);
-        addTlv(buffer, DHCP_ROUTER, mGateway);
+        addTlv(buffer, DHCP_ROUTER, mGateways);
         addTlv(buffer, DHCP_DOMAIN_NAME, mDomainName);
         addTlv(buffer, DHCP_BROADCAST_ADDRESS, mBroadcastAddress);
         addTlv(buffer, DHCP_DNS_SERVER, mDnsServers);
diff --git a/services/net/java/android/net/dhcp/DhcpPacket.java b/services/net/java/android/net/dhcp/DhcpPacket.java
index cbf8fc2..8927bfa 100644
--- a/services/net/java/android/net/dhcp/DhcpPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpPacket.java
@@ -114,6 +114,11 @@ abstract class DhcpPacket {
     protected static final int MAX_LENGTH = 1500;
 
     /**
+     * The magic cookie that identifies this as a DHCP packet instead of BOOTP.
+     */
+    private static final int DHCP_MAGIC_COOKIE = 0x63825363;
+
+    /**
      * DHCP Optional Type: DHCP Subnet Mask
      */
     protected static final byte DHCP_SUBNET_MASK = 1;
@@ -123,7 +128,7 @@ abstract class DhcpPacket {
      * DHCP Optional Type: DHCP Router
      */
     protected static final byte DHCP_ROUTER = 3;
-    protected Inet4Address mGateway;
+    protected List <Inet4Address> mGateways;
 
     /**
      * DHCP Optional Type: DHCP DNS Server
@@ -403,7 +408,7 @@ abstract class DhcpPacket {
                      (HWADDR_LEN - mClientMac.length) // pad addr to 16 bytes
                      + 64     // empty server host name (64 bytes)
                      + 128);  // empty boot file name (128 bytes)
-        buf.putInt(0x63825363); // magic number
+        buf.putInt(DHCP_MAGIC_COOKIE); // magic number
         finishPacket(buf);
 
         // round up to an even number of octets
@@ -668,6 +673,20 @@ abstract class DhcpPacket {
         return new String(bytes, 0, length, StandardCharsets.US_ASCII);
     }
 
+    private static boolean isPacketToOrFromClient(short udpSrcPort, short udpDstPort) {
+        return (udpSrcPort == DHCP_CLIENT) || (udpDstPort == DHCP_CLIENT);
+    }
+
+    private static boolean isPacketServerToServer(short udpSrcPort, short udpDstPort) {
+        return (udpSrcPort == DHCP_SERVER) && (udpDstPort == DHCP_SERVER);
+    }
+
+    public static class ParseException extends Exception {
+        public ParseException(String msg, Object... args) {
+            super(String.format(msg, args));
+        }
+    }
+
     /**
      * Creates a concrete DhcpPacket from the supplied ByteBuffer.  The
      * buffer may have an L2 encapsulation (which is the full EthernetII
@@ -677,7 +696,7 @@ abstract class DhcpPacket {
      * A subset of the optional parameters are parsed and are stored
      * in object fields.
      */
-    public static DhcpPacket decodeFullPacket(ByteBuffer packet, int pktType)
+    public static DhcpPacket decodeFullPacket(ByteBuffer packet, int pktType) throws ParseException
     {
         // bootp parameters
         int transactionId;
@@ -687,8 +706,8 @@ abstract class DhcpPacket {
         Inet4Address nextIp;
         Inet4Address relayIp;
         byte[] clientMac;
-        List<Inet4Address> dnsServers = new ArrayList<Inet4Address>();
-        Inet4Address gateway = null; // aka router
+        List<Inet4Address> dnsServers = new ArrayList<>();
+        List<Inet4Address> gateways = new ArrayList<>();  // aka router
         Inet4Address serverIdentifier = null;
         Inet4Address netMask = null;
         String message = null;
@@ -720,7 +739,8 @@ abstract class DhcpPacket {
         // check to see if we need to parse L2, IP, and UDP encaps
         if (pktType == ENCAP_L2) {
             if (packet.remaining() < MIN_PACKET_LENGTH_L2) {
-                return null;
+                throw new ParseException("L2 packet too short, %d < %d",
+                        packet.remaining(), MIN_PACKET_LENGTH_L2);
             }
 
             byte[] l2dst = new byte[6];
@@ -732,18 +752,20 @@ abstract class DhcpPacket {
             short l2type = packet.getShort();
 
             if (l2type != OsConstants.ETH_P_IP)
-                return null;
+                throw new ParseException("Unexpected L2 type 0x%04x, expected 0x%04x",
+                        l2type, OsConstants.ETH_P_IP);
         }
 
         if (pktType <= ENCAP_L3) {
             if (packet.remaining() < MIN_PACKET_LENGTH_L3) {
-                return null;
+                throw new ParseException("L3 packet too short, %d < %d",
+                        packet.remaining(), MIN_PACKET_LENGTH_L3);
             }
 
             byte ipTypeAndLength = packet.get();
             int ipVersion = (ipTypeAndLength & 0xf0) >> 4;
             if (ipVersion != 4) {
-                return null;
+                throw new ParseException("Invalid IP version %d", ipVersion);
             }
 
             // System.out.println("ipType is " + ipType);
@@ -759,8 +781,9 @@ abstract class DhcpPacket {
             ipSrc = readIpAddress(packet);
             ipDst = readIpAddress(packet);
 
-            if (ipProto != IP_TYPE_UDP) // UDP
-                return null;
+            if (ipProto != IP_TYPE_UDP) {
+                throw new ParseException("Protocol not UDP: %d", ipProto);
+            }
 
             // Skip options. This cannot cause us to read beyond the end of the buffer because the
             // IPv4 header cannot be more than (0x0f * 4) = 60 bytes long, and that is less than
@@ -776,13 +799,19 @@ abstract class DhcpPacket {
             short udpLen = packet.getShort();
             short udpChkSum = packet.getShort();
 
-            if ((udpSrcPort != DHCP_SERVER) && (udpSrcPort != DHCP_CLIENT))
+            // Only accept packets to or from the well-known client port (expressly permitting
+            // packets from ports other than the well-known server port; http://b/24687559), and
+            // server-to-server packets, e.g. for relays.
+            if (!isPacketToOrFromClient(udpSrcPort, udpDstPort) &&
+                !isPacketServerToServer(udpSrcPort, udpDstPort)) {
                 return null;
+            }
         }
 
         // We need to check the length even for ENCAP_L3 because the IPv4 header is variable-length.
         if (pktType > ENCAP_BOOTP || packet.remaining() < MIN_PACKET_LENGTH_BOOTP) {
-            return null;
+            throw new ParseException("Invalid type or BOOTP packet too short, %d < %d",
+                        packet.remaining(), MIN_PACKET_LENGTH_BOOTP);
         }
 
         byte type = packet.get();
@@ -805,7 +834,7 @@ abstract class DhcpPacket {
             packet.get(ipv4addr);
             relayIp = (Inet4Address) Inet4Address.getByAddress(ipv4addr);
         } catch (UnknownHostException ex) {
-            return null;
+            throw new ParseException("Invalid IPv4 address: %s", Arrays.toString(ipv4addr));
         }
 
         // Some DHCP servers have been known to announce invalid client hardware address values such
@@ -828,8 +857,10 @@ abstract class DhcpPacket {
 
         int dhcpMagicCookie = packet.getInt();
 
-        if (dhcpMagicCookie !=  0x63825363)
-            return null;
+        if (dhcpMagicCookie != DHCP_MAGIC_COOKIE) {
+            throw new ParseException("Bad magic cookie 0x%08x, should be 0x%08x", dhcpMagicCookie,
+                    DHCP_MAGIC_COOKIE);
+        }
 
         // parse options
         boolean notFinishedOptions = true;
@@ -852,8 +883,9 @@ abstract class DhcpPacket {
                             expectedLen = 4;
                             break;
                         case DHCP_ROUTER:
-                            gateway = readIpAddress(packet);
-                            expectedLen = 4;
+                            for (expectedLen = 0; expectedLen < optionLen; expectedLen += 4) {
+                                gateways.add(readIpAddress(packet));
+                            }
                             break;
                         case DHCP_DNS_SERVER:
                             for (expectedLen = 0; expectedLen < optionLen; expectedLen += 4) {
@@ -937,18 +969,20 @@ abstract class DhcpPacket {
                     }
 
                     if (expectedLen != optionLen) {
-                        return null;
+                        throw new ParseException("Invalid length %d for option %d, expected %d",
+                                optionLen, optionType, expectedLen);
                     }
                 }
             } catch (BufferUnderflowException e) {
-                return null;
+                throw new ParseException("BufferUnderflowException");
             }
         }
 
         DhcpPacket newPacket;
 
         switch(dhcpType) {
-            case -1: return null;
+            case (byte) 0xFF:
+                throw new ParseException("No DHCP message type option");
             case DHCP_MESSAGE_TYPE_DISCOVER:
                 newPacket = new DhcpDiscoverPacket(
                     transactionId, secs, clientMac, broadcast);
@@ -981,14 +1015,13 @@ abstract class DhcpPacket {
                     clientMac);
                 break;
             default:
-                System.out.println("Unimplemented type: " + dhcpType);
-                return null;
+                throw new ParseException("Unimplemented DHCP type %d", dhcpType);
         }
 
         newPacket.mBroadcastAddress = bcAddr;
         newPacket.mDnsServers = dnsServers;
         newPacket.mDomainName = domainName;
-        newPacket.mGateway = gateway;
+        newPacket.mGateways = gateways;
         newPacket.mHostName = hostName;
         newPacket.mLeaseTime = leaseTime;
         newPacket.mMessage = message;
@@ -1009,7 +1042,7 @@ abstract class DhcpPacket {
      * Parse a packet from an array of bytes, stopping at the given length.
      */
     public static DhcpPacket decodeFullPacket(byte[] packet, int length, int pktType)
-    {
+            throws ParseException {
         ByteBuffer buffer = ByteBuffer.wrap(packet, 0, length).order(ByteOrder.BIG_ENDIAN);
         return decodeFullPacket(buffer, pktType);
     }
@@ -1044,7 +1077,11 @@ abstract class DhcpPacket {
         } catch (IllegalArgumentException e) {
             return null;
         }
-        results.gateway = mGateway;
+
+        if (mGateways.size() > 0) {
+            results.gateway = mGateways.get(0);
+        }
+
         results.dnsServers.addAll(mDnsServers);
         results.domains = mDomainName;
         results.serverAddress = mServerIdentifier;
@@ -1086,11 +1123,11 @@ abstract class DhcpPacket {
     public static ByteBuffer buildOfferPacket(int encap, int transactionId,
         boolean broadcast, Inet4Address serverIpAddr, Inet4Address clientIpAddr,
         byte[] mac, Integer timeout, Inet4Address netMask, Inet4Address bcAddr,
-        Inet4Address gateway, List<Inet4Address> dnsServers,
+        List<Inet4Address> gateways, List<Inet4Address> dnsServers,
         Inet4Address dhcpServerIdentifier, String domainName) {
         DhcpPacket pkt = new DhcpOfferPacket(
             transactionId, (short) 0, broadcast, serverIpAddr, INADDR_ANY, clientIpAddr, mac);
-        pkt.mGateway = gateway;
+        pkt.mGateways = gateways;
         pkt.mDnsServers = dnsServers;
         pkt.mLeaseTime = timeout;
         pkt.mDomainName = domainName;
@@ -1106,11 +1143,11 @@ abstract class DhcpPacket {
     public static ByteBuffer buildAckPacket(int encap, int transactionId,
         boolean broadcast, Inet4Address serverIpAddr, Inet4Address clientIpAddr,
         byte[] mac, Integer timeout, Inet4Address netMask, Inet4Address bcAddr,
-        Inet4Address gateway, List<Inet4Address> dnsServers,
+        List<Inet4Address> gateways, List<Inet4Address> dnsServers,
         Inet4Address dhcpServerIdentifier, String domainName) {
         DhcpPacket pkt = new DhcpAckPacket(
             transactionId, (short) 0, broadcast, serverIpAddr, INADDR_ANY, clientIpAddr, mac);
-        pkt.mGateway = gateway;
+        pkt.mGateways = gateways;
         pkt.mDnsServers = dnsServers;
         pkt.mLeaseTime = timeout;
         pkt.mDomainName = domainName;
diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
index cd3b8bb..7e60bf1 100644
--- a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
+++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
@@ -117,7 +117,7 @@ public class DhcpPacketTest extends TestCase {
 
     private void assertDomainAndVendorInfoParses(
             String expectedDomain, byte[] domainBytes,
-            String expectedVendorInfo, byte[] vendorInfoBytes) {
+            String expectedVendorInfo, byte[] vendorInfoBytes) throws Exception {
         ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER)
                 .setDomainBytes(domainBytes)
                 .setVendorInfoBytes(vendorInfoBytes)
@@ -158,17 +158,25 @@ public class DhcpPacketTest extends TestCase {
     }
 
     private void assertLeaseTimeParses(boolean expectValid, Integer rawLeaseTime,
-                                       long leaseTimeMillis, byte[] leaseTimeBytes) {
+            long leaseTimeMillis, byte[] leaseTimeBytes) throws Exception {
         TestDhcpPacket testPacket = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER);
         if (leaseTimeBytes != null) {
             testPacket.setLeaseTimeBytes(leaseTimeBytes);
         }
         ByteBuffer packet = testPacket.build();
-        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
+        DhcpPacket offerPacket = null;
+
         if (!expectValid) {
-            assertNull(offerPacket);
+            try {
+                offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
+                fail("Invalid packet parsed successfully: " + offerPacket);
+            } catch (ParseException expected) {
+            }
             return;
         }
+
+        offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
+        assertNotNull(offerPacket);
         assertEquals(rawLeaseTime, offerPacket.mLeaseTime);
         DhcpResults dhcpResults = offerPacket.toDhcpResults();  // Just check this doesn't crash.
         assertEquals(leaseTimeMillis, offerPacket.getLeaseTimeMillis());
@@ -200,14 +208,14 @@ public class DhcpPacketTest extends TestCase {
     }
 
     private void checkIpAddress(String expected, Inet4Address clientIp, Inet4Address yourIp,
-                                byte[] netmaskBytes) {
+                                byte[] netmaskBytes) throws Exception {
         checkIpAddress(expected, DHCP_MESSAGE_TYPE_OFFER, clientIp, yourIp, netmaskBytes);
         checkIpAddress(expected, DHCP_MESSAGE_TYPE_ACK, clientIp, yourIp, netmaskBytes);
     }
 
     private void checkIpAddress(String expected, byte type,
                                 Inet4Address clientIp, Inet4Address yourIp,
-                                byte[] netmaskBytes) {
+                                byte[] netmaskBytes) throws Exception {
         ByteBuffer packet = new TestDhcpPacket(type, clientIp, yourIp)
                 .setNetmaskBytes(netmaskBytes)
                 .build();
@@ -506,4 +514,74 @@ public class DhcpPacketTest extends TestCase {
         assertDhcpResults("10.32.158.205/20", "10.32.144.1", "148.88.65.52,148.88.65.53",
                 "lancs.ac.uk", "10.32.255.128", null, 7200, false, dhcpResults);
     }
+
+    @SmallTest
+    public void testUdpServerAnySourcePort() throws Exception {
+        final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
+            // Ethernet header.
+            "9cd917000000001c2e0000000800" +
+            // IP header.
+            "45a00148000040003d115087d18194fb0a0f7af2" +
+            // UDP header. TODO: fix invalid checksum (due to MAC address obfuscation).
+            // NOTE: The server source port is not the canonical port 67.
+            "C29F004401341268" +
+            // BOOTP header.
+            "02010600d628ba8200000000000000000a0f7af2000000000a0fc818" +
+            // MAC address.
+            "9cd91700000000000000000000000000" +
+            // Server name.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // File.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // Options.
+            "6382536335010236040a0169fc3304000151800104ffff000003040a0fc817060cd1818003d1819403" +
+            "d18180060f0777766d2e6564751c040a0fffffff000000"
+        ).toCharArray(), false));
+
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L2);
+        assertTrue(offerPacket instanceof DhcpOfferPacket);
+        assertEquals("9CD917000000", HexDump.toHexString(offerPacket.getClientMac()));
+        DhcpResults dhcpResults = offerPacket.toDhcpResults();
+        assertDhcpResults("10.15.122.242/16", "10.15.200.23",
+                "209.129.128.3,209.129.148.3,209.129.128.6",
+                "wvm.edu", "10.1.105.252", null, 86400, false, dhcpResults);
+    }
+
+    @SmallTest
+    public void testMultipleRouters() throws Exception {
+        final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
+            // Ethernet header.
+            "fc3d93000000" + "081735000000" + "0800" +
+            // IP header.
+            "45000148c2370000ff117ac2c0a8bd02ffffffff" +
+            // UDP header. TODO: fix invalid checksum (due to MAC address obfuscation).
+            "0043004401343beb" +
+            // BOOTP header.
+            "0201060027f518e20000800000000000c0a8bd310000000000000000" +
+            // MAC address.
+            "fc3d9300000000000000000000000000" +
+            // Server name.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // File.
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            "0000000000000000000000000000000000000000000000000000000000000000" +
+            // Options.
+            "638253633501023604c0abbd023304000070803a04000038403b04000062700104ffffff00" +
+            "0308c0a8bd01ffffff0006080808080808080404ff000000000000"
+        ).toCharArray(), false));
+
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L2);
+        assertTrue(offerPacket instanceof DhcpOfferPacket);
+        assertEquals("FC3D93000000", HexDump.toHexString(offerPacket.getClientMac()));
+        DhcpResults dhcpResults = offerPacket.toDhcpResults();
+        assertDhcpResults("192.168.189.49/24", "192.168.189.1", "8.8.8.8,8.8.4.4",
+                null, "192.171.189.2", null, 28800, false, dhcpResults);
+    }
 }