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

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

Fix DHCP lease time parsing. 

Currently we treat a lease time larger than 2**31-1 as a negative
value, which causes DhcpClient to attempt to renew its IP address
constantly. Fix this by properly handling large and infinite
lifetimes, and while we're at it, impose a minimum lease time of
60 seconds.

Bug: 21352084
Change-Id: If62c9efeffad6222e2fe0c110f77d0e4c70de96d
parent 99fbb56a

services/net/java/android/net/dhcp/DhcpClient.java

+17 −11
Original line number Diff line number Diff line
@@ -390,11 +390,15 @@ public class DhcpClient extends BaseDhcpStateMachine { 
    }



    private void scheduleRenew() {

        mAlarmManager.cancel(mRenewIntent);

        if (mDhcpLeaseExpiry != 0) {

            long now = SystemClock.elapsedRealtime();

            long alarmTime = (now + mDhcpLeaseExpiry) / 2;

        mAlarmManager.cancel(mRenewIntent);

            mAlarmManager.setExact(AlarmManager.ELAPSED_REALTIME_WAKEUP, alarmTime, mRenewIntent);

            Log.d(TAG, "Scheduling renewal in " + ((alarmTime - now) / 1000) + "s");

        } else {

            Log.d(TAG, "Infinite lease, no renewal needed");

        }

    }



    private void notifyLease() {
@@ -586,6 +590,12 @@ public class DhcpClient extends BaseDhcpStateMachine { 
        return true;

    }



    public void setDhcpLeaseExpiry(DhcpPacket packet) {

        long leaseTimeMillis = packet.getLeaseTimeMillis();

        mDhcpLeaseExpiry =

                (leaseTimeMillis > 0) ? SystemClock.elapsedRealtime() + leaseTimeMillis : 0;

    }



    /**

     * Retransmits packets using jittered exponential backoff with an optional timeout. Packet

     * transmission is triggered by CMD_KICK, which is sent by an AlarmManager alarm.
@@ -723,10 +733,9 @@ public class DhcpClient extends BaseDhcpStateMachine { 
                DhcpResults results = packet.toDhcpResults();

                if (results != null) {

                    mDhcpLease = results;

                    Log.d(TAG, "Confirmed lease: " + mDhcpLease);

                    mDhcpLeaseExpiry = SystemClock.elapsedRealtime() +

                            mDhcpLease.leaseDuration * 1000;

                    mOffer = null;

                    Log.d(TAG, "Confirmed lease: " + mDhcpLease);

                    setDhcpLeaseExpiry(packet);

                    transitionTo(mDhcpBoundState);

                }

            } else if (packet instanceof DhcpNakPacket) {
@@ -797,10 +806,7 @@ public class DhcpClient extends BaseDhcpStateMachine { 
        protected void receivePacket(DhcpPacket packet) {

            if (!isValidPacket(packet)) return;

            if ((packet instanceof DhcpAckPacket)) {

                DhcpResults results = packet.toDhcpResults();

                mDhcpLease.leaseDuration = results.leaseDuration;

                mDhcpLeaseExpiry = SystemClock.elapsedRealtime() +

                        mDhcpLease.leaseDuration * 1000;

                setDhcpLeaseExpiry(packet);

                transitionTo(mDhcpBoundState);

            } else if (packet instanceof DhcpNakPacket) {

                transitionTo(mDhcpInitState);

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

+21 −1
Original line number Diff line number Diff line
@@ -28,6 +28,12 @@ import java.util.List; 
abstract class DhcpPacket {

    protected static final String TAG = "DhcpPacket";



    // dhcpcd has a minimum lease of 20 seconds, but DhcpStateMachine would refuse to wake up the

    // CPU for anything shorter than 5 minutes. For sanity's sake, this must be higher than the

    // DHCP client timeout.

    public static final int MINIMUM_LEASE = 60;

    public static final int INFINITE_LEASE = (int) 0xffffffff;



    public static final Inet4Address INADDR_ANY = (Inet4Address) Inet4Address.ANY;

    public static final Inet4Address INADDR_BROADCAST = (Inet4Address) Inet4Address.ALL;

    public static final byte[] ETHER_BROADCAST = new byte[] {
@@ -1006,10 +1012,24 @@ abstract class DhcpPacket { 
        results.domains = mDomainName;

        results.serverAddress = mServerIdentifier;

        results.vendorInfo = mVendorId;

        results.leaseDuration = mLeaseTime;

        results.leaseDuration = (mLeaseTime != null) ? mLeaseTime : INFINITE_LEASE;

        return results;

    }



    /**

     * Returns the parsed lease time, in milliseconds, or 0 for infinite.

     */

    public long getLeaseTimeMillis() {

        // dhcpcd treats the lack of a lease time option as an infinite lease.

        if (mLeaseTime == null || mLeaseTime == INFINITE_LEASE) {

            return 0;

        } else if (0 <= mLeaseTime && mLeaseTime < MINIMUM_LEASE) {

            return MINIMUM_LEASE * 1000;

        } else {

            return (mLeaseTime & 0xffffffffL) * 1000;

        }

    }



    /**

     * Builds a DHCP-DISCOVER packet from the required specified

     * parameters.

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

+65 −4
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.net.dhcp;



import android.net.NetworkUtils;

import android.net.DhcpResults;

import android.system.OsConstants;

import android.test.suitebuilder.annotation.SmallTest;

import junit.framework.TestCase;
@@ -38,14 +39,27 @@ public class DhcpPacketTest extends TestCase { 
    class TestDhcpPacket extends DhcpPacket {

        private byte mType;

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

        private byte[] mDomainBytes, mVendorInfoBytes;

        private byte[] mDomainBytes, mVendorInfoBytes, mLeaseTimeBytes;



        public TestDhcpPacket(byte type, byte[] domainBytes, byte[] vendorInfoBytes) {

        public TestDhcpPacket(byte type) {

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

                  CLIENT_MAC, true);

            mType = type;

        }



        public TestDhcpPacket setDomainBytes(byte[] domainBytes) {

            mDomainBytes = domainBytes;

            return this;

        }



        public TestDhcpPacket setVendorInfoBytes(byte[] vendorInfoBytes) {

            mVendorInfoBytes = vendorInfoBytes;

            return this;

        }



        public TestDhcpPacket setLeaseTimeBytes(byte[] leaseTimeBytes) {

            mLeaseTimeBytes = leaseTimeBytes;

            return this;

        }



        public ByteBuffer buildPacket(int encap, short unusedDestUdp, short unusedSrcUdp) {
@@ -63,6 +77,9 @@ public class DhcpPacketTest extends TestCase { 
            if (mVendorInfoBytes != null) {

                addTlv(buffer, DHCP_VENDOR_CLASS_ID, mVendorInfoBytes);

            }

            if (mLeaseTimeBytes != null) {

                addTlv(buffer, DHCP_LEASE_TIME, mLeaseTimeBytes);

            }

            addTlvEnd(buffer);

        }
@@ -78,8 +95,10 @@ public class DhcpPacketTest extends TestCase { 
    private void assertDomainAndVendorInfoParses(

            String expectedDomain, byte[] domainBytes,

            String expectedVendorInfo, byte[] vendorInfoBytes) {

        ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER,

                domainBytes, vendorInfoBytes).build();

        ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER)

                .setDomainBytes(domainBytes)

                .setVendorInfoBytes(vendorInfoBytes)

                .build();

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

        assertEquals(expectedDomain, offerPacket.mDomainName);

        assertEquals(expectedVendorInfo, offerPacket.mVendorId);
@@ -114,4 +133,46 @@ public class DhcpPacketTest extends TestCase { 
        assertDomainAndVendorInfoParses("goo.gl", trailingNullDomain,

                                        "ANDROID_METERE\u0000", meteredTrailingNull);

    }



    private void assertLeaseTimeParses(boolean expectValid, Integer rawLeaseTime,

                                       long leaseTimeMillis, byte[] leaseTimeBytes) {

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

        if (!expectValid) {

            assertNull(offerPacket);

            return;

        }

        assertEquals(rawLeaseTime, offerPacket.mLeaseTime);

        DhcpResults dhcpResults = offerPacket.toDhcpResults();  // Just check this doesn't crash.

        assertEquals(leaseTimeMillis, offerPacket.getLeaseTimeMillis());

    }



    @SmallTest

    public void testLeaseTime() throws Exception {

        byte[] noLease = null;

        byte[] tooShortLease = new byte[] { 0x00, 0x00 };

        byte[] tooLongLease = new byte[] { 0x00, 0x00, 0x00, 60, 0x01 };

        byte[] zeroLease = new byte[] { 0x00, 0x00, 0x00, 0x00 };

        byte[] tenSecondLease = new byte[] { 0x00, 0x00, 0x00, 10 };

        byte[] oneMinuteLease = new byte[] { 0x00, 0x00, 0x00, 60 };

        byte[] fiveMinuteLease = new byte[] { 0x00, 0x00, 0x01, 0x2c };

        byte[] oneDayLease = new byte[] { 0x00, 0x01, 0x51, (byte) 0x80 };

        byte[] maxIntPlusOneLease = new byte[] { (byte) 0x80, 0x00, 0x00, 0x01 };

        byte[] infiniteLease = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff };



        assertLeaseTimeParses(true, null, 0, noLease);

        assertLeaseTimeParses(false, null, 0, tooShortLease);

        assertLeaseTimeParses(false, null, 0, tooLongLease);

        assertLeaseTimeParses(true, 0, 60 * 1000, zeroLease);

        assertLeaseTimeParses(true, 10, 60 * 1000, tenSecondLease);

        assertLeaseTimeParses(true, 60, 60 * 1000, oneMinuteLease);

        assertLeaseTimeParses(true, 300, 300 * 1000, fiveMinuteLease);

        assertLeaseTimeParses(true, 86400, 86400 * 1000, oneDayLease);

        assertLeaseTimeParses(true, -2147483647, 2147483649L * 1000, maxIntPlusOneLease);

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

    }

}