sqlcipher / android-database-sqlcipher

Android SQLite API based on SQLCipher
https://www.zetetic.net/sqlcipher/sqlcipher-for-android/
Other
2.76k stars 568 forks source link

net.sqlcipher.database.SQLiteException: file is not a database: , while compiling: select count(*) from sqlite_master #560

Closed sunilcnair closed 2 years ago

sunilcnair commented 3 years ago

Expected Behavior

The app should open.

Actual Behavior

The app crashes before starting

Steps to Reproduce

Open the app on Android 12 - os S version

SQLCipher version (can be identified by executing PRAGMA cipher_version;):

SQLCipher for Android version: 4.4.2@aar

Are you able to reproduce this issue within the SQLCipher for Android test suite?

We are not able to get that test suite running. Need some support on the running test for library compatibility on Android 12 OS S.

Note: If you are not posting a specific issue for the SQLCipher library, please post your question to the SQLCipher discuss site. Thanks!

E/AndroidRuntime: FATAL EXCEPTION: DefaultDispatcher-worker-3 Process: com.mobile.android.debug, PID: 26682 net.sqlcipher.database.SQLiteException: file is not a database: , while compiling: select count(*) from sqlite_master; at net.sqlcipher.database.SQLiteCompiledSql.native_compile(Native Method) at net.sqlcipher.database.SQLiteCompiledSql.compile(SQLiteCompiledSql.java:89) at net.sqlcipher.database.SQLiteCompiledSql.(SQLiteCompiledSql.java:62) at net.sqlcipher.database.SQLiteProgram.(SQLiteProgram.java:91) at net.sqlcipher.database.SQLiteQuery.(SQLiteQuery.java:48) at net.sqlcipher.database.SQLiteDirectCursorDriver.query(SQLiteDirectCursorDriver.java:60) at net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory(SQLiteDatabase.java:2016) at net.sqlcipher.database.SQLiteDatabase.rawQuery(SQLiteDatabase.java:1902) at net.sqlcipher.database.SQLiteDatabase.keyDatabase(SQLiteDatabase.java:2669) at net.sqlcipher.database.SQLiteDatabase.openDatabaseInternal(SQLiteDatabase.java:2599) at net.sqlcipher.database.SQLiteDatabase.openDatabase(SQLiteDatabase.java:1247) at net.sqlcipher.database.SQLiteDatabase.openOrCreateDatabase(SQLiteDatabase.java:1322) at net.sqlcipher.database.SQLiteOpenHelper.getWritableDatabase(SQLiteOpenHelper.java:166) at net.sqlcipher.database.SupportHelper.getWritableDatabase(SupportHelper.java:83) at androidx.room.RoomDatabase.inTransaction(RoomDatabase.java:476) at androidx.room.RoomDatabase.assertNotSuspendingTransaction(RoomDatabase.java:281) at com.android.repository.login.UserAccountDao_Impl.getUserByStatus(UserAccountDao_Impl.java:)

we are using a keypair generator for android

val keyPairGenerator = KeyPairGenerator.getInstance( TYPE_RSA, PROVIDER_ANDROID_KEYSTORE ) spec = KeyGenParameterSpec.Builder( KEY_ALIAS, KeyProperties.PURPOSE_SIGN or KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT ) .setCertificateSerialNumber(BigInteger.ONE) .setKeyValidityEnd(endDate.time) .setCertificateSubject( X500Principal( context.getString(R.string.certificate).format( KEY_ALIAS ) ) ) .setUserAuthenticationRequired(false) .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA1) .setSignaturePaddings(KeyProperties.SIGNATURE_PADDING_RSA_PKCS1) .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1).build()

We also use the SupportFactory using the SQLiteDatabaseHook as mentioned in your document.

