Revert "WifiConfigStore: Limit integrity checks to single user devices"
This reverts commit 8e70909c098f29b008d062e0cb30f313d300542d.
Removing workaround. Proper fix in CL above.
Bug: 138482990
Test: N/A
Change-Id: Iaee7746a3afc0fd6e80daa77ce5034e660261e8b
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index 8da2d2c..bda1eb7 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -3094,7 +3094,7 @@
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 @@
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 6989e72..a618eb5 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.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 @@
* @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 @@
/**
* 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 @@
* @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 @@
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 @@
* 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 @@
* 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 @@
*/
private String mFileName;
/**
+ * The integrity file storing integrity checking data for the store file.
+ */
+ private DataIntegrityChecker mDataIntegrityChecker;
+ /**
* {@link StoreFileId} Type of store file.
*/
private @StoreFileId int mFileId;
- /**
- * The integrity file storing integrity checking data for the store file.
- * Note: This is only turned on for single user devices.
- */
- private @Nullable DataIntegrityChecker mDataIntegrityChecker;
- 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 @@
return mAtomicFile.exists();
}
-
/**
* Read the entire raw data from the store file and return in a byte array.
*
@@ -709,24 +691,20 @@
byte[] bytes = null;
try {
bytes = mAtomicFile.readFully();
+ // Check that the file has not been altered since last writeBufferedRawData()
+ if (!mDataIntegrityChecker.isOk(bytes)) {
+ Log.wtf(TAG, "Data integrity problem with file: " + mFileName);
+ return null;
+ }
} 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);
- }
+ } 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 @@
}
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 762b24b..8095bfa 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -241,7 +241,7 @@
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 28964b1..686b209 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 @@
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 6fdcce8..64e762b 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 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 @@
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 @@
}
/**
- * 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 @@
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
