From a339df080d7dfacbec7915f869115dfa91ce4b12 Mon Sep 17 00:00:00 2001
From: Rubin Xu <rubinxu@google.com>
Date: Tue, 10 Apr 2018 09:44:18 +0100
Subject: [PATCH] Fix unintended deletion of reused weaver slots during user
 removal

During user deletion, getNextAvailableWeaverSlot() might return a slot
that's currently taken by the to-be-removed user, because the user is
marked as partial so it's not returned from UserManager in the list of
users and SyntheticPasswordManager will not attempt to track which slot
that user has used. This slot might then be reused by a newly-created
user. Meanwhile SyntheticPasswordManager will also try to explicitly
delete slots for the deleted user during ACTION_USER_REMOVED, and if this
happens after the slot is reused by the new user, the weaver slot will
be corrupted, leading to failure to unlock the new user later.

Quick fix by double checking if slot is reused during user deletion.

Test: Add device lock; create work profile; reboot device; delete the
      current profile and quickly re-create one. Verify the new profile
      can still be unlocked after another reboot
Bug: 76134651
Change-Id: I89769b7cbcf64f2562717c98e3f15ab0fbb38bf0
---
 .../SyntheticPasswordManager.java             | 37 +++++++++++++++----
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
index 88b2a36850884..0700ab35df1b3 100644
--- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
+++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
@@ -534,17 +534,33 @@ public class SyntheticPasswordManager {
 
     private void destroyWeaverSlot(long handle, int userId) {
         int slot = loadWeaverSlot(handle, userId);
+        destroyState(WEAVER_SLOT_NAME, handle, userId);
         if (slot != INVALID_WEAVER_SLOT) {
-            try {
-                weaverEnroll(slot, null, null);
-            } catch (RemoteException e) {
-                Log.w(TAG, "Failed to destroy slot", e);
+            Set<Integer> usedSlots = getUsedWeaverSlots();
+            if (!usedSlots.contains(slot)) {
+                Log.i(TAG, "Destroy weaver slot " + slot + " for user " + userId);
+                try {
+                    weaverEnroll(slot, null, null);
+                } catch (RemoteException e) {
+                    Log.w(TAG, "Failed to destroy slot", e);
+                }
+            } else {
+                Log.w(TAG, "Skip destroying reused weaver slot " + slot + " for user " + userId);
             }
         }
-        destroyState(WEAVER_SLOT_NAME, handle, userId);
     }
 
-    private int getNextAvailableWeaverSlot() {
+    /**
+     * Return the set of weaver slots that are currently in use by all users on the device.
+     * <p>
+     * <em>Note:</em> Users who are in the process of being deleted are not tracked here
+     * (due to them being marked as partial in UserManager so not visible from
+     * {@link UserManager#getUsers}). As a result their weaver slots will not be considered
+     * taken and can be reused by new users. Care should be taken when cleaning up the
+     * deleted user in {@link #removeUser}, to prevent a reused slot from being erased
+     * unintentionally.
+     */
+    private Set<Integer> getUsedWeaverSlots() {
         Map<Integer, List<Long>> slotHandles = mStorage.listSyntheticPasswordHandlesForAllUsers(
                 WEAVER_SLOT_NAME);
         HashSet<Integer> slots = new HashSet<>();
@@ -554,8 +570,13 @@ public class SyntheticPasswordManager {
                 slots.add(slot);
             }
         }
+        return slots;
+    }
+
+    private int getNextAvailableWeaverSlot() {
+        Set<Integer> usedSlots = getUsedWeaverSlots();
         for (int i = 0; i < mWeaverConfig.slots; i++) {
-            if (!slots.contains(i)) {
+            if (!usedSlots.contains(i)) {
                 return i;
             }
         }
@@ -592,6 +613,7 @@ public class SyntheticPasswordManager {
         if (isWeaverAvailable()) {
             // Weaver based user password
             int weaverSlot = getNextAvailableWeaverSlot();
+            Log.i(TAG, "Weaver enroll password to slot " + weaverSlot + " for user " + userId);
             byte[] weaverSecret = weaverEnroll(weaverSlot, passwordTokenToWeaverKey(pwdToken), null);
             if (weaverSecret == null) {
                 Log.e(TAG, "Fail to enroll user password under weaver " + userId);
@@ -749,6 +771,7 @@ public class SyntheticPasswordManager {
         if (isWeaverAvailable()) {
             int slot = getNextAvailableWeaverSlot();
             try {
+                Log.i(TAG, "Weaver enroll token to slot " + slot + " for user " + userId);
                 weaverEnroll(slot, null, tokenData.weaverSecret);
             } catch (RemoteException e) {
                 Log.e(TAG, "Failed to enroll weaver secret when activating token", e);