val factory = SupportFactory(SQLiteDatabase.getBytes(key.toCharArray()), object : SQLiteDatabaseHook { override fun preKey(database: SQLiteDatabase) { // Nothing specific needs to be done for before loading database }

            override fun postKey(database: SQLiteDatabase) {
                val cursor = database.rawQuery("PRAGMA cipher_migrate", arrayOf())
                var value: String? = ""
                cursor?.let {
                    it.moveToFirst()
                    value = it.getString(0)
                    it.close()
                }
                Logger.debug("CMS : Database cipher_migrate: $value")
            }
        })

On launch of the app, we have few parallel calls,

We are seeing the crash everytime when we launch the app on either of the 3 scenarios getting this crash.

sjlombardo commented 3 years ago

Hello @sunilcnair - thanks for getting in touch about this. As I'm sure you have seen from multiple other threads, the most common cause of this error is incorrect key material. You have a complicated scenario with key storage, databases, users and threads. Before doing anything else to troubleshoot this, you should rule out that there is a problem with your application. Take your application and modify it so it uses a hard coded key for testing i.e. the same key, statically defined in the code, is used for every key operation. Then run your application and see if it works on a test device under a clean install (i.e. without pre-existing databases. To be clear we are not suggesting you deploy or release an application using a hard-coded key, rather we want you to test this to determine whether there is an issue with your management outside of SQLCipher.

Please do this and report back the results.

sunilcnair commented 3 years ago

@sjlombardo , thanks for your response. I will check that out. Meanwhile , i wanted to clarify that our app always works on clean install ( first time install ) . But in production users are already logged in and persistant.

So each time they launch the app,

we do a fetch from firebase config to get the latest values for the toggles and update it in DB if any new toggle we also check for current user in DB and then take user in the app

so i havent seen issues, when we install app fresh for first time which does the same mechanism as mentioned above. Every subsequent operation gives this error on Android 12.

Hope the library is compatible with Android 12. As the library mentioned support upto Android 10 in the read me notes.

karan4c6 commented 3 years ago

Also worth noting that executing PRAGMA cipher_migrate returns 1, which means migration did not happen successfully. database.rawQuery("PRAGMA cipher_migrate", arrayOf())

We are executing this in postKey() method from SQLiteDatabaseHook callback.

sunilcnair commented 3 years ago

@sjlombardo , based on your suggestion, i added a hardcoded key and app is not crashing any more. i am also adding a few point here from the crash .. where we got database.rawQuery("PRAGMA cipher_migrate", arrayOf()). - > as 1 when there is no crash , it returns 0

So I am wondering even if we use a key, does this have any effect on the migration ?

private fun buildDatabase(context: Context, key: String): MyDatabase { val factory = SupportFactory(SQLiteDatabase.getBytes(key.toCharArray()), object : SQLiteDatabaseHook { override fun preKey(database: SQLiteDatabase) { // Nothing specific needs to be done for before loading database }

            override fun postKey(database: SQLiteDatabase) {
                val cursor = database.rawQuery("PRAGMA cipher_migrate", arrayOf())
                var value: String? = ""
                cursor?.let {
                    it.moveToFirst()
                    value = it.getString(0)
                    it.close()
                }
                Logger.debug("CMS : MyDatabase cipher_migrate: $value").  -> prints 1 during crash else 0 for success
            }
        })
slavkoder commented 3 years ago

Hey sunilcnair@ could you please provide a snippet of how you're creating the key object?

developernotes@ It seems that this issue affects other apps too and I'd like to find out whether there is a specific change in Android 12 that causes this crash to happen. Please note that in Android 12 we updated SQLite version to 3.32.2.

sunilcnair commented 3 years ago

@slavkoder , @developernotes yes i can share the create key snippet

      private fun createKey() {
      val endDate = Calendar.getInstance()
      endDate.add(Calendar.YEAR, 25) // key validity till 25 year
       try {
        // Initialize a KeyPair generator using the the intended algorithm RSAand the KeyStore. here using the AndroidKeyStore.
        val keyPairGenerator = KeyPairGenerator.getInstance("RSA", "AndroidKeyStore")

        // The KeyPairGeneratorSpec object is how parameters for your key pair are passed to the KeyPairGenerator.
        val spec: AlgorithmParameterSpec

        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
            // Below Android M, use the KeyPairGeneratorSpec.Builder.
            spec = KeyPairGeneratorSpec.Builder(context)
                .setAlias("MyKey")
                .setSerialNumber(BigInteger.ONE)
                .setSubject(X500Principal(context.getString(R.string.certificate).format("MyKey")))
                .setStartDate(Calendar.getInstance().time)
                .setEndDate(endDate.time).build()
        } else {
            spec = KeyGenParameterSpec.Builder(
                "MyKey", KeyProperties.PURPOSE_SIGN or KeyProperties.PURPOSE_ENCRYPT or
                    KeyProperties.PURPOSE_DECRYPT
            )
                .setCertificateSerialNumber(BigInteger.ONE)
                .setKeyValidityEnd(endDate.time)
                .setCertificateSubject(
                    X500Principal(
                        context.getString(R.string.certificate).format(
                            "MyKey"
                        )
                    )
                )
                .setUserAuthenticationRequired(false)
                .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA1)
                .setSignaturePaddings(KeyProperties.SIGNATURE_PADDING_RSA_PKCS1)
                .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1).build()
        }
        keyPairGenerator.initialize(spec)
        keyPairGenerator.generateKeyPair()
    } catch (ex: Exception) {
        Logger.error("EncryptionUtil - error in createKey", ex)
    }
}

