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
Merged-In: I247bd1f095dd27335e671981f9e2d77e149af84f

9 files changed, 63 insertions, 18 deletions

diff --git a/core/java/android/os/storage/IStorageManager.aidl b/core/java/android/os/storage/IStorageManager.aidl
index 2cc4b7170f38..92f7f319a14b 100644
--- a/core/java/android/os/storage/IStorageManager.aidl
+++ b/core/java/android/os/storage/IStorageManager.aidl
@@ -295,4 +295,5 @@ interface IStorageManager {
     long getCacheSizeBytes(String volumeUuid, int uid) = 76;
     long getAllocatableBytes(String volumeUuid, int flags) = 77;
     void allocateBytes(String volumeUuid, long bytes, int flags) = 78;
+    void secdiscard(in String path) = 79;
 }
diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java
index bd43d6a3c218..f361c549f016 100644
--- a/core/java/android/os/storage/StorageManager.java
+++ b/core/java/android/os/storage/StorageManager.java
@@ -1252,6 +1252,15 @@ public class StorageManager {
     }
 
     /** {@hide} */
+    public void secdiscard(String path) {
+        try {
+            mStorageManager.secdiscard(path);
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
+    }
+
+    /** {@hide} */
     public static boolean isUserKeyUnlocked(int userId) {
         if (sStorageManager == null) {
             sStorageManager = IStorageManager.Stub
diff --git a/services/core/java/com/android/server/LockSettingsStorage.java b/services/core/java/com/android/server/LockSettingsStorage.java
index f5bae7c14d97..fe9bfd04f707 100644
--- a/services/core/java/com/android/server/LockSettingsStorage.java
+++ b/services/core/java/com/android/server/LockSettingsStorage.java
@@ -26,6 +26,7 @@ import android.database.sqlite.SQLiteDatabase;
 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;
@@ -336,8 +337,11 @@ class LockSettingsStorage {
         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);
@@ -432,12 +436,17 @@ class LockSettingsStorage {
         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/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index 452fe1d70ddc..cffb158a3f21 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -3009,6 +3009,18 @@ class StorageManagerService extends IStorageManager.Stub
         }
     }
 
+    @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/SyntheticPasswordManager.java b/services/core/java/com/android/server/SyntheticPasswordManager.java
index f797517744e6..1d17ff72940d 100644
--- a/services/core/java/com/android/server/SyntheticPasswordManager.java
+++ b/services/core/java/com/android/server/SyntheticPasswordManager.java
@@ -283,7 +283,7 @@ public class SyntheticPasswordManager {
 
     // 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) {
@@ -318,8 +318,8 @@ public class SyntheticPasswordManager {
     }
 
     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);
     }
 
     /**
@@ -584,17 +584,17 @@ public class SyntheticPasswordManager {
 
     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));
     }
 
@@ -629,8 +629,8 @@ public class SyntheticPasswordManager {
         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/BaseLockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java
index ca7752819b9c..d9d06ae7d70c 100644
--- a/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java
+++ b/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java
@@ -30,6 +30,7 @@ import android.content.pm.UserInfo;
 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;
 
@@ -82,7 +83,7 @@ public class BaseLockSettingsServiceTests extends AndroidTestCase {
         mActivityManager = mock(IActivityManager.class);
         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/LockSettingsStorageTests.java b/services/tests/servicestests/src/com/android/server/LockSettingsStorageTests.java
index 46779048e9db..f242b26b641f 100644
--- a/services/tests/servicestests/src/com/android/server/LockSettingsStorageTests.java
+++ b/services/tests/servicestests/src/com/android/server/LockSettingsStorageTests.java
@@ -28,6 +28,7 @@ import android.content.pm.UserInfo;
 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 @@ public class LockSettingsStorageTests extends AndroidTestCase {
         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 @@ public class LockSettingsStorageTests extends AndroidTestCase {
         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/MockLockSettingsContext.java b/services/tests/servicestests/src/com/android/server/MockLockSettingsContext.java
index 8bceed45ab5f..9dede3bdf0d1 100644
--- a/services/tests/servicestests/src/com/android/server/MockLockSettingsContext.java
+++ b/services/tests/servicestests/src/com/android/server/MockLockSettingsContext.java
@@ -21,19 +21,23 @@ import android.app.admin.DevicePolicyManager;
 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 @@ public class MockLockSettingsContext extends ContextWrapper {
             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/MockStorageManager.java b/services/tests/servicestests/src/com/android/server/MockStorageManager.java
index 17c8ec2fac13..3a177186e226 100644
--- a/services/tests/servicestests/src/com/android/server/MockStorageManager.java
+++ b/services/tests/servicestests/src/com/android/server/MockStorageManager.java
@@ -500,4 +500,9 @@ public class MockStorageManager implements IStorageManager {
         throw new UnsupportedOperationException();
     }
 
+    @Override
+    public void secdiscard(String path) throws RemoteException {
+        throw new UnsupportedOperationException();
+    }
+
 }