Merge "Clean up the arguments annotation and verify items on IpMemoryStoreTest." into qt-dev
diff --git a/packages/NetworkStack/src/android/net/apf/ApfFilter.java b/packages/NetworkStack/src/android/net/apf/ApfFilter.java
index 359c859..f054319 100644
--- a/packages/NetworkStack/src/android/net/apf/ApfFilter.java
+++ b/packages/NetworkStack/src/android/net/apf/ApfFilter.java
@@ -155,7 +155,8 @@
         DROPPED_ETHERTYPE_BLACKLISTED,
         DROPPED_ARP_REPLY_SPA_NO_HOST,
         DROPPED_IPV4_KEEPALIVE_ACK,
-        DROPPED_IPV6_KEEPALIVE_ACK;
+        DROPPED_IPV6_KEEPALIVE_ACK,
+        DROPPED_IPV4_NATT_KEEPALIVE;
 
         // Returns the negative byte offset from the end of the APF data segment for
         // a given counter.
@@ -857,12 +858,104 @@
         }
     }
 
-    // A class to hold keepalive ack information.
-    private abstract static class TcpKeepaliveAck {
+    // TODO: Refactor these subclasses to avoid so much repetition.
+    private abstract static class KeepalivePacket {
         // Note that the offset starts from IP header.
         // These must be added ether header length when generating program.
         static final int IP_HEADER_OFFSET = 0;
+        static final int IPV4_SRC_ADDR_OFFSET = IP_HEADER_OFFSET + 12;
 
+        // Append a filter for this keepalive ack to {@code gen}.
+        // Jump to drop if it matches the keepalive ack.
+        // Jump to the next filter if packet doesn't match the keepalive ack.
+        abstract void generateFilterLocked(ApfGenerator gen) throws IllegalInstructionException;
+    }
+
+    // A class to hold NAT-T keepalive ack information.
+    private class NattKeepaliveResponse extends KeepalivePacket {
+        static final int UDP_LENGTH_OFFSET = 4;
+        static final int UDP_HEADER_LEN = 8;
+
+        protected class NattKeepaliveResponseData {
+            public final byte[] srcAddress;
+            public final int srcPort;
+            public final byte[] dstAddress;
+            public final int dstPort;
+
+            NattKeepaliveResponseData(final NattKeepalivePacketDataParcelable sentKeepalivePacket) {
+                srcAddress = sentKeepalivePacket.dstAddress;
+                srcPort = sentKeepalivePacket.dstPort;
+                dstAddress = sentKeepalivePacket.srcAddress;
+                dstPort = sentKeepalivePacket.srcPort;
+            }
+        }
+
+        protected final NattKeepaliveResponseData mPacket;
+        protected final byte[] mSrcDstAddr;
+        protected final byte[] mPortFingerprint;
+        // NAT-T keepalive packet
+        protected final byte[] mPayload = {(byte) 0xff};
+
+        NattKeepaliveResponse(final NattKeepalivePacketDataParcelable sentKeepalivePacket) {
+            mPacket = new NattKeepaliveResponseData(sentKeepalivePacket);
+            mSrcDstAddr = concatArrays(mPacket.srcAddress, mPacket.dstAddress);
+            mPortFingerprint = generatePortFingerprint(mPacket.srcPort, mPacket.dstPort);
+        }
+
+        byte[] generatePortFingerprint(int srcPort, int dstPort) {
+            final ByteBuffer fp = ByteBuffer.allocate(4);
+            fp.order(ByteOrder.BIG_ENDIAN);
+            fp.putShort((short) srcPort);
+            fp.putShort((short) dstPort);
+            return fp.array();
+        }
+
+        @Override
+        void generateFilterLocked(ApfGenerator gen) throws IllegalInstructionException {
+            final String nextFilterLabel = "natt_keepalive_filter" + getUniqueNumberLocked();
+
+            gen.addLoadImmediate(Register.R0, ETH_HEADER_LEN + IPV4_SRC_ADDR_OFFSET);
+            gen.addJumpIfBytesNotEqual(Register.R0, mSrcDstAddr, nextFilterLabel);
+
+            // A NAT-T keepalive packet contains 1 byte payload with the value 0xff
+            // Check payload length is 1
+            gen.addLoadFromMemory(Register.R0, gen.IPV4_HEADER_SIZE_MEMORY_SLOT);
+            gen.addAdd(UDP_HEADER_LEN);
+            gen.addSwap();
+            gen.addLoad16(Register.R0, IPV4_TOTAL_LENGTH_OFFSET);
+            gen.addNeg(Register.R1);
+            gen.addAddR1();
+            gen.addJumpIfR0NotEquals(1, nextFilterLabel);
+
+            // Check that the ports match
+            gen.addLoadFromMemory(Register.R0, gen.IPV4_HEADER_SIZE_MEMORY_SLOT);
+            gen.addAdd(ETH_HEADER_LEN);
+            gen.addJumpIfBytesNotEqual(Register.R0, mPortFingerprint, nextFilterLabel);
+
+            // Payload offset = R0 + UDP header length
+            gen.addAdd(UDP_HEADER_LEN);
+            gen.addJumpIfBytesNotEqual(Register.R0, mPayload, nextFilterLabel);
+
+            maybeSetupCounter(gen, Counter.DROPPED_IPV4_NATT_KEEPALIVE);
+            gen.addJump(mCountAndDropLabel);
+            gen.defineLabel(nextFilterLabel);
+        }
+
+        public String toString() {
+            try {
+                return String.format("%s -> %s",
+                        NetworkStackUtils.addressAndPortToString(
+                                InetAddress.getByAddress(mPacket.srcAddress), mPacket.srcPort),
+                        NetworkStackUtils.addressAndPortToString(
+                                InetAddress.getByAddress(mPacket.dstAddress), mPacket.dstPort));
+            } catch (UnknownHostException e) {
+                return "Unknown host";
+            }
+        }
+    }
+
+    // A class to hold TCP keepalive ack information.
+    private abstract static class TcpKeepaliveAck extends KeepalivePacket {
         protected static class TcpKeepaliveAckData {
             public final byte[] srcAddress;
             public final int srcPort;
@@ -870,6 +963,7 @@
             public final int dstPort;
             public final int seq;
             public final int ack;
+
             // Create the characteristics of the ack packet from the sent keepalive packet.
             TcpKeepaliveAckData(final TcpKeepalivePacketDataParcelable sentKeepalivePacket) {
                 srcAddress = sentKeepalivePacket.dstAddress;
@@ -902,28 +996,18 @@
             return fp.array();
         }
 
-        static byte[] concatArrays(final byte[]... arr) {
-            int size = 0;
-            for (byte[] a : arr) {
-                size += a.length;
-            }
-            final byte[] result = new byte[size];
-            int offset = 0;
-            for (byte[] a : arr) {
-                System.arraycopy(a, 0, result, offset, a.length);
-                offset += a.length;
-            }
-            return result;
-        }
-
         public String toString() {
-            return String.format("%s(%d) -> %s(%d), seq=%d, ack=%d",
-                    mPacket.srcAddress,
-                    mPacket.srcPort,
-                    mPacket.dstAddress,
-                    mPacket.dstPort,
-                    mPacket.seq,
-                    mPacket.ack);
+            try {
+                return String.format("%s -> %s , seq=%d, ack=%d",
+                        NetworkStackUtils.addressAndPortToString(
+                                InetAddress.getByAddress(mPacket.srcAddress), mPacket.srcPort),
+                        NetworkStackUtils.addressAndPortToString(
+                                InetAddress.getByAddress(mPacket.dstAddress), mPacket.dstPort),
+                        Integer.toUnsignedLong(mPacket.seq),
+                        Integer.toUnsignedLong(mPacket.ack));
+            } catch (UnknownHostException e) {
+                return "Unknown host";
+            }
         }
 
         // Append a filter for this keepalive ack to {@code gen}.
@@ -933,7 +1017,6 @@
     }
 
     private class TcpKeepaliveAckV4 extends TcpKeepaliveAck {
-        private static final int IPV4_SRC_ADDR_OFFSET = IP_HEADER_OFFSET + 12;
 
         TcpKeepaliveAckV4(final TcpKeepalivePacketDataParcelable sentKeepalivePacket) {
             this(new TcpKeepaliveAckData(sentKeepalivePacket));
@@ -987,7 +1070,7 @@
 
         @Override
         void generateFilterLocked(ApfGenerator gen) throws IllegalInstructionException {
-            throw new UnsupportedOperationException("IPv6 Keepalive is not supported yet");
+            throw new UnsupportedOperationException("IPv6 TCP Keepalive is not supported yet");
         }
     }
 
@@ -997,7 +1080,7 @@
     @GuardedBy("this")
     private ArrayList<Ra> mRas = new ArrayList<>();
     @GuardedBy("this")
-    private SparseArray<TcpKeepaliveAck> mKeepaliveAcks = new SparseArray<>();
+    private SparseArray<KeepalivePacket> mKeepalivePackets = new SparseArray<>();
 
     // There is always some marginal benefit to updating the installed APF program when an RA is
     // seen because we can extend the program's lifetime slightly, but there is some cost to
@@ -1171,9 +1254,12 @@
                 gen.addJumpIfR0Equals(broadcastAddr, mCountAndDropLabel);
             }
 
-            // If any keepalive filter matches, drop
+            // If any TCP keepalive filter matches, drop
             generateV4KeepaliveFilters(gen);
 
+            // If any NAT-T keepalive filter matches, drop
+            generateV4NattKeepaliveFilters(gen);
+
             // Otherwise, this is an IPv4 unicast, pass
             // If L2 broadcast packet, drop.
             // TODO: can we invert this condition to fall through to the common pass case below?
@@ -1184,6 +1270,7 @@
             gen.addJump(mCountAndDropLabel);
         } else {
             generateV4KeepaliveFilters(gen);
+            generateV4NattKeepaliveFilters(gen);
         }
 
         // Otherwise, pass
@@ -1191,25 +1278,36 @@
         gen.addJump(mCountAndPassLabel);
     }
 
