Merge "[Autofill Framework] Fix authentication id by limiting request id to only 15 bits." into main

3 files changed, 221 insertions, 109 deletions

diff --git a/services/autofill/java/com/android/server/autofill/RequestId.java b/services/autofill/java/com/android/server/autofill/RequestId.java
index 29ad786dbd4b..d8069a840156 100644
--- a/services/autofill/java/com/android/server/autofill/RequestId.java
+++ b/services/autofill/java/com/android/server/autofill/RequestId.java
@@ -16,8 +16,14 @@
 
 package com.android.server.autofill;
 
-import java.util.List;
+import static com.android.server.autofill.Helper.sDebug;
+
+import android.util.Slog;
+import android.util.SparseArray;
+
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.List;
+import java.util.Random;
 
 // Helper class containing various methods to deal with FillRequest Ids.
 // For authentication flows, there needs to be a way to know whether to retrieve the Fill
@@ -25,56 +31,97 @@ import java.util.concurrent.atomic.AtomicInteger;
 // way to achieve this is by assigning odd number request ids to secondary provider and
 // even numbers to primary provider.
 public class RequestId {
+    private AtomicInteger sIdCounter;
+
+    // The minimum request id is 2 to avoid possible authentication issues.
+    static final int MIN_REQUEST_ID = 2;
+    // The maximum request id is 0x7FFF to make sure the 16th bit is 0.
+    // This is to make sure the authentication id is always positive.
+    static final int MAX_REQUEST_ID = 0x7FFF; // 32767
+
+    // The maximum start id is made small to best avoid wrapping around.
+    static final int MAX_START_ID = 1000;
+    // The magic number is used to determine if a wrap has happened.
+    // The underlying assumption of MAGIC_NUMBER is that there can't be as many as MAGIC_NUMBER
+    // of fill requests in one session. so there can't be as many as MAGIC_NUMBER of fill requests
+    // getting dropped.
+    static final int MAGIC_NUMBER = 5000;
+
+    static final int MIN_PRIMARY_REQUEST_ID = 2;
+    static final int MAX_PRIMARY_REQUEST_ID = 0x7FFE; // 32766
+
+    static final int MIN_SECONDARY_REQUEST_ID = 3;
+    static final int MAX_SECONDARY_REQUEST_ID = 0x7FFF; // 32767
+
+    private static final String TAG = "RequestId";
+
+    // WARNING: This constructor should only be used for testing
+    RequestId(int startId) {
+        if (startId < MIN_REQUEST_ID || startId > MAX_REQUEST_ID) {
+            throw new IllegalArgumentException("startId must be between " + MIN_REQUEST_ID +
+                                                   " and " + MAX_REQUEST_ID);
+        }
+        if (sDebug) {
+            Slog.d(TAG, "RequestId(int): startId= " + startId);
+        }
+        sIdCounter = new AtomicInteger(startId);
+    }
 
-  private AtomicInteger sIdCounter;
-
-  // Mainly used for tests
-  RequestId(int start) {
-    sIdCounter = new AtomicInteger(start);
-  }
-
-  public RequestId() {
-    this((int) (Math.floor(Math.random() * 0xFFFF)));
-  }
-
-  public static int getLastRequestIdIndex(List<Integer> requestIds) {
-    int lastId = -1;
-    int indexOfBiggest = -1;
-    // Biggest number is usually the latest request, since IDs only increase
-    // The only exception is when the request ID wraps around back to 0
-      for (int i = requestIds.size() - 1; i >= 0; i--) {
-        if (requestIds.get(i) > lastId) {
-        lastId = requestIds.get(i);
-        indexOfBiggest = i;
-      }
+    // WARNING: This get method should only be used for testing
+    int getRequestId() {
+        return sIdCounter.get();
     }
 
-    // 0xFFFE + 2 == 0x1 (for secondary)
-    // 0xFFFD + 2 == 0x0 (for primary)
-    // Wrap has occurred
-    if (lastId >= 0xFFFD) {
-      // Calculate the biggest size possible
-      // If list only has one kind of request ids - we need to multiple by 2
-      // (since they skip odd ints)
-      // Also subtract one from size because at least one integer exists pre-wrap
-      int calcSize = (requestIds.size()) * 2;
-      //Biggest possible id after wrapping
-      int biggestPossible = (lastId + calcSize) % 0xFFFF;
-      lastId = -1;
-      indexOfBiggest = -1;
-      for (int i = 0; i < requestIds.size(); i++) {
-        int currentId = requestIds.get(i);
-        if (currentId <= biggestPossible && currentId > lastId) {
-          lastId = currentId;
-          indexOfBiggest = i;
+    public RequestId() {
+        Random random = new Random();
+        int low = MIN_REQUEST_ID;
+        int high = MAX_START_ID + 1; // nextInt is exclusive on upper limit
+
+        // Generate a random start request id that >= MIN_REQUEST_ID and <= MAX_START_ID
+        int startId = random.nextInt(high - low) + low;
+        if (sDebug) {
+            Slog.d(TAG, "RequestId(): startId= " + startId);
         }
-      }
+        sIdCounter = new AtomicInteger(startId);
     }
 
-    return indexOfBiggest;
-  }
+    // Given a list of request ids, find the index of the last request id.
+    // Note: Since the request id wraps around, the largest request id may not be
+    // the latest request id.
+    //
+    // @param requestIds List of request ids in ascending order with at least one element.
+    // @return Index of the last request id.
+    public static int getLastRequestIdIndex(List<Integer> requestIds) {
+        // If there is only one request id, return index as 0.
+        if (requestIds.size() == 1) {
+            return 0;
+        }
+
+        // We have to use a magical number to determine if a wrap has happened because
+        // the request id could be lost. The underlying assumption of MAGIC_NUMBER is that
+        // there can't be as many as MAGIC_NUMBER of fill requests in one session.
+        boolean wrapHasHappened = false;
+        int latestRequestIdIndex = -1;
+
+        for (int i = 0; i < requestIds.size() - 1; i++) {
+            if (requestIds.get(i+1) - requestIds.get(i) > MAGIC_NUMBER) {
+                wrapHasHappened = true;
+                latestRequestIdIndex = i;
+                break;
+            }
+        }
+
+        // If there was no wrap, the last request index is the last index.
+        if (!wrapHasHappened) {
+            latestRequestIdIndex = requestIds.size() - 1;
+        }
+        if (sDebug) {
+            Slog.d(TAG, "getLastRequestIdIndex(): latestRequestIdIndex = " + latestRequestIdIndex);
+        }
+        return latestRequestIdIndex;
+    }
 
-  public int nextId(boolean isSecondary) {
+    public int nextId(boolean isSecondary) {
         // For authentication flows, there needs to be a way to know whether to retrieve the Fill
         // Response from the primary provider or the secondary provider from the requestId. A simple
         // way to achieve this is by assigning odd number request ids to secondary provider and
@@ -82,13 +129,20 @@ public class RequestId {
         int requestId;
 
         do {
-            requestId = sIdCounter.incrementAndGet() % 0xFFFF;
+            requestId = sIdCounter.incrementAndGet() % (MAX_REQUEST_ID + 1);
+            // Skip numbers smaller than MIN_REQUEST_ID to avoid possible authentication issue
+            if (requestId < MIN_REQUEST_ID) {
+                requestId = MIN_REQUEST_ID;
+            }
             sIdCounter.set(requestId);
         } while (isSecondaryProvider(requestId) != isSecondary);
+        if (sDebug) {
+            Slog.d(TAG, "nextId(): requestId = " + requestId);
+        }
         return requestId;
-  }
+    }
 
-  public static boolean isSecondaryProvider(int requestId) {
-      return requestId % 2 == 1;
-  }
+    public static boolean isSecondaryProvider(int requestId) {
+        return requestId % 2 == 1;
+    }
 }
diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index 494e956c413f..c6ddc16211bc 100644
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -6902,17 +6902,18 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         return mPendingSaveUi != null && mPendingSaveUi.getState() == PendingUi.STATE_PENDING;
     }
 
+    // Return latest response index in mResponses SparseArray.
     @GuardedBy("mLock")
     private int getLastResponseIndexLocked() {
-        if (mResponses != null) {
-            List<Integer> requestIdList = new ArrayList<>();
-            final int responseCount = mResponses.size();
-            for (int i = 0; i < responseCount; i++) {
-                requestIdList.add(mResponses.keyAt(i));
-            }
-            return mRequestId.getLastRequestIdIndex(requestIdList);
+        if (mResponses == null  || mResponses.size() == 0) {
+          return -1;
+        }
+        List<Integer> requestIdList = new ArrayList<>();
+        final int responseCount = mResponses.size();
+        for (int i = 0; i < responseCount; i++) {
+            requestIdList.add(mResponses.keyAt(i));
         }
-        return -1;
+        return mRequestId.getLastRequestIdIndex(requestIdList);
     }
 
     private LogMaker newLogMaker(int category) {
diff --git a/services/tests/servicestests/src/com/android/server/autofill/RequestIdTest.java b/services/tests/servicestests/src/com/android/server/autofill/RequestIdTest.java
index 6d56c417f789..60c3659202be 100644
--- a/services/tests/servicestests/src/com/android/server/autofill/RequestIdTest.java
+++ b/services/tests/servicestests/src/com/android/server/autofill/RequestIdTest.java
@@ -17,17 +17,25 @@ package com.android.server.autofill;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import android.util.Slog;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.util.ArrayList;
-import java.util.List;
-
 @RunWith(JUnit4.class)
 public class RequestIdTest {
 
+    private static final int TEST_DATASET_SIZE = 300;
+    private static final int TEST_WRAP_SIZE = 50; // Number of request ids before wrap happens
+    private static final String TAG = "RequestIdTest";
+
     List<Integer> datasetPrimaryNoWrap = new ArrayList<>();
     List<Integer> datasetPrimaryWrap = new ArrayList<>();
     List<Integer> datasetSecondaryNoWrap = new ArrayList<>();
@@ -35,151 +43,200 @@ public class RequestIdTest {
     List<Integer> datasetMixedNoWrap = new ArrayList<>();
     List<Integer> datasetMixedWrap = new ArrayList<>();
 
-    @Before
-    public void setup() throws Exception {
-      int datasetSize = 300;
+    List<Integer> manualWrapRequestIdList = Arrays.asList(3, 9, 15,
+                                                            RequestId.MAX_SECONDARY_REQUEST_ID - 5,
+                                                            RequestId.MAX_SECONDARY_REQUEST_ID - 3);
+    List<Integer> manualNoWrapRequestIdList =Arrays.asList(2, 6, 10, 14, 18, 22, 26, 30);
 
+    List<Integer> manualOneElementRequestIdList = Arrays.asList(1);
+
+    @Before
+    public void setup() throws IllegalArgumentException {
+        Slog.d(TAG, "setup()");
         { // Generate primary only ids that do not wrap
-            RequestId requestId = new RequestId(0);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MIN_PRIMARY_REQUEST_ID);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetPrimaryNoWrap.add(requestId.nextId(false));
             }
+            Collections.sort(datasetPrimaryNoWrap);
         }
 
         { // Generate primary only ids that wrap
-            RequestId requestId = new RequestId(0xff00);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MAX_PRIMARY_REQUEST_ID -
+                                                    TEST_WRAP_SIZE * 2);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetPrimaryWrap.add(requestId.nextId(false));
             }
+            Collections.sort(datasetPrimaryWrap);
         }
 
         { // Generate SECONDARY only ids that do not wrap
-            RequestId requestId = new RequestId(0);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MIN_SECONDARY_REQUEST_ID);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetSecondaryNoWrap.add(requestId.nextId(true));
             }
+            Collections.sort(datasetSecondaryNoWrap);
         }
 
         { // Generate SECONDARY only ids that wrap
-            RequestId requestId = new RequestId(0xff00);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MAX_SECONDARY_REQUEST_ID -
+                                                    TEST_WRAP_SIZE * 2);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetSecondaryWrap.add(requestId.nextId(true));
             }
+            Collections.sort(datasetSecondaryWrap);
         }
 
         { // Generate MIXED only ids that do not wrap
-            RequestId requestId = new RequestId(0);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MIN_REQUEST_ID);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetMixedNoWrap.add(requestId.nextId(i % 2 != 0));
             }
