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

Commit 7ae2ec39 authored by Xiao Ma's avatar Xiao Ma
Browse files

Rollback to DHCPDISCOVER w/o Rapid Commit if handshake doesn't succeeds.

To prevent the potential interoperability issue with Rapid Commit
option, limit the first 3 DHCPDISCOVER packets taking this option.
If DhcpClient doesn't receive any valid DHCPACK within the first
3 packet (re)transmission, then just rollback to DHCPDISCOVER
without Rapid Commit option in the subsequent transmission.

Bug: 198226242
Test: atest NetworkStackIntegrationTests
Change-Id: Id926c94aa96a5a36d91f82371e472eac7361114e
parent 0d79a43a
Loading
Loading
Loading
Loading
+6 −1
Original line number Diff line number Diff line
@@ -747,9 +747,14 @@ public class DhcpClient extends StateMachine {
    }

    private boolean sendDiscoverPacket() {
        // When Rapid Commit option is enabled, limit only the first 3 DHCPDISCOVER packets
        // taking Rapid Commit option, in order to prevent the potential interoperability issue
        // and be able to rollback later. See {@link DHCP_TIMEOUT_MS} for the (re)transmission
        // schedule with 10% jitter.
        final boolean requestRapidCommit = isDhcpRapidCommitEnabled() && (getSecs() <= 4);
        final ByteBuffer packet = DhcpPacket.buildDiscoverPacket(
                DhcpPacket.ENCAP_L2, mTransactionId, getSecs(), mHwAddr,
                DO_UNICAST, getRequestedParams(), isDhcpRapidCommitEnabled(), mHostname,
                DO_UNICAST, getRequestedParams(), requestRapidCommit, mHostname,
                mConfiguration.options);
        mMetrics.incrementCountForDiscover();
        return transmitPacket(packet, "DHCPDISCOVER", DhcpPacket.ENCAP_L2, INADDR_BROADCAST);
+2 −1
Original line number Diff line number Diff line
@@ -321,7 +321,8 @@ public abstract class DhcpPacket {
     * packet may include this option.
     */
    public static final byte DHCP_RAPID_COMMIT = 80;
    protected boolean mRapidCommit;
    @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
    public boolean mRapidCommit;

    /**
     * DHCP IPv6-Only Preferred Option(RFC 8925).
+35 −0
Original line number Diff line number Diff line
@@ -71,6 +71,7 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.contains;
import static org.mockito.ArgumentMatchers.longThat;
import static org.mockito.Mockito.any;
@@ -592,6 +593,8 @@ public abstract class IpClientIntegrationTestCommon {
        if (useNetworkStackSignature()) {
            setUpMocks();
            setUpIpClient();
            // Enable packet retransmit alarm in DhcpClient.
            enableRealAlarm("DhcpClient." + mIfaceName + ".KICK");
        }

        mIIpClient = makeIIpClient(mIfaceName, mCb);
@@ -688,6 +691,17 @@ public abstract class IpClientIntegrationTestCommon {
        }
    }

    private void enableRealAlarm(String cmdName) {
        doAnswer((inv) -> {
            final Context context = InstrumentationRegistry.getTargetContext();
            final AlarmManager alarmManager = context.getSystemService(AlarmManager.class);
            alarmManager.setExact(inv.getArgument(0), inv.getArgument(1), inv.getArgument(2),
                    inv.getArgument(3), inv.getArgument(4));
            return null;
        }).when(mAlarm).setExact(anyInt(), anyLong(), eq(cmdName), any(OnAlarmListener.class),
                any(Handler.class));
    }

    private IpClient makeIpClient() throws Exception {
        IpClient ipc = new IpClient(mContext, mIfaceName, mCb, mNetworkObserverRegistry,
                mNetworkStackServiceManager, mDependencies);
@@ -1388,6 +1402,27 @@ public abstract class IpClientIntegrationTestCommon {
        assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
    }

    @Test
    public void testRollbackFromRapidCommitOption() throws Exception {
        startIpClientProvisioning(false /* isDhcpLeaseCacheEnabled */,
                true /* isDhcpRapidCommitEnabled */, false /* isPreConnectionEnabled */,
                false /* isDhcpIpConflictDetectEnabled */, false /* isIPv6OnlyPreferredEnabled */);

        final List<DhcpPacket> discoverList = new ArrayList<DhcpPacket>();
        DhcpPacket packet;
        do {
            packet = getNextDhcpPacket();
            assertTrue(packet instanceof DhcpDiscoverPacket);
            discoverList.add(packet);
        } while (discoverList.size() < 4);

        // Check the only first 3 DHCPDISCOVERs take rapid commit option.
        assertTrue(discoverList.get(0).mRapidCommit);
        assertTrue(discoverList.get(1).mRapidCommit);
        assertTrue(discoverList.get(2).mRapidCommit);
        assertFalse(discoverList.get(3).mRapidCommit);
    }

    @Test @SignatureRequiredTest(reason = "TODO: evaluate whether signature perms are required")
    public void testDhcpClientStartWithCachedInfiniteLease() throws Exception {
        final DhcpPacket packet = getReplyFromDhcpLease(