From 5be4058e19d5b5244224c6d8580b801567cb9ac7 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 30 Jul 2019 15:16:31 -0700 Subject: WifiConfigStore: Limit integrity checks to single user devices Bug: 138482990 Test: atest com.android.server.wifi Test: Device boots up & verified that config store files are not integrity protected. Test: Will send for full wifi regression tests. Change-Id: I1bf8ae320935cc1bf70625792c4c1a5e0d54f034 Merged-In: I1bf8ae320935cc1bf70625792c4c1a5e0d54f034 --- .../com/android/server/wifi/WifiConfigManager.java | 4 +- .../com/android/server/wifi/WifiConfigStore.java | 82 ++++++++++++++-------- .../java/com/android/server/wifi/WifiInjector.java | 2 +- 3 files changed, 56 insertions(+), 32 deletions(-) (limited to 'service') 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 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 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 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 /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 createUserFiles(int userId) { + public static @Nullable List createUserFiles(int userId, UserManager userManager) { List 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 userStores) throws XmlPullParserException, IOException { @@ -654,20 +669,22 @@ public class WifiConfigStore { * Store the file name for setting the file permissions/logging purposes. */ 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 -- cgit v1.2.3