Support dropping RDNSS options with a low lifetime.

Some routers have been known to use RDNSS lifetimes of 10
seconds(!). This causes APF filters to be set to very low
lifetimes, which substantially impacts battery life.

There are two parts to this:
1. Ignore low RDNSS option lifetimes for the purpose of
   calculating APF filter lifetimes.
2. Do not add DNS servers to the repository if their lifetimes
   are too low.

If we do #1 without #2, the servers will expire because APF will
drop the RAs that refresh them, potentially causing outages or
disconnections.

The behaviour is enabled by default starting from R and can be
enabled on all builds using a flag.

Bug: 66928272
Test: New unit test passes
Change-Id: Ib2e2555026da3e81ea3d50767a30092413b4e4f5

5 files changed, 123 insertions, 10 deletions

diff --git a/src/android/net/apf/ApfFilter.java b/src/android/net/apf/ApfFilter.java
index 75a737d..165e37b 100644
--- a/src/android/net/apf/ApfFilter.java
+++ b/src/android/net/apf/ApfFilter.java
@@ -105,6 +105,7 @@ public class ApfFilter {
         public boolean multicastFilter;
         public boolean ieee802_3Filter;
         public int[] ethTypeBlackList;
+        public int minRdnssLifetimeSec;
     }
 
     // Enums describing the outcome of receiving an RA packet.