+            Collections.sort(datasetMixedNoWrap);
         }
 
         { // Generate MIXED only ids that wrap
-            RequestId requestId = new RequestId(0xff00);
-            for (int i = 0; i < datasetSize; i++) {
+            RequestId requestId = new RequestId(RequestId.MAX_REQUEST_ID -
+                                                    TEST_WRAP_SIZE);
+            for (int i = 0; i < TEST_DATASET_SIZE; i++) {
                 datasetMixedWrap.add(requestId.nextId(i % 2 != 0));
             }
+            Collections.sort(datasetMixedWrap);
         }
+        Slog.d(TAG, "finishing setup()");
     }
 
     @Test
     public void testRequestIdLists() {
+        Slog.d(TAG, "testRequestIdLists()");
         for (int id : datasetPrimaryNoWrap) {
             assertThat(RequestId.isSecondaryProvider(id)).isFalse();
-            assertThat(id >= 0).isTrue();
-            assertThat(id < 0xffff).isTrue();
+            assertThat(id).isAtLeast(RequestId.MIN_PRIMARY_REQUEST_ID);
+            assertThat(id).isAtMost(RequestId.MAX_PRIMARY_REQUEST_ID);
         }
 
         for (int id : datasetPrimaryWrap) {
             assertThat(RequestId.isSecondaryProvider(id)).isFalse();
-            assertThat(id >= 0).isTrue();
-            assertThat(id < 0xffff).isTrue();
+            assertThat(id).isAtLeast(RequestId.MIN_PRIMARY_REQUEST_ID);
+            assertThat(id).isAtMost(RequestId.MAX_PRIMARY_REQUEST_ID);
         }
 
         for (int id : datasetSecondaryNoWrap) {
             assertThat(RequestId.isSecondaryProvider(id)).isTrue();
-            assertThat(id >= 0).isTrue();
-            assertThat(id < 0xffff).isTrue();
+            assertThat(id).isAtLeast(RequestId.MIN_SECONDARY_REQUEST_ID);
+            assertThat(id).isAtMost(RequestId.MAX_SECONDARY_REQUEST_ID);
         }
 
         for (int id : datasetSecondaryWrap) {
             assertThat(RequestId.isSecondaryProvider(id)).isTrue();
-            assertThat(id >= 0).isTrue();
-            assertThat(id < 0xffff).isTrue();
+            assertThat(id).isAtLeast(RequestId.MIN_SECONDARY_REQUEST_ID);
+            assertThat(id).isAtMost(RequestId.MAX_SECONDARY_REQUEST_ID);
         }
     }
 
     @Test
-    public void testRequestIdGeneration() {
-        RequestId requestId = new RequestId(0);
+    public void testCreateNewRequestId() {
+        Slog.d(TAG, "testCreateNewRequestId()");
+        for (int i = 0; i < 100000; i++) {
+            RequestId requestId = new RequestId();
+            assertThat(requestId.getRequestId()).isAtLeast(RequestId.MIN_REQUEST_ID);
+            assertThat(requestId.getRequestId()).isAtMost(RequestId.MAX_START_ID);
+        }
+    }
 
+    @Test
+    public void testGetNextRequestId() throws IllegalArgumentException{
+        Slog.d(TAG, "testGetNextRequestId()");
+        RequestId requestId = new RequestId();
         // Large Primary
         for (int i = 0; i < 100000; i++) {
             int y = requestId.nextId(false);
             assertThat(RequestId.isSecondaryProvider(y)).isFalse();
-            assertThat(y >= 0).isTrue();
-            assertThat(y < 0xffff).isTrue();
+            assertThat(y).isAtLeast(RequestId.MIN_PRIMARY_REQUEST_ID);
+            assertThat(y).isAtMost(RequestId.MAX_PRIMARY_REQUEST_ID);
         }
 
         // Large Secondary
-        requestId = new RequestId(0);
+        requestId = new RequestId();
         for (int i = 0; i < 100000; i++) {
             int y = requestId.nextId(true);
             assertThat(RequestId.isSecondaryProvider(y)).isTrue();
-            assertThat(y >= 0).isTrue();
-            assertThat(y < 0xffff).isTrue();
+            assertThat(y).isAtLeast(RequestId.MIN_SECONDARY_REQUEST_ID);
+            assertThat(y).isAtMost(RequestId.MAX_SECONDARY_REQUEST_ID);
         }
 
         // Large Mixed
-        requestId = new RequestId(0);
+        requestId = new RequestId();
         for (int i = 0; i < 50000; i++) {
             int y = requestId.nextId(i % 2 != 0);
-            assertThat(RequestId.isSecondaryProvider(y)).isEqualTo(i % 2 == 0);
-            assertThat(y >= 0).isTrue();
-            assertThat(y < 0xffff).isTrue();
+            assertThat(y).isAtLeast(RequestId.MIN_REQUEST_ID);
+            assertThat(y).isAtMost(RequestId.MAX_REQUEST_ID);
         }
     }
 
     @Test
     public void testGetLastRequestId() {
-        // In this test, request ids are generated FIFO, so the last entry is also the last
-        // request
+        Slog.d(TAG, "testGetLastRequestId()");
 
-        { // Primary no wrap
-          int lastIdIndex = datasetPrimaryNoWrap.size() - 1;
-          int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetPrimaryNoWrap);
-          assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
+        {   // Primary no wrap
+            int lastIdIndex = datasetPrimaryNoWrap.size() - 1;
+            int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetPrimaryNoWrap);
+            assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
         }
 
-        { // Primary wrap
-            int lastIdIndex = datasetPrimaryWrap.size() - 1;
+        {   // Primary wrap
+            // The last index would be the # of request ids left after wrap
+            // minus 1 (index starts at 0)
+            int lastIdIndex = TEST_DATASET_SIZE - TEST_WRAP_SIZE - 1;
             int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetPrimaryWrap);
-            assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
+            assertThat(lastComputedIdIndex).isEqualTo(lastIdIndex);
         }
 
-        { // Secondary no wrap
+        {   // Secondary no wrap
             int lastIdIndex = datasetSecondaryNoWrap.size() - 1;
             int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetSecondaryNoWrap);
             assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
         }
 
-        { // Secondary wrap
-            int lastIdIndex = datasetSecondaryWrap.size() - 1;
+        {   // Secondary wrap
+            int lastIdIndex = TEST_DATASET_SIZE - TEST_WRAP_SIZE - 1;
             int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetSecondaryWrap);
             assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
         }
 
-        { // Mixed no wrap
+        {   // Mixed no wrap
             int lastIdIndex = datasetMixedNoWrap.size() - 1;
             int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetMixedNoWrap);
             assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
         }
 
-        { // Mixed wrap
-            int lastIdIndex = datasetMixedWrap.size() - 1;
+        {   // Mixed wrap
+            int lastIdIndex = TEST_DATASET_SIZE - TEST_WRAP_SIZE - 1;
             int lastComputedIdIndex = RequestId.getLastRequestIdIndex(datasetMixedWrap);
             assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
         }
 
+        {   // Manual wrap
+            int lastIdIndex = 2; // [3, 9, 15,
+                                 // MAX_SECONDARY_REQUEST_ID - 5, MAX_SECONDARY_REQUEST_ID - 3]
+            int lastComputedIdIndex = RequestId.getLastRequestIdIndex(manualWrapRequestIdList);
+            assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
+        }
+
+        {   // Manual no wrap
+            int lastIdIndex = manualNoWrapRequestIdList.size() - 1; // [2, 6, 10, 14,
+                                                                    // 18, 22, 26, 30]
+            int lastComputedIdIndex = RequestId.getLastRequestIdIndex(manualNoWrapRequestIdList);
+            assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
+
+        }
+
+        {   // Manual one element
+            int lastIdIndex = 0; // [1]
+            int lastComputedIdIndex = RequestId.getLastRequestIdIndex(
+                manualOneElementRequestIdList);
+            assertThat(lastIdIndex).isEqualTo(lastComputedIdIndex);
+
+        }
     }
 }