Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f68edb16 authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

Actually fall back from yiaddr to ciaddr. 

The initial implementation of toDhcpResults attempted to get the
leased IP address from ciaddr if yiaddr was 0.0.0.0, but it never
actually did so because a) it used == instead of equals(), and b)
the parsing code never populated mClientIp for a DhcpOfferPacket
or DhcpAckPacket.

Fix this and add a test for it.. Technically DHCP does not use
ciaddr (only bootp uses it), but in 5.0 we would use ciaddr if
yiaddr was 0.0.0.0 and a bit more compatibility shouldn't hurt.

Bug: 19704592
Change-Id: I1f58555f0c10b9c576995a6edb759a83d8938ea0
parent 3a40b0de

services/net/java/android/net/dhcp/DhcpAckPacket.java

+2 −2
Original line number Diff line number Diff line
@@ -30,8 +30,8 @@ class DhcpAckPacket extends DhcpPacket { 
    private final Inet4Address mSrcIp;



    DhcpAckPacket(int transId, short secs, boolean broadcast, Inet4Address serverAddress,

                  Inet4Address clientIp, byte[] clientMac) {

        super(transId, secs, INADDR_ANY, clientIp, serverAddress, INADDR_ANY, clientMac, broadcast);

                  Inet4Address clientIp, Inet4Address yourIp, byte[] clientMac) {

        super(transId, secs, clientIp, yourIp, serverAddress, INADDR_ANY, clientMac, broadcast);

        mBroadcast = broadcast;

        mSrcIp = serverAddress;

    }

services/net/java/android/net/dhcp/DhcpOfferPacket.java

+2 −2
Original line number Diff line number Diff line
@@ -32,8 +32,8 @@ class DhcpOfferPacket extends DhcpPacket { 
     * Generates a OFFER packet with the specified parameters.

     */

    DhcpOfferPacket(int transId, short secs, boolean broadcast, Inet4Address serverAddress,

                    Inet4Address clientIp, byte[] clientMac) {

        super(transId, secs, INADDR_ANY, clientIp, INADDR_ANY, INADDR_ANY, clientMac, broadcast);

                    Inet4Address clientIp, Inet4Address yourIp, byte[] clientMac) {

        super(transId, secs, clientIp, yourIp, INADDR_ANY, INADDR_ANY, clientMac, broadcast);

        mSrcIp = serverAddress;

    }

services/net/java/android/net/dhcp/DhcpPacket.java

+6 −6
Original line number Diff line number Diff line
@@ -919,7 +919,7 @@ abstract class DhcpPacket { 
                break;

            case DHCP_MESSAGE_TYPE_OFFER:

                newPacket = new DhcpOfferPacket(

                    transactionId, secs, broadcast, ipSrc, yourIp, clientMac);

                    transactionId, secs, broadcast, ipSrc, clientIp, yourIp, clientMac);

                break;

            case DHCP_MESSAGE_TYPE_REQUEST:

                newPacket = new DhcpRequestPacket(
@@ -932,7 +932,7 @@ abstract class DhcpPacket { 
                break;

            case DHCP_MESSAGE_TYPE_ACK:

                newPacket = new DhcpAckPacket(

                    transactionId, secs, broadcast, ipSrc, yourIp, clientMac);

                    transactionId, secs, broadcast, ipSrc, clientIp, yourIp, clientMac);

                break;

            case DHCP_MESSAGE_TYPE_NAK:

                newPacket = new DhcpNakPacket(
@@ -982,9 +982,9 @@ abstract class DhcpPacket { 
     */

    public DhcpResults toDhcpResults() {

        Inet4Address ipAddress = mYourIp;

        if (ipAddress == Inet4Address.ANY) {

        if (ipAddress.equals(Inet4Address.ANY)) {

            ipAddress = mClientIp;

            if (ipAddress == Inet4Address.ANY) {

            if (ipAddress.equals(Inet4Address.ANY)) {

                return null;

            }

        }
@@ -1052,7 +1052,7 @@ abstract class DhcpPacket { 
        Inet4Address gateway, List<Inet4Address> dnsServers,

        Inet4Address dhcpServerIdentifier, String domainName) {

        DhcpPacket pkt = new DhcpOfferPacket(

            transactionId, (short) 0, broadcast, serverIpAddr, clientIpAddr, mac);

            transactionId, (short) 0, broadcast, serverIpAddr, INADDR_ANY, clientIpAddr, mac);

        pkt.mGateway = gateway;

        pkt.mDnsServers = dnsServers;

        pkt.mLeaseTime = timeout;
@@ -1072,7 +1072,7 @@ abstract class DhcpPacket { 
        Inet4Address gateway, List<Inet4Address> dnsServers,

        Inet4Address dhcpServerIdentifier, String domainName) {

        DhcpPacket pkt = new DhcpAckPacket(

            transactionId, (short) 0, broadcast, serverIpAddr, clientIpAddr, mac);

            transactionId, (short) 0, broadcast, serverIpAddr, INADDR_ANY, clientIpAddr, mac);

        pkt.mGateway = gateway;

        pkt.mDnsServers = dnsServers;

        pkt.mLeaseTime = timeout;

services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java

+66 −3
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.net.dhcp; 


import android.net.NetworkUtils;

import android.net.DhcpResults;

import android.net.LinkAddress;

import android.system.OsConstants;

import android.test.suitebuilder.annotation.SmallTest;

import junit.framework.TestCase;
@@ -34,19 +35,27 @@ public class DhcpPacketTest extends TestCase { 
            (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");

    private static Inet4Address CLIENT_ADDR =

            (Inet4Address) NetworkUtils.numericToInetAddress("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 byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };



    class TestDhcpPacket extends DhcpPacket {

        private byte mType;

        // TODO: Make this a map of option numbers to bytes instead.

        private byte[] mDomainBytes, mVendorInfoBytes, mLeaseTimeBytes;

        private byte[] mDomainBytes, mVendorInfoBytes, mLeaseTimeBytes, mNetmaskBytes;



        public TestDhcpPacket(byte type) {

            super(0xdeadbeef, (short) 0, INADDR_ANY, CLIENT_ADDR, INADDR_ANY, INADDR_ANY,

        public TestDhcpPacket(byte type, Inet4Address clientIp, Inet4Address yourIp) {

            super(0xdeadbeef, (short) 0, clientIp, yourIp, INADDR_ANY, INADDR_ANY,

                  CLIENT_MAC, true);

            mType = type;

        }



        public TestDhcpPacket(byte type) {

            this(type, INADDR_ANY, CLIENT_ADDR);

        }



        public TestDhcpPacket setDomainBytes(byte[] domainBytes) {

            mDomainBytes = domainBytes;

            return this;
@@ -62,6 +71,11 @@ public class DhcpPacketTest extends TestCase { 
            return this;

        }



        public TestDhcpPacket setNetmaskBytes(byte[] netmaskBytes) {

            mNetmaskBytes = netmaskBytes;

            return this;

        }



        public ByteBuffer buildPacket(int encap, short unusedDestUdp, short unusedSrcUdp) {

            ByteBuffer result = ByteBuffer.allocate(MAX_LENGTH);

            fillInPacket(encap, CLIENT_ADDR, SERVER_ADDR,
@@ -80,6 +94,9 @@ public class DhcpPacketTest extends TestCase { 
            if (mLeaseTimeBytes != null) {

                addTlv(buffer, DHCP_LEASE_TIME, mLeaseTimeBytes);

            }

            if (mNetmaskBytes != null) {

                addTlv(buffer, DHCP_SUBNET_MASK, mNetmaskBytes);

            }

            addTlvEnd(buffer);

        }
@@ -175,4 +192,50 @@ public class DhcpPacketTest extends TestCase { 
        assertLeaseTimeParses(true, -2147483647, 2147483649L * 1000, maxIntPlusOneLease);

        assertLeaseTimeParses(true, DhcpPacket.INFINITE_LEASE, 0, infiniteLease);

    }



    private void checkIpAddress(String expected, Inet4Address clientIp, Inet4Address yourIp,

                                byte[] netmaskBytes) {

        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) {

        ByteBuffer packet = new TestDhcpPacket(type, clientIp, yourIp)

                .setNetmaskBytes(netmaskBytes)

                .build();

        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);

        DhcpResults results = offerPacket.toDhcpResults();



        if (expected != null) {

            LinkAddress expectedAddress = new LinkAddress(expected);

            assertEquals(expectedAddress, results.ipAddress);

        } else {

            assertNull(results);

        }

    }



    @SmallTest

    public void testIpAddress() throws Exception {

        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");



        // A packet without any addresses is not valid.

        checkIpAddress(null, ANY, ANY, slash24Netmask);



        // ClientIP is used iff YourIP is not present.

        checkIpAddress("192.0.2.1/24", example2, example1, slash24Netmask);

        checkIpAddress("192.0.2.43/11", example2, ANY, slash11Netmask);

        checkIpAddress("192.0.2.43/11", ANY, example2, slash11Netmask);



        // Invalid netmasks are ignored.

        checkIpAddress(null, example2, ANY, invalidNetmask);



        // If there is no netmask, implicit netmasks are used.

        checkIpAddress("192.0.2.43/24", ANY, example2, null);

    }

}