-    private void generateV4KeepaliveFilters(ApfGenerator gen) throws IllegalInstructionException {
-        final String skipV4KeepaliveFilter = "skip_v4_keepalive_filter";
-        final boolean haveV4KeepaliveAcks = NetworkStackUtils.any(mKeepaliveAcks,
-                ack -> ack instanceof TcpKeepaliveAckV4);
+    private void generateKeepaliveFilters(ApfGenerator gen, Class<?> filterType, int proto,
+            int offset, String label) throws IllegalInstructionException {
+        final boolean haveKeepaliveResponses = NetworkStackUtils.any(mKeepalivePackets,
+                ack -> filterType.isInstance(ack));
 
-        // If no keepalive acks
-        if (!haveV4KeepaliveAcks) return;
+        // If no keepalive packets of this type
+        if (!haveKeepaliveResponses) return;
 
-        // If not tcp, skip keepalive filters
-        gen.addLoad8(Register.R0, IPV4_PROTOCOL_OFFSET);
-        gen.addJumpIfR0NotEquals(IPPROTO_TCP, skipV4KeepaliveFilter);
+        // If not the right proto, skip keepalive filters
+        gen.addLoad8(Register.R0, offset);
+        gen.addJumpIfR0NotEquals(proto, label);
 
-        // Drop IPv4 Keepalive acks
-        for (int i = 0; i < mKeepaliveAcks.size(); ++i) {
-            final TcpKeepaliveAck ack = mKeepaliveAcks.valueAt(i);
-            if (ack instanceof TcpKeepaliveAckV4) ack.generateFilterLocked(gen);
+        // Drop Keepalive responses
+        for (int i = 0; i < mKeepalivePackets.size(); ++i) {
+            final KeepalivePacket response = mKeepalivePackets.valueAt(i);
+            if (filterType.isInstance(response)) response.generateFilterLocked(gen);
         }
 
-        gen.defineLabel(skipV4KeepaliveFilter);
+        gen.defineLabel(label);
+    }
+
+    private void generateV4KeepaliveFilters(ApfGenerator gen) throws IllegalInstructionException {
+        generateKeepaliveFilters(gen, TcpKeepaliveAckV4.class, IPPROTO_TCP, IPV4_PROTOCOL_OFFSET,
+                "skip_v4_keepalive_filter");
+    }
+
+    private void generateV4NattKeepaliveFilters(ApfGenerator gen)
+            throws IllegalInstructionException {
+        generateKeepaliveFilters(gen, NattKeepaliveResponse.class,
+                IPPROTO_UDP, IPV4_PROTOCOL_OFFSET, "skip_v4_nattkeepalive_filter");
     }
 
     /**
@@ -1294,24 +1392,8 @@
     }
 
     private void generateV6KeepaliveFilters(ApfGenerator gen) throws IllegalInstructionException {
-        final String skipV6KeepaliveFilter = "skip_v6_keepalive_filter";
-        final boolean haveV6KeepaliveAcks = NetworkStackUtils.any(mKeepaliveAcks,
-                ack -> ack instanceof TcpKeepaliveAckV6);
-
-        // If no keepalive acks
-        if (!haveV6KeepaliveAcks) return;
-
-        // If not tcp, skip keepalive filters
-        gen.addLoad8(Register.R0, IPV6_NEXT_HEADER_OFFSET);
-        gen.addJumpIfR0NotEquals(IPPROTO_TCP, skipV6KeepaliveFilter);
-
-        // Drop IPv6 Keepalive acks
-        for (int i = 0; i < mKeepaliveAcks.size(); ++i) {
-            final TcpKeepaliveAck ack = mKeepaliveAcks.valueAt(i);
-            if (ack instanceof TcpKeepaliveAckV6) ack.generateFilterLocked(gen);
-        }
-
-        gen.defineLabel(skipV6KeepaliveFilter);
+        generateKeepaliveFilters(gen, TcpKeepaliveAckV6.class, IPPROTO_TCP, IPV6_NEXT_HEADER_OFFSET,
+                "skip_v6_keepalive_filter");
     }
 
     /**
@@ -1701,26 +1783,34 @@
     public synchronized void addTcpKeepalivePacketFilter(final int slot,
             final TcpKeepalivePacketDataParcelable sentKeepalivePacket) {
         log("Adding keepalive ack(" + slot + ")");
-        if (null != mKeepaliveAcks.get(slot)) {
+        if (null != mKeepalivePackets.get(slot)) {
             throw new IllegalArgumentException("Keepalive slot " + slot + " is occupied");
         }
         final int ipVersion = sentKeepalivePacket.srcAddress.length == 4 ? 4 : 6;
-        mKeepaliveAcks.put(slot, (ipVersion == 4)
+        mKeepalivePackets.put(slot, (ipVersion == 4)
                 ? new TcpKeepaliveAckV4(sentKeepalivePacket)
                 : new TcpKeepaliveAckV6(sentKeepalivePacket));
         installNewProgramLocked();
     }
 
     /**
-     * Add NATT keepalive packet filter.
-     * This will add a filter to drop NATT keepalive packet which is passed as an argument.
+     * Add NAT-T keepalive packet filter.
+     * This will add a filter to drop NAT-T keepalive packet which is passed as an argument.
      *
      * @param slot The index used to access the filter.
      * @param sentKeepalivePacket The attributes of the sent keepalive packet.
      */
     public synchronized void addNattKeepalivePacketFilter(final int slot,
             final NattKeepalivePacketDataParcelable sentKeepalivePacket) {
-        Log.e(TAG, "APF add NATT keepalive filter is not implemented");
+        log("Adding NAT-T keepalive packet(" + slot + ")");
+        if (null != mKeepalivePackets.get(slot)) {
+            throw new IllegalArgumentException("NAT-T Keepalive slot " + slot + " is occupied");
+        }
+        if (sentKeepalivePacket.srcAddress.length != 4) {
+            throw new IllegalArgumentException("NAT-T keepalive is only supported on IPv4");
+        }
+        mKeepalivePackets.put(slot, new NattKeepaliveResponse(sentKeepalivePacket));
+        installNewProgramLocked();
     }
 
     /**
@@ -1729,7 +1819,8 @@
      * @param slot The index used to access the filter.
      */
     public synchronized void removeKeepalivePacketFilter(int slot) {
-        mKeepaliveAcks.remove(slot);
+        log("Removing keepalive packet(" + slot + ")");
+        mKeepalivePackets.remove(slot);
         installNewProgramLocked();
     }
 
@@ -1785,14 +1876,29 @@
         }
         pw.decreaseIndent();
 
-        pw.println("Keepalive filters:");
+        pw.println("TCP Keepalive filters:");
         pw.increaseIndent();
-        for (int i = 0; i < mKeepaliveAcks.size(); ++i) {
-            final TcpKeepaliveAck keepaliveAck = mKeepaliveAcks.valueAt(i);
-            pw.print("Slot ");
-            pw.print(mKeepaliveAcks.keyAt(i));
-            pw.print(" : ");
-            pw.println(keepaliveAck);
+        for (int i = 0; i < mKeepalivePackets.size(); ++i) {
+            final KeepalivePacket keepalivePacket = mKeepalivePackets.valueAt(i);
+            if (keepalivePacket instanceof TcpKeepaliveAck) {
+                pw.print("Slot ");
+                pw.print(mKeepalivePackets.keyAt(i));
+                pw.print(": ");
+                pw.println(keepalivePacket);
+            }
+        }
+        pw.decreaseIndent();
+
+        pw.println("NAT-T Keepalive filters:");
+        pw.increaseIndent();
+        for (int i = 0; i < mKeepalivePackets.size(); ++i) {
+            final KeepalivePacket keepalivePacket = mKeepalivePackets.valueAt(i);
+            if (keepalivePacket instanceof NattKeepaliveResponse) {
+                pw.print("Slot ");
+                pw.print(mKeepalivePackets.keyAt(i));
+                pw.print(": ");
+                pw.println(keepalivePacket);
+            }
         }
         pw.decreaseIndent();
 
@@ -1858,4 +1964,18 @@
                 + (uint8(bytes[2]) << 8)
                 + (uint8(bytes[3]));
     }
+
+    private static byte[] concatArrays(final byte[]... arr) {
+        int size = 0;
+        for (byte[] a : arr) {
+            size += a.length;
+        }
+        final byte[] result = new byte[size];
+        int offset = 0;
+        for (byte[] a : arr) {
+            System.arraycopy(a, 0, result, offset, a.length);
+            offset += a.length;
+        }
+        return result;
+    }
 }