After this , each time we launch the app, we fetch the key as with below function.

  fun getKeyAsString(arg: String): String {
    val key = getKey(arg)
    return key?.let {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
            (key as PrivateKey).toString()
        } else {
            (key as KeyStore.PrivateKeyEntry).toString()
        }
    } ?: run { "" }
 }

  private fun getKeyStore(): KeyStore {
    return KeyStore.getInstance("AndroidKeyStore").apply { load(null) }
}

open fun getKey(arg: String): Any? {
    val store = getKeyStore()
    var keyEntry: Any? = null

    if (!store.isKeyEntry("MyKey")) createKey()

    if (store.isKeyEntry("MyKey")) {
        keyEntry = try {
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
                getKeyStore().getKey("MyKey", null)
            } else {
                getKeyStore().getEntry("MyKey", null)
            }
        } catch (ex: Exception) {
            Logger.error("EncryptionUtil - error in getKey - ", ex)
            null
        }
    }
    return keyEntry
}

I feel this is where we may need to dig deep.

nicbell commented 3 years ago

@slavkoder we're also getting a higher crash rate on Android 12. There seems to be a massive increase in SQLiteDiskIOException errors. We're going to improve our logging and try provide more information.

sunilcnair commented 3 years ago

@developernotes this is message we got from google

Possible root cause: net.sqlcipher.database seems to rely on an unsupported and undocumented feature of sqlite3: SQLITE_HAS_CODEC. SQLite (upstream) has removed this feature. Apparently, we updated the SQLite library to 3.32.2 in Android S which no longer has this feature

sjlombardo commented 3 years ago

@sunilcnair SQLite and SQLITE_HAS_CODEC are unrelated here and not the root cause. SQLCipher is completely self-contained fork of SQLite which does include support for all the codec features. Provided that your application is integrating SQLCipher properly it will not be using the version of SQLite included with Android at all.

From your earlier comment the hardcoded key is working with a statically defined key. This makes it extremely likely that the root cause of the problem in how the key material is retrieved. I think a reasonable debugging step would be to under debug, capture the key material from the createKey procedures and the getKey / getKeyAsString to make sure that is operating as expected, and returning consistent values under all scenarios (initial creation, first run, subsequent invocations, etc).

With respect to cipher_migrate, that function is intended to migrate a database created with an older version of SQLCipher to the latest format. If the key material is incorrect it won't be able to do that and will return the error code. However, it's worth noting that PRAGMA only needs to be run one time on a database. It is not recommended to run it every time a database is opened. Here is some information about that funciton:

https://www.zetetic.net/sqlcipher/sqlcipher-api/#cipher_migrate

werwurm commented 3 years ago

Hi @sunilcnair,

Thank you for sharing this code snippet. If I understand correctly, you are essentially using getKeyAsString("ignored").toCharArray() as the passphrase for sqlcipher.

I think I can shed some light on what is going on here. (Also, if you happen to have found this pattern in another public forum, please share the link with me. Thank you.)

