rn-secure-storage icon indicating copy to clipboard operation
rn-secure-storage copied to clipboard

getting error in Google Pre-launch report

Open Tanuja0049 opened this issue 4 years ago • 4 comments

Version of rn-secure-storage 2.0.6

Version of react-native 0.63.2

Platforms you faced the error (IOS or Android or both?) Android

Details:

We are using this library to store user details at app side. The app gets deployed successfully in Google play console with internal/closed track. But we are getting below error in Pre-launch report: Unsafe Cipher Mode Your app contains a less secure encryption mode. com.securepreferences.SecurePreferencesOld.encrypt

Reference link related to issue: https://support.google.com/faqs/answer/10046138

We checked and found that the native code in library is using 'AES/ECB/PKCS5Padding' encryption which seems to cause the issue.

Tanuja0049 avatar Jun 22 '21 13:06 Tanuja0049

Thanks for the issue. I'm aware about these problems. Currently I couldn't spare time to maintain my packages. But I'll handle all of these problems as soon as possible.

talut avatar Jun 22 '21 14:06 talut

@talut any news about this issue ?

mlecoq avatar Nov 14 '21 18:11 mlecoq

I had the same issue and end up fixing the code locally. I tried to create a pull request but I can't push my branch. If anyone is interested here are patches to fix the issue:

  1. migrate to more secure AES
From: rakiop <[email protected]>
Date: Wed, 24 Nov 2021 12:06:29 +0100
Subject: [PATCH] Change android AES to AES/GCM/NoPadding

---
 .../rnsecurestorage/Constants.java            |  3 ++-
 .../rnsecurestorage/RNKeyStore.java           | 27 ++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
index ef46392..319f2ad 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
@@ -6,11 +6,12 @@ public class Constants {
     public static final String KEYSTORE_PROVIDER_3 = "AndroidOpenSSL";
 
     public static final String RSA_ALGORITHM = "RSA/ECB/PKCS1Padding";
-    public static final String AES_ALGORITHM = "AES/ECB/PKCS5Padding";
+    public static final String AES_ALGORITHM = "AES/GCM/NoPadding";
 
     public static final String TAG = "RNSecureStorage";
 
     // Internal storage file
     public static final String SKS_KEY_FILENAME = "SKS_KEY_FILE";
     public static final String SKS_DATA_FILENAME = "SKS_DATA_FILE";
+    public static final String AES_IV_FILENAME = "__iv";
 }
\ No newline at end of file
diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
index 7c49d15..325796b 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
@@ -15,6 +15,7 @@ import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.PrivateKey;
 import java.security.PublicKey;
+import java.security.SecureRandom;
 import java.util.Calendar;
 
 import javax.crypto.Cipher;
@@ -23,10 +24,24 @@ import javax.crypto.CipherOutputStream;
 import javax.crypto.KeyGenerator;
 import javax.crypto.SecretKey;
 import javax.crypto.spec.SecretKeySpec;
