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

Commit e1a1dcc9 authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN
Browse files

Ignore DHCP discover and request w/ invalid giaddr 

This matches previous behavior, and there could be situations where
another server on the network would be configured to reply to such
requests, so not replying is better than sending NAKs as done now.

Also refactoring requests in DhcpLeaseRepositoryTest and replacing some
INETADDR_UNSPEC usages for giaddr with INET4_ANY (giaddr is a BOOTP
field and can't be unspecified, only empty).

Test: Following DhcpServerTest.py regression tests pass:
      test_request_selecting_giaddr_outside_subnet
      test_discover_requestaddress_giaddr_outside_subnet
      test_discover_knownaddress_giaddr_outside_subnet
      test_discover_giaddr_outside_subnet
      Also: atest FrameworksNetTests passes
Change-Id: I4decffccfc64d5e0e29c9ce1cf1446644fcf8190
parent 8975ab63

services/net/java/android/net/dhcp/DhcpLeaseRepository.java

+27 −15
Original line number Diff line number Diff line
@@ -99,6 +99,12 @@ class DhcpLeaseRepository { 
        }

    }



    static class InvalidSubnetException extends DhcpLeaseException {

        InvalidSubnetException(String message) {

            super(message);

        }

    }



    /**

     * Leases by IP address

     */
