diff options
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 |