5

I'm encrypting the password for firebase sign in, it's working well but I received a warning in google play console that your app contains unsafe cryptographic encryption patterns how can I get rid of it ??

I'm trying it on android studio.

public static class AESCrypt
{
    private static final String ALGORITHM = "AES";
    private static final String KEY = "1Hbfh667adfDEJ78";

    public static String encrypt(String value) throws Exception
    {
        Key key = generateKey();
        Cipher cipher = Cipher.getInstance(AESCrypt.ALGORITHM);
        cipher.init(Cipher.ENCRYPT_MODE, key);
        byte [] encryptedByteValue = cipher.doFinal(value.getBytes("utf-8"));
        String encryptedValue64 = Base64.encodeToString(encryptedByteValue, Base64.DEFAULT);
        return encryptedValue64;

    }

    public static String decrypt(String value) throws Exception
    {
        Key key = generateKey();
        Cipher cipher = Cipher.getInstance(AESCrypt.ALGORITHM);
        cipher.init(Cipher.DECRYPT_MODE, key);
        byte[] decryptedValue64 = Base64.decode(value, Base64.DEFAULT);
        byte [] decryptedByteValue = cipher.doFinal(decryptedValue64);
        String decryptedValue = new String(decryptedByteValue,"utf-8");
        return decryptedValue;

    }

    private static Key generateKey() throws Exception
    {
        Key key = new SecretKeySpec(AESCrypt.KEY.getBytes(),AESCrypt.ALGORITHM);
        return key;
    }
Abdul Hanan
  • 93
  • 1
  • 7

1 Answers1

4

The main issues are that you use a cipher with no integrity and a hard coded cryptographic key. If you analyse your source with Find Security Bugs you get CIPHER_INTEGRITY and HARD_CODE_KEY warning:

The cipher does not provide data integrity [com.lloyds.keystorage.AESCrypt] At AESCrypt.java:[line 25] CIPHER_INTEGRITY
The cipher does not provide data integrity [com.lloyds.keystorage.AESCrypt] At AESCrypt.java:[line 15] CIPHER_INTEGRITY
Hard coded cryptographic key found [com.lloyds.keystorage.AESCrypt] At AESCrypt.java:[line 35] HARD_CODE_KEY

The solution is to use a cipher that includes a Hash based Message Authentication Code (HMAC) to sign the data:

Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");

And to store the secret key in separate configuration files or keystores.

Below is the whole class after a full refactoring:

import android.util.Base64
import static java.nio.charset.StandardCharsets.UTF_8;
import java.security.Key;
import javax.crypto.Cipher;
import javax.crypto.spec.SecretKeySpec;

public class AESCrypt {
  private static final String TRANSFORMATION = "AES/GCM/NoPadding";

  public static String encrypt(String value) throws Exception {
    Key key = generateKey();
    Cipher cipher = Cipher.getInstance(TRANSFORMATION);
    cipher.init(Cipher.ENCRYPT_MODE, key);
    byte[] encryptedByteValue = cipher.doFinal(value.getBytes(UTF_8));
    return Base64.encodeToString(encryptedByteValue, Base64.DEFAULT);
  }

  public static String decrypt(String value) throws Exception {
    Key key = generateKey();
    Cipher cipher = Cipher.getInstance(TRANSFORMATION);
    cipher.init(Cipher.DECRYPT_MODE, key);
    byte[] decryptedValue64 = Base64.decode(value, Base64.DEFAULT);
    byte[] decryptedByteValue = cipher.doFinal(decryptedValue64);
    return new String(decryptedByteValue, UTF_8);
  }

  private static Key generateKey() {
    return new SecretKeySpec(Configuration.getKey().getBytes(UTF_8), TRANSFORMATION);
  }
}
Boris
  • 22,667
  • 16
  • 50
  • 71
  • But, `java.util.Base64` requires API level 26. What's the reason of using `java.util.Base64` or `android.util.Base64`? – Cheok Yan Cheng Sep 22 '19 at 22:32
  • 1
    Also, what is the implementation of `Configuration.getKey()`? If it is always returning the same value for different devices, will Google flag warning again? – Cheok Yan Cheng Sep 22 '19 at 22:37
  • @CheokYanCheng, can you share a link to the resource showing that you need the level 26 for a Java 8 class `java.util.Base64`? – Boris Sep 23 '19 at 10:51
  • 1
    See https://developer.android.com/reference/java/util/Base64.Encoder (Added in API level 26) – Cheok Yan Cheng Sep 23 '19 at 10:58
  • @CheokYanCheng, I believe, as long as cryptographic keys are not kept in the source code, there shouldn't be any warnings. The implementation would be to read the value from a config file. AbdulHanan can you test? – Boris Sep 23 '19 at 11:17
  • @Boris, firstly who you are calling config file?? can i put the key in string.xml also ? Secondly, I tried but with this 'AES/GCM/NoPadding' every time password is changed and once the user logout from firbase then can't login again because password is modified ... i'm new at this, feeling so frustrated don't know what to do . Plz help – Abdul Hanan Sep 24 '19 at 11:25
  • I'm not calling anyone a config file. You should see [HARD_CODE_KEY](https://find-sec-bugs.github.io/bugs.htm#HARD_CODE_KEY) bug pattern for the details. And I'm not sure `AES/GCM/NoPadding` transformation is responsible for the password change. Can you provide any proof? – Boris Sep 24 '19 at 13:02
  • 1
    @Boris i remove hard code key now the warning gone thanks. :-) – Abdul Hanan Oct 10 '19 at 06:24
  • What if you need to decrypt some data that was previously encrypted with a former key? – Basit Ali Jan 20 '22 at 10:45
  • @BasitAli that's a valid sceanario. I'd recommend to create a class for legacy data and configure `SpotBugsExcludeFilter.xml`: ` ` – Boris Jan 20 '22 at 14:37