diff --git a/packages/NetworkStack/src/android/net/util/NetworkStackUtils.java b/packages/NetworkStack/src/android/net/util/NetworkStackUtils.java
index 2934e1c..9bf1b96 100644
--- a/packages/NetworkStack/src/android/net/util/NetworkStackUtils.java
+++ b/packages/NetworkStack/src/android/net/util/NetworkStackUtils.java
@@ -24,6 +24,8 @@
 import java.io.FileDescriptor;
 import java.io.IOException;
 import java.net.Inet4Address;
+import java.net.Inet6Address;
+import java.net.InetAddress;
 import java.net.SocketException;
 import java.util.List;
 import java.util.function.Predicate;
@@ -228,4 +230,13 @@
 
     private static native void addArpEntry(byte[] ethAddr, byte[] netAddr, String ifname,
             FileDescriptor fd) throws IOException;
+
+    /**
+     * Return IP address and port in a string format.
+     */
+    public static String addressAndPortToString(InetAddress address, int port) {
+        return String.format(
+                (address instanceof Inet6Address) ? "[%s]:%d" : "%s:%d",
+                        address.getHostAddress(), port);
+    }
 }
diff --git a/packages/NetworkStack/tests/src/android/net/apf/ApfTest.java b/packages/NetworkStack/tests/src/android/net/apf/ApfTest.java
index 93ab3be..8f2b968 100644
--- a/packages/NetworkStack/tests/src/android/net/apf/ApfTest.java
+++ b/packages/NetworkStack/tests/src/android/net/apf/ApfTest.java
@@ -40,6 +40,7 @@
 import android.content.Context;
 import android.net.LinkAddress;
 import android.net.LinkProperties;