Note that AndroidKeyStore never exposes any private key material to the apps, not even the OS can see this material. The PrivateKey object is merely a reference to the actual key material that is stored in secure hardware. So (key as PrivateKey).toString() is not actually related to the RSA private key that your code generates earlier. In fact what your code uses as passphrase is "android.security.keystore.AndroidKeyStoreRsaPrivateKey@<obj hash>" where the object hash depends entirely on the alias (here "MyKey") and the algorithm (here "RSA"). So for a given alias you can trivially compute the passphrase used.

Here is the algorithm that would generate the exact passphrase. (This code snippet is a slightly modified version of android.security.keystore.AndroidKeyStoreKey#hashCode() from the Android 11 code base.)

    String passphrase = keyString("MyKey", "RSA");

    private String keyString(String alias, String algorithm) {
        alias = "USRPKEY_" + alias;
        final int uid = -1;
        final int prime = 31;
        int hash = 1;
        hash = prime * hash + ((algorithm == null) ? 0 : algorithm.hashCode());
        hash = prime * hash + ((alias == null) ? 0 : alias.hashCode());
        hash = prime * hash + uid;

        return "android.security.keystore.AndroidKeyStoreRsaPrivateKey@" + Integer.toHexString(hash);
    }

Now in Android 12 the package name has changed to android.security.keystore2.AndroidKeyStoreRsaPrivateKey and the hash code depends on some data that changes every time the key is loaded from Keystore. So it is no longer stable.

If you really want to use a passphrase that is cryptographically bound to the hardware Keystore, you can do the following:

  1. Generate an HMAC key in AndroidKeyStore.
  2. Chose some context string, e.g., "MyDatabaseKey". This string can be hard coded. It is not sensitive.
  3. Sign the context string with the HMAC key and use the signature as the passphrase.

This method is guaranteed to be stable and yields a high-entropy passphrase.

However, this does not really protect your data any better. The database file is already stored on an encrypted file system that is bound to the hardware and the user's lock screen credential. And the same mechanism that protects your file from access by a third party also makes sure that the Keystore key can only be used by your app.

You can add value, though, if you make your HMAC key authentication bound. In that case, it can only be used when the user authorizes the operation, e.g., by presenting a fingerprint. Or if you mix a secret into the context, that is not always on the device and must be obtained, e.g., from your server ...

Regarding the update to SQLite 3.32.2 that @slavkoder mentioned: This version of SQLite removed the crypto features (sqlite3_key/sqlite3_rekey...) that sqlcipher exposes through jni. So if you dynamically link against libsqlite3 on Android 12 you will get UnsatisfiedLinkExceptions.

developernotes commented 3 years ago

Hi @werwurm

Thank you for your reply and interest in SQLCipher, I think that will be very helpful for @sunilcnair. I want to clarify that SQLCipher for Android dynamically links against libsqlcipher here as a standalone dependency. While SQLCipher is implemented as a codec extension to SQLite, the library itself is a superset of SQLite and thus has no dependency on a system provided version of libsqlite3.

werwurm commented 3 years ago

Hi @developernotes,

Thank you for pointing that out. This was very helpful.

sunilcnair commented 3 years ago

Hi @werwurm , @slavkoder , @developernotes

Is this the way you want me to change it ?

   import android.annotation.SuppressLint
  import android.security.keystore.KeyGenParameterSpec
 import android.security.keystore.KeyProperties
import java.lang.Exception
import java.security.KeyStore
import java.security.KeyStoreException
import java.security.SecureRandom
import javax.crypto.Cipher
import javax.crypto.KeyGenerator
import javax.crypto.SecretKey
import javax.crypto.spec.GCMParameterSpec

 @SuppressLint("NewApi")
 @Singleton
 open class EncryptionUtil @Inject constructor(val context: Context) { {

private val CIPHER_TYPE = "AES/GCM/NoPadding"

private val GCM_TAG_LENGTH = 128

private var keyStore: KeyStore? = null
var iv: ByteArray? = null
    private set

init {
    try {
        keyStore = KeyStore.getInstance("AndroidKeyStore")
        keyStore!!.load(null)
    } catch (e: KeyStoreException) {
        if (BuildConfig.DEBUG) {
            e.printStackTrace()
        }
    }

}

fun generateKey(alias: String) {
    try {
        if (!keyStore!!.containsAlias(alias)) {
            val keyGenerator = KeyGenerator.getInstance(
                    KeyProperties.KEY_ALGORITHM_AES, "AndroidKeyStore")

            keyGenerator.init(
                    KeyGenParameterSpec.Builder(alias,
                            KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT)
                            .setBlockModes(KeyProperties.BLOCK_MODE_GCM)
                            .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)
                            .setKeySize(256)
                            .setUserAuthenticationRequired(false)
                            .build())
            keyGenerator.generateKey()
        }
    } catch (e: Exception) {
        if (BuildConfig.DEBUG) {
            e.printStackTrace()
        }
    }

}

fun encrypt(alias: String, plainText: ByteArray): ByteArray? {
    try {
        val key = keyStore!!.getKey(alias, null) as SecretKey
        val cipher = Cipher.getInstance(CIPHER_TYPE)
        cipher.init(Cipher.ENCRYPT_MODE, key)
        iv = cipher.iv
        return cipher.doFinal(plainText)
    } catch (e: Exception) {
        if (BuildConfig.DEBUG) {
            e.printStackTrace()
        }
    }

    return null
}

fun decrypt(alias: String, cipherText: ByteArray, IV: ByteArray): ByteArray? {
    try {
        val key = keyStore!!.getKey(alias, null) as SecretKey
        val cipher = Cipher.getInstance(CIPHER_TYPE)
        val gcmSpec = GCMParameterSpec(GCM_TAG_LENGTH, IV)
        cipher.init(Cipher.DECRYPT_MODE, key, gcmSpec)
        return cipher.doFinal(cipherText)
    } catch (e: Exception) {
        if (BuildConfig.DEBUG) {
            e.printStackTrace()
        }
    }

    return null
}

}

In this case, how can i use getKeyAsString function ( which i was using till now in my AppModule) @Provides @Singleton fun providesDatabase(context: Context): BaseDatabase { return MyDatabase.getInstance(context, EncryptionUtil(context).getKeyAsString()) } This change to use a new key generation will impact the customers out there using the app , as they would have already generated the key with the old mechanism.

sunilcnair commented 3 years ago

@developernotes , @werwurm , @slavkoder , @sjlombardo guys need your expert advice on few points.

Point 1: As suggested by all, we may need to go with new key generation mechanism. In that case how will we able to use our existing DB which is currently encrypted with the existing old key.

is there any way to handle this scenario. like a DB which is encrypted with a key and now because of this issue, we need to use a new key.

Kindly support to understand the best possible way from App side

Point 2 We may not be able to use AES mechanism here , as it supports only from Android 6, Hence we may have to use HMAC256 which also shows added in Api 23 . Is that the way forward. Our app supports from Api level 22.

werwurm commented 3 years ago

Hi @sunilcnair,

Before I address your questions some free code review.

  1. I would suggest you return the IV from encrypt() along with the ciphertext instead of storing it in the EncryptionUtil object. Otherwise you open your implementation up to data races.
  2. The generate key function checks if the key exists and creates it if it does not. Unfortunately, there is no atomic way to in the Java Crypto API to get-or-generate a key. If there is any chance that your app calls this function concurrently with the same alias, even when called on different instances of EncryptionUtil, you must use some form of synchronization.

Regarding keyAsString: The SQLCipher API requires a byte array for a pass phrase. So give it a byte array. The problem with turning it into a String as an intermediate representation is that (depending on the method of interpretation) not all bytes may be valid code points, so you'll loose entropy. Don't do it.

Regarding Point 1: You can use the function I posted above to recover the original passphrase, which, if your key alias is constant, is a constant and the same on all devices. So you may just hard code it as a fallback. Next, you have to determine if the DB was encrypted with the old or the new method. You could store this information out-of-band. Since you chose to use AES-GCM instead of HMAC, you actually have to store the encrypted pass phrase. The presence of the stored artifact tells you that the new method was used. Otherwise, I would defer to the maintainers of SQLcipher on how to validate if the passphrase was correct.

Regarding Point 2: Although it is a little cringe worthy, I guess you could use an RSA signing key similar to the HMAC method I described above. Sign your non sensitive context with the key using no padding, and use the signature as passphrase. Better ask your friendly neighborhood crypto expert if this a viable solution. However, RSA has been supported by AndroidKeyStore since API level 18.

sunilcnair commented 3 years ago

@werwurm for point 2, Since our app supports from Api level 22, using AES / HMAC looks only feasible from Api 23 onwards. In that case we may have to resort to our existing RSA implementation. So in that case, we need to modify our KeyAsString function to return bytearray instead of toString. I feel we also need to move outof this old implementation and go with new way as you mentioned. Let me also try HMAC and add the snippet here.

For point 1, thats where the complexity lies. Our app is already out in prod with old key. Now When we go with new key, we need to find a way : if we can keep the same DB and pass the new key and get it working. else we may need to find a way to use both keys and open the DB if nothing works, we may need to create new DB and migrate the data from old to new. @developernotes @sjlombardo guys can you help understand if the SQLcipher can validate 2 keys to update to the latest key

werwurm commented 3 years ago

Hi @sunilcnair,

I think you missed an important point of my original post. You should be clear about what your attack model is.

If you store your database in getFilesDir(), the data is stored on an encrypted file system (since Android 4.4 API 19). And the data is protected from access by a third party through file system access control and SELinux policy. So to read that data, an attacker would have to circumvent these access control mechanisms. Now, access to the Keystore key is protected by the same mechanism. I say "access is protected", because they would not be able to ex-filtrate the key material, but they would be able to use the key if they compromised your app's protection domain. So adding another layer of encryption does not raise the bar for an attacker even if the key is stored in AndroidKeyStore. That is, unless you use features like authentication binding, which adds another aspect into the mix making the data inaccessible unless the user authenticates, and thereby, authorizes the key usage. But this has not been available until API 23.

I don't mean to say that that AndroidKeyStore or SQLCipher are useless - they are not. But I don't think they add any value in the way your app is using them.

I would focus on making sure that you don't have any regressions when upgrading to Android 12 by hard coding the passphrase that was used before. As long as the database is stored at a location where your app has exclusive access, e.g., getFilesDir(), your data is well protected, and you are not playing fast-and-loose with your users' data. You'll be fine. [1]

Then you take a deep breath and think carefully about what your security model is, and if and how you want to improve it.

I know it may be hard to wrap your head around at first, so let me say it again: Regardless of whether you store your database in plain text, encrypted with a hard coded passphrase (as is currently the case), or with a high entropy passphrase bound to a hardware protected key in AndroidKeyStore, the only thing that stands between an attacker and your data are the Linux access control mechanisms.

[1] If you store the database at a shared location, encrypting it with a Keystore key extends the umbrella of exclusive access to the database. In that case, you should really get this fixed. ... The problems with making conjectures about what the security model is :-) .

