Cleanup password change and removal logic.

Replace password with notifyUserPasswordChanged for password changes,
unlock should now be used to unlock keystore instead of calling password
with the current password.

When the user removes their password now only keystore entries that were
created with FLAG_ENCRYPTED will be deleted. Unencrypted entries will
remain. This makes it more concrete that the keystore could be non-empty
while in STATE_UNINITIALIZED, though this was previously possible due to
the state only being checked if FLAG_ENCRYPTED was set.

Change-Id: I324914c00195d762cbaa8c63084e41fa796b7df8

4 files changed, 107 insertions, 67 deletions

diff --git a/keystore/IKeystoreService.cpp b/keystore/IKeystoreService.cpp
index df1c5dd..64530f5 100644
--- a/keystore/IKeystoreService.cpp
+++ b/keystore/IKeystoreService.cpp
@@ -555,20 +555,22 @@ public:
         return ret;
     }
 
-    virtual int32_t password(const String16& password)
+    virtual int32_t onUserPasswordChanged(int32_t userId, const String16& password)
     {
         Parcel data, reply;
         data.writeInterfaceToken(IKeystoreService::getInterfaceDescriptor());
+        data.writeInt32(userId);
         data.writeString16(password);
-        status_t status = remote()->transact(BnKeystoreService::PASSWORD, data, &reply);
+        status_t status = remote()->transact(BnKeystoreService::ON_USER_PASSWORD_CHANGED, data,
+                                             &reply);
         if (status != NO_ERROR) {
-            ALOGD("password() could not contact remote: %d\n", status);
+            ALOGD("onUserPasswordChanged() could not contact remote: %d\n", status);
             return -1;
         }
         int32_t err = reply.readExceptionCode();
         int32_t ret = reply.readInt32();
         if (err < 0) {
-            ALOGD("password() caught exception %d\n", err);
+            ALOGD("onUserPasswordChanged() caught exception %d\n", err);
             return -1;
         }
         return ret;
@@ -592,10 +594,11 @@ public:
         return ret;
     }
 
-    virtual int32_t unlock(const String16& password)
+    virtual int32_t unlock(int32_t userId, const String16& password)
     {
         Parcel data, reply;
         data.writeInterfaceToken(IKeystoreService::getInterfaceDescriptor());
+        data.writeInt32(userId);
         data.writeString16(password);
         status_t status = remote()->transact(BnKeystoreService::UNLOCK, data, &reply);
         if (status != NO_ERROR) {
@@ -1380,10 +1383,11 @@ status_t BnKeystoreService::onTransact(
             reply->writeInt32(ret);
             return NO_ERROR;
         } break;
-        case PASSWORD: {
+        case ON_USER_PASSWORD_CHANGED: {
             CHECK_INTERFACE(IKeystoreService, data, reply);
+            int32_t userId = data.readInt32();
             String16 pass = data.readString16();
-            int32_t ret = password(pass);
+            int32_t ret = onUserPasswordChanged(userId, pass);
             reply->writeNoException();
             reply->writeInt32(ret);
             return NO_ERROR;
@@ -1397,8 +1401,9 @@ status_t BnKeystoreService::onTransact(
         } break;
         case UNLOCK: {
             CHECK_INTERFACE(IKeystoreService, data, reply);
+            int32_t userId = data.readInt32();
             String16 pass = data.readString16();
-            int32_t ret = unlock(pass);
+            int32_t ret = unlock(userId, pass);
             reply->writeNoException();
             reply->writeInt32(ret);
             return NO_ERROR;
diff --git a/keystore/include/keystore/IKeystoreService.h b/keystore/include/keystore/IKeystoreService.h
index 8b5b5d3..d0cf6f5 100644
--- a/keystore/include/keystore/IKeystoreService.h
+++ b/keystore/include/keystore/IKeystoreService.h
@@ -105,7 +105,7 @@ public:
         EXIST = IBinder::FIRST_CALL_TRANSACTION + 4,
         SAW = IBinder::FIRST_CALL_TRANSACTION + 5,
         RESET = IBinder::FIRST_CALL_TRANSACTION + 6,
-        PASSWORD = IBinder::FIRST_CALL_TRANSACTION + 7,
+        ON_USER_PASSWORD_CHANGED = IBinder::FIRST_CALL_TRANSACTION + 7,
         LOCK = IBinder::FIRST_CALL_TRANSACTION + 8,
         UNLOCK = IBinder::FIRST_CALL_TRANSACTION + 9,
         ZERO = IBinder::FIRST_CALL_TRANSACTION + 10,
@@ -154,11 +154,11 @@ public:
 
     virtual int32_t reset() = 0;
 
-    virtual int32_t password(const String16& password) = 0;
+    virtual int32_t onUserPasswordChanged(int32_t userId, const String16& newPassword) = 0;
 
     virtual int32_t lock() = 0;
 
-    virtual int32_t unlock(const String16& password) = 0;
+    virtual int32_t unlock(int32_t userId, const String16& password) = 0;
 
     virtual int32_t zero() = 0;
 
diff --git a/keystore/keystore.cpp b/keystore/keystore.cpp
index e8c48d1..906992c 100644
--- a/keystore/keystore.cpp
+++ b/keystore/keystore.cpp
@@ -249,6 +249,10 @@ static uid_t get_user_id(uid_t uid) {
     return uid / AID_USER;
 }
 
+static uid_t get_uid(uid_t userId, uid_t appId) {
+    return userId * AID_USER + get_app_id(appId);
+}
+
 static bool keystore_selinux_check_access(uid_t /*uid*/, perm_t perm, pid_t spid) {
     if (!ks_is_selinux_enabled) {
         return true;
@@ -771,6 +775,12 @@ public:
         memset(&mMasterKeyDecryption, 0, sizeof(mMasterKeyDecryption));
     }
 
+    bool deleteMasterKey() {
+        setState(STATE_UNINITIALIZED);
+        zeroizeMasterKeysInMemory();
+        return unlink(mMasterKeyFile) == 0 || errno == ENOENT;
+    }
+
     ResponseCode initialize(const android::String8& pw, Entropy* entropy) {
         if (!generateMasterKey(entropy)) {
             return SYSTEM_ERROR;
@@ -873,19 +883,18 @@ public:
     bool reset() {
         DIR* dir = opendir(getUserDirName());
         if (!dir) {
+            // If the directory doesn't exist then nothing to do.
+            if (errno == ENOENT) {
+                return true;
+            }
             ALOGW("couldn't open user directory: %s", strerror(errno));
             return false;
         }
 
         struct dirent* file;
         while ((file = readdir(dir)) != NULL) {
-            // We only care about files.
-            if (file->d_type != DT_REG) {
-                continue;
-            }
-
-            // Skip anything that starts with a "."
-            if (file->d_name[0] == '.' && strcmp(".masterkey", file->d_name)) {
+            // skip . and ..
+            if (!strcmp(".", file->d_name) || !strcmp("..", file->d_name)) {
                 continue;
             }
 
@@ -1053,29 +1062,53 @@ public:
                 encoded);
     }
 
-    bool reset(uid_t uid) {
+    /*
+     * Delete entries owned by userId. If keepUnencryptedEntries is true
+     * then only encrypted entries will be removed, otherwise all entries will
+     * be removed.
+     */
+    void resetUser(uid_t userId, bool keepUnenryptedEntries) {
         android::String8 prefix("");
         android::Vector<android::String16> aliases;
+        uid_t uid = get_uid(userId, AID_SYSTEM);
+        UserState* userState = getUserState(uid);
         if (saw(prefix, &aliases, uid) != ::NO_ERROR) {
-            return ::SYSTEM_ERROR;
+            return;
         }
-
-        UserState* userState = getUserState(uid);
         for (uint32_t i = 0; i < aliases.size(); i++) {
             android::String8 filename(aliases[i]);
             filename = android::String8::format("%s/%s", userState->getUserDirName(),
-                    getKeyName(filename).string());
-            del(filename, ::TYPE_ANY, uid);
+                                                getKeyName(filename).string());
+            bool shouldDelete = true;
+            if (keepUnenryptedEntries) {
+                Blob blob;
+                ResponseCode rc = get(filename, &blob, ::TYPE_ANY, uid);
+
+                /* get can fail if the blob is encrypted and the state is
+                 * not unlocked, only skip deleting blobs that were loaded and
+                 * who are not encrypted. If there are blobs we fail to read for
+                 * other reasons err on the safe side and delete them since we
+                 * can't tell if they're encrypted.
+                 */
+                shouldDelete = !(rc == ::NO_ERROR && !blob.isEncrypted());
+            }
+            if (shouldDelete) {
+                del(filename, ::TYPE_ANY, uid);
+            }
+        }
+        if (!userState->deleteMasterKey()) {
+            ALOGE("Failed to delete user %d's master key", userId);
+        }
+        if (!keepUnenryptedEntries) {
+            if(!userState->reset()) {
+                ALOGE("Failed to remove user %d's directory", userId);
+            }
         }
-
-        userState->zeroizeMasterKeysInMemory();
-        userState->setState(STATE_UNINITIALIZED);
-        return userState->reset();
     }
 
     bool isEmpty(uid_t uid) const {
         const UserState* userState = getUserState(uid);
-        if (userState == NULL || userState->getState() == STATE_UNINITIALIZED) {
+        if (userState == NULL) {
             return true;
         }
 
@@ -1732,39 +1765,46 @@ public:
         }
 
         uid_t callingUid = IPCThreadState::self()->getCallingUid();
-        return mKeyStore->reset(callingUid) ? ::NO_ERROR : ::SYSTEM_ERROR;
+        mKeyStore->resetUser(get_user_id(callingUid), false);
+        return ::NO_ERROR;
     }
 
-    /*
-     * Here is the history. To improve the security, the parameters to generate the
-     * master key has been changed. To make a seamless transition, we update the
-     * file using the same password when the user unlock it for the first time. If
-     * any thing goes wrong during the transition, the new file will not overwrite
-     * the old one. This avoids permanent damages of the existing data.
-     */
-    int32_t password(const String16& password) {
+    int32_t onUserPasswordChanged(int32_t userId, const String16& password) {
         if (!checkBinderPermission(P_PASSWORD)) {
             return ::PERMISSION_DENIED;
         }
 
         const String8 password8(password);
-        uid_t callingUid = IPCThreadState::self()->getCallingUid();
+        uid_t uid = get_uid(userId, AID_SYSTEM);
 
-        switch (mKeyStore->getState(callingUid)) {
-            case ::STATE_UNINITIALIZED: {
-                // generate master key, encrypt with password, write to file, initialize mMasterKey*.
-                return mKeyStore->initializeUser(password8, callingUid);
-            }
-            case ::STATE_NO_ERROR: {
-                // rewrite master key with new password.
-                return mKeyStore->writeMasterKey(password8, callingUid);
-            }
-            case ::STATE_LOCKED: {
-                // read master key, decrypt with password, initialize mMasterKey*.
-                return mKeyStore->readMasterKey(password8, callingUid);
+        // Flush the auth token table to prevent stale tokens from sticking
+        // around.
+        mAuthTokenTable.Clear();
+
+        if (password.size() == 0) {
+            ALOGI("Secure lockscreen for user %d removed, deleting encrypted entries", userId);
+            mKeyStore->resetUser(static_cast<uid_t>(userId), true);
+            return ::NO_ERROR;
+        } else {
+            switch (mKeyStore->getState(uid)) {
+                case ::STATE_UNINITIALIZED: {
+                    // generate master key, encrypt with password, write to file,
+                    // initialize mMasterKey*.
+                    return mKeyStore->initializeUser(password8, uid);
+                }
+                case ::STATE_NO_ERROR: {
+                    // rewrite master key with new password.
+                    return mKeyStore->writeMasterKey(password8, uid);
+                }
+                case ::STATE_LOCKED: {
+                    ALOGE("Changing user %d's password while locked, clearing old encryption",
+                          userId);
+                    mKeyStore->resetUser(static_cast<uid_t>(userId), true);
+                    return mKeyStore->initializeUser(password8, uid);
+                }
             }
+            return ::SYSTEM_ERROR;
         }
-        return ::SYSTEM_ERROR;
     }
 
     int32_t lock() {
@@ -1783,20 +1823,21 @@ public:
         return ::NO_ERROR;
     }
 
-    int32_t unlock(const String16& pw) {
+    int32_t unlock(int32_t userId, const String16& pw) {
         if (!checkBinderPermission(P_UNLOCK)) {
             return ::PERMISSION_DENIED;
         }
 
-        uid_t callingUid = IPCThreadState::self()->getCallingUid();
-        State state = mKeyStore->getState(callingUid);
+        uid_t uid = get_uid(userId, AID_SYSTEM);
+        State state = mKeyStore->getState(uid);
         if (state != ::STATE_LOCKED) {
-            ALOGD("calling unlock when not locked");
+            ALOGI("calling unlock when not locked, ignoring.");
             return state;
         }
 
         const String8 password8(pw);
-        return password(pw);
+        // read master key, decrypt with password, initialize mMasterKey*.
+        return mKeyStore->readMasterKey(password8, uid);
     }
 
     int32_t zero() {
@@ -2249,15 +2290,9 @@ public:
     }
 
     int32_t reset_uid(int32_t targetUid) {
+        // TODO: Remove this method from the binder interface
         targetUid = getEffectiveUid(targetUid);
-        if (!checkBinderPermissionSelfOrSystem(P_RESET_UID, targetUid)) {
-            return ::PERMISSION_DENIED;
-        }
-        // Flush the auth token table to prevent stale tokens from sticking
-        // around.
-        mAuthTokenTable.Clear();
-
-        return mKeyStore->reset(targetUid) ? ::NO_ERROR : ::SYSTEM_ERROR;
+        return onUserPasswordChanged(get_user_id(targetUid), String16(""));
     }
 
     int32_t sync_uid(int32_t sourceUid, int32_t targetUid) {
diff --git a/keystore/keystore_cli.cpp b/keystore/keystore_cli.cpp
index 1e19890..1391abf 100644
--- a/keystore/keystore_cli.cpp
+++ b/keystore/keystore_cli.cpp
@@ -200,11 +200,11 @@ int main(int argc, char* argv[])
 
     NO_ARG_INT_RETURN(reset);
 
-    SINGLE_ARG_INT_RETURN(password);
+    // TODO: notifyUserPasswordChanged
 
     NO_ARG_INT_RETURN(lock);
 
-    SINGLE_ARG_INT_RETURN(unlock);
+    // TODO: unlock
 
     NO_ARG_INT_RETURN(zero);