Use RecoverySession object to hide session IDs

Session IDs are an implementation detail that the framework can (and should)
abstract away.

Test: adb shell am instrument -w -e package
com.android.server.locksettings.recoverablekeystore
com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner

Change-Id: Ieba641a9b54ac9bba197a6e9749b621a07e40c67

9 files changed, 255 insertions, 12 deletions

diff --git a/core/java/android/security/keystore/RecoveryClaim.java b/core/java/android/security/keystore/RecoveryClaim.java
new file mode 100644
index 000000000000..6f566af1dc7d
--- /dev/null
+++ b/core/java/android/security/keystore/RecoveryClaim.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.security.keystore;
+
+/**
+ * An attempt to recover a keychain protected by remote secure hardware.
+ *
+ * @hide
+ */
+public class RecoveryClaim {
+
+    private final RecoverySession mRecoverySession;
+    private final byte[] mClaimBytes;
+
+    RecoveryClaim(RecoverySession recoverySession, byte[] claimBytes) {
+        mRecoverySession = recoverySession;
+        mClaimBytes = claimBytes;
+    }
+
+    /**
+     * Returns the session associated with the recovery attempt. This is used to match the symmetric
+     * key, which remains internal to the framework, for decrypting the claim response.
+     *
+     * @return The session data.
+     */
+    public RecoverySession getRecoverySession() {
+        return mRecoverySession;
+    }
+
+    /**
+     * Returns the encrypted claim's bytes.
+     *
+     * <p>This should be sent by the recovery agent to the remote secure hardware, which will use
+     * it to decrypt the keychain, before sending it re-encrypted with the session's symmetric key
+     * to the device.
+     */
+    public byte[] getClaimBytes() {
+        return mClaimBytes;
+    }
+}
diff --git a/core/java/android/security/keystore/RecoveryManager.java b/core/java/android/security/keystore/RecoveryManager.java
index 6177cd76bed4..05c931efb0b2 100644
--- a/core/java/android/security/keystore/RecoveryManager.java
+++ b/core/java/android/security/keystore/RecoveryManager.java
@@ -23,6 +23,7 @@ import android.content.pm.PackageManager.NameNotFoundException;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.ServiceSpecificException;
+import android.util.Log;
 
 import com.android.internal.widget.ILockSettings;
 