sjlombardo commented 3 years ago

@sunilcnair you can't simultaneously open a database with two keys. However, you can open it with the original key, then use changePassword() to re-encrypt the database using a new key. That will likely work in your scenario, where you can be certain of the previous key value and then generate a new key for use going forward to support a migration.

sunilcnair commented 3 years ago

@sjlombardo , do you have a snippet of this change password function ? or is it that we pass a db.rawExecSQL(PRAGMA_REKEY); correct me if i am wrong in my understanding.

This would be the best possible outcome, if i can pass a new key to re-ecrnypt the DB. Kindly support me to understand better how to achieve this.

should i add it in the support factory with the SQLDatabasehook?

      private fun buildDatabase(context: Context, key: String): MyDatabase {
        val factory = SupportFactory(SQLiteDatabase.getBytes(key.toCharArray()), object : SQLiteDatabaseHook {
            override fun preKey(database: SQLiteDatabase) {
                // Nothing specific needs to be done for before loading database
            }

            override fun postKey(database: SQLiteDatabase) {
                database.changePassword("My123")
                val cursor = database.rawQuery("PRAGMA cipher_migrate", arrayOf())
                var value: String? = ""
                cursor?.let {
                    it.moveToFirst()
                    value = it.getString(0)
                    it.close()
                }
                Logger.debug("CMS : MyDatabase cipher_migrate: $value")
            }
        })

This one didnt work. where to add this particular changePassword call? is it inside the Room builder?

sunilcnair commented 3 years ago

@werwurm Hi good morning. Really appreciate for the explanation given above. Real gold and worth reading :) As everything we have started re doing , few bits to clarify .