@@ -153,25 +159,17 @@ class DhcpLeaseRepository { 
     * @param reqAddr Requested address by the client (option 50), or {@link #INETADDR_UNSPEC}

     * @param hostname Client-provided hostname, or {@link DhcpLease#HOSTNAME_NONE}

     * @throws OutOfAddressesException The server does not have any available address

     * @throws InvalidAddressException The lease was requested from an unsupported subnet

     * @throws InvalidSubnetException The lease was requested from an unsupported subnet

     */

    @NonNull

    public DhcpLease getOffer(@Nullable byte[] clientId, @NonNull MacAddress hwAddr,

            @NonNull Inet4Address relayAddr,

            @Nullable Inet4Address reqAddr, @Nullable String hostname)

            throws OutOfAddressesException, InvalidAddressException {

            @NonNull Inet4Address relayAddr, @Nullable Inet4Address reqAddr,

            @Nullable String hostname) throws OutOfAddressesException, InvalidSubnetException {

        final long currentTime = mClock.elapsedRealtime();

        final long expTime = currentTime + mLeaseTimeMs;



        removeExpiredLeases(currentTime);



        // As per #4.3.1, addresses are assigned based on the relay address if present. This

        // implementation only assigns addresses if the relayAddr is inside our configured subnet.

        // This also applies when the client requested a specific address for consistency between

        // requests, and with older behavior.

        if (isIpAddrOutsidePrefix(mPrefix, relayAddr)) {

            throw new InvalidAddressException("Lease requested by relay from outside of subnet");

        }

        checkValidRelayAddr(relayAddr);



        final DhcpLease currentLease = findByClient(clientId, hwAddr);

        final DhcpLease newLease;
@@ -189,7 +187,19 @@ class DhcpLeaseRepository { 
        return newLease;

    }



    private static boolean isIpAddrOutsidePrefix(IpPrefix prefix, Inet4Address addr) {

    private void checkValidRelayAddr(@Nullable Inet4Address relayAddr)

            throws InvalidSubnetException {

        // As per #4.3.1, addresses are assigned based on the relay address if present. This

        // implementation only assigns addresses if the relayAddr is inside our configured subnet.

        // This also applies when the client requested a specific address for consistency between

        // requests, and with older behavior.

        if (isIpAddrOutsidePrefix(mPrefix, relayAddr)) {

            throw new InvalidSubnetException("Lease requested by relay from outside of subnet");

        }

    }



    private static boolean isIpAddrOutsidePrefix(@NonNull IpPrefix prefix,

            @Nullable Inet4Address addr) {

        return addr != null && !addr.equals(Inet4Address.ANY) && !prefix.contains(addr);

    }
@@ -223,10 +233,12 @@ class DhcpLeaseRepository { 
     */

    @NonNull

    public DhcpLease requestLease(@Nullable byte[] clientId, @NonNull MacAddress hwAddr,

            @NonNull Inet4Address clientAddr, @Nullable Inet4Address reqAddr, boolean sidSet,

            @Nullable String hostname) throws InvalidAddressException {

            @NonNull Inet4Address clientAddr, @NonNull Inet4Address relayAddr,

            @Nullable Inet4Address reqAddr, boolean sidSet, @Nullable String hostname)

            throws InvalidAddressException, InvalidSubnetException {

        final long currentTime = mClock.elapsedRealtime();

        removeExpiredLeases(currentTime);

        checkValidRelayAddr(relayAddr);

        final DhcpLease assignedLease = findByClient(clientId, hwAddr);



        final Inet4Address leaseAddr = reqAddr != null ? reqAddr : clientAddr;

services/net/java/android/net/dhcp/DhcpServer.java

+14 −5
Original line number Diff line number Diff line
@@ -269,6 +269,11 @@ public class DhcpServer { 
        }

    }



    private void logIgnoredPacketInvalidSubnet(DhcpLeaseRepository.InvalidSubnetException e) {

        // Not an internal error: only logging exception message, not stacktrace

        mLog.e("Ignored packet from invalid subnet: " + e.getMessage());

    }



    private void processDiscover(@NonNull DhcpDiscoverPacket packet)

            throws MalformedPacketException {

        final DhcpLease lease;
@@ -279,8 +284,8 @@ public class DhcpServer { 
        } catch (DhcpLeaseRepository.OutOfAddressesException e) {

            transmitNak(packet, "Out of addresses to offer");

            return;

        } catch (DhcpLeaseRepository.InvalidAddressException e) {

            transmitNak(packet, "Lease requested from an invalid subnet");

        } catch (DhcpLeaseRepository.InvalidSubnetException e) {

            logIgnoredPacketInvalidSubnet(e);

            return;

        }
@@ -294,16 +299,20 @@ public class DhcpServer { 
        final MacAddress clientMac = getMacAddr(packet);

        try {

            lease = mLeaseRepo.requestLease(packet.getExplicitClientIdOrNull(), clientMac,

                    packet.mClientIp, packet.mRequestedIp, sidSet, packet.mHostName);

                    packet.mClientIp, packet.mRelayIp, packet.mRequestedIp, sidSet,

                    packet.mHostName);

        } catch (DhcpLeaseRepository.InvalidAddressException e) {

            transmitNak(packet, "Invalid requested address");

            return;

        } catch (DhcpLeaseRepository.InvalidSubnetException e) {

            logIgnoredPacketInvalidSubnet(e);

            return;

        }



        transmitAck(packet, lease, clientMac);

    }



    private void processRelease(@Nullable DhcpReleasePacket packet)

    private void processRelease(@NonNull DhcpReleasePacket packet)

            throws MalformedPacketException {

        final byte[] clientId = packet.getExplicitClientIdOrNull();

        final MacAddress macAddr = getMacAddr(packet);
@@ -464,7 +473,7 @@ public class DhcpServer { 
        }

    }



    private static boolean isEmpty(@NonNull Inet4Address address) {

    private static boolean isEmpty(@Nullable Inet4Address address) {

        return address == null || Inet4Address.ANY.equals(address);

    }

tests/net/java/android/net/dhcp/DhcpLeaseRepositoryTest.java

+118 −98

File changed.

Preview size limit exceeded, changes collapsed.

tests/net/java/android/net/dhcp/DhcpServerTest.java

+6 −5
Original line number Diff line number Diff line
@@ -216,8 +216,8 @@ public class DhcpServerTest { 
    @Test

    public void testRequest_Selecting_Ack() throws Exception {

        when(mRepository.requestLease(isNull() /* clientId */, eq(TEST_CLIENT_MAC),

                eq(INADDR_ANY) /* clientAddr */, eq(TEST_CLIENT_ADDR) /* reqAddr */,

                eq(true) /* sidSet */, isNull() /* hostname */))

                eq(INADDR_ANY) /* clientAddr */, eq(INADDR_ANY) /* relayAddr */,

                eq(TEST_CLIENT_ADDR) /* reqAddr */, eq(true) /* sidSet */, isNull() /* hostname */))

                .thenReturn(TEST_LEASE);



        final DhcpRequestPacket request = makeRequestSelectingPacket();
@@ -231,8 +231,8 @@ public class DhcpServerTest { 
    @Test

    public void testRequest_Selecting_Nak() throws Exception {

        when(mRepository.requestLease(isNull(), eq(TEST_CLIENT_MAC),

                eq(INADDR_ANY) /* clientAddr */, eq(TEST_CLIENT_ADDR) /* reqAddr */,

                eq(true) /* sidSet */, isNull() /* hostname */))

                eq(INADDR_ANY) /* clientAddr */, eq(INADDR_ANY) /* relayAddr */,

                eq(TEST_CLIENT_ADDR) /* reqAddr */, eq(true) /* sidSet */, isNull() /* hostname */))

                .thenThrow(new InvalidAddressException("Test error"));



        final DhcpRequestPacket request = makeRequestSelectingPacket();
@@ -248,7 +248,8 @@ public class DhcpServerTest { 
        final DhcpRequestPacket request = makeRequestSelectingPacket();

        mServer.processPacket(request, 50000);



        verify(mRepository, never()).requestLease(any(), any(), any(), any(), anyBoolean(), any());

        verify(mRepository, never())

                .requestLease(any(), any(), any(), any(), any(), anyBoolean(), any());

        verify(mDeps, never()).sendPacket(any(), any(), any());

    }