From fd58c9bca2e84c6384ff2248c94bc0da801495f7 Mon Sep 17 00:00:00 2001 From: Robert Berry Date: Mon, 18 Dec 2017 18:56:22 +0000 Subject: [PATCH] Revert "Add platform key generation ID to WrappedKey instances" This reverts commit 9fa18c621e82d4a6e2b647fc3268ddc89e64b73c. Reason for revert: broke the build, sorry Change-Id: I6425160e9ac565664e25ee5c92ce1a5813dd4c28 --- .../recoverablekeystore/PlatformEncryptionKey.java | 62 ---------------------- .../RecoverableKeyGenerator.java | 7 +-- .../recoverablekeystore/WrappedKey.java | 20 +++---- .../RecoverableKeyGeneratorTest.java | 8 +-- .../recoverablekeystore/WrappedKeyTest.java | 45 ++++++---------- 5 files changed, 33 insertions(+), 109 deletions(-) delete mode 100644 services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformEncryptionKey.java diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformEncryptionKey.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformEncryptionKey.java deleted file mode 100644 index 38f5b45ea190..000000000000 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformEncryptionKey.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (C) 2017 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 com.android.server.locksettings.recoverablekeystore; - -import android.security.keystore.AndroidKeyStoreSecretKey; - -/** - * Private key stored in AndroidKeyStore. Used to wrap recoverable keys before writing them to disk. - * - *

Identified by a generation ID, which increments whenever a new platform key is generated. A - * new key must be generated whenever the user disables their lock screen, as the decryption key is - * tied to lock-screen authentication. - * - *

One current platform key exists per profile on the device. (As each must be tied to a - * different user's lock screen.) - * - * @hide - */ -public class PlatformEncryptionKey { - - private final int mGenerationId; - private final AndroidKeyStoreSecretKey mKey; - - /** - * A new instance. - * - * @param generationId The generation ID of the key. - * @param key The secret key handle. Can be used to encrypt WITHOUT requiring screen unlock. - */ - public PlatformEncryptionKey(int generationId, AndroidKeyStoreSecretKey key) { - mGenerationId = generationId; - mKey = key; - } - - /** - * Returns the generation ID of the key. - */ - public int getGenerationId() { - return mGenerationId; - } - - /** - * Returns the actual key, which can only be used to encrypt. - */ - public AndroidKeyStoreSecretKey getKey() { - return mKey; - } -} diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java index b22ba4ec8bde..40c788997ba5 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGenerator.java @@ -16,6 +16,7 @@ package com.android.server.locksettings.recoverablekeystore; +import android.security.keystore.AndroidKeyStoreSecretKey; import android.security.keystore.KeyProperties; import android.security.keystore.KeyProtection; import android.util.Log; @@ -55,7 +56,7 @@ public class RecoverableKeyGenerator { * @hide */ public static RecoverableKeyGenerator newInstance( - PlatformEncryptionKey platformKey, RecoverableKeyStorage recoverableKeyStorage) + AndroidKeyStoreSecretKey platformKey, RecoverableKeyStorage recoverableKeyStorage) throws NoSuchAlgorithmException { // NB: This cannot use AndroidKeyStore as the provider, as we need access to the raw key // material, so that it can be synced to disk in encrypted form. @@ -65,11 +66,11 @@ public class RecoverableKeyGenerator { private final KeyGenerator mKeyGenerator; private final RecoverableKeyStorage mRecoverableKeyStorage; - private final PlatformEncryptionKey mPlatformKey; + private final AndroidKeyStoreSecretKey mPlatformKey; private RecoverableKeyGenerator( KeyGenerator keyGenerator, - PlatformEncryptionKey platformKey, + AndroidKeyStoreSecretKey platformKey, RecoverableKeyStorage recoverableKeyStorage) { mKeyGenerator = keyGenerator; mRecoverableKeyStorage = recoverableKeyStorage; diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/WrappedKey.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/WrappedKey.java index a0e34c35b314..f18e7961de5f 100644 --- a/services/core/java/com/android/server/locksettings/recoverablekeystore/WrappedKey.java +++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/WrappedKey.java @@ -44,7 +44,6 @@ public class WrappedKey { private static final String APPLICATION_KEY_ALGORITHM = "AES"; private static final int GCM_TAG_LENGTH_BITS = 128; - private final int mPlatformKeyGenerationId; private final byte[] mNonce; private final byte[] mKeyMaterial; @@ -56,8 +55,8 @@ public class WrappedKey { * {@link android.security.keystore.AndroidKeyStoreKey} for an example of a key that does * not expose its key material. */ - public static WrappedKey fromSecretKey(PlatformEncryptionKey wrappingKey, SecretKey key) - throws InvalidKeyException, KeyStoreException { + public static WrappedKey fromSecretKey( + SecretKey wrappingKey, SecretKey key) throws InvalidKeyException, KeyStoreException { if (key.getEncoded() == null) { throw new InvalidKeyException( "key does not expose encoded material. It cannot be wrapped."); @@ -71,7 +70,7 @@ public class WrappedKey { "Android does not support AES/GCM/NoPadding. This should never happen."); } - cipher.init(Cipher.WRAP_MODE, wrappingKey.getKey()); + cipher.init(Cipher.WRAP_MODE, wrappingKey); byte[] encryptedKeyMaterial; try { encryptedKeyMaterial = cipher.wrap(key); @@ -91,10 +90,7 @@ public class WrappedKey { } } - return new WrappedKey( - /*nonce=*/ cipher.getIV(), - /*keyMaterial=*/ encryptedKeyMaterial, - /*platformKeyGenerationId=*/ wrappingKey.getGenerationId()); + return new WrappedKey(/*mNonce=*/ cipher.getIV(), /*mKeyMaterial=*/ encryptedKeyMaterial); } /** @@ -102,14 +98,12 @@ public class WrappedKey { * * @param nonce The nonce with which the key material was encrypted. * @param keyMaterial The encrypted bytes of the key material. - * @param platformKeyGenerationId The generation ID of the key used to wrap this key. * * @hide */ - public WrappedKey(byte[] nonce, byte[] keyMaterial, int platformKeyGenerationId) { + public WrappedKey(byte[] nonce, byte[] keyMaterial) { mNonce = nonce; mKeyMaterial = keyMaterial; - mPlatformKeyGenerationId = platformKeyGenerationId; } /** @@ -130,13 +124,15 @@ public class WrappedKey { return mKeyMaterial; } + /** * Returns the generation ID of the platform key, with which this key was wrapped. * * @hide */ public int getPlatformKeyGenerationId() { - return mPlatformKeyGenerationId; + // TODO(robertberry) Implement. See ag/3362855. + return 1; } /** diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGeneratorTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGeneratorTest.java index c13c779f2934..298a98822caa 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGeneratorTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyGeneratorTest.java @@ -48,24 +48,24 @@ import javax.crypto.SecretKey; @SmallTest @RunWith(AndroidJUnit4.class) public class RecoverableKeyGeneratorTest { - private static final int TEST_GENERATION_ID = 3; private static final String ANDROID_KEY_STORE_PROVIDER = "AndroidKeyStore"; private static final String KEY_ALGORITHM = "AES"; private static final String TEST_ALIAS = "karlin"; private static final String WRAPPING_KEY_ALIAS = "RecoverableKeyGeneratorTestWrappingKey"; - @Mock RecoverableKeyStorage mRecoverableKeyStorage; + @Mock + RecoverableKeyStorage mRecoverableKeyStorage; @Captor ArgumentCaptor mKeyProtectionArgumentCaptor; - private PlatformEncryptionKey mPlatformKey; + private AndroidKeyStoreSecretKey mPlatformKey; private SecretKey mKeyHandle; private RecoverableKeyGenerator mRecoverableKeyGenerator; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - mPlatformKey = new PlatformEncryptionKey(TEST_GENERATION_ID, generateAndroidKeyStoreKey()); + mPlatformKey = generateAndroidKeyStoreKey(); mKeyHandle = generateKey(); mRecoverableKeyGenerator = RecoverableKeyGenerator.newInstance( mPlatformKey, mRecoverableKeyStorage); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/WrappedKeyTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/WrappedKeyTest.java index c056160e7f11..8371fe5e376c 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/WrappedKeyTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/WrappedKeyTest.java @@ -61,8 +61,7 @@ public class WrappedKeyTest { @Test public void fromSecretKey_createsWrappedKeyThatCanBeUnwrapped() throws Exception { - PlatformEncryptionKey wrappingKey = new PlatformEncryptionKey( - GENERATION_ID, generateAndroidKeyStoreKey()); + SecretKey wrappingKey = generateAndroidKeyStoreKey(); SecretKey rawKey = generateKey(); WrappedKey wrappedKey = WrappedKey.fromSecretKey(wrappingKey, rawKey); @@ -70,7 +69,7 @@ public class WrappedKeyTest { Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM); cipher.init( Cipher.UNWRAP_MODE, - wrappingKey.getKey(), + wrappingKey, new GCMParameterSpec(GCM_TAG_LENGTH_BITS, wrappedKey.getNonce())); SecretKey unwrappedKey = (SecretKey) cipher.unwrap( wrappedKey.getKeyMaterial(), KEY_ALGORITHM, Cipher.SECRET_KEY); @@ -78,28 +77,15 @@ public class WrappedKeyTest { } @Test - public void fromSecretKey_returnsAKeyWithTheGenerationIdOfTheWrappingKey() throws Exception { - PlatformEncryptionKey wrappingKey = new PlatformEncryptionKey( - GENERATION_ID, generateAndroidKeyStoreKey()); - SecretKey rawKey = generateKey(); - - WrappedKey wrappedKey = WrappedKey.fromSecretKey(wrappingKey, rawKey); - - assertEquals(GENERATION_ID, wrappedKey.getPlatformKeyGenerationId()); - } - - @Test public void decryptWrappedKeys_decryptsWrappedKeys() throws Exception { String alias = "karlin"; - AndroidKeyStoreSecretKey platformKey = generateAndroidKeyStoreKey(); + PlatformDecryptionKey platformKey = generatePlatformDecryptionKey(); SecretKey appKey = generateKey(); - WrappedKey wrappedKey = WrappedKey.fromSecretKey( - new PlatformEncryptionKey(GENERATION_ID, platformKey), appKey); + WrappedKey wrappedKey = WrappedKey.fromSecretKey(platformKey.getKey(), appKey); HashMap keysByAlias = new HashMap<>(); keysByAlias.put(alias, wrappedKey); - Map unwrappedKeys = WrappedKey.unwrapKeys( - new PlatformDecryptionKey(GENERATION_ID, platformKey), keysByAlias); + Map unwrappedKeys = WrappedKey.unwrapKeys(platformKey, keysByAlias); assertEquals(1, unwrappedKeys.size()); assertTrue(unwrappedKeys.containsKey(alias)); @@ -109,31 +95,26 @@ public class WrappedKeyTest { @Test public void decryptWrappedKeys_doesNotDieIfSomeKeysAreUnwrappable() throws Exception { String alias = "karlin"; - AndroidKeyStoreSecretKey platformKey = generateAndroidKeyStoreKey(); SecretKey appKey = generateKey(); - WrappedKey wrappedKey = WrappedKey.fromSecretKey( - new PlatformEncryptionKey(GENERATION_ID, platformKey), appKey); + WrappedKey wrappedKey = WrappedKey.fromSecretKey(generateKey(), appKey); HashMap keysByAlias = new HashMap<>(); keysByAlias.put(alias, wrappedKey); Map unwrappedKeys = WrappedKey.unwrapKeys( - new PlatformDecryptionKey(GENERATION_ID, platformKey), keysByAlias); + generatePlatformDecryptionKey(), keysByAlias); assertEquals(0, unwrappedKeys.size()); } @Test public void decryptWrappedKeys_throwsIfPlatformKeyGenerationIdDoesNotMatch() throws Exception { - AndroidKeyStoreSecretKey platformKey = generateAndroidKeyStoreKey(); - WrappedKey wrappedKey = WrappedKey.fromSecretKey( - new PlatformEncryptionKey(GENERATION_ID, platformKey), generateKey()); + WrappedKey wrappedKey = WrappedKey.fromSecretKey(generateKey(), generateKey()); HashMap keysByAlias = new HashMap<>(); keysByAlias.put("benji", wrappedKey); try { WrappedKey.unwrapKeys( - new PlatformDecryptionKey(/*generationId=*/ 2, platformKey), - keysByAlias); + generatePlatformDecryptionKey(/*generationId=*/ 2), keysByAlias); fail("Should have thrown."); } catch (BadPlatformKeyException e) { assertEquals( @@ -161,4 +142,12 @@ public class WrappedKeyTest { .build()); return (AndroidKeyStoreSecretKey) keyGenerator.generateKey(); } + + private PlatformDecryptionKey generatePlatformDecryptionKey() throws Exception { + return generatePlatformDecryptionKey(GENERATION_ID); + } + + private PlatformDecryptionKey generatePlatformDecryptionKey(int generationId) throws Exception { + return new PlatformDecryptionKey(generationId, generateAndroidKeyStoreKey()); + } } -- 2.11.0