Revert "WifiConfigStore: Limit integrity checks to single user devices"

am: e9adc4e4c0

Change-Id: I53d385d1224a6aff6a3e17387847eb7e6d01d261

5 files changed, 34 insertions, 108 deletions

diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 8da2d2cb4..bda1eb7d2 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -3094,7 +3094,7 @@ public class WifiConfigManager {
         if (mDeferredUserUnlockRead) {
             Log.i(TAG, "Handling user unlock before loading from store.");
             List<WifiConfigStore.StoreFile> userStoreFiles =
-                    WifiConfigStore.createUserFiles(mCurrentUserId, UserManager.get(mContext));
+                    WifiConfigStore.createUserFiles(mCurrentUserId);
             if (userStoreFiles == null) {
                 Log.wtf(TAG, "Failed to create user store files");
                 return false;
@@ -3133,7 +3133,7 @@ public class WifiConfigManager {
     private boolean loadFromUserStoreAfterUnlockOrSwitch(int userId) {
         try {
             List<WifiConfigStore.StoreFile> userStoreFiles =
-                    WifiConfigStore.createUserFiles(userId, UserManager.get(mContext));
+                    WifiConfigStore.createUserFiles(userId);
             if (userStoreFiles == null) {
                 Log.e(TAG, "Failed to create user store files");
                 return false;
diff --git a/service/java/com/android/server/wifi/WifiConfigStore.java b/service/java/com/android/server/wifi/WifiConfigStore.java
index 6989e728f..a618eb5b5 100644
--- a/service/java/com/android/server/wifi/WifiConfigStore.java
+++ b/service/java/com/android/server/wifi/WifiConfigStore.java
@@ -27,7 +27,6 @@ import android.os.Environment;
 import android.os.FileUtils;
 import android.os.Handler;
 import android.os.Looper;
-import android.os.UserManager;
 import android.util.Log;
 import android.util.SparseArray;
 import android.util.Xml;
@@ -209,7 +208,7 @@ public class WifiConfigStore {
      * @param clock       clock instance to retrieve timestamps for alarms.
      * @param wifiMetrics Metrics instance.
      * @param sharedStore StoreFile instance pointing to the shared store file. This should
-     *                    be retrieved using {@link #createSharedFile(UserManager)} method.
+     *                    be retrieved using {@link #createSharedFile()} method.
      */
     public WifiConfigStore(Context context, Looper looper, Clock clock, WifiMetrics wifiMetrics,
             StoreFile sharedStore) {
@@ -230,8 +229,7 @@ public class WifiConfigStore {
     /**
      * Set the user store files.
      * (Useful for mocking in unit tests).
-     * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int,
-     * UserManager)}.
+     * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int)}.
      */
     public void setUserStores(@NonNull List<StoreFile> userStores) {
         Preconditions.checkNotNull(userStores);
@@ -268,11 +266,9 @@ public class WifiConfigStore {
      * @param storeBaseDir Base directory under which the store file is to be stored. The store file
      *                     will be at <storeBaseDir>/wifi/WifiConfigStore.xml.
      * @param fileId Identifier for the file. See {@link StoreFileId}.
-     * @param userManager Instance of UserManager to check if the device is in single user mode.
      * @return new instance of the store file or null if the directory cannot be created.
      */
-    private static @Nullable StoreFile createFile(File storeBaseDir, @StoreFileId int fileId,
-                                                  UserManager userManager) {
+    private static @Nullable StoreFile createFile(File storeBaseDir, @StoreFileId int fileId) {
         File storeDir = new File(storeBaseDir, STORE_DIRECTORY_NAME);
         if (!storeDir.exists()) {
             if (!storeDir.mkdir()) {
@@ -280,24 +276,16 @@ public class WifiConfigStore {
                 return null;
             }
         }
-        File file = new File(storeDir, STORE_ID_TO_FILE_NAME.get(fileId));
-        DataIntegrityChecker dataIntegrityChecker = null;
-        // Turn on integrity checking only for single user mode devices.
-        if (userManager.hasUserRestriction(UserManager.DISALLOW_ADD_USER)) {
-            dataIntegrityChecker = new DataIntegrityChecker(file.getAbsolutePath());
-        }
-        return new StoreFile(file, fileId, dataIntegrityChecker);
+        return new StoreFile(new File(storeDir, STORE_ID_TO_FILE_NAME.get(fileId)), fileId);
     }
 
     /**
      * Create a new instance of the shared store file.
      *
-     * @param userManager Instance of UserManager to check if the device is in single user mode.
      * @return new instance of the store file or null if the directory cannot be created.
      */
-    public static @Nullable StoreFile createSharedFile(UserManager userManager) {
-        return createFile(
-                Environment.getDataMiscDirectory(), STORE_FILE_SHARED_GENERAL, userManager);
+    public static @Nullable StoreFile createSharedFile() {
+        return createFile(Environment.getDataMiscDirectory(), STORE_FILE_SHARED_GENERAL);
     }
 
     /**
@@ -305,16 +293,14 @@ public class WifiConfigStore {
      * The user store file is inside the user's encrypted data directory.
      *
      * @param userId userId corresponding to the currently logged-in user.
-     * @param userManager Instance of UserManager to check if the device is in single user mode.
      * @return List of new instances of the store files created or null if the directory cannot be
      * created.
      */
-    public static @Nullable List<StoreFile> createUserFiles(int userId, UserManager userManager) {
+    public static @Nullable List<StoreFile> createUserFiles(int userId) {
         List<StoreFile> storeFiles = new ArrayList<>();
         for (int fileId : Arrays.asList(
                 STORE_FILE_USER_GENERAL, STORE_FILE_USER_NETWORK_SUGGESTIONS)) {
-            StoreFile storeFile =
-                    createFile(Environment.getDataMiscCeDirectory(userId), fileId, userManager);
+            StoreFile storeFile = createFile(Environment.getDataMiscCeDirectory(userId), fileId);
             if (storeFile == null) {
                 return null;
             }
@@ -516,8 +502,7 @@ public class WifiConfigStore {
      * Handles a user switch. This method changes the user specific store files and reads from the
      * new user's store files.
      *
-     * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int,
-     * UserManager)}.
+     * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int)}.
      */
     public void switchUserStoresAndRead(@NonNull List<StoreFile> userStores)
             throws XmlPullParserException, IOException {
@@ -670,21 +655,19 @@ public class WifiConfigStore {
          */
         private String mFileName;
         /**
-         * {@link StoreFileId} Type of store file.
+         * The integrity file storing integrity checking data for the store file.
          */
-        private @StoreFileId int mFileId;
+        private DataIntegrityChecker mDataIntegrityChecker;
         /**
-         * The integrity file storing integrity checking data for the store file.
-         * Note: This is only turned on for single user devices.
+         * {@link StoreFileId} Type of store file.
          */
-        private @Nullable DataIntegrityChecker mDataIntegrityChecker;
+        private @StoreFileId int mFileId;
 
-        public StoreFile(File file, @StoreFileId int fileId,
-                         @Nullable DataIntegrityChecker dataIntegrityChecker) {
+        public StoreFile(File file, @StoreFileId int fileId) {
             mAtomicFile = new AtomicFile(file);
             mFileName = mAtomicFile.getBaseFile().getAbsolutePath();
+            mDataIntegrityChecker = new DataIntegrityChecker(mFileName);
             mFileId = fileId;
-            mDataIntegrityChecker = dataIntegrityChecker;
         }
 
         /**
@@ -696,7 +679,6 @@ public class WifiConfigStore {
             return mAtomicFile.exists();
         }
 
-
         /**
          * Read the entire raw data from the store file and return in a byte array.
          *
@@ -709,24 +691,20 @@ public class WifiConfigStore {
             byte[] bytes = null;
             try {
                 bytes = mAtomicFile.readFully();
-            } catch (FileNotFoundException e) {
-                return null;
-            }
-            if (mDataIntegrityChecker != null) {
                 // Check that the file has not been altered since last writeBufferedRawData()
-                try {
-                    if (!mDataIntegrityChecker.isOk(bytes)) {
-                        Log.wtf(TAG, "Data integrity problem with file: " + mFileName);
-                        return null;
-                    }
-                } catch (DigestException e) {
-                    // When integrity checking is introduced. The existing data will have no
-                    // related integrity file for validation. Thus, we will assume the existing
-                    // data is correct and immediately create the integrity file.
-                    Log.i(TAG, "isOK() had no integrity data to check; thus vacuously "
-                            + "true. Running update now.");
-                    mDataIntegrityChecker.update(bytes);
+                if (!mDataIntegrityChecker.isOk(bytes)) {
+                    Log.wtf(TAG, "Data integrity problem with file: " + mFileName);
+                    return null;
                 }
+            } catch (FileNotFoundException e) {
+                return null;
+            } catch (DigestException e) {
+                // When integrity checking is introduced. The existing data will have no related
+                // integrity file for validation. Thus, we will assume the existing data is correct
+                // and immediately create the integrity file.
+                Log.i(TAG, "isOK() had no integrity data to check; thus vacuously "
+                        + "true. Running update now.");
+                mDataIntegrityChecker.update(bytes);
             }
             return bytes;
         }
@@ -763,10 +741,8 @@ public class WifiConfigStore {
                 }
                 throw e;
             }
-            if (mDataIntegrityChecker != null) {
-                // There was a legitimate change and update the integrity checker.
-                mDataIntegrityChecker.update(mWriteData);
-            }
+            // There was a legitimate change and update the integrity checker.
+            mDataIntegrityChecker.update(mWriteData);
             // Reset the pending write data after write.
             mWriteData = null;
         }
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 762b24b0a..8095bfac6 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -241,7 +241,7 @@ public class WifiInjector {
         mWifiKeyStore = new WifiKeyStore(mKeyStore);
         mWifiConfigStore = new WifiConfigStore(
                 mContext, clientModeImplLooper, mClock, mWifiMetrics,
-                WifiConfigStore.createSharedFile(UserManager.get(mContext)));
+                WifiConfigStore.createSharedFile());
         SubscriptionManager subscriptionManager =
                 mContext.getSystemService(SubscriptionManager.class);
         // Config Manager
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
index 28964b109..686b2098d 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
@@ -229,8 +229,7 @@ public class WifiConfigManagerTest {
         mSession = ExtendedMockito.mockitoSession()
                 .mockStatic(WifiConfigStore.class, withSettings().lenient())
                 .startMocking();
-        when(WifiConfigStore.createUserFiles(anyInt(), any(UserManager.class)))
-                .thenReturn(mock(List.class));
+        when(WifiConfigStore.createUserFiles(anyInt())).thenReturn(mock(List.class));
         when(mTelephonyManager.createForSubscriptionId(anyInt())).thenReturn(mDataTelephonyManager);
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java
index 6fdcce80c..64e762bec 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java
@@ -31,7 +31,6 @@ import androidx.test.filters.SmallTest;
 import com.android.internal.util.ArrayUtils;
 import com.android.server.wifi.WifiConfigStore.StoreData;
 import com.android.server.wifi.WifiConfigStore.StoreFile;
-import com.android.server.wifi.util.DataIntegrityChecker;
 import com.android.server.wifi.util.XmlUtil;
 
 import org.junit.After;
@@ -148,7 +147,6 @@ public class WifiConfigStoreTest {
     private TestLooper mLooper;
     @Mock private Clock mClock;
     @Mock private WifiMetrics mWifiMetrics;
-    @Mock private DataIntegrityChecker mDataIntegrityChecker;
     private MockStoreFile mSharedStore;
     private MockStoreFile mUserStore;
     private MockStoreFile mUserNetworkSuggestionsStore;
@@ -381,48 +379,6 @@ public class WifiConfigStoreTest {
     }
 
     /**
-     * Tests the read API behaviour after a write to the store files (with no integrity checks).
-     * Expected behaviour: The read should return the same data that was last written.
-     */
-    @Test
-    public void testReadAfterWriteWithNoIntegrityCheck() throws Exception {
-        // Recreate the mock store files with no data integrity checking.
-        mUserStores.clear();
-        mSharedStore = new MockStoreFile(WifiConfigStore.STORE_FILE_SHARED_GENERAL, null);
-        mUserStore = new MockStoreFile(WifiConfigStore.STORE_FILE_USER_GENERAL, null);
-        mUserNetworkSuggestionsStore =
-                new MockStoreFile(WifiConfigStore.STORE_FILE_USER_NETWORK_SUGGESTIONS, null);
-        mUserStores.add(mUserStore);
-        mUserStores.add(mUserNetworkSuggestionsStore);
-        mWifiConfigStore = new WifiConfigStore(mContext, mLooper.getLooper(), mClock, mWifiMetrics,
-                mSharedStore);
-
-        // Register data container.
-        mWifiConfigStore.registerStoreData(mSharedStoreData);
-        mWifiConfigStore.registerStoreData(mUserStoreData);
-
-        // Read both share and user config store.
-        mWifiConfigStore.switchUserStoresAndRead(mUserStores);
-
-        // Verify no data is read.
-        assertNull(mUserStoreData.getData());
-        assertNull(mSharedStoreData.getData());
-
-        // Write share and user data.
-        mUserStoreData.setData(TEST_USER_DATA);
-        mSharedStoreData.setData(TEST_SHARE_DATA);
-        mWifiConfigStore.write(true);
-
-        // Read and verify the data content in the data container.
-        mWifiConfigStore.read();
-        assertEquals(TEST_USER_DATA, mUserStoreData.getData());
-        assertEquals(TEST_SHARE_DATA, mSharedStoreData.getData());
-
-        verify(mWifiMetrics, times(2)).noteWifiConfigStoreReadDuration(anyInt());
-        verify(mWifiMetrics).noteWifiConfigStoreWriteDuration(anyInt());
-    }
-
-    /**
      * Tests the read API behaviour when the shared store file is empty and the user store
      * is not yet visible (user not yet unlocked).
      * Expected behaviour: The read should return an empty store data instance when the file not
@@ -805,12 +761,7 @@ public class WifiConfigStoreTest {
         private boolean mStoreWritten;
 
         MockStoreFile(@WifiConfigStore.StoreFileId int fileId) {
-            super(new File("MockStoreFile"), fileId, mDataIntegrityChecker);
-        }
-
-        MockStoreFile(@WifiConfigStore.StoreFileId int fileId,
-                      DataIntegrityChecker dataIntegrityChecker) {
-            super(new File("MockStoreFile"), fileId, dataIntegrityChecker);
+            super(new File("MockStoreFile"), fileId);
         }
 
         @Override