@@ -358,6 +359,9 @@ public class ApfFilter {
     private final boolean mDrop802_3Frames;
     private final int[] mEthTypeBlackList;
 
+    // Ignore non-zero RDNSS lifetimes below this value.
+    private final int mMinRdnssLifetimeSec;
+
     // Detects doze mode state transitions.
     private final BroadcastReceiver mDeviceIdleReceiver = new BroadcastReceiver() {
         @Override
@@ -388,6 +392,7 @@ public class ApfFilter {
         mInterfaceParams = ifParams;
         mMulticastFilter = config.multicastFilter;
         mDrop802_3Frames = config.ieee802_3Filter;
+        mMinRdnssLifetimeSec = config.minRdnssLifetimeSec;
         mContext = context;
 
         if (mApfCapabilities.hasDataAccess()) {
@@ -748,6 +753,19 @@ public class ApfFilter {
             return lifetime;
         }
 
+        // http://b/66928272 http://b/65056012
+        // DnsServerRepository ignores RDNSS servers with lifetimes that are too low. Ignore these
+        // lifetimes for the purpose of filter lifetime calculations.
+        private boolean shouldIgnoreLifetime(int optionType, long lifetime) {
+            return optionType == ICMP6_RDNSS_OPTION_TYPE
+                    && lifetime != 0 && lifetime < mMinRdnssLifetimeSec;
+        }
+
+        private boolean isRelevantLifetime(PacketSection section) {
+            return section.type == PacketSection.Type.LIFETIME
+                    && !shouldIgnoreLifetime(section.option, section.lifetime);
+        }
+
         // Note that this parses RA and may throw InvalidRaException (from
         // Buffer.position(int) or due to an invalid-length option) or IndexOutOfBoundsException
         // (from ByteBuffer.get(int) ) if parsing encounters something non-compliant with
@@ -862,7 +880,7 @@ public class ApfFilter {
         long minLifetime() {
             long minLifetime = Long.MAX_VALUE;
             for (PacketSection section : mPacketSections) {
-                if (section.type == PacketSection.Type.LIFETIME) {
+                if (isRelevantLifetime(section)) {
                     minLifetime = Math.min(minLifetime, section.lifetime);
                 }
             }
@@ -902,9 +920,10 @@ public class ApfFilter {
                                     section.start + section.length),
                             nextFilterLabel);
                 }
+
                 // Generate code to test the lifetimes haven't gone down too far.
-                // The packet is accepted if any of its lifetimes are lower than filterLifetime.
-                if (section.type == PacketSection.Type.LIFETIME) {
+                // The packet is accepted if any non-ignored lifetime is lower than filterLifetime.
+                if (isRelevantLifetime(section)) {
                     switch (section.length) {
                         case 4: gen.addLoad32(Register.R0, section.start); break;
                         case 2: gen.addLoad16(Register.R0, section.start); break;
@@ -1913,6 +1932,7 @@ public class ApfFilter {
         pw.println("Capabilities: " + mApfCapabilities);
         pw.println("Receive thread: " + (mReceiveThread != null ? "RUNNING" : "STOPPED"));
         pw.println("Multicast: " + (mMulticastFilter ? "DROP" : "ALLOW"));
+        pw.println("Minimum RDNSS lifetime: " + mMinRdnssLifetimeSec);
         try {
             pw.println("IPv4 address: " + InetAddress.getByAddress(mIPv4Address).getHostAddress());
         } catch (UnknownHostException|NullPointerException e) {}
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index 9f1f17c..8808ee2 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -18,6 +18,7 @@ package android.net.ip;
 
 import static android.net.RouteInfo.RTN_UNICAST;
 import static android.net.shared.IpConfigurationParcelableUtil.toStableParcelable;
+import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
 
 import static com.android.server.util.PermissionUtil.enforceNetworkStackCallingPermission;
 
@@ -42,7 +43,9 @@ import android.net.metrics.IpManagerEvent;
 import android.net.shared.InitialConfiguration;
 import android.net.shared.ProvisioningConfiguration;
 import android.net.util.InterfaceParams;
+import android.net.util.NetworkStackUtils;
 import android.net.util.SharedLog;
+import android.os.Build;
 import android.os.ConditionVariable;
 import android.os.IBinder;
 import android.os.Message;
@@ -65,6 +68,7 @@ import com.android.internal.util.Preconditions;
 import com.android.internal.util.State;
 import com.android.internal.util.StateMachine;
 import com.android.internal.util.WakeupMessage;
+import com.android.networkstack.apishim.ShimUtils;
 import com.android.server.NetworkObserverRegistry;
 import com.android.server.NetworkStackService.NetworkStackServiceManager;
 
@@ -313,9 +317,15 @@ public class IpClient extends StateMachine {
     // IpClient shares a handler with DhcpClient: commands must not overlap
     public static final int DHCPCLIENT_CMD_BASE = 1000;
 
+    // Settings and default values.
     private static final int MAX_LOG_RECORDS = 500;
     private static final int MAX_PACKET_RECORDS = 100;
 
+    @VisibleForTesting
+    static final String CONFIG_MIN_RDNSS_LIFETIME = "ipclient_min_rdnss_lifetime";
+    private static final int DEFAULT_MIN_RDNSS_LIFETIME =
+            ShimUtils.isReleaseOrDevelopmentApiAbove(Build.VERSION_CODES.Q) ? 120 : 0;
+
     private static final boolean NO_CALLBACKS = false;
     private static final boolean SEND_CALLBACKS = true;
 
@@ -355,6 +365,9 @@ public class IpClient extends StateMachine {
     private final IpConnectivityLog mMetricsLog = new IpConnectivityLog();
     private final InterfaceController mInterfaceCtrl;
 
+    // Ignore nonzero RDNSS option lifetimes below this value. 0 = disabled.
+    private final int mMinRdnssLifetimeSec;
+
     private InterfaceParams mInterfaceParams;
 
     /**
@@ -411,6 +424,14 @@ public class IpClient extends StateMachine {
                 NetworkStackIpMemoryStore ipMemoryStore) {
             return new DhcpClient.Dependencies(ipMemoryStore);
         }
+
+        /**
+         * Read an integer DeviceConfig property.
+         */
+        public int getDeviceConfigPropertyInt(String name, int defaultValue) {
+            return NetworkStackUtils.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY, name,
+                    defaultValue);
+        }
     }
 
     public IpClient(Context context, String ifName, IIpClientCallbacks callback,
@@ -449,9 +470,15 @@ public class IpClient extends StateMachine {
         mNetd = deps.getNetd(mContext);
         mInterfaceCtrl = new InterfaceController(mInterfaceName, mNetd, mLog);
 
+        mMinRdnssLifetimeSec = mDependencies.getDeviceConfigPropertyInt(
+                CONFIG_MIN_RDNSS_LIFETIME, DEFAULT_MIN_RDNSS_LIFETIME);
+
+        IpClientLinkObserver.Configuration config = new IpClientLinkObserver.Configuration(
+                mMinRdnssLifetimeSec);
+
         mLinkObserver = new IpClientLinkObserver(
                 mInterfaceName,
-                () -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED)) {
+                () -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED), config) {
             @Override
             public void onInterfaceAdded(String iface) {
                 super.onInterfaceAdded(iface);
@@ -1500,6 +1527,7 @@ public class IpClient extends StateMachine {
             // Get the Configuration for ApfFilter from Context
             apfConfig.ieee802_3Filter = ApfCapabilities.getApfDrop8023Frames();
             apfConfig.ethTypeBlackList = ApfCapabilities.getApfEtherTypeBlackList();
+            apfConfig.minRdnssLifetimeSec = mMinRdnssLifetimeSec;
             mApfFilter = ApfFilter.maybeCreate(mContext, apfConfig, mInterfaceParams, mCallback);
             // TODO: investigate the effects of any multicast filtering racing/interfering with the
             // rest of this IP configuration startup.
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java
index 3f399e9..02bf5f0 100644
--- a/src/android/net/ip/IpClientLinkObserver.java
+++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -71,20 +71,31 @@ public class IpClientLinkObserver implements NetworkObserver {
         void update();
     }
 
+    /** Configuration parameters for IpClientLinkObserver. */
+    public static class Configuration {
+        public final int minRdnssLifetime;
+
+        public Configuration(int minRdnssLifetime) {
+            this.minRdnssLifetime = minRdnssLifetime;
+        }
+    }
+
     private final String mInterfaceName;
     private final Callback mCallback;
     private final LinkProperties mLinkProperties;
     private DnsServerRepository mDnsServerRepository;
+    private final Configuration mConfig;
 
     private static final boolean DBG = false;
 
-    public IpClientLinkObserver(String iface, Callback callback) {
+    public IpClientLinkObserver(String iface, Callback callback, Configuration config) {
         mTag = "NetlinkTracker/" + iface;
         mInterfaceName = iface;
         mCallback = callback;
         mLinkProperties = new LinkProperties();
         mLinkProperties.setInterfaceName(mInterfaceName);
-        mDnsServerRepository = new DnsServerRepository();
+        mConfig = config;
+        mDnsServerRepository = new DnsServerRepository(config.minRdnssLifetime);
     }
 
     private void maybeLog(String operation, String iface, LinkAddress address) {
@@ -197,7 +208,7 @@ public class IpClientLinkObserver implements NetworkObserver {
         // Clear the repository before clearing mLinkProperties. That way, if a clear() happens
         // while interfaceDnsServerInfo() is being called, we'll end up with no DNS servers in
         // mLinkProperties, as desired.
-        mDnsServerRepository = new DnsServerRepository();
+        mDnsServerRepository = new DnsServerRepository(mConfig.minRdnssLifetime);
         mLinkProperties.clear();
         mLinkProperties.setInterfaceName(mInterfaceName);
     }
@@ -260,10 +271,16 @@ public class IpClientLinkObserver implements NetworkObserver {
          */
         private HashMap<InetAddress, DnsServerEntry> mIndex;
 
-        DnsServerRepository() {
+        /**
+         * Minimum (non-zero) RDNSS lifetime to accept.
+         */
+        private final int mMinLifetime;
+
+        DnsServerRepository(int minLifetime) {
             mCurrentServers = new HashSet<>();
             mAllServers = new ArrayList<>(NUM_SERVERS);
             mIndex = new HashMap<>(NUM_SERVERS);
+            mMinLifetime = minLifetime;
         }
 
         /** Sets the DNS servers of the provided LinkProperties object to the current servers. */
@@ -277,6 +294,9 @@ public class IpClientLinkObserver implements NetworkObserver {
          * @param addresses the string representations of the IP addresses of DNS servers to use.
          */
         public synchronized boolean addServers(long lifetime, String[] addresses) {
+            // If the servers are below the minimum lifetime, don't change anything.
+            if (lifetime != 0 && lifetime < mMinLifetime) return false;
+
             // The lifetime is actually an unsigned 32-bit number, but Java doesn't have unsigned.
             // Technically 0xffffffff (the maximum) is special and means "forever", but 2^32 seconds
             // (136 years) is close enough.
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index 63f4319..f34c26e 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -123,6 +123,7 @@ import java.net.NetworkInterface;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 
@@ -235,6 +236,8 @@ public class IpClientIntegrationTest {
     private class Dependencies extends IpClient.Dependencies {
         private boolean mIsDhcpLeaseCacheEnabled;
         private boolean mIsDhcpRapidCommitEnabled;
+        // Can't use SparseIntArray, it doesn't have an easy way to know if a key is not present.
+        private HashMap<String, Integer> mIntConfigProperties = new HashMap<>();
 
         public void setDhcpLeaseCacheEnabled(final boolean enable) {
             mIsDhcpLeaseCacheEnabled = enable;
@@ -274,6 +277,19 @@ public class IpClientIntegrationTest {
                 }
             };
         }
+
+        @Override
+        public int getDeviceConfigPropertyInt(String name, int defaultValue) {
+            Integer value = mIntConfigProperties.get(name);
+            if (value == null) {
+                throw new IllegalStateException("Non-mocked device config property " + name);
+            }
+            return value;
+        }
+
+        public void setDeviceConfigProperty(String name, int value) {
+            mIntConfigProperties.put(name, value);
+        }
     }
 
     @Before
@@ -288,6 +304,8 @@ public class IpClientIntegrationTest {
         when(mNetworkStackServiceManager.getIpMemoryStoreService())
                 .thenReturn(mIpMemoryStoreService);
 
+        mDependencies.setDeviceConfigProperty(IpClient.CONFIG_MIN_RDNSS_LIFETIME, 67);
+
         setUpTapInterface();
         setUpIpClient();
     }
@@ -410,6 +428,7 @@ public class IpClientIntegrationTest {
         try (FileOutputStream out = new FileOutputStream(mPacketReader.createFd())) {
             byte[] packetBytes = new byte[packet.limit()];
             packet.get(packetBytes);
+            packet.flip();  // So we can reuse it in the future.
             out.write(packetBytes);
         }
     }
@@ -868,11 +887,22 @@ public class IpClientIntegrationTest {
         verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
         LinkProperties lp = captor.getValue();
 
+        // Expect that DNS servers with lifetimes below CONFIG_MIN_RDNSS_LIFETIME are not accepted.
+        assertNotNull(lp);
+        assertEquals(1, lp.getDnsServers().size());
+        assertTrue(lp.getDnsServers().contains(InetAddress.getByName(dnsServer)));
+        reset(mCb);
+
+        // If the RDNSS lifetime is above the minimum, the DNS server is accepted.
+        rdnss1 = buildRdnssOption(68, lowlifeDnsServer);
+        ra = buildRaPacket(pio, rdnss1, rdnss2);
+        sendResponse(ra);
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(captor.capture());
+        lp = captor.getValue();
         assertNotNull(lp);
         assertEquals(2, lp.getDnsServers().size());
         assertTrue(lp.getDnsServers().contains(InetAddress.getByName(dnsServer)));
         assertTrue(lp.getDnsServers().contains(InetAddress.getByName(lowlifeDnsServer)));
-
         reset(mCb);
 
         // Expect that setting RDNSS lifetime of 0 causes loss of provisioning.
diff --git a/tests/unit/src/android/net/apf/ApfTest.java b/tests/unit/src/android/net/apf/ApfTest.java
index ab92892..006ad19 100644
--- a/tests/unit/src/android/net/apf/ApfTest.java
+++ b/tests/unit/src/android/net/apf/ApfTest.java
@@ -130,6 +130,8 @@ public class ApfTest {
     private static final boolean DROP_802_3_FRAMES = true;
     private static final boolean ALLOW_802_3_FRAMES = false;
 
+    private static final int MIN_RDNSS_LIFETIME_SEC = 0;
+
     // Constants for opcode encoding
     private static final byte LI_OP   = (byte)(13 << 3);
     private static final byte LDDW_OP = (byte)(22 << 3);
@@ -146,6 +148,8 @@ public class ApfTest {
         config.multicastFilter = ALLOW_MULTICAST;
         config.ieee802_3Filter = ALLOW_802_3_FRAMES;
         config.ethTypeBlackList = new int[0];
+        config.minRdnssLifetimeSec = MIN_RDNSS_LIFETIME_SEC;
+        config.minRdnssLifetimeSec = 67;
         return config;
     }
 
@@ -2090,6 +2094,16 @@ public class ApfTest {
         verifyRaLifetime(apfFilter, ipClientCallback, rdnssOptionPacket, RDNSS_LIFETIME);
         verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, -1, RDNSS_LIFETIME, -1));
 
+        final int lowLifetime = 60;
+        ByteBuffer lowLifetimeRdnssOptionPacket = ByteBuffer.wrap(
+                new byte[ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_OPTION_LEN + IPV6_ADDR_LEN]);
+        basePacket.clear();
+        lowLifetimeRdnssOptionPacket.put(basePacket);
+        addRdnssOption(lowLifetimeRdnssOptionPacket, lowLifetime, "2620:fe::9");
+        verifyRaLifetime(apfFilter, ipClientCallback, lowLifetimeRdnssOptionPacket,
+                ROUTER_LIFETIME);
+        verifyRaEvent(new RaEvent(ROUTER_LIFETIME, -1, -1, -1, lowLifetime, -1));
+
         ByteBuffer routeInfoOptionPacket = ByteBuffer.wrap(
                 new byte[ICMP6_RA_OPTION_OFFSET + ICMP6_4_BYTE_OPTION_LEN + IPV6_ADDR_LEN]);
         basePacket.clear();
@@ -2123,12 +2137,13 @@ public class ApfTest {
         verifyRaLifetime(apfFilter, ipClientCallback, largeRaPacket, 300);
         verifyRaEvent(new RaEvent(1800, 600, 300, 1200, 7200, -1));
 
-        // Verify that current program filters all the RAs:
+        // Verify that current program filters all the RAs (note: ApfFilter.MAX_RAS == 10).
         program = ipClientCallback.getApfProgram();
         verifyRaLifetime(program, basePacket, ROUTER_LIFETIME);
         verifyRaLifetime(program, newFlowLabelPacket, ROUTER_LIFETIME);
         verifyRaLifetime(program, prefixOptionPacket, PREFIX_PREFERRED_LIFETIME);
         verifyRaLifetime(program, rdnssOptionPacket, RDNSS_LIFETIME);
+        verifyRaLifetime(program, lowLifetimeRdnssOptionPacket, ROUTER_LIFETIME);
         verifyRaLifetime(program, routeInfoOptionPacket, ROUTE_LIFETIME);
         verifyRaLifetime(program, dnsslOptionPacket, ROUTER_LIFETIME);
         verifyRaLifetime(program, largeRaPacket, 300);