+import android.net.NattKeepalivePacketDataParcelable;
 import android.net.TcpKeepalivePacketDataParcelable;
 import android.net.apf.ApfFilter.ApfConfiguration;
 import android.net.apf.ApfGenerator.IllegalInstructionException;
@@ -998,47 +999,54 @@
         }
     }
 
-    private static final int ETH_HEADER_LEN = 14;
-    private static final int ETH_DEST_ADDR_OFFSET = 0;
-    private static final int ETH_ETHERTYPE_OFFSET = 12;
+    private static final int ETH_HEADER_LEN               = 14;
+    private static final int ETH_DEST_ADDR_OFFSET         = 0;
+    private static final int ETH_ETHERTYPE_OFFSET         = 12;
     private static final byte[] ETH_BROADCAST_MAC_ADDRESS =
             {(byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff, (byte) 0xff };
 
-    private static final int IPV4_HEADER_LEN = 20;
-    private static final int IPV4_VERSION_IHL_OFFSET = ETH_HEADER_LEN + 0;
+    private static final int IPV4_HEADER_LEN          = 20;
+    private static final int IPV4_VERSION_IHL_OFFSET  = ETH_HEADER_LEN + 0;
     private static final int IPV4_TOTAL_LENGTH_OFFSET = ETH_HEADER_LEN + 2;
-    private static final int IPV4_PROTOCOL_OFFSET = ETH_HEADER_LEN + 9;
-    private static final int IPV4_SRC_ADDR_OFFSET = ETH_HEADER_LEN + 12;
-    private static final int IPV4_DEST_ADDR_OFFSET = ETH_HEADER_LEN + 16;
-    private static final int IPV4_TCP_HEADER_LEN = 20;
-    private static final int IPV4_TCP_HEADER_OFFSET = ETH_HEADER_LEN + IPV4_HEADER_LEN;
-    private static final int IPV4_TCP_SRC_PORT_OFFSET = IPV4_TCP_HEADER_OFFSET + 0;
-    private static final int IPV4_TCP_DEST_PORT_OFFSET = IPV4_TCP_HEADER_OFFSET + 2;
-    private static final int IPV4_TCP_SEQ_NUM_OFFSET = IPV4_TCP_HEADER_OFFSET + 4;
-    private static final int IPV4_TCP_ACK_NUM_OFFSET = IPV4_TCP_HEADER_OFFSET + 8;
+    private static final int IPV4_PROTOCOL_OFFSET     = ETH_HEADER_LEN + 9;
+    private static final int IPV4_SRC_ADDR_OFFSET     = ETH_HEADER_LEN + 12;
+    private static final int IPV4_DEST_ADDR_OFFSET    = ETH_HEADER_LEN + 16;
+
+    private static final int IPV4_TCP_HEADER_LEN           = 20;
+    private static final int IPV4_TCP_HEADER_OFFSET        = ETH_HEADER_LEN + IPV4_HEADER_LEN;
+    private static final int IPV4_TCP_SRC_PORT_OFFSET      = IPV4_TCP_HEADER_OFFSET + 0;
+    private static final int IPV4_TCP_DEST_PORT_OFFSET     = IPV4_TCP_HEADER_OFFSET + 2;
+    private static final int IPV4_TCP_SEQ_NUM_OFFSET       = IPV4_TCP_HEADER_OFFSET + 4;
+    private static final int IPV4_TCP_ACK_NUM_OFFSET       = IPV4_TCP_HEADER_OFFSET + 8;
     private static final int IPV4_TCP_HEADER_LENGTH_OFFSET = IPV4_TCP_HEADER_OFFSET + 12;
-    private static final int IPV4_TCP_HEADER_FLAG_OFFSET = IPV4_TCP_HEADER_OFFSET + 13;
+    private static final int IPV4_TCP_HEADER_FLAG_OFFSET   = IPV4_TCP_HEADER_OFFSET + 13;
+
+    private static final int IPV4_UDP_HEADER_OFFSET    = ETH_HEADER_LEN + IPV4_HEADER_LEN;;
+    private static final int IPV4_UDP_SRC_PORT_OFFSET  = IPV4_UDP_HEADER_OFFSET + 0;
+    private static final int IPV4_UDP_DEST_PORT_OFFSET = IPV4_UDP_HEADER_OFFSET + 2;
+    private static final int IPV4_UDP_LENGTH_OFFSET    = IPV4_UDP_HEADER_OFFSET + 4;
+    private static final int IPV4_UDP_PAYLOAD_OFFSET   = IPV4_UDP_HEADER_OFFSET + 8;
     private static final byte[] IPV4_BROADCAST_ADDRESS =
             {(byte) 255, (byte) 255, (byte) 255, (byte) 255};
 
-    private static final int IPV6_NEXT_HEADER_OFFSET = ETH_HEADER_LEN + 6;
-    private static final int IPV6_HEADER_LEN = 40;
-    private static final int IPV6_SRC_ADDR_OFFSET = ETH_HEADER_LEN + 8;
-    private static final int IPV6_DEST_ADDR_OFFSET = ETH_HEADER_LEN + 24;
-    private static final int IPV6_TCP_HEADER_OFFSET = ETH_HEADER_LEN + IPV6_HEADER_LEN;
-    private static final int IPV6_TCP_SRC_PORT_OFFSET = IPV6_TCP_HEADER_OFFSET + 0;
-    private static final int IPV6_TCP_DEST_PORT_OFFSET = IPV6_TCP_HEADER_OFFSET + 2;
-    private static final int IPV6_TCP_SEQ_NUM_OFFSET = IPV6_TCP_HEADER_OFFSET + 4;
-    private static final int IPV6_TCP_ACK_NUM_OFFSET = IPV6_TCP_HEADER_OFFSET + 8;
+    private static final int IPV6_HEADER_LEN             = 40;
+    private static final int IPV6_NEXT_HEADER_OFFSET     = ETH_HEADER_LEN + 6;
+    private static final int IPV6_SRC_ADDR_OFFSET        = ETH_HEADER_LEN + 8;
+    private static final int IPV6_DEST_ADDR_OFFSET       = ETH_HEADER_LEN + 24;
+    private static final int IPV6_TCP_HEADER_OFFSET      = ETH_HEADER_LEN + IPV6_HEADER_LEN;
+    private static final int IPV6_TCP_SRC_PORT_OFFSET    = IPV6_TCP_HEADER_OFFSET + 0;
+    private static final int IPV6_TCP_DEST_PORT_OFFSET   = IPV6_TCP_HEADER_OFFSET + 2;
+    private static final int IPV6_TCP_SEQ_NUM_OFFSET     = IPV6_TCP_HEADER_OFFSET + 4;
+    private static final int IPV6_TCP_ACK_NUM_OFFSET     = IPV6_TCP_HEADER_OFFSET + 8;
     // The IPv6 all nodes address ff02::1
-    private static final byte[] IPV6_ALL_NODES_ADDRESS =
+    private static final byte[] IPV6_ALL_NODES_ADDRESS   =
             { (byte) 0xff, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 };
     private static final byte[] IPV6_ALL_ROUTERS_ADDRESS =
             { (byte) 0xff, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2 };
 
-    private static final int ICMP6_TYPE_OFFSET = ETH_HEADER_LEN + IPV6_HEADER_LEN;
-    private static final int ICMP6_ROUTER_SOLICITATION = 133;
-    private static final int ICMP6_ROUTER_ADVERTISEMENT = 134;
+    private static final int ICMP6_TYPE_OFFSET           = ETH_HEADER_LEN + IPV6_HEADER_LEN;
+    private static final int ICMP6_ROUTER_SOLICITATION   = 133;
+    private static final int ICMP6_ROUTER_ADVERTISEMENT  = 134;
     private static final int ICMP6_NEIGHBOR_SOLICITATION = 135;
     private static final int ICMP6_NEIGHBOR_ANNOUNCEMENT = 136;
 
@@ -1050,9 +1058,9 @@
     private static final int ICMP6_RA_OPTION_OFFSET =
             ETH_HEADER_LEN + IPV6_HEADER_LEN + ICMP6_RA_HEADER_LEN;
 
-    private static final int ICMP6_PREFIX_OPTION_TYPE = 3;
-    private static final int ICMP6_PREFIX_OPTION_LEN = 32;
-    private static final int ICMP6_PREFIX_OPTION_VALID_LIFETIME_OFFSET = 4;
+    private static final int ICMP6_PREFIX_OPTION_TYPE                      = 3;
+    private static final int ICMP6_PREFIX_OPTION_LEN                       = 32;
+    private static final int ICMP6_PREFIX_OPTION_VALID_LIFETIME_OFFSET     = 4;
     private static final int ICMP6_PREFIX_OPTION_PREFERRED_LIFETIME_OFFSET = 8;
 
     // From RFC6106: Recursive DNS Server option
@@ -1063,17 +1071,17 @@
     // From RFC4191: Route Information option
     private static final int ICMP6_ROUTE_INFO_OPTION_TYPE = 24;
     // Above three options all have the same format:
-    private static final int ICMP6_4_BYTE_OPTION_LEN = 8;
+    private static final int ICMP6_4_BYTE_OPTION_LEN      = 8;
     private static final int ICMP6_4_BYTE_LIFETIME_OFFSET = 4;
-    private static final int ICMP6_4_BYTE_LIFETIME_LEN = 4;
+    private static final int ICMP6_4_BYTE_LIFETIME_LEN    = 4;
 
-    private static final int UDP_HEADER_LEN = 8;
+    private static final int UDP_HEADER_LEN              = 8;
     private static final int UDP_DESTINATION_PORT_OFFSET = ETH_HEADER_LEN + 22;
 
-    private static final int DHCP_CLIENT_PORT = 68;
+    private static final int DHCP_CLIENT_PORT       = 68;
     private static final int DHCP_CLIENT_MAC_OFFSET = ETH_HEADER_LEN + UDP_HEADER_LEN + 48;
 
-    private static final int ARP_HEADER_OFFSET = ETH_HEADER_LEN;
+    private static final int ARP_HEADER_OFFSET          = ETH_HEADER_LEN;
     private static final byte[] ARP_IPV4_REQUEST_HEADER = {
             0, 1, // Hardware type: Ethernet (1)
             8, 0, // Protocol type: IP (0x0800)
@@ -1714,6 +1722,83 @@
         return packet.array();
     }
 
+    @Test
+    public void testApfFilterNattKeepalivePacket() throws Exception {
+        final MockIpClientCallback cb = new MockIpClientCallback();
+        final ApfConfiguration config = getDefaultConfig();
+        config.multicastFilter = DROP_MULTICAST;
+        config.ieee802_3Filter = DROP_802_3_FRAMES;
+        final TestApfFilter apfFilter = new TestApfFilter(mContext, config, cb, mLog);
+        byte[] program;
+        final int srcPort = 1024;
+        final int dstPort = 4500;
+        final int slot1 = 1;
+        // NAT-T keepalive
+        final byte[] kaPayload = {(byte) 0xff};
+        final byte[] nonKaPayload = {(byte) 0xfe};
+
+        // src: 10.0.0.5, port: 1024
+        // dst: 10.0.0.6, port: 4500
+        InetAddress srcAddr = InetAddress.getByAddress(IPV4_KEEPALIVE_SRC_ADDR);
+        InetAddress dstAddr = InetAddress.getByAddress(IPV4_KEEPALIVE_DST_ADDR);
+
+        final NattKeepalivePacketDataParcelable parcel = new NattKeepalivePacketDataParcelable();
+        parcel.srcAddress = srcAddr.getAddress();
+        parcel.srcPort = srcPort;
+        parcel.dstAddress = dstAddr.getAddress();
+        parcel.dstPort = dstPort;
+
+        apfFilter.addNattKeepalivePacketFilter(slot1, parcel);
+        program = cb.getApfProgram();
+
+        // Verify IPv4 keepalive packet is dropped
+        // src: 10.0.0.6, port: 4500
+        // dst: 10.0.0.5, port: 1024
+        byte[] pkt = ipv4UdpPacket(IPV4_KEEPALIVE_DST_ADDR,
+                    IPV4_KEEPALIVE_SRC_ADDR, dstPort, srcPort, 1 /* dataLength */);
+        System.arraycopy(kaPayload, 0, pkt, IPV4_UDP_PAYLOAD_OFFSET, kaPayload.length);
+        assertDrop(program, pkt);
+
+        // Verify a packet with payload length 1 byte but it is not 0xff will pass the filter.
+        System.arraycopy(nonKaPayload, 0, pkt, IPV4_UDP_PAYLOAD_OFFSET, nonKaPayload.length);
+        assertPass(program, pkt);
+
+        // Verify IPv4 non-keepalive response packet from the same source address is passed
+        assertPass(program,
+                ipv4UdpPacket(IPV4_KEEPALIVE_DST_ADDR, IPV4_KEEPALIVE_SRC_ADDR,
+                        dstPort, srcPort, 10 /* dataLength */));
+
+        // Verify IPv4 non-keepalive response packet from other source address is passed
+        assertPass(program,
+                ipv4UdpPacket(IPV4_ANOTHER_ADDR, IPV4_KEEPALIVE_SRC_ADDR,
+                        dstPort, srcPort, 10 /* dataLength */));
+
+        apfFilter.removeKeepalivePacketFilter(slot1);
+        apfFilter.shutdown();
+    }
+
+    private static byte[] ipv4UdpPacket(byte[] sip, byte[] dip, int sport,
+            int dport, int dataLength) {
+        final int totalLength = dataLength + IPV4_HEADER_LEN + UDP_HEADER_LEN;
+        final int udpLength = UDP_HEADER_LEN + dataLength;
+        ByteBuffer packet = ByteBuffer.wrap(new byte[totalLength + ETH_HEADER_LEN]);
+
+        // ether type
+        packet.putShort(ETH_ETHERTYPE_OFFSET, (short) ETH_P_IP);
+
+        // IPv4 header
+        packet.put(IPV4_VERSION_IHL_OFFSET, (byte) 0x45);
+        packet.putShort(IPV4_TOTAL_LENGTH_OFFSET, (short) totalLength);
+        packet.put(IPV4_PROTOCOL_OFFSET, (byte) IPPROTO_UDP);
+        put(packet, IPV4_SRC_ADDR_OFFSET, sip);
+        put(packet, IPV4_DEST_ADDR_OFFSET, dip);
+        packet.putShort(IPV4_UDP_SRC_PORT_OFFSET, (short) sport);
+        packet.putShort(IPV4_UDP_DEST_PORT_OFFSET, (short) dport);
+        packet.putShort(IPV4_UDP_LENGTH_OFFSET, (short) udpLength);
+
+        return packet.array();
+    }
+
     // Verify that the last program pushed to the IpClient.Callback properly filters the
     // given packet for the given lifetime.
     private void verifyRaLifetime(byte[] program, ByteBuffer packet, int lifetime) {
diff --git a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
index 64c1830..2552f04 100644
--- a/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
+++ b/telephony/java/com/android/internal/telephony/TelephonyPermissions.java
@@ -209,18 +209,9 @@
                 context.enforcePermission(
                         android.Manifest.permission.READ_PHONE_STATE, pid, uid, message);
             } catch (SecurityException phoneStateException) {
-                SubscriptionManager sm = (SubscriptionManager) context.getSystemService(
-                        Context.TELEPHONY_SUBSCRIPTION_SERVICE);
-                int[] activeSubIds = sm.getActiveSubscriptionIdList();
-                for (int activeSubId : activeSubIds) {
-                    // If we don't have the runtime permission, but do have carrier privileges, that
-                    // suffices for reading phone state.
-                    if (getCarrierPrivilegeStatus(telephonySupplier, activeSubId, uid)
-                            == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) {
-                        return true;
-                    }
-                }
-                return false;
+                // If we don't have the runtime permission, but do have carrier privileges, that
+                // suffices for reading phone state.
+                return checkCarrierPrivilegeForAnySubId(context, telephonySupplier, uid);
             }
         }
 
