Invoke BLKSECDISCARD to securely delete sensitive data
Bug: 34600579
Test: manual - change device lock under synthetic password, verify
old data on disk is erased.
Change-Id: I247bd1f095dd27335e671981f9e2d77e149af84f
diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index 452fe1d..cffb158 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -3009,6 +3009,18 @@
}
}
+ @Override
+ public void secdiscard(String path) {
+ enforcePermission(android.Manifest.permission.STORAGE_INTERNAL);
+ waitForReady();
+
+ try {
+ mCryptConnector.execute("cryptfs", "secdiscard", escapeNull(path));
+ } catch (NativeDaemonConnectorException e) {
+ throw e.rethrowAsParcelableException();
+ }
+ }
+
class AppFuseMountScope extends AppFuseBridge.MountScope {
boolean opened = false;
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
index 861209d..b928241 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
@@ -26,6 +26,7 @@
import android.database.sqlite.SQLiteOpenHelper;
import android.os.Environment;
import android.os.UserManager;
+import android.os.storage.StorageManager;
import android.util.ArrayMap;
import android.util.Log;
import android.util.Slog;
@@ -339,8 +340,11 @@
synchronized (mFileWriteLock) {
RandomAccessFile raf = null;
try {
- // Write the hash to file
- raf = new RandomAccessFile(name, "rw");
+ // Write the hash to file, requiring each write to be synchronized to the
+ // underlying storage device immediately to avoid data loss in case of power loss.
+ // This also ensures future secdiscard operation on the file succeeds since the
+ // file would have been allocated on flash.
+ raf = new RandomAccessFile(name, "rws");
// Truncate the file if pattern is null, to clear the lock
if (hash == null || hash.length == 0) {
raf.setLength(0);
@@ -435,12 +439,17 @@
return readFile(getSynthenticPasswordStateFilePathForUser(userId, handle, name));
}
- public void deleteSyntheticPasswordState(int userId, long handle, String name, boolean secure) {
+ public void deleteSyntheticPasswordState(int userId, long handle, String name) {
String path = getSynthenticPasswordStateFilePathForUser(userId, handle, name);
File file = new File(path);
if (file.exists()) {
- //TODO: (b/34600579) invoke secdiscardable
- file.delete();
+ try {
+ mContext.getSystemService(StorageManager.class).secdiscard(file.getAbsolutePath());
+ } catch (Exception e) {
+ Slog.w(TAG, "Failed to secdiscard " + path, e);
+ } finally {
+ file.delete();
+ }
mCache.putFile(path, null);
}
}
diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
index e2ec92b..2f8a1b4 100644
--- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
+++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
@@ -452,7 +452,7 @@
// Nuke the SP handle (and as a result, its SID) for the given user.
public void clearSidForUser(int userId) {
- destroyState(SP_HANDLE_NAME, true, DEFAULT_HANDLE, userId);
+ destroyState(SP_HANDLE_NAME, DEFAULT_HANDLE, userId);
}
public boolean hasSidForUser(int userId) {
@@ -487,8 +487,8 @@
}
public void destroyEscrowData(int userId) {
- destroyState(SP_E0_NAME, true, DEFAULT_HANDLE, userId);
- destroyState(SP_P1_NAME, true, DEFAULT_HANDLE, userId);
+ destroyState(SP_E0_NAME, DEFAULT_HANDLE, userId);
+ destroyState(SP_P1_NAME, DEFAULT_HANDLE, userId);
}
private int loadWeaverSlot(long handle, int userId) {
@@ -523,7 +523,7 @@
Log.w(TAG, "Failed to destroy slot", e);
}
}
- destroyState(WEAVER_SLOT_NAME, true, handle, userId);
+ destroyState(WEAVER_SLOT_NAME, handle, userId);
}
private int getNextAvailableWeaverSlot() {
@@ -877,17 +877,17 @@
public void destroyTokenBasedSyntheticPassword(long handle, int userId) {
destroySyntheticPassword(handle, userId);
- destroyState(SECDISCARDABLE_NAME, true, handle, userId);
+ destroyState(SECDISCARDABLE_NAME, handle, userId);
}
public void destroyPasswordBasedSyntheticPassword(long handle, int userId) {
destroySyntheticPassword(handle, userId);
- destroyState(SECDISCARDABLE_NAME, true, handle, userId);
- destroyState(PASSWORD_DATA_NAME, true, handle, userId);
+ destroyState(SECDISCARDABLE_NAME, handle, userId);
+ destroyState(PASSWORD_DATA_NAME, handle, userId);
}
private void destroySyntheticPassword(long handle, int userId) {
- destroyState(SP_BLOB_NAME, true, handle, userId);
+ destroyState(SP_BLOB_NAME, handle, userId);
destroySPBlobKey(getHandleName(handle));
if (hasState(WEAVER_SLOT_NAME, handle, userId)) {
destroyWeaverSlot(handle, userId);
@@ -938,8 +938,8 @@
mStorage.writeSyntheticPasswordState(userId, handle, stateName, data);
}
- private void destroyState(String stateName, boolean secure, long handle, int userId) {
- mStorage.deleteSyntheticPasswordState(userId, handle, stateName, secure);
+ private void destroyState(String stateName, long handle, int userId) {
+ mStorage.deleteSyntheticPasswordState(userId, handle, stateName);
}
protected byte[] decryptSPBlob(String blobKeyName, byte[] blob, byte[] applicationId) {
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/BaseLockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/BaseLockSettingsServiceTests.java
index a2b4568..7de46d9 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/BaseLockSettingsServiceTests.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/BaseLockSettingsServiceTests.java
@@ -31,6 +31,7 @@
import android.os.FileUtils;
import android.os.IProgressListener;
import android.os.UserManager;
+import android.os.storage.StorageManager;
import android.security.KeyStore;
import android.test.AndroidTestCase;
@@ -85,7 +86,7 @@
mDevicePolicyManager = mock(DevicePolicyManager.class);
mContext = new MockLockSettingsContext(getContext(), mUserManager, mNotificationManager,
- mDevicePolicyManager);
+ mDevicePolicyManager, mock(StorageManager.class));
mStorage = new LockSettingsStorageTestable(mContext,
new File(getContext().getFilesDir(), "locksettings"));
File storageDir = mStorage.mStorageDir;
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java
index 4665441..449a54c 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsStorageTests.java
@@ -28,6 +28,7 @@
import android.database.sqlite.SQLiteDatabase;
import android.os.FileUtils;
import android.os.UserManager;
+import android.os.storage.StorageManager;
import android.test.AndroidTestCase;
import com.android.internal.widget.LockPatternUtils;
@@ -69,7 +70,8 @@
when(mockUserManager.getProfileParent(eq(3))).thenReturn(new UserInfo(0, "name", 0));
MockLockSettingsContext context = new MockLockSettingsContext(getContext(), mockUserManager,
- mock(NotificationManager.class), mock(DevicePolicyManager.class));
+ mock(NotificationManager.class), mock(DevicePolicyManager.class),
+ mock(StorageManager.class));
mStorage = new LockSettingsStorageTestable(context,
new File(getContext().getFilesDir(), "locksettings"));
mStorage.setDatabaseOnCreateCallback(new LockSettingsStorage.Callback() {
@@ -336,7 +338,7 @@
assertArrayEquals(data, mStorage.readSyntheticPasswordState(10, 1234L, "state"));
assertEquals(null, mStorage.readSyntheticPasswordState(0, 1234L, "state"));
- mStorage.deleteSyntheticPasswordState(10, 1234L, "state", true);
+ mStorage.deleteSyntheticPasswordState(10, 1234L, "state");
assertEquals(null, mStorage.readSyntheticPasswordState(10, 1234L, "state"));
}
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/MockLockSettingsContext.java b/services/tests/servicestests/src/com/android/server/locksettings/MockLockSettingsContext.java
index c76a83e..8da33a8 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/MockLockSettingsContext.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/MockLockSettingsContext.java
@@ -21,19 +21,23 @@
import android.content.Context;
import android.content.ContextWrapper;
import android.os.UserManager;
+import android.os.storage.StorageManager;
public class MockLockSettingsContext extends ContextWrapper {
private UserManager mUserManager;
private NotificationManager mNotificationManager;
private DevicePolicyManager mDevicePolicyManager;
+ private StorageManager mStorageManager;
public MockLockSettingsContext(Context base, UserManager userManager,
- NotificationManager notificationManager, DevicePolicyManager devicePolicyManager) {
+ NotificationManager notificationManager, DevicePolicyManager devicePolicyManager,
+ StorageManager storageManager) {
super(base);
mUserManager = userManager;
mNotificationManager = notificationManager;
mDevicePolicyManager = devicePolicyManager;
+ mStorageManager = storageManager;
}
@Override
@@ -44,6 +48,8 @@
return mNotificationManager;
} else if (DEVICE_POLICY_SERVICE.equals(name)) {
return mDevicePolicyManager;
+ } else if (STORAGE_SERVICE.equals(name)) {
+ return mStorageManager;
} else {
throw new RuntimeException("System service not mocked: " + name);
}
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/MockStorageManager.java b/services/tests/servicestests/src/com/android/server/locksettings/MockStorageManager.java
index ac46bae..89e18b4 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/MockStorageManager.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/MockStorageManager.java
@@ -500,4 +500,9 @@
throw new UnsupportedOperationException();
}
+ @Override
+ public void secdiscard(String path) throws RemoteException {
+ throw new UnsupportedOperationException();
+ }
+
}