@@ -36,6 +37,7 @@ import java.util.Map;
  * @hide
  */
 public class RecoveryManager {
+    private static final String TAG = "RecoveryController";
 
     /** Key has been successfully synced. */
     public static final int RECOVERY_STATUS_SYNCED = 0;
@@ -371,7 +373,6 @@ public class RecoveryManager {
      * The method generates symmetric key for a session, which trusted remote device can use to
      * return recovery key.
      *
-     * @param sessionId ID for recovery session.
      * @param verifierPublicKey Encoded {@code java.security.cert.X509Certificate} with Public key
      * used to create the recovery blob on the source device.
      * Keystore will verify the certificate using root of trust.
@@ -380,30 +381,31 @@ public class RecoveryManager {
      * @param vaultChallenge Data passed from server for this recovery session and used to prevent
      *     replay attacks
      * @param secrets Secrets provided by user, the method only uses type and secret fields.
-     * @return Binary blob with recovery claim. It is encrypted with verifierPublicKey and contains
-     *     a proof of user secrets, session symmetric key and parameters necessary to identify the
-     *     counter with the number of failed recovery attempts.
+     * @return The recovery claim. Claim provides a binary blob with recovery claim. It is
+     *     encrypted with verifierPublicKey and contains a proof of user secrets, session symmetric
+     *     key and parameters necessary to identify the counter with the number of failed recovery
+     *     attempts.
      * @throws BadCertificateFormatException if the {@code verifierPublicKey} is in an incorrect
      *     format.
      * @throws InternalRecoveryServiceException if an unexpected error occurred in the recovery
      *     service.
      */
-    public @NonNull byte[] startRecoverySession(
-            @NonNull String sessionId,
+    @NonNull public RecoveryClaim startRecoverySession(
             @NonNull byte[] verifierPublicKey,
             @NonNull byte[] vaultParams,
             @NonNull byte[] vaultChallenge,
             @NonNull List<KeychainProtectionParameter> secrets)
             throws BadCertificateFormatException, InternalRecoveryServiceException {
         try {
+            RecoverySession recoverySession = RecoverySession.newInstance(this);
             byte[] recoveryClaim =
                     mBinder.startRecoverySession(
-                            sessionId,
+                            recoverySession.getSessionId(),
                             verifierPublicKey,
                             vaultParams,
                             vaultChallenge,
                             secrets);
-            return recoveryClaim;
+            return new RecoveryClaim(recoverySession, recoveryClaim);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         } catch (ServiceSpecificException e) {
@@ -417,8 +419,8 @@ public class RecoveryManager {
     /**
      * Imports keys.
      *
-     * @param sessionId Id for recovery session, same as in
-     *     {@link #startRecoverySession(String, byte[], byte[], byte[], List)}.
+     * @param session Related recovery session, as originally created by invoking
+     *        {@link #startRecoverySession(byte[], byte[], byte[], List)}.
      * @param recoveryKeyBlob Recovery blob encrypted by symmetric key generated for this session.
      * @param applicationKeys Application keys. Key material can be decrypted using recoveryKeyBlob
      *     and session. KeyStore only uses package names from the application info in {@link
@@ -429,14 +431,14 @@ public class RecoveryManager {
      * @throws InternalRecoveryServiceException if an error occurs internal to the recovery service.
      */
     public Map<String, byte[]> recoverKeys(
-            @NonNull String sessionId,
+            @NonNull RecoverySession session,
             @NonNull byte[] recoveryKeyBlob,
             @NonNull List<WrappedApplicationKey> applicationKeys)
             throws SessionExpiredException, DecryptionFailedException,
             InternalRecoveryServiceException {
         try {
             return (Map<String, byte[]>) mBinder.recoverKeys(
-                    sessionId, recoveryKeyBlob, applicationKeys);
+                    session.getSessionId(), recoveryKeyBlob, applicationKeys);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         } catch (ServiceSpecificException e) {
@@ -451,6 +453,20 @@ public class RecoveryManager {
     }
 
     /**
+     * Deletes all data associated with {@code session}. Should not be invoked directly but via
+     * {@link RecoverySession#close()}.
+     *
+     * @hide
+     */
+    void closeSession(RecoverySession session) {
+        try {
+            mBinder.closeSession(session.getSessionId());
+        } catch (RemoteException | ServiceSpecificException e) {
+            Log.e(TAG, "Unexpected error trying to close session", e);
+        }
+    }
+
+    /**
      * Generates a key called {@code alias} and loads it into the recoverable key store. Returns the
      * raw material of the key.
      *
diff --git a/core/java/android/security/keystore/RecoverySession.java b/core/java/android/security/keystore/RecoverySession.java
new file mode 100644
index 000000000000..f78551fdf194
--- /dev/null
+++ b/core/java/android/security/keystore/RecoverySession.java
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.security.keystore;
+
+import java.security.SecureRandom;
+
+/**
+ * Session to recover a {@link KeychainSnapshot} from the remote trusted hardware, initiated by a
+ * recovery agent.
+ *
+ * @hide
+ */
+public class RecoverySession implements AutoCloseable {
+
+    private static final int SESSION_ID_LENGTH_BYTES = 16;
+
+    private final String mSessionId;
+    private final RecoveryManager mRecoveryManager;
+
+    private RecoverySession(RecoveryManager recoveryManager, String sessionId) {
+        mRecoveryManager = recoveryManager;
+        mSessionId = sessionId;
+    }
+
+    /**
+     * A new session, started by {@code recoveryManager}.
+     */
+    static RecoverySession newInstance(RecoveryManager recoveryManager) {
+        return new RecoverySession(recoveryManager, newSessionId());
+    }
+
+    /**
+     * Returns a new random session ID.
+     */
+    private static String newSessionId() {
+        SecureRandom secureRandom = new SecureRandom();
+        byte[] sessionId = new byte[SESSION_ID_LENGTH_BYTES];
+        secureRandom.nextBytes(sessionId);
+        StringBuilder sb = new StringBuilder();
+        for (byte b : sessionId) {
+            sb.append(Byte.toHexString(b, /*upperCase=*/ false));
+        }
+        return sb.toString();
+    }
+
+    /**
+     * An internal session ID, used by the framework to match recovery claims to snapshot responses.
+     */
+    String getSessionId() {
+        return mSessionId;
+    }
+
+    @Override
+    public void close() {
+        mRecoveryManager.closeSession(this);
+    }
+}
diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl
index b2bab6f7d58f..b3ef0f2b6ba1 100644
--- a/core/java/com/android/internal/widget/ILockSettings.aidl
+++ b/core/java/com/android/internal/widget/ILockSettings.aidl
@@ -81,4 +81,5 @@ interface ILockSettings {
             in List<KeychainProtectionParameter> secrets);
     Map/*<String, byte[]>*/ recoverKeys(in String sessionId, in byte[] recoveryKeyBlob,
             in List<WrappedApplicationKey> applicationKeys);
+     void closeSession(in String sessionId);
 }
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index 879c0242d936..50ef8e12b50d 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -2029,6 +2029,11 @@ public class LockSettingsService extends ILockSettings.Stub {
     }
 
     @Override
+    public void closeSession(@NonNull String sessionId) throws RemoteException {
+        mRecoverableKeyStoreManager.closeSession(sessionId);
+    }
+
+    @Override
     public Map<String, byte[]> recoverKeys(@NonNull String sessionId,
             @NonNull byte[] recoveryKeyBlob, @NonNull List<WrappedApplicationKey> applicationKeys)
             throws RemoteException {
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
index 0371c7e8df01..01fd95c6198b 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -434,6 +434,13 @@ public class RecoverableKeyStoreManager {
         }
     }
 
+    /**
+     * Destroys the session with the given {@code sessionId}.
+     */
+    public void closeSession(@NonNull String sessionId) throws RemoteException {
+        mRecoverySessionStorage.remove(Binder.getCallingUid(), sessionId);
+    }
+
     public void removeKey(@NonNull String alias) throws RemoteException {
         int uid = Binder.getCallingUid();
         int userId = UserHandle.getCallingUserId();
diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorage.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorage.java
index f7633e4cee8d..4c8f66b94f7f 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorage.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorage.java
@@ -91,6 +91,16 @@ public class RecoverySessionStorage implements Destroyable {
     }
 
     /**
+     * Deletes the session with {@code sessionId} created by app with {@code uid}.
+     */
+    public void remove(int uid, String sessionId) {
+        if (mSessionsByUid.get(uid) == null) {
+            return;
+        }
+        mSessionsByUid.get(uid).removeIf(session -> session.mSessionId.equals(sessionId));
+    }
+
+    /**
      * Returns the total count of pending sessions.
      *
      * @hide
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
index 37157428683a..5244c7ac3571 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
@@ -283,6 +283,44 @@ public class RecoverableKeyStoreManagerTest {
     }
 
     @Test
+    public void closeSession_closesASession() throws Exception {
+        mRecoverableKeyStoreManager.startRecoverySession(
+                TEST_SESSION_ID,
+                TEST_PUBLIC_KEY,
+                TEST_VAULT_PARAMS,
+                TEST_VAULT_CHALLENGE,
+                ImmutableList.of(
+                        new KeychainProtectionParameter(
+                                TYPE_LOCKSCREEN,
+                                TYPE_PASSWORD,
+                                KeyDerivationParams.createSha256Params(TEST_SALT),
+                                TEST_SECRET)));
+
+        mRecoverableKeyStoreManager.closeSession(TEST_SESSION_ID);
+
+        assertEquals(0, mRecoverySessionStorage.size());
+    }
+
+    @Test
+    public void closeSession_doesNotCloseUnrelatedSessions() throws Exception {
+        mRecoverableKeyStoreManager.startRecoverySession(
+                TEST_SESSION_ID,
+                TEST_PUBLIC_KEY,
+                TEST_VAULT_PARAMS,
+                TEST_VAULT_CHALLENGE,
+                ImmutableList.of(
+                        new KeychainProtectionParameter(
+                                TYPE_LOCKSCREEN,
+                                TYPE_PASSWORD,
+                                KeyDerivationParams.createSha256Params(TEST_SALT),
+                                TEST_SECRET)));
+
+        mRecoverableKeyStoreManager.closeSession("some random session");
+
+        assertEquals(1, mRecoverySessionStorage.size());
+    }
+
+    @Test
     public void startRecoverySession_throwsIfBadNumberOfSecrets() throws Exception {
         try {
             mRecoverableKeyStoreManager.startRecoverySession(
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorageTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorageTest.java
index 6f93fe409e48..0f95748a8ea7 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorageTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/storage/RecoverySessionStorageTest.java
@@ -19,6 +19,8 @@ package com.android.server.locksettings.recoverablekeystore.storage;
 import static junit.framework.Assert.fail;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
@@ -88,6 +90,45 @@ public class RecoverySessionStorageTest {
     }
 
     @Test
+    public void remove_deletesSpecificSession() {
+        RecoverySessionStorage storage = new RecoverySessionStorage();
+        storage.add(TEST_USER_ID, new RecoverySessionStorage.Entry(
+                TEST_SESSION_ID,
+                lskfHashFixture(),
+                keyClaimantFixture(),
+                vaultParamsFixture()));
+        storage.add(TEST_USER_ID, new RecoverySessionStorage.Entry(
+                "some other session",
+                lskfHashFixture(),
+                keyClaimantFixture(),
+                vaultParamsFixture()));
+
+        storage.remove(TEST_USER_ID, TEST_SESSION_ID);
+
+        assertNull(storage.get(TEST_USER_ID, TEST_SESSION_ID));
+    }
+
+    @Test
+    public void remove_doesNotDeleteOtherSessions() {
+        String otherSessionId = "some other session";
+        RecoverySessionStorage storage = new RecoverySessionStorage();
+        storage.add(TEST_USER_ID, new RecoverySessionStorage.Entry(
+                TEST_SESSION_ID,
+                lskfHashFixture(),
+                keyClaimantFixture(),
+                vaultParamsFixture()));
+        storage.add(TEST_USER_ID, new RecoverySessionStorage.Entry(
+                otherSessionId,
+                lskfHashFixture(),
+                keyClaimantFixture(),
+                vaultParamsFixture()));
+
+        storage.remove(TEST_USER_ID, TEST_SESSION_ID);
+
+        assertNotNull(storage.get(TEST_USER_ID, TEST_SESSION_ID));
+    }
+
+    @Test
     public void destroy_overwritesLskfHashMemory() {
         RecoverySessionStorage storage = new RecoverySessionStorage();
         RecoverySessionStorage.Entry entry = new RecoverySessionStorage.Entry(