As per your recommendation a good fix is :

    1. Generate an HMAC key in Keystore.
    1. Choose a fixed context string e.g. “MySuperDuperSqlCipherKey”. This can be hardcoded. It is not sensitive.
    1. Sign the context string with the HMAC key and use the signature as a passphrase for sqlcipher.

Step 1 - as our app support from API level 22, we are still inclined to reusing the RSA mechanism. HMAC we wish to go, but support from 23 makes it tough for us as we still have users out there on 22. So the createKey() function remains same.

  private fun createKey() {
  val endDate = Calendar.getInstance()
  endDate.add(Calendar.YEAR, 25) // key validity till 25 year
   try {
    // Initialize a KeyPair generator using the the intended algorithm RSAand the KeyStore. here using the AndroidKeyStore.
    val keyPairGenerator = KeyPairGenerator.getInstance("RSA", "AndroidKeyStore")

    // The KeyPairGeneratorSpec object is how parameters for your key pair are passed to the KeyPairGenerator.
    val spec: AlgorithmParameterSpec

    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
        // Below Android M, use the KeyPairGeneratorSpec.Builder.
        spec = KeyPairGeneratorSpec.Builder(context)
            .setAlias("MyKey")
            .setSerialNumber(BigInteger.ONE)
            .setSubject(X500Principal(context.getString(R.string.certificate).format("MyKeyAlias")))
            .setStartDate(Calendar.getInstance().time)
            .setEndDate(endDate.time).build()
    } else {
        spec = KeyGenParameterSpec.Builder(
            "MyKeyAlias", KeyProperties.PURPOSE_SIGN or KeyProperties.PURPOSE_ENCRYPT or
                KeyProperties.PURPOSE_DECRYPT
        )
            .setCertificateSerialNumber(BigInteger.ONE)
            .setKeyValidityEnd(endDate.time)
            .setCertificateSubject(
                X500Principal(
                    context.getString(R.string.certificate).format(
                        "MyKeyAlias"
                    )
                )
            )
            .setUserAuthenticationRequired(false)
            .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA1)
            .setSignaturePaddings(KeyProperties.SIGNATURE_PADDING_RSA_PKCS1)
            .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1).build()
      }
      keyPairGenerator.initialize(spec)
      keyPairGenerator.generateKeyPair()
      } catch (ex: Exception) {
         Logger.error("EncryptionUtil - error in createKey", ex)
   }
 }

