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

Commit 4e4d59ee authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

Connect the DHCP UDP socket to the server.

This makes it so that the socket cannot receive datagrams from
anybody except the DHCP server. This does not improve security,
because we never read from the UDP socket anyway, but it does
make ListeningPortsTest pass.

Bug: 23906864
Bug: 23933386
Change-Id: Ib090273a417f7eb2ac1ee3309260249b72fb8345
parent eb0bbd45
Loading
Loading
Loading
Loading
+4 −5
Original line number Diff line number Diff line
@@ -21,7 +21,6 @@ import android.os.Parcel;
import android.text.TextUtils;
import android.util.Log;

import java.net.InetAddress;
import java.net.Inet4Address;
import java.util.Objects;

@@ -34,7 +33,7 @@ import java.util.Objects;
public class DhcpResults extends StaticIpConfiguration {
    private static final String TAG = "DhcpResults";

    public InetAddress serverAddress;
    public Inet4Address serverAddress;

    /** Vendor specific information (from RFC 2132). */
    public String vendorInfo;
@@ -142,7 +141,7 @@ public class DhcpResults extends StaticIpConfiguration {
    private static void readFromParcel(DhcpResults dhcpResults, Parcel in) {
        StaticIpConfiguration.readFromParcel(dhcpResults, in);
        dhcpResults.leaseDuration = in.readInt();
        dhcpResults.serverAddress = NetworkUtils.unparcelInetAddress(in);
        dhcpResults.serverAddress = (Inet4Address) NetworkUtils.unparcelInetAddress(in);
        dhcpResults.vendorInfo = in.readString();
    }

@@ -183,8 +182,8 @@ public class DhcpResults extends StaticIpConfiguration {

    public boolean setServerAddress(String addrString) {
        try {
            serverAddress = NetworkUtils.numericToInetAddress(addrString);
        } catch (IllegalArgumentException e) {
            serverAddress = (Inet4Address) NetworkUtils.numericToInetAddress(addrString);
        } catch (IllegalArgumentException|ClassCastException e) {
            Log.e(TAG, "setServerAddress failed with addrString " + addrString);
            return true;
        }
+19 −7
Original line number Diff line number Diff line
@@ -299,6 +299,7 @@ public class DhcpClient extends BaseDhcpStateMachine {
            Os.setsockoptInt(mUdpSock, SOL_SOCKET, SO_REUSEADDR, 1);
            Os.setsockoptIfreq(mUdpSock, SOL_SOCKET, SO_BINDTODEVICE, mIfaceName);
            Os.setsockoptInt(mUdpSock, SOL_SOCKET, SO_BROADCAST, 1);
            Os.setsockoptInt(mUdpSock, SOL_SOCKET, SO_RCVBUF, 0);
            Os.bind(mUdpSock, Inet4Address.ANY, DhcpPacket.DHCP_CLIENT);
            NetworkUtils.protectFromVpn(mUdpSock);
        } catch(SocketException|ErrnoException e) {
@@ -308,6 +309,16 @@ public class DhcpClient extends BaseDhcpStateMachine {
        return true;
    }

    private boolean connectUdpSock(Inet4Address to) {
        try {
            Os.connect(mUdpSock, to, DhcpPacket.DHCP_SERVER);
            return true;
        } catch (SocketException|ErrnoException e) {
            Log.e(TAG, "Error connecting UDP socket", e);
            return false;
        }
    }

    private static void closeQuietly(FileDescriptor fd) {
        try {
            IoBridge.closeAndSignalBlockedThreads(fd);
@@ -325,7 +336,7 @@ public class DhcpClient extends BaseDhcpStateMachine {
        try {
            mNMService.setInterfaceConfig(mIfaceName, ifcg);
        } catch (RemoteException|IllegalStateException e) {
            Log.e(TAG, "Error configuring IP address : " + e);
            Log.e(TAG, "Error configuring IP address " + address + ": ", e);
            return false;
        }
        return true;
@@ -377,8 +388,10 @@ public class DhcpClient extends BaseDhcpStateMachine {
                maybeLog("Broadcasting " + description);
                Os.sendto(mPacketSock, buf.array(), 0, buf.limit(), 0, mInterfaceBroadcastAddr);
            } else {
                maybeLog("Unicasting " + description + " to " + to.getHostAddress());
                Os.sendto(mUdpSock, buf, 0, to, DhcpPacket.DHCP_SERVER);
                // It's safe to call getpeername here, because we only send unicast packets if we
                // have an IP address, and we connect the UDP socket in DhcpHaveAddressState#enter.
                maybeLog("Unicasting " + description + " to " + Os.getpeername(mUdpSock));
                Os.write(mUdpSock, buf);
            }
        } catch(ErrnoException|IOException e) {
            Log.e(TAG, "Can't send packet: ", e);
@@ -790,6 +803,7 @@ public class DhcpClient extends BaseDhcpStateMachine {
                    transitionTo(mDhcpBoundState);
                }
            } else if (packet instanceof DhcpNakPacket) {
                // TODO: Wait a while before returning into INIT state.
                Log.d(TAG, "Received NAK, returning to INIT");
                mOffer = null;
                transitionTo(mDhcpInitState);
@@ -807,10 +821,8 @@ public class DhcpClient extends BaseDhcpStateMachine {
        @Override
        public void enter() {
            super.enter();
            if (setIpAddress(mDhcpLease.ipAddress)) {
                maybeLog("Configured IP address " + mDhcpLease.ipAddress);
            } else {
                Log.e(TAG, "Failed to configure IP address " + mDhcpLease.ipAddress);
            if (!setIpAddress(mDhcpLease.ipAddress) ||
                    !connectUdpSock((mDhcpLease.serverAddress))) {
                notifyFailure();
                // There's likely no point in going into DhcpInitState here, we'll probably just
                // repeat the transaction, get the same IP address as before, and fail.