diff options
5 files changed, 108 insertions, 34 deletions
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index 1d287f02d..0c175acd2 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -3088,7 +3088,7 @@ public class WifiConfigManager { if (mDeferredUserUnlockRead) { Log.i(TAG, "Handling user unlock before loading from store."); List<WifiConfigStore.StoreFile> userStoreFiles = - WifiConfigStore.createUserFiles(mCurrentUserId); + WifiConfigStore.createUserFiles(mCurrentUserId, UserManager.get(mContext)); if (userStoreFiles == null) { Log.wtf(TAG, "Failed to create user store files"); return false; @@ -3127,7 +3127,7 @@ public class WifiConfigManager { private boolean loadFromUserStoreAfterUnlockOrSwitch(int userId) { try { List<WifiConfigStore.StoreFile> userStoreFiles = - WifiConfigStore.createUserFiles(userId); + WifiConfigStore.createUserFiles(userId, UserManager.get(mContext)); 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 a618eb5b5..6989e728f 100644 --- a/service/java/com/android/server/wifi/WifiConfigStore.java +++ b/service/java/com/android/server/wifi/WifiConfigStore.java @@ -27,6 +27,7 @@ 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; @@ -208,7 +209,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()} method. + * be retrieved using {@link #createSharedFile(UserManager)} method. */ public WifiConfigStore(Context context, Looper looper, Clock clock, WifiMetrics wifiMetrics, StoreFile sharedStore) { @@ -229,7 +230,8 @@ 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)}. + * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int, + * UserManager)}. */ public void setUserStores(@NonNull List<StoreFile> userStores) { Preconditions.checkNotNull(userStores); @@ -266,9 +268,11 @@ 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) { + private static @Nullable StoreFile createFile(File storeBaseDir, @StoreFileId int fileId, + UserManager userManager) { File storeDir = new File(storeBaseDir, STORE_DIRECTORY_NAME); if (!storeDir.exists()) { if (!storeDir.mkdir()) { @@ -276,16 +280,24 @@ public class WifiConfigStore { return null; } } - return new StoreFile(new File(storeDir, STORE_ID_TO_FILE_NAME.get(fileId)), fileId); + 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); } /** * 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() { - return createFile(Environment.getDataMiscDirectory(), STORE_FILE_SHARED_GENERAL); + public static @Nullable StoreFile createSharedFile(UserManager userManager) { + return createFile( + Environment.getDataMiscDirectory(), STORE_FILE_SHARED_GENERAL, userManager); } /** @@ -293,14 +305,16 @@ 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) { + public static @Nullable List<StoreFile> createUserFiles(int userId, UserManager userManager) { 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); + StoreFile storeFile = + createFile(Environment.getDataMiscCeDirectory(userId), fileId, userManager); if (storeFile == null) { return null; } @@ -502,7 +516,8 @@ 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)}. + * @param userStores List of {@link StoreFile} created using {@link #createUserFiles(int, + * UserManager)}. */ public void switchUserStoresAndRead(@NonNull List<StoreFile> userStores) throws XmlPullParserException, IOException { @@ -655,19 +670,21 @@ public class WifiConfigStore { */ 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) { + public StoreFile(File file, @StoreFileId int fileId, + @Nullable DataIntegrityChecker dataIntegrityChecker) { mAtomicFile = new AtomicFile(file); mFileName = mAtomicFile.getBaseFile().getAbsolutePath(); - mDataIntegrityChecker = new DataIntegrityChecker(mFileName); mFileId = fileId; + mDataIntegrityChecker = dataIntegrityChecker; } /** @@ -679,6 +696,7 @@ public class WifiConfigStore { return mAtomicFile.exists(); } + /** * Read the entire raw data from the store file and return in a byte array. * @@ -691,20 +709,24 @@ public class WifiConfigStore { 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; - } 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 != 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); + } } return bytes; } @@ -741,8 +763,10 @@ public class WifiConfigStore { } throw e; } - // There was a legitimate change and update the integrity checker. - mDataIntegrityChecker.update(mWriteData); + if (mDataIntegrityChecker != null) { + // 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 178c98f6d..474f27cdd 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -239,7 +239,7 @@ public class WifiInjector { mWifiKeyStore = new WifiKeyStore(mKeyStore); mWifiConfigStore = new WifiConfigStore( mContext, clientModeImplLooper, mClock, mWifiMetrics, - WifiConfigStore.createSharedFile()); + WifiConfigStore.createSharedFile(UserManager.get(mContext))); 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 3b86be887..9d5ed04c7 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -228,7 +228,8 @@ public class WifiConfigManagerTest { mSession = ExtendedMockito.mockitoSession() .mockStatic(WifiConfigStore.class, withSettings().lenient()) .startMocking(); - when(WifiConfigStore.createUserFiles(anyInt())).thenReturn(mock(List.class)); + when(WifiConfigStore.createUserFiles(anyInt(), any(UserManager.class))) + .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 64e762bec..6fdcce80c 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigStoreTest.java @@ -31,6 +31,7 @@ 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; @@ -147,6 +148,7 @@ 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; @@ -379,6 +381,48 @@ 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 @@ -761,7 +805,12 @@ public class WifiConfigStoreTest { private boolean mStoreWritten; MockStoreFile(@WifiConfigStore.StoreFileId int fileId) { - super(new File("MockStoreFile"), fileId); + super(new File("MockStoreFile"), fileId, mDataIntegrityChecker); + } + + MockStoreFile(@WifiConfigStore.StoreFileId int fileId, + DataIntegrityChecker dataIntegrityChecker) { + super(new File("MockStoreFile"), fileId, dataIntegrityChecker); } @Override |