@@ -236,9 +227,9 @@
      *
      * <p>This method behaves in one of the following ways:
      * <ul>
-     *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission or the
-     *       calling package passes a DevicePolicyManager Device Owner / Profile Owner device
-     *       identifier access check,
+     *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
+     *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
+     *       access check, or the calling package has carrier privileges.
      *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
      *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission.
      *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
@@ -258,9 +249,9 @@
      *
      * <p>This method behaves in one of the following ways:
      * <ul>
-     *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission or the
-     *       calling package passes a DevicePolicyManager Device Owner / Profile Owner device
-     *       identifier access check,
+     *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
+     *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
+     *       access check, or the calling package has carrier privileges.
      *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
      *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission
      *       or carrier privileges.
@@ -272,23 +263,8 @@
      */
     public static boolean checkCallingOrSelfReadDeviceIdentifiers(Context context, int subId,
             String callingPackage, String message) {
-        int pid = Binder.getCallingPid();
-        int uid = Binder.getCallingUid();
-        // if the device identifier check completes successfully then grant access.
-        if (checkReadDeviceIdentifiers(context, pid, uid, callingPackage)) {
-            return true;
-        }
-        // Calling packages with carrier privileges will also have access to device identifiers, but
-        // this may be removed in a future release.
-        if (checkCarrierPrivilegeForSubId(subId)) {
-            return true;
-        }
-        // else the calling package is not authorized to access the device identifiers; call
-        // a central method to report the failure based on the target SDK and if the calling package
-        // has the READ_PHONE_STATE permission or carrier privileges that were previously required
-        // to access the identifiers.
-        return reportAccessDeniedToReadIdentifiers(context, subId, pid, uid, callingPackage,
-                message);
+        return checkReadDeviceIdentifiers(context, TELEPHONY_SUPPLIER, subId,
+                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, message);
     }
 
     /**
@@ -298,7 +274,7 @@
      * <ul>
      *   <li>return true: if the caller has the READ_PRIVILEGED_PHONE_STATE permission, the calling
      *       package passes a DevicePolicyManager Device Owner / Profile Owner device identifier
-     *       access check, or the calling package has carrier privleges.
+     *       access check, or the calling package has carrier privileges.
      *   <li>throw SecurityException: if the caller does not meet any of the requirements and is
      *       targeting Q or is targeting pre-Q and does not have the READ_PHONE_STATE permission.
      *   <li>return false: if the caller is targeting pre-Q and does have the READ_PHONE_STATE
@@ -309,19 +285,8 @@
      */
     public static boolean checkCallingOrSelfReadSubscriberIdentifiers(Context context, int subId,
             String callingPackage, String message) {
-        int pid = Binder.getCallingPid();
-        int uid = Binder.getCallingUid();
-        // if the device identifiers can be read then grant access to the subscriber identifiers
-        if (checkReadDeviceIdentifiers(context, pid, uid, callingPackage)) {
-            return true;
-        }
-        // If the calling package has carrier privileges then allow access to the subscriber
-        // identifiers.
-        if (checkCarrierPrivilegeForSubId(subId)) {
-            return true;
-        }
-        return reportAccessDeniedToReadIdentifiers(context, subId, pid, uid, callingPackage,
-                message);
+        return checkReadDeviceIdentifiers(context, TELEPHONY_SUPPLIER, subId,
+                Binder.getCallingPid(), Binder.getCallingUid(), callingPackage, message);
     }
 
     /**
@@ -331,8 +296,10 @@
      * package passes a DevicePolicyManager Device Owner / Profile Owner device identifier access
      * check.
      */
