Skip to content

Commit 3f151f1

Browse files
p3dr0rvCopilotsomalaya
authored
Add KeyStoreBackedSecretKeyProvider, Fixes AB#3274176 (#2674)
## Summary This PR has two main goals: to seamlessly update the encryption padding used in the Android KeyStore, and to lay the groundwork for integrating future cryptographic requirements with minimal changes. --- ### Phase 1: Replace `AndroidWrappedKeyProvider` with `KeyStoreBackedSecretKeyProvider` - **Controller Flag:** `ENABLE_KEYSTORE_BACKED_SECRET_KEY_PROVIDER` - **Behavior When Enabled:** - The factory will return `KeyStoreBackedSecretKeyProvider` instead of `AndroidWrappedKeyProvider`. - The new class behaves similarly to the previous one but introduces the ability to: - Retrieve the paddings supported by the KeyStore `KeyPair` - Maintain a list of available ciphers (`oaepCipherSpec` and `pkcs1CipherSpec`) - Select a compatible cipher for encryption/decryption based on what the `KeyPair` supports. - In this case, if the `KeyPair` only supports `PKCS1`, then all keys will be encrypted and decrypted using `PKCS1`. - **Goal:** - Validate that the new provider functions equivalently to the existing one. > Note: No encryption changes are introduced in this phase. ### Tests - `AndroidWrappedKeyProviderTest` has been parameterized to confirm both providers behave identically. - `KeyStoreBackedSecretKeyProviderRolloutTest.java` includes test to validate migration and rollback --- ### Phase 2: Enable New Wrapped Secret Key Format with Metadata Support Before introducing new key types, we need to ensure that all new keys include additional metadata that specifies how they were encrypted. - **Controller Flag:** `ENABLE_NEW_WRAPPED_SECRET_KEY_FORMAT` - **Behavior When Enabled:** - `WrappedSecretKey` will use an enhanced binary format with the following structure: - MAGIC BYTES (4 bytes) - SERIALIZER ID (4bytes) - Metadata length (4 bytes) - Metadata (format is decided by the serializer) - Raw encrypted key bytes - **Goal:** - Future-proof the wrapped key by including metadata that specifies: - Key size - Algorithm - Transformation used for encryption - Key version - This allows the system to handle multiple key types if formats change in the future. - Validate `WrappedSecretKey` can read both new and old keys. ### Tests - `WrappedSecretKeyTest` --- ### Phase 3: Add OAEP encryption padding. - **Controller Flag:** `ENABLE_OAEP_WITH_SHA_AND_MGF1_PADDING` - **Behavior When Enabled:** - `CryptoParameterSpecFactory` will now return two paddings: `ENCRYPTION_PADDING_RSA_PKCS1` and `ENCRYPTION_PADDING_RSA_OAEP` for the KeyGen specifications. - `KeyStoreBackedSecretKeyProvider` will generate a `KeyPair` that supports `ENCRYPTION_PADDING_RSA_OAEP`. - As a result, when a `SecretKey` needs to be wrapped, the provider will select OAEP, since the `KeyPair` now supports this padding and OAEP cipher support was already introduced in Phase 1. - **Goal:** - All new keys will be encrypted /decrypted with OAEP - Existing keys will still be encrypted /decrypted with PKCS1 ### Tests - `KeyStoreBackedSecretKeyProviderRolloutTest.java` includes test to validate migration and rollback --- ### Telemetry | span_name | key_pair_gen_successful_method | elapsed_time_keypair_generation | secret_key_wrapping_cipher | secret_key_wrapping_operation | |-------------------|----------------------------------------------------------------------------------------------|----------------------------------|------------------------------------------------------------|--------------------------------------------------------------| | SecretKeyWrapping | null | | CipherSpec(transformation='RSA/ECB/PKCS1Padding') | KeyStoreBackedSecretKeyProvider:unwrapSecretKey | | KeyPairGeneration | KeyGenSpec(description='modern_spec_with_wrap_key', algorithm='RSA', encryptionPaddings='[PKCS1Padding]') | 581 | | | | KeyPairGeneration | KeyGenSpec(description='modern_spec_with_wrap_key', algorithm='RSA', encryptionPaddings='[PKCS1Padding]') | 294 | | | [AB#3274176](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3274176) AzureAD/ad-accounts-for-android#3166 --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Sowmya Malayanur <[email protected]>
1 parent 4bfba98 commit 3f151f1

28 files changed

+4304
-46
lines changed

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [MAJOR] Add KeyStoreBackedSecretKeyProvider (#2674)
34
- [MINOR] Add Open Id configuration issuer validation reporting in OpenIdProviderConfigurationClient (#2751)
45
- [MINOR] Add helper method to record elapsed time (#2768)
56
- [MINOR] Implement TenantUtil (#2761)

common/src/androidTest/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyProviderTest.java

Lines changed: 91 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,90 @@
3131

3232
import com.microsoft.identity.common.adal.internal.AuthenticationSettings;
3333
import com.microsoft.identity.common.internal.util.AndroidKeyStoreUtil;
34+
import com.microsoft.identity.common.java.crypto.key.ISecretKeyProvider;
3435
import com.microsoft.identity.common.java.exception.ClientException;
3536
import com.microsoft.identity.common.java.util.FileUtil;
3637

3738
import org.junit.Assert;
3839
import org.junit.Before;
3940
import org.junit.Ignore;
4041
import org.junit.Test;
42+
import org.junit.runner.RunWith;
43+
import org.junit.runners.Parameterized;
4144

4245
import java.io.File;
4346
import java.math.BigInteger;
4447
import java.security.KeyPair;
4548
import java.security.spec.AlgorithmParameterSpec;
4649
import java.util.Arrays;
50+
import java.util.Collection;
4751
import java.util.Date;
4852

4953
import javax.crypto.SecretKey;
5054
import javax.security.auth.x500.X500Principal;
5155

56+
@RunWith(Parameterized.class)
5257
public class AndroidWrappedKeyProviderTest {
5358

59+
@Parameterized.Parameter(0)
60+
public String providerType;
61+
62+
@Parameterized.Parameters(name = "{0}")
63+
public static Collection<Object[]> data() {
64+
return Arrays.asList(new Object[][] {
65+
{"KEYSTORE_BACKED"},
66+
{"ANDROID_WRAPPED"}
67+
// Add other provider types here as keywords
68+
});
69+
}
70+
71+
private ISecretKeyProvider createProvider() {
72+
if ("KEYSTORE_BACKED".equals(providerType)) {
73+
return new KeyStoreBackedSecretKeyProvider(context, MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH);
74+
} else if ("ANDROID_WRAPPED".equals(providerType)) {
75+
return new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
76+
} else {
77+
throw new IllegalArgumentException("Unsupported provider type: " + providerType);
78+
}
79+
}
80+
81+
private SecretKey getKeyFromCache(ISecretKeyProvider keyProvider) {
82+
if (keyProvider instanceof AndroidWrappedKeyProvider) {
83+
return ((AndroidWrappedKeyProvider) keyProvider).getKeyFromCache();
84+
} else if (keyProvider instanceof KeyStoreBackedSecretKeyProvider) {
85+
return ((KeyStoreBackedSecretKeyProvider) keyProvider).getKeyFromCache();
86+
}
87+
throw new IllegalArgumentException("Unsupported key provider type: " + keyProvider.getClass().getName());
88+
}
89+
90+
private void clearKeyFromCache(ISecretKeyProvider keyProvider) {
91+
if (keyProvider instanceof AndroidWrappedKeyProvider) {
92+
((AndroidWrappedKeyProvider) keyProvider).clearKeyFromCache();
93+
} else if (keyProvider instanceof KeyStoreBackedSecretKeyProvider) {
94+
((KeyStoreBackedSecretKeyProvider) keyProvider).clearKeyFromCache();
95+
} else {
96+
throw new IllegalArgumentException("Unsupported key provider type: " + keyProvider.getClass().getName());
97+
}
98+
}
99+
100+
private SecretKey readSecretKeyFromStorage(ISecretKeyProvider keyProvider) throws ClientException {
101+
if (keyProvider instanceof AndroidWrappedKeyProvider) {
102+
return ((AndroidWrappedKeyProvider) keyProvider).readSecretKeyFromStorage();
103+
} else if (keyProvider instanceof KeyStoreBackedSecretKeyProvider) {
104+
return ((KeyStoreBackedSecretKeyProvider) keyProvider).readSecretKeyFromStorage();
105+
}
106+
throw new IllegalArgumentException("Unsupported key provider type: " + keyProvider.getClass().getName());
107+
}
108+
109+
private SecretKey generateNewSecretKey(ISecretKeyProvider keyProvider) throws ClientException {
110+
if (keyProvider instanceof AndroidWrappedKeyProvider) {
111+
return ((AndroidWrappedKeyProvider) keyProvider).generateRandomKey();
112+
} else if (keyProvider instanceof KeyStoreBackedSecretKeyProvider) {
113+
return ((KeyStoreBackedSecretKeyProvider) keyProvider).generateNewSecretKey();
114+
}
115+
throw new IllegalArgumentException("Unsupported key provider type: " + keyProvider.getClass().getName());
116+
}
117+
54118
final Context context = ApplicationProvider.getApplicationContext();
55119
final String MOCK_KEY_ALIAS = "MOCK_KEY_ALIAS";
56120
final String MOCK_KEY_FILE_PATH = "MOCK_KEY_FILE_PATH";
@@ -111,17 +175,17 @@ private AlgorithmParameterSpec getMockKeyPairGeneratorSpec(final String alias) {
111175

112176
@Test
113177
public void testGenerateKey() throws ClientException {
114-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
115-
final SecretKey secretKey = keyProvider.generateRandomKey();
178+
final ISecretKeyProvider keyProvider = createProvider();
179+
final SecretKey secretKey = generateNewSecretKey(keyProvider);
116180

117181
Assert.assertEquals(AES_ALGORITHM, secretKey.getAlgorithm());
118182
}
119183

120184
@Test
121185
public void testReadKeyDirectly() throws ClientException {
122-
final AndroidWrappedKeyProvider keyProvider = initkeyProviderWithKeyEntry();
186+
final ISecretKeyProvider keyProvider = initkeyProviderWithKeyEntry();
123187
final SecretKey secretKey = keyProvider.getKey();
124-
final SecretKey storedSecretKey = keyProvider.readSecretKeyFromStorage();
188+
final SecretKey storedSecretKey = readSecretKeyFromStorage(keyProvider);
125189

126190
// They're not the same object!
127191
Assert.assertNotSame(secretKey, storedSecretKey);
@@ -139,12 +203,12 @@ public void testReadKeyDirectly() throws ClientException {
139203
public void testLoadKey() throws ClientException {
140204
// Nothing exists. This load key function should generate a key if the key hasn't exist.
141205
Assert.assertNull(AndroidKeyStoreUtil.readKey(MOCK_KEY_ALIAS));
142-
Assert.assertNull(FileUtil.readFromFile(getKeyFile(), AndroidWrappedKeyProvider.KEY_FILE_SIZE));
206+
Assert.assertNull(FileUtil.readFromFile(getKeyFile(), KeyStoreBackedSecretKeyProvider.KEY_FILE_SIZE));
143207

144-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
208+
final ISecretKeyProvider keyProvider = createProvider();
145209
final SecretKey secretKey = keyProvider.getKey();
146210

147-
final SecretKey key = keyProvider.getKeyFromCache();
211+
final SecretKey key = getKeyFromCache(keyProvider);
148212
Assert.assertNotNull(key);
149213
Assert.assertEquals(AES_ALGORITHM, secretKey.getAlgorithm());
150214
Assert.assertArrayEquals(secretKey.getEncoded(), key.getEncoded());
@@ -154,18 +218,18 @@ public void testLoadKey() throws ClientException {
154218
@Test
155219
public void testLoadKeyFromCorruptedFile_TruncatedExisingKey() throws ClientException {
156220
// Create a new Keystore-wrapped key.
157-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
158-
keyProvider.generateRandomKey();
221+
final ISecretKeyProvider keyProvider = createProvider();
222+
generateNewSecretKey(keyProvider);
159223

160-
final byte[] wrappedKey = FileUtil.readFromFile(getKeyFile(), AndroidWrappedKeyProvider.KEY_FILE_SIZE);
224+
final byte[] wrappedKey = FileUtil.readFromFile(getKeyFile(), KeyStoreBackedSecretKeyProvider.KEY_FILE_SIZE);
161225
Assert.assertNotNull(wrappedKey);
162226

163227
// Overwrite the key file with corrupted data.
164228
FileUtil.writeDataToFile(Arrays.copyOfRange(wrappedKey, 0, wrappedKey.length/2), getKeyFile());
165229

166230
// It should fail to read, with an exception, and everything should be wiped.
167231
try{
168-
keyProvider.readSecretKeyFromStorage();
232+
readSecretKeyFromStorage(keyProvider);
169233
Assert.fail();
170234
} catch (ClientException e){
171235
Assert.assertEquals(INVALID_KEY, e.getErrorCode());
@@ -175,24 +239,24 @@ public void testLoadKeyFromCorruptedFile_TruncatedExisingKey() throws ClientExce
175239
Assert.assertFalse(getKeyFile().exists());
176240

177241
// the next read should be unblocked.
178-
Assert.assertNull(keyProvider.readSecretKeyFromStorage());
242+
Assert.assertNull(readSecretKeyFromStorage(keyProvider));
179243
}
180244

181245
@Test
182246
public void testLoadKeyFromCorruptedFile_InjectGarbage() throws ClientException {
183247
// Create a new Keystore-wrapped key.
184-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
185-
keyProvider.generateRandomKey();
248+
final ISecretKeyProvider keyProvider = createProvider();
249+
generateNewSecretKey(keyProvider);
186250

187-
final byte[] wrappedKey = FileUtil.readFromFile(getKeyFile(), AndroidWrappedKeyProvider.KEY_FILE_SIZE);
251+
final byte[] wrappedKey = FileUtil.readFromFile(getKeyFile(), KeyStoreBackedSecretKeyProvider.KEY_FILE_SIZE);
188252
Assert.assertNotNull(wrappedKey);
189253

190254
// Overwrite the key file with corrupted data.
191255
FileUtil.writeDataToFile(new byte[]{10, 20, 30, 40}, getKeyFile());
192256

193257
// It should fail to read, with an exception, and everything should be wiped.
194258
try{
195-
keyProvider.readSecretKeyFromStorage();
259+
readSecretKeyFromStorage(keyProvider);
196260
Assert.fail();
197261
} catch (ClientException e){
198262
Assert.assertEquals(INVALID_KEY, e.getErrorCode());
@@ -202,14 +266,14 @@ public void testLoadKeyFromCorruptedFile_InjectGarbage() throws ClientException
202266
Assert.assertFalse(getKeyFile().exists());
203267

204268
// the next read should be unblocked.
205-
Assert.assertNull(keyProvider.readSecretKeyFromStorage());
269+
Assert.assertNull(readSecretKeyFromStorage(keyProvider));
206270
}
207271

208272
// 1s With Google Pixel XL, OS Version 29 (100 loop)
209273
@Test
210274
@Ignore
211275
public void testPerf_WithCachedKey() throws ClientException {
212-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
276+
final ISecretKeyProvider keyProvider = createProvider();
213277

214278
long timeStartLoop = System.nanoTime();
215279
for (int i = 0; i < TEST_LOOP; i++) {
@@ -224,11 +288,11 @@ public void testPerf_WithCachedKey() throws ClientException {
224288
@Test
225289
@Ignore("Performance test, ignore in normal test run")
226290
public void testPerf_NoCachedKey() throws ClientException {
227-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
291+
final ISecretKeyProvider keyProvider = createProvider();
228292

229293
long timeStartLoopNotCached = System.nanoTime();
230294
for (int i = 0; i < 100; i++) {
231-
keyProvider.clearKeyFromCache();
295+
clearKeyFromCache(keyProvider);
232296
keyProvider.getKey();
233297
}
234298
long timeFinishLoopNotCached = System.nanoTime();
@@ -241,31 +305,31 @@ public void testPerf_NoCachedKey() throws ClientException {
241305
*/
242306
@Test
243307
public void testLoadDeletedKeyStoreKey() throws ClientException {
244-
final AndroidWrappedKeyProvider keyProvider = initkeyProviderWithKeyEntry();
308+
final ISecretKeyProvider keyProvider = initkeyProviderWithKeyEntry();
245309

246310
AndroidKeyStoreUtil.deleteKey(MOCK_KEY_ALIAS);
247311

248312
// Cached key also be wiped.
249-
final SecretKey key = keyProvider.getKeyFromCache();
313+
final SecretKey key = getKeyFromCache(keyProvider);
250314
Assert.assertNull(key);
251315
}
252316

253317
@Test
254318
public void testLoadDeletedKeyFile() throws ClientException {
255-
final AndroidWrappedKeyProvider keyProvider = initkeyProviderWithKeyEntry();
319+
final ISecretKeyProvider keyProvider = initkeyProviderWithKeyEntry();
256320

257321
FileUtil.deleteFile(getKeyFile());
258322

259323
// Cached key also be wiped.
260-
final SecretKey key = keyProvider.getKeyFromCache();
324+
final SecretKey key = getKeyFromCache(keyProvider);
261325
Assert.assertNull(key);
262326
}
263327

264-
private AndroidWrappedKeyProvider initkeyProviderWithKeyEntry() throws ClientException {
265-
final AndroidWrappedKeyProvider keyProvider = new AndroidWrappedKeyProvider(MOCK_KEY_ALIAS, MOCK_KEY_FILE_PATH, context);
328+
private ISecretKeyProvider initkeyProviderWithKeyEntry() throws ClientException {
329+
final ISecretKeyProvider keyProvider = createProvider();
266330
final SecretKey key = keyProvider.getKey();
267331
Assert.assertNotNull(key);
268-
Assert.assertNotNull(keyProvider.getKeyFromCache());
332+
Assert.assertNotNull(getKeyFromCache(keyProvider));
269333
return keyProvider;
270334
}
271335
}

0 commit comments

Comments
 (0)