GetKey function remains same.

open fun getKey(arg: String): Any? { val store = getKeyStore() var keyEntry: Any? = null if (!store.isKeyEntry("MyKey")) createKey()

  if (store.isKeyEntry("MyKey")) {
    keyEntry = try {
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
            getKeyStore().getKey("MyKey", null)
        } else {
            getKeyStore().getEntry("MyKey", null)
        }
    } catch (ex: Exception) {
        Logger.error("EncryptionUtil - error in getKey - ", ex)
        null
    }
   }
     return keyEntry
  }

As the issue was with out getKeyAsString() function, so we decided to use your Step 2 and 3. For Step 2 and 3, we made this change to the getKeyAsString()

  fun getKeyAsString(): String {
    return encrypt("MyDatabaseKey")  ( as per step 2, we pass a hardcoded constant string)
   } 

Step 3 - we use the encrypt() function as is

  / * Encrypts data using the key*/
   open fun encrypt(data: String): String {
     val cipIn = Cipher.getInstance("RSA/ECB/PKCS1Padding")
     cipIn?.let {
        return try {
            val pubKey: RSAPublicKey = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
                getKeyStore().getCertificate("MyKeyAlias).publicKey as RSAPublicKey
            } else {
                (getKey() as KeyStore.PrivateKeyEntry).certificate?.publicKey as RSAPublicKey
            }
            it.init(Cipher.ENCRYPT_MODE, pubKey)
            val encryptBytes = it.doFinal(data.toByteArray())
            Base64.encodeToString(encryptBytes, Base64.NO_WRAP)
        } catch (ex: Exception) {
            CrashlyticsTrackingAgent.recordException(ex)
            Logger.error("error in encrypt", ex)
            data
        }
      } ?: run { return data }
   }

After following a new key is generated succesfully . But what we see is, each time the encryption value is different. Still something is wrong :(

   @Provides
   @Singleton
   fun providesDatabase(context: Context): BaseDatabase {
    return MyDatabase.getInstance(context, EncryptionUtil(context).getKeyAsString())
   }

Kindly help to understand where we are going wrong again on Step 3 - Sign the context string with the HMAC/RSA key and use the signature as a passphrase for sqlcipher.

werwurm commented 3 years ago

Hi @sunilcnair ,

I appreciate your appreciation :-).

  1. RSA encryption is a public key operation. So your passphrase is still not bound to the private key that is actually hardware protected but bound to the public key that is not.
  2. You are using PKCS1 padding which adds randomness. So the operation is not idempotent and yields a different result every time.

I specifically wrote: "Sign your non sensitive context with the key using no padding". (scroll up - this is what I wrote - well, without the emphasis.) This addresses both issues.

  1. RSA signing is a private key operation, so you can recover the signature, which you use as passphrase, only if you have access to the private key material which is hardware protected.
  2. Using no padding makes the operation idempotent. Normally, this is not desirable if you actually use the signature as such. But here you want to get the same passphrase every time you sign the context string.
stale[bot] commented 3 years ago

Hello, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "bug", "enhancement", or "security" and I will leave it open. Thank you for your contributions.

sumit269 commented 2 years ago

Is this fixed? I am getting this in my app too. https://stackoverflow.com/q/70077997/610837

developernotes commented 2 years ago

Hi @sumit269,

Are you able to reproduce the error consistently outside of your application (i.e., creating a separate standalone reproduction case as a separate application)?

stale[bot] commented 2 years ago

Hello, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "bug", "enhancement", or "security" and I will leave it open. Thank you for your contributions.

stale[bot] commented 2 years ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to reopen with up-to-date information.

AChep commented 2 years ago

@developernotes I have a code that consistently triggers this issue after changePassword(...) call. It's not open source, but I can share it directly with the team

sjlombardo commented 2 years ago

@AChep - you can reach out to support@zetetic.net with your reproduction case.

AChep commented 2 years ago

@sjlombardo Sorry folks, in the end, it turned out that I used a not intended password to rekey the database :)

Apparently

SQLiteDatabase.getBytes(SQLiteDatabase.getChars(data)) != data

which was a little bit confusing

Natzee commented 1 year ago

Hi team Can I know how to proceed if test suite fails in Encrypt Bytes Test

developernotes commented 1 year ago

Hi @Natzee,

The Encrypted Bytes Test is only applicable with our Commercial/Enterprise as it interfaces with the SQLCipher VLE. The VLE is not available on the Community edition builds. If you are using the Community edition library you may disregard that test.

Natzee commented 1 year ago

Hi @developernotes then in that case, shouldn't we use community version in our production, because same above crash is occurring. Also we are not using value level encryption as you mentioned

sjlombardo commented 1 year ago

@Natzee If you are seeing that crash in an application, the most common cause is incorrect key material. The error in the test is unrelated, though they have the same error message.

diegodantas commented 1 year ago

First of all, I apologize for reopening the post, but I'm encountering the same error.

Here are the configurations:

implementation "net.zetetic:android-database-sqlcipher:4.5.2"
implementation "androidx.sqlite:sqlite:2.2.0"
companion object {
    lateinit var instance: AppDatabase
    fun createInstance(context: Context) {
        val saved = AppDatabaseShared(context).getKey().toCharArray()
        val factory = SupportFactory(SQLiteDatabase.getBytes(saved))
        instance = Room.databaseBuilder(
            context,
            AppDatabase::class.java,
            "database-name"
        ).openHelperFactory(factory).build()
    }
}

My question is whether there's something wrong with my implementation?