-    private static boolean checkReadDeviceIdentifiers(Context context, int pid, int uid,
-            String callingPackage) {
+    @VisibleForTesting
+    public static boolean checkReadDeviceIdentifiers(Context context,
+            Supplier<ITelephony> telephonySupplier, int subId, int pid, int uid,
+            String callingPackage, String message) {
         // Allow system and root access to the device identifiers.
         final int appId = UserHandle.getAppId(uid);
         if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID) {
@@ -343,31 +310,36 @@
                 uid) == PackageManager.PERMISSION_GRANTED) {
             return true;
         }
-        // if the calling package is null then return now as there's no way to perform the
-        // DevicePolicyManager device / profile owner and AppOp checks
-        if (callingPackage == null) {
-            return false;
-        }
-        // Allow access to an app that has been granted the READ_DEVICE_IDENTIFIERS app op.
-        long token = Binder.clearCallingIdentity();
-        AppOpsManager appOpsManager = (AppOpsManager) context.getSystemService(
-                Context.APP_OPS_SERVICE);
-        try {
-            if (appOpsManager.noteOpNoThrow(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS, uid,
-                    callingPackage) == AppOpsManager.MODE_ALLOWED) {
-                return true;
-            }
-        } finally {
-            Binder.restoreCallingIdentity(token);
-        }
-        // Allow access to a device / profile owner app.
-        DevicePolicyManager devicePolicyManager = (DevicePolicyManager) context.getSystemService(
-                Context.DEVICE_POLICY_SERVICE);
-        if (devicePolicyManager != null && devicePolicyManager.checkDeviceIdentifierAccess(
-                callingPackage, pid, uid)) {
+        // If the calling package has carrier privileges for any subscription then allow access.
+        if (checkCarrierPrivilegeForAnySubId(context, telephonySupplier, uid)) {
             return true;
         }
-        return false;
+        // if the calling package is not null then perform the DevicePolicyManager device /
+        // profile owner and Appop checks.
+        if (callingPackage != null) {
+            // Allow access to an app that has been granted the READ_DEVICE_IDENTIFIERS app op.
+            long token = Binder.clearCallingIdentity();
+            AppOpsManager appOpsManager = (AppOpsManager) context.getSystemService(
+                    Context.APP_OPS_SERVICE);
+            try {
+                if (appOpsManager.noteOpNoThrow(AppOpsManager.OPSTR_READ_DEVICE_IDENTIFIERS, uid,
+                        callingPackage) == AppOpsManager.MODE_ALLOWED) {
+                    return true;
+                }
+            } finally {
+                Binder.restoreCallingIdentity(token);
+            }
+            // Allow access to a device / profile owner app.
+            DevicePolicyManager devicePolicyManager =
+                    (DevicePolicyManager) context.getSystemService(
+                            Context.DEVICE_POLICY_SERVICE);
+            if (devicePolicyManager != null && devicePolicyManager.checkDeviceIdentifierAccess(
+                    callingPackage, pid, uid)) {
+                return true;
+            }
+        }
+        return reportAccessDeniedToReadIdentifiers(context, subId, pid, uid, callingPackage,
+            message);
     }
 
     /**
@@ -403,10 +375,12 @@
         try {
             callingPackageInfo = context.getPackageManager().getApplicationInfoAsUser(
                     callingPackage, 0, UserHandle.getUserId(uid));
-            if (callingPackageInfo.isSystemApp()) {
-                isPreinstalled = true;
-                if (callingPackageInfo.isPrivilegedApp()) {
-                    isPrivApp = true;
+            if (callingPackageInfo != null) {
+                if (callingPackageInfo.isSystemApp()) {
+                    isPreinstalled = true;
+                    if (callingPackageInfo.isPrivilegedApp()) {
+                        isPrivApp = true;
+                    }
                 }
             }
         } catch (PackageManager.NameNotFoundException e) {
@@ -651,6 +625,26 @@
         }
     }
 
+    /**
+     * Returns whether the provided uid has carrier privileges for any active subscription ID.
+     */
+    private static boolean checkCarrierPrivilegeForAnySubId(Context context,
+            Supplier<ITelephony> telephonySupplier, int uid) {
+        SubscriptionManager sm = (SubscriptionManager) context.getSystemService(
+                Context.TELEPHONY_SUBSCRIPTION_SERVICE);
+        int[] activeSubIds = sm.getActiveSubscriptionIdList();
+        if (activeSubIds != null) {
+            for (int activeSubId : activeSubIds) {
+                if (getCarrierPrivilegeStatus(telephonySupplier, activeSubId, uid)
+                        == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+
     private static int getCarrierPrivilegeStatus(
             Supplier<ITelephony> telephonySupplier, int subId, int uid) {
         ITelephony telephony = telephonySupplier.get();