+import javax.crypto.spec.IvParameterSpec;
 import javax.security.auth.x500.X500Principal;
 
 public class RNKeyStore {
 
+    private IvParameterSpec getIv(Context context) throws IOException {
+        byte[] iv;
+        if(Storage.exists(context, Constants.AES_IV_FILENAME)){
+            iv = Storage.readValues(context, Constants.AES_IV_FILENAME );
+        }
+        else {
+            iv = new byte[16];
+            new SecureRandom().nextBytes(iv);
+            Storage.writeValues(context, Constants.AES_IV_FILENAME, iv);
+        }
+        return new IvParameterSpec(iv);
+    }
+
     private PublicKey getOrCreatePublicKey(Context context, String alias) throws GeneralSecurityException, IOException {
         KeyStore keyStore = getKeyStore();
         keyStore.load(null);
@@ -61,9 +76,9 @@ public class RNKeyStore {
         return encryptCipherText(cipher, plainTextBytes);
     }
 
-    private byte[] encryptAesPlainText(SecretKey secretKey, String plainText) throws GeneralSecurityException, IOException {
+    private byte[] encryptAesPlainText(SecretKey secretKey, IvParameterSpec iv, String plainText) throws GeneralSecurityException, IOException {
         Cipher cipher = Cipher.getInstance(Constants.AES_ALGORITHM);
-        cipher.init(Cipher.ENCRYPT_MODE, secretKey);
+        cipher.init(Cipher.ENCRYPT_MODE, secretKey, iv);
         return encryptCipherText(cipher, plainText);
     }
 
@@ -100,7 +115,7 @@ public class RNKeyStore {
 
     public void setCipherText(Context context, String alias, String input) throws GeneralSecurityException, IOException {
         Storage.writeValues(context, Constants.SKS_DATA_FILENAME + alias,
-                encryptAesPlainText(getOrCreateSecretKey(context, alias), input));
+                encryptAesPlainText(getOrCreateSecretKey(context, alias), getIv(context), input));
     }
 
     private PrivateKey getPrivateKey(String alias) throws GeneralSecurityException, IOException {
@@ -115,9 +130,9 @@ public class RNKeyStore {
         return decryptCipherText(cipher, cipherTextBytes);
     }
 
-    private byte[] decryptAesCipherText(SecretKey secretKey, byte[] cipherTextBytes) throws GeneralSecurityException, IOException {
+    private byte[] decryptAesCipherText(SecretKey secretKey, IvParameterSpec iv, byte[] cipherTextBytes) throws GeneralSecurityException, IOException {
         Cipher cipher = Cipher.getInstance(Constants.AES_ALGORITHM);
-        cipher.init(Cipher.DECRYPT_MODE, secretKey);
+        cipher.init(Cipher.DECRYPT_MODE, secretKey, iv);
         return decryptCipherText(cipher, cipherTextBytes);
     }
 
@@ -142,7 +157,7 @@ public class RNKeyStore {
     public String getPlainText(Context context, String alias) throws GeneralSecurityException, IOException {
         SecretKey secretKey = getSymmetricKey(context, alias);
         byte[] cipherTextBytes = Storage.readValues(context, Constants.SKS_DATA_FILENAME + alias);
-        return new String(decryptAesCipherText(secretKey, cipherTextBytes), "UTF-8");
+        return new String(decryptAesCipherText(secretKey, getIv(context), cipherTextBytes), "UTF-8");
     }
 
     public boolean exists(Context context, String alias) throws IOException {
-- 
2.30.0.windows.1

  1. Remove secure-preferences library (that also uses less secure AES and fails GoogleConsole check)(technically this requires minSdkVersion to be at least 23 which ends up with always using the KeyStore, but this patch also updates to EncryptedSharedPreferences):
From 7301b43125566b93ca31de386b1cca382c3842a9 Mon Sep 17 00:00:00 2001
From: rakiop <[email protected]>
Date: Thu, 25 Nov 2021 11:38:56 +0100
Subject: [PATCH 2/2] Remove connection to secure-preferences

---
 android/build.gradle                              |  4 ++--
 .../rnsecurestorage/RNSecureStorageModule.java    | 15 +++++++++++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/android/build.gradle b/android/build.gradle
index 3b8f297..a9ca261 100644
--- a/android/build.gradle
+++ b/android/build.gradle
@@ -20,7 +20,7 @@ apply plugin: 'com.android.library'
 android {
     compileSdkVersion 28
     defaultConfig {
-        minSdkVersion 16
+        minSdkVersion 23
         targetSdkVersion 28
         versionCode 4
         versionName "2.0.7"
@@ -37,8 +37,8 @@ repositories {
 }
 
 dependencies {
-    implementation "com.scottyab:secure-preferences-lib:0.1.4"
     implementation "com.facebook.react:react-native:+"
+    implementation "androidx.security:security-crypto:1.0.0"
 }
 task copyDownloadableDepsToLibs(type: Copy) {
     from configurations.compile
diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
index d8d88d1..2f035a2 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
@@ -3,15 +3,18 @@ package com.taluttasgiran.rnsecurestorage;
 import android.content.SharedPreferences;
 import android.os.Build;
 import androidx.annotation.Nullable;
+import androidx.security.crypto.EncryptedSharedPreferences;
+import androidx.security.crypto.MasterKeys;
 import com.facebook.react.bridge.Promise;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactContextBaseJavaModule;
 import com.facebook.react.bridge.ReactMethod;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.uimanager.IllegalViewOperationException;
-import com.securepreferences.SecurePreferences;
 
 import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
 import java.util.ArrayList;
 import java.util.Locale;
 
@@ -31,7 +34,15 @@ public class RNSecureStorageModule extends ReactContextBaseJavaModule {
         if (useKeystore()) {
             rnKeyStore = new RNKeyStore();
         } else {
-            prefs = new SecurePreferences(getReactApplicationContext(), (String) null, "e4b001df9a082298dd090bb7455c45d92fbd5ddd.xml");
+            try {
+                prefs = EncryptedSharedPreferences.create("e4b001df9a082298dd090bb7455c45d92fbd5dda.xml", 
+                    MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC),
+                    getReactApplicationContext(),
+                    EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
+                    EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM);
+            }catch(GeneralSecurityException|IOException e){
+                ;
+            }
         }
     }
 
-- 
2.30.0.windows.1

rakiop avatar Nov 24 '21 11:11 rakiop

@talut Do you have any update on this? Could the scheme be updated to GCM also keeping the migration path in mind?

I believe this issue is quite severe considering that it is a secure storage library, and the chosen scheme is flagged as unsecure (which indeed is an error in every pre-publish report). Thanks!

cdraeger avatar Jan 27 '22 14:01 cdraeger

I had the same issue and end up fixing the code locally. I tried to create a pull request but I can't push my branch. If anyone is interested here are patches to fix the issue:

  1. migrate to more secure AES
From: rakiop <[email protected]>
Date: Wed, 24 Nov 2021 12:06:29 +0100
Subject: [PATCH] Change android AES to AES/GCM/NoPadding

---
 .../rnsecurestorage/Constants.java            |  3 ++-
 .../rnsecurestorage/RNKeyStore.java           | 27 ++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
index ef46392..319f2ad 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/Constants.java
@@ -6,11 +6,12 @@ public class Constants {
     public static final String KEYSTORE_PROVIDER_3 = "AndroidOpenSSL";
 
     public static final String RSA_ALGORITHM = "RSA/ECB/PKCS1Padding";
-    public static final String AES_ALGORITHM = "AES/ECB/PKCS5Padding";
+    public static final String AES_ALGORITHM = "AES/GCM/NoPadding";
 
     public static final String TAG = "RNSecureStorage";
 
     // Internal storage file
     public static final String SKS_KEY_FILENAME = "SKS_KEY_FILE";
     public static final String SKS_DATA_FILENAME = "SKS_DATA_FILE";
+    public static final String AES_IV_FILENAME = "__iv";
 }
\ No newline at end of file
diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
index 7c49d15..325796b 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNKeyStore.java
@@ -15,6 +15,7 @@ import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.PrivateKey;
 import java.security.PublicKey;
+import java.security.SecureRandom;
 import java.util.Calendar;
 
 import javax.crypto.Cipher;
@@ -23,10 +24,24 @@ import javax.crypto.CipherOutputStream;
 import javax.crypto.KeyGenerator;
 import javax.crypto.SecretKey;
 import javax.crypto.spec.SecretKeySpec;
+import javax.crypto.spec.IvParameterSpec;
 import javax.security.auth.x500.X500Principal;
 
 public class RNKeyStore {
 
+    private IvParameterSpec getIv(Context context) throws IOException {
+        byte[] iv;
+        if(Storage.exists(context, Constants.AES_IV_FILENAME)){
+            iv = Storage.readValues(context, Constants.AES_IV_FILENAME );
+        }
+        else {
+            iv = new byte[16];
+            new SecureRandom().nextBytes(iv);
+            Storage.writeValues(context, Constants.AES_IV_FILENAME, iv);
+        }
+        return new IvParameterSpec(iv);
+    }
+
     private PublicKey getOrCreatePublicKey(Context context, String alias) throws GeneralSecurityException, IOException {
         KeyStore keyStore = getKeyStore();
         keyStore.load(null);
@@ -61,9 +76,9 @@ public class RNKeyStore {
         return encryptCipherText(cipher, plainTextBytes);
     }
 
-    private byte[] encryptAesPlainText(SecretKey secretKey, String plainText) throws GeneralSecurityException, IOException {
+    private byte[] encryptAesPlainText(SecretKey secretKey, IvParameterSpec iv, String plainText) throws GeneralSecurityException, IOException {
         Cipher cipher = Cipher.getInstance(Constants.AES_ALGORITHM);
-        cipher.init(Cipher.ENCRYPT_MODE, secretKey);
+        cipher.init(Cipher.ENCRYPT_MODE, secretKey, iv);
         return encryptCipherText(cipher, plainText);
     }
 
@@ -100,7 +115,7 @@ public class RNKeyStore {
 
     public void setCipherText(Context context, String alias, String input) throws GeneralSecurityException, IOException {
         Storage.writeValues(context, Constants.SKS_DATA_FILENAME + alias,
-                encryptAesPlainText(getOrCreateSecretKey(context, alias), input));
+                encryptAesPlainText(getOrCreateSecretKey(context, alias), getIv(context), input));
     }
 
     private PrivateKey getPrivateKey(String alias) throws GeneralSecurityException, IOException {
@@ -115,9 +130,9 @@ public class RNKeyStore {
         return decryptCipherText(cipher, cipherTextBytes);
     }
 
-    private byte[] decryptAesCipherText(SecretKey secretKey, byte[] cipherTextBytes) throws GeneralSecurityException, IOException {
+    private byte[] decryptAesCipherText(SecretKey secretKey, IvParameterSpec iv, byte[] cipherTextBytes) throws GeneralSecurityException, IOException {
         Cipher cipher = Cipher.getInstance(Constants.AES_ALGORITHM);
-        cipher.init(Cipher.DECRYPT_MODE, secretKey);
+        cipher.init(Cipher.DECRYPT_MODE, secretKey, iv);
         return decryptCipherText(cipher, cipherTextBytes);
     }
 
@@ -142,7 +157,7 @@ public class RNKeyStore {
     public String getPlainText(Context context, String alias) throws GeneralSecurityException, IOException {
         SecretKey secretKey = getSymmetricKey(context, alias);
         byte[] cipherTextBytes = Storage.readValues(context, Constants.SKS_DATA_FILENAME + alias);
-        return new String(decryptAesCipherText(secretKey, cipherTextBytes), "UTF-8");
+        return new String(decryptAesCipherText(secretKey, getIv(context), cipherTextBytes), "UTF-8");
     }
 
     public boolean exists(Context context, String alias) throws IOException {
-- 
2.30.0.windows.1
  1. Remove secure-preferences library (that also uses less secure AES and fails GoogleConsole check)(technically this requires minSdkVersion to be at least 23 which ends up with always using the KeyStore, but this patch also updates to EncryptedSharedPreferences):
From 7301b43125566b93ca31de386b1cca382c3842a9 Mon Sep 17 00:00:00 2001
From: rakiop <[email protected]>
Date: Thu, 25 Nov 2021 11:38:56 +0100
Subject: [PATCH 2/2] Remove connection to secure-preferences

---
 android/build.gradle                              |  4 ++--
 .../rnsecurestorage/RNSecureStorageModule.java    | 15 +++++++++++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/android/build.gradle b/android/build.gradle
index 3b8f297..a9ca261 100644
--- a/android/build.gradle
+++ b/android/build.gradle
@@ -20,7 +20,7 @@ apply plugin: 'com.android.library'
 android {
     compileSdkVersion 28
     defaultConfig {
-        minSdkVersion 16
+        minSdkVersion 23
         targetSdkVersion 28
         versionCode 4
         versionName "2.0.7"
@@ -37,8 +37,8 @@ repositories {
 }
 
 dependencies {
-    implementation "com.scottyab:secure-preferences-lib:0.1.4"
     implementation "com.facebook.react:react-native:+"
+    implementation "androidx.security:security-crypto:1.0.0"
 }
 task copyDownloadableDepsToLibs(type: Copy) {
     from configurations.compile
diff --git a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
index d8d88d1..2f035a2 100644
--- a/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
+++ b/android/src/main/java/com/taluttasgiran/rnsecurestorage/RNSecureStorageModule.java
@@ -3,15 +3,18 @@ package com.taluttasgiran.rnsecurestorage;
 import android.content.SharedPreferences;
 import android.os.Build;
 import androidx.annotation.Nullable;
+import androidx.security.crypto.EncryptedSharedPreferences;
+import androidx.security.crypto.MasterKeys;
 import com.facebook.react.bridge.Promise;
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactContextBaseJavaModule;
 import com.facebook.react.bridge.ReactMethod;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.uimanager.IllegalViewOperationException;
-import com.securepreferences.SecurePreferences;
 
 import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.security.GeneralSecurityException;
 import java.util.ArrayList;
 import java.util.Locale;
 
@@ -31,7 +34,15 @@ public class RNSecureStorageModule extends ReactContextBaseJavaModule {
         if (useKeystore()) {
             rnKeyStore = new RNKeyStore();
         } else {
-            prefs = new SecurePreferences(getReactApplicationContext(), (String) null, "e4b001df9a082298dd090bb7455c45d92fbd5ddd.xml");
+            try {
+                prefs = EncryptedSharedPreferences.create("e4b001df9a082298dd090bb7455c45d92fbd5dda.xml", 
+                    MasterKeys.getOrCreate(MasterKeys.AES256_GCM_SPEC),
+                    getReactApplicationContext(),
+                    EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
+                    EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM);
+            }catch(GeneralSecurityException|IOException e){
+                ;
+            }
         }
     }
 
-- 
2.30.0.windows.1

Hey, can you confirm if this method doesn't break the access to already stored data and migration is safe?

I'm getting: "mac check in GCM failed" error after the migration to your code.

paveltar avatar Dec 25 '22 10:12 paveltar

Hi! Is there an update on this?

abstractk avatar Dec 15 '23 02:12 abstractk

This is still an issue, any updates?

McBlaise avatar Dec 15 '23 03:12 McBlaise

Update

I just released v3.0.0 I hope it'll solve your problems.

Please check the new version and Readme file. It also includes a few new features as well.

It's tested with the newest RN versions.

Disclaimer

https://github.com/talut/rn-secure-storage?tab=readme-ov-file#disclaimer

talut avatar Dec 23 '23 00:12 talut