Missing Android P support for SecureCredentialsManager class

[sebaslogen]

I continuously see the following warning in the logcat logs when using Auth0 with the SecureCredentialsManager in Android 9 (P)

W/KeyStore: KeyStore exception android.os.ServiceSpecificException: (code 7) at android.os.Parcel.createException(Parcel.java:1956) at android.os.Parcel.readException(Parcel.java:1910) at android.os.Parcel.readException(Parcel.java:1860) at android.security.IKeystoreService$Stub$Proxy.get(IKeystoreService.java:786) at android.security.KeyStore.get(KeyStore.java:195) at android.security.keystore.AndroidKeyStoreSpi.engineGetCertificateChain(AndroidKeyStoreSpi.java:118) at java.security.KeyStoreSpi.engineGetEntry(KeyStoreSpi.java:484) at java.security.KeyStore.getEntry(KeyStore.java:1560) at com.auth0.android.authentication.storage.CryptoUtil.getRSAKeyEntry(CryptoUtil.java:88) at com.auth0.android.authentication.storage.CryptoUtil.RSADecrypt(CryptoUtil.java:166) at com.auth0.android.authentication.storage.CryptoUtil.getAESKey(CryptoUtil.java:204) at com.auth0.android.authentication.storage.CryptoUtil.decrypt(CryptoUtil.java:225) at com.auth0.android.authentication.storage.SecureCredentialsManager.continueGetCredentials(SecureCredentialsManager.java:208) at com.auth0.android.authentication.storage.SecureCredentialsManager.getCredentials(SecureCredentialsManager.java:175)

The root cause is probably the same as in this other library´s issue: adorsys/secure-storage-android#30

[lbalmaceda]

Thanks for the report 🙆‍♂️

[shriakhilc]

@lbalmaceda, can I take this up?

[lbalmaceda]

@TheGamer007 sure. Make sure that versions pre-P still work by checking the version code on runtime, and add tests 👌

[shriakhilc]

@lbalmaceda I've added the version code check along with the suggested solution as per this SO answer. Instead of using getEntry, we should now be fetching the PrivateKey and Certificate objects individually via getPrivateKey and getCertificate.

For P and above, I am fetching those, then constructing a KeyStore.PrivateKeyEntry object and returning that, as follows:

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
    //Following code is for API 28+
    PrivateKey privateKey = (PrivateKey) keyStore.getKey(KEY_ALIAS, null);
    Certificate[] certificateChain = keyStore.getCertificateChain(KEY_ALIAS);
    return new KeyStore.PrivateKeyEntry(privateKey, certificateChain);
} else {
    return (KeyStore.PrivateKeyEntry) keyStore.getEntry(KEY_ALIAS, null);
}

All the current tests are passing. For adding a new test, initially I duplicated the shouldCreateRSAKeyPairIfMissingOnAPI23AndUp test, renamed it to Api28AndUp, and set the corresponding sdk levels to 28 in order to trigger the new part of the code. (I am new to automated testing, so if this is wrong, then please let me know what approach should be taken)

The execution is taking the expected route, but the test fails when the getRSAKeyEntry method is called because the privateKey and certificateChain variables are null, and the KeyStore.PrivateKeyEntry object can't be made.

Upon debugging the passing API 23 test, I saw that the expectedEntry and returned entry are both marked as a "Private key entry and Certificate chain with 0 elements" , and the privKey inside them is marked as "null" (not null).

I'm guessing the automated test for API 28 needs to be modified for it to work properly? Could you give me some pointers as to where I might be going wrong? Meanwhile, I plan to see if it fails on a device test as well, in which case I'd have to modify my code logic.

[lbalmaceda]

@TheGamer007 There's an annotation you can use for robolectric to run a test under a different API level than the one declared by the project. That is being used on the 23AndUp test you mentioned.

@Config(sdk = Build.VERSION_CODES.{code})
public void shouldDoX(){
}

That test is mocking what the returned object should be in this line, so for this new test you'll have to make the mock return what you expect. I suggest you take a real device running that API level and see how the output looks like, so you emulate the real behavior on the test.

Feel free to post the PR so I can follow your code changes.

[shriakhilc]

I tested the method on an API 27 device and an API 28 emulator. Even in my solution, it seems the ServiceSpecificException warning is being triggered when I call getCertificateChain(). However, it is not triggered if I directly get the Certificate using getCertificate().

From what I can see, the only usages of getRSAKeyEntry (apart from in tests) are in the methods RSADecrypt and RSAEncrypt in the same file, where they are just accessing the PrivateKey and the Certificate respectively. The entire chain doesn't seem to be in use.

Would it be fine to use the following approach then? It does not yield warnings / exceptions on Android P, and the code for older versions is unchanged.

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
    //Following code is for API 28+
    PrivateKey privateKey = (PrivateKey) keyStore.getKey(KEY_ALIAS, null);
    Certificate certificate = keyStore.getCertificate(KEY_ALIAS);
    return new KeyStore.PrivateKeyEntry(privateKey, new Certificate[] {certificate});
} else {
    return (KeyStore.PrivateKeyEntry) keyStore.getEntry(KEY_ALIAS, null);
}