Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to store keys in Java Keystore (JKS) / Android Keystore (AKS) #43

Closed
PunchyRascal opened this issue Jan 26, 2018 · 14 comments
Closed

Comments

@PunchyRascal
Copy link

@PunchyRascal PunchyRascal commented Jan 26, 2018

Hi, this library looks very nice (unlike the native API), but I have some trouble storing the resulting keys.

I tried to store the keys in AndroidKeyStore for its hardware-backed security, but I am getting an exception saying I cannot store secretKeys at all. Is there a way how to achieve this?

Thanks.

keyStore = KeyStore.getInstance("AndroidKeyStore");
keyStore.load(null);

AesCbcWithIntegrity.SecretKeys keys = AesCbcWithIntegrity.generateKey();
KeyStore.SecretKeyEntry entryConfidential = new KeyStore.SecretKeyEntry(keys.getConfidentialityKey());
keyStore.setEntry(confidentialAlias, entryConfidential, null); //throws exception

this ↑ throws an exception java.security.KeyStoreException: Entry must be a PrivateKeyEntry or TrustedCertificateEntry; was SecretKeyEntry: algorithm - AES

@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Jan 26, 2018

We would like to integrate the library more completely with the Android Keystore - (AKS), as there's a lot of subtle details around the Android version, we'd like our users to have easier access to good key storage.

This should be possible today - AES CBC is available in AKS after API level 23 (6.0 / marshmallow), and so is the HMAC algorithm we use, I believe. You can even generate the key inside the AKS, which probably provides an additional level of protection.

You can also do fun things like associate the key with the user's fingerprint so they have to present that biometric to unlock the key. This can add a nice extra layer of protection. The lock screen is also a great choice.

However, some caveats are that the API level 23+ might only be half of the Android phones out there, so you probably have to handle other methods of key storage to accommodate all users. Also, note the key destruction rules (eg. the user removes the lock screen, adds fingerprints, etc) to determine if they are suitable for your application. You wouldn't want the user to lose access to e.g. their photo collection because they decided to enroll another fingerprint.

As for your particular exception, I can only say we're not quite supporting this yet, but my best guess is that you're operating on a platform where AKS doesn't have AES support. You definitely need to check API level before trying this funciton.

PRs welcome for some saveKeyToAKS function or similar :)

Isaac

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

You're absolutely right, I didn't realize the AKS does not support AES on API 22 for which I develop. So for now I will have to store the key on my own somewhere. Sorry for that.

As for PR for this, I will consider it after familiarizing myself more with this :)

Thanks for the response.

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

Just a note regarding said PR:

It would be great if you would be able to just say:

  • generate and store key with this alias
  • encrypt using key with this alias
  • decrypt using key with this alias
@SyntaxPolice SyntaxPolice changed the title How to store the keys in AndroidKeyStore? Add feature to store keys in Java Keystore (JKS) / Android Keystore (AKS) Jan 29, 2018
@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Jan 29, 2018

FYI, in case it's not clear, you can already use the Java KeyStore (JKS) for this type of workflow on all or nearly all platforms.

Also, if you're encrypting a key with e.g. a passphrase in JKS, you might get similar security if you generate a key from a password and use that directly, which we already provide. AKS is much nicer because the OS protects the storage.

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

But not with this library that uses AES, is that right? Because I want to use this library because I am not an expert on cryptography and want to use a library that makes the good choices for me.

@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Jan 29, 2018

Can you explain what you're trying to do? I might be able to give some guidance. I believe JKS works on all recent platforms with AES, but it may not be trivial to get it working.

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

Well I have a string that I want to encrypt and decrypt in-place, on the device.

@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Jan 29, 2018

Is there a user password that you could use to encrypt the string?

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

Well I was thinking about not involving the user side in it. But theoretically I could use it.

@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Jan 29, 2018

Up to you. Without AKS, there's not a great choice for secure key storage, but generating the key from the user password (we provide a function for that) is a pretty good approach. As I said, we'll aim to make this easier in future releases.

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Jan 29, 2018

I want to use AKS, but it does not support AES in API < 23:
https://developer.android.com/training/articles/keystore.html#SupportedCiphers

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Feb 2, 2018

I am trying to decrypt a text; after retrieving the keys (generated with this library) from the AndroidKeyStore I get a null pointer exception: (testing it on API 27),

cipherTextIvMac = AesCbcWithIntegrity.encrypt(text, keys);

throws

java.lang.NullPointerException: Attempt to get length of null array

  • com.android.org.bouncycastle.crypto.params.KeyParameter.(KeyParameter.java:13)
  • com.android.org.bouncycastle.jcajce.provider.symmetric.util.BaseBlockCipher.engineInit(BaseBlockCipher.java:693)
  • javax.crypto.Cipher.tryTransformWithProvider(Cipher.java:2664)
  • javax.crypto.Cipher.tryCombinations(Cipher.java:2575)
  • javax.crypto.Cipher$SpiAndProviderUpdater.updateAndGetSpiAndProvider(Cipher.java:2480)
  • javax.crypto.Cipher.chooseProvider(Cipher.java:567)
  • javax.crypto.Cipher.init(Cipher.java:975)
  • javax.crypto.Cipher.init(Cipher.java:910)
  • com.tozny.crypto.android.AesCbcWithIntegrity.encrypt(AesCbcWithIntegrity.java:282)
  • com.tozny.crypto.android.AesCbcWithIntegrity.encrypt(AesCbcWithIntegrity.java:266)
  • com.tozny.crypto.android.AesCbcWithIntegrity.encrypt(AesCbcWithIntegrity.java:251)

Debugger view:
image

When creating the key store entries to save in AndroidKeyStore, I use the following protection params:

new KeyProtection
        .Builder(KeyProperties.PURPOSE_DECRYPT | KeyProperties.PURPOSE_ENCRYPT)
        .setUserAuthenticationRequired(false)
        .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
        .build();

Any idea what I am doing wrong?

@PunchyRascal
Copy link
Author

@PunchyRascal PunchyRascal commented Mar 25, 2019

In the end I used the native API:

keyStore = KeyStore.getInstance(BaseActivity.ANDROID_KEY_STORE);
keyStore.load(null);
Cipher cipher = Cipher.getInstance(AES_MODE);
Key key = keyStore.getKey(BaseActivity.CIPHER_KEY_ALIAS, null);
cipher.init(Cipher.ENCRYPT_MODE, key);
byte[] encrypted = cipher.doFinal(textToEncrypt.getBytes());
return Base64.encodeToString(cipher.getIV(), Base64.DEFAULT)  + ":" + Base64.encodeToString(encrypted, Base64.DEFAULT);

and wrapped it in an interface:

public class Encryption implements EncryptDecryptInterface {
    public static EncryptDecryptInterface getEncrypter();
    public String encrypt(String textToEncrypt);
    public String decrypt(String textToDecrypt);
}

I am not sure if this is something you wouldbe interested in somehow including in the library.

@SyntaxPolice
Copy link
Contributor

@SyntaxPolice SyntaxPolice commented Oct 14, 2019

For this library, we're going to leave it relatively simple w/ backward compatibility. Our product, TozStore has secure key storage and lots more compatibility across platforms. At some point, we may release a new version of the simple AES library that will be restricted to more modern Android platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants