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

Commit c651fcc9 authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN Committed by Gerrit Code Review
Browse files

Merge changes I4decffcc,I283e88b1

* changes:
  Ignore DHCP discover and request w/ invalid giaddr
  Send ciaddr in DHCP server ACK if set by client
parents eb5f4588 e1a1dcc9
Loading
Loading
Loading
Loading
+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;
+4 −4
Original line number Diff line number Diff line
@@ -1281,12 +1281,12 @@ public abstract class DhcpPacket {
     */
    public static ByteBuffer buildAckPacket(int encap, int transactionId,
        boolean broadcast, Inet4Address serverIpAddr, Inet4Address relayIp, Inet4Address yourIp,
        byte[] mac, Integer timeout, Inet4Address netMask, Inet4Address bcAddr,
        List<Inet4Address> gateways, List<Inet4Address> dnsServers,
        Inet4Address requestClientIp, byte[] mac, Integer timeout, Inet4Address netMask,
        Inet4Address bcAddr, List<Inet4Address> gateways, List<Inet4Address> dnsServers,
        Inet4Address dhcpServerIdentifier, String domainName, boolean metered) {
        DhcpPacket pkt = new DhcpAckPacket(
                transactionId, (short) 0, broadcast, serverIpAddr, relayIp,
                INADDR_ANY /* clientIp */, yourIp, mac);
                transactionId, (short) 0, broadcast, serverIpAddr, relayIp, requestClientIp, yourIp,
                mac);
        pkt.mGateways = gateways;
        pkt.mDnsServers = dnsServers;
        pkt.mLeaseTime = timeout;
+15 −6
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);
@@ -367,7 +376,7 @@ public class DhcpServer {
        final int timeout = getLeaseTimeout(lease);
        final ByteBuffer ackPacket = DhcpPacket.buildAckPacket(ENCAP_BOOTP, request.mTransId,
                broadcastFlag, mServingParams.getServerInet4Addr(), request.mRelayIp,
                lease.getNetAddr(), request.mClientMac, timeout,
                lease.getNetAddr(), request.mClientIp, request.mClientMac, timeout,
                mServingParams.getPrefixMaskAsAddress(), mServingParams.getBroadcastAddress(),
                new ArrayList<>(mServingParams.defaultRouters),
                new ArrayList<>(mServingParams.dnsServers),
@@ -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);
    }

+118 −98

File changed.

Preview size limit exceeded, changes collapsed.

+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());
    }