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

Force quiting our app throws file is not a database error at startup of the app #1810

Closed
vanlooverenkoen opened this issue Apr 19, 2022 · 56 comments

Comments

@vanlooverenkoen
Copy link

We are using an encrypted database but I found that when using a long encryption key it is possible to find other combinations that will decrypt the database.

Db key = 3d80ac84-6dff-40bb-b803-abeed6c5e297

3d80ac84-6dff-40bb-b803-abeed6c5e297 + test -> ✅
test + 3d80ac84-6dff-40bb-b803-abeed6c5e297 + test -> ❌
test + 3d80ac84-6dff-40bb-b803-abeed6c5e297 -> ❌
3d80ac84-6dff-4 -> ❌
3d80ac84-6dff-40 -> ✅

It seems that the max lenght for the encryption pragma key is 16. Other chars will be ignored. If this is intended behaviour this should be documented somewhere

@simolus3
Copy link
Owner

simolus3 commented Apr 19, 2022

This sounds worrying - SQLCipher uses PBKDF2 to derive the key from a string, so it shouldn't consider a subset of the key only. This is definitely not the intended behavior.

On what platforms are you running into this? I can't reproduce this on Android with this driver test:

  test('key test', () async {
    final dir = await getApplicationDocumentsDirectory();
    dir.createSync();
    final file = File('${dir.path}/test.db');
    if (file.existsSync()) file.deleteSync();

    var db = sqlite3.open('${dir.path}/test.db');

    void reOpen() {
      db.dispose();
      db = sqlite3.open('${dir.path}/test.db');
    }

    db.execute("PRAGMA key = '3d80ac84-6dff-40bb-b803-abeed6c5e297'");
    db.execute('create table foo (bar);');
    reOpen();

    db.execute("PRAGMA key = '3d80ac84-6dff-40bb-b803-abeed6c5e297test'");
    expect(
        () => db.execute('select * from foo;'),
        throwsA(isA<SqliteException>()
            .having((e) => e.message, 'message', 'file is not a database')));
  });

I tried running this and it gave me the expected exception.

@vanlooverenkoen
Copy link
Author

I have been testing on iOS today. Tomorrow I will do android. We do have a could apps that use this implementation with 8 million users. But they are all still using moor. Tomorrow I will do some extra checks & check out the moor implementation of our other projects. Maybe I did forget something when migrating from moor to drift

@simolus3
Copy link
Owner

Same test works for me in the iOS simulator, I don't have an actual device to test on unfortunately. Let me know if there's more I can try out, so far I've only tested the underlying sqlite3 package but I don't think moor/drift would cause this as custom statements are directly passed down to that package.

@North101
Copy link
Contributor

Same test works for me in the iOS simulator, I don't have an actual device to test on unfortunately. Let me know if there's more I can try out, so far I've only tested the underlying sqlite3 package but I don't think moor/drift would cause this as custom statements are directly passed down to that package.

Could it be a difference between sqflite vs flutter_sql_libs?

@davidmartos96
Copy link
Contributor

The iOS sqflite_sqlcipher implementation uses the FMDB library.

@simolus3
Copy link
Owner

@vanlooverenkoen Are you using encrypted_moor or the VmDatabase/NativeDatabase backend with sqlcipher_flutter_libs? I can test the other one too (but it would surprise me if they somehow cut the key pragma).

@vanlooverenkoen
Copy link
Author

We are using NativeDatabase

@vanlooverenkoen
Copy link
Author

We are also using isolates, maybe that has something to do with it?

@vanlooverenkoen
Copy link
Author

Just tested on our application with 8 million users. (using moor). It does throw an error file not a database when adding test at the end.

I am now trying to figure out what the difference is between 2 codebases.

@vanlooverenkoen
Copy link
Author

We are experiencing a lot of strange issues with our current implementation. sometimes we get

SqliteException(26): file is not a database, file is not a database (code 26)
Causing statement: PRAGMA user_version;

But if we wait some time or restart of do some other work on your phone. and open the app again everything is file.
The database key does not change. (So I will be doing more investigation why this is happening)

@vanlooverenkoen
Copy link
Author

The issue is happening on android & iOS

@vanlooverenkoen
Copy link
Author

vanlooverenkoen commented Apr 21, 2022

  final executor = NativeDatabase(
    File(request.targetPath),
    logStatements: true,
    setup: (db) async {
      final cipherVersion = db.select('PRAGMA cipher_version;');
      if (cipherVersion.isEmpty) {
        print('No cipher version');
      } else {
        print(cipherVersion.first);
      }
      db.execute("PRAGMA key = '${request.password}';");
    },
  );

@simolus3 is it normal that I get no cipher version here?

Also after a restart. (file is not removed)

@vanlooverenkoen
Copy link
Author

I also can't open my sqlite db in table plus. A password is requested. but after entering the password I can't do anything.
No select
Don't see any tables

@simolus3
Copy link
Owner

We are also using isolates, maybe that has something to do with it?

Isolates shouldn't have an impact on this.

is it normal that I get no cipher version here?

Absolutely not! I means that the database is running against the regular sqlite3 implementation instead of SQLCipher. That would also explain why the key pragma isn't doing anything. If you're getting a "file is not a database" error sometimes, it sounds like you're not always getting the SQLCipher implementation at runtime.

Just to be sure, you're using open.overrideFor(OperatingSystem.android, openCipherOnAndroid) on Android, right? And you're also doing this on all isolates where you're opening a database connection?

On iOS, you might have another pod linking the regular sqlite3 library. Linking both SQLCipher and sqlite3 into the same executable leads to undefined behavior (which could include a broken mixture of both libraries). Could you check something like this? (unfortunately I don't have too much native iOS development experience).

@vanlooverenkoen
Copy link
Author

This is our current implementation

import 'dart:ffi';
import 'dart:io';
import 'dart:isolate';

import 'package:drift/drift.dart';
import 'package:drift/isolate.dart';
import 'package:drift/native.dart';
import 'package:firebase_crashlytics/firebase_crashlytics.dart';
import 'package:my_app/repository/secure_storage/auth/auth_storage.dart';
import 'package:my_app/util/env/flavor_config.dart';
import 'package:icapps_architecture/icapps_architecture.dart';
import 'package:path/path.dart';
import 'package:path_provider/path_provider.dart';
import 'package:sqlcipher_flutter_libs/sqlcipher_flutter_libs.dart';
import 'package:sqlite3/open.dart';

Future<DatabaseConnection> createDriftDatabaseConnection(AuthStorage authStorage, String name) async {
  if (FlavorConfig.isInTest()) {
    return DatabaseConnection.fromExecutor(NativeDatabase.memory());
  }
  final dbFolder = await getApplicationDocumentsDirectory();
  final dbName = '$name.sqlite';
  final file = File(join(dbFolder.path, dbName));
  final dbKey = await authStorage.databaseKey;
  staticLogger.debug('DB Key: $dbKey');
  await FirebaseCrashlytics.instance.log('DB Key: $dbKey');
  final receivePort = ReceivePort();
  await applyWorkaroundToOpenSqlCipherOnOldAndroidVersions();
  await Isolate.spawn(
    _startBackground,
    _IsolateStartRequest(receivePort.sendPort, file.path, dbKey),
  );

  final isolate = await receivePort.first as DriftIsolate;
  return isolate.connect();
}

Future<void> _startBackground(_IsolateStartRequest request) async {
  if (Platform.isIOS) {
    open.overrideFor(OperatingSystem.iOS, DynamicLibrary.process);
  } else if (Platform.isAndroid) {
    await applyWorkaroundToOpenSqlCipherOnOldAndroidVersions();
    open.overrideFor(OperatingSystem.android, openCipherOnAndroid);
  }

  final executor = NativeDatabase(
    File(request.targetPath),
    logStatements: true,
    setup: (db) => db.execute("PRAGMA key = '${request.password}'"),
  );
  final isolate = DriftIsolate.inCurrent(
    () => DatabaseConnection.fromExecutor(executor),
  );
  request.sendIsolate.send(isolate);
}

class _IsolateStartRequest {
  final SendPort sendIsolate;
  final String targetPath;
  final String password;

  _IsolateStartRequest(
    this.sendIsolate,
    this.targetPath,
    this.password,
  );
}

@vanlooverenkoen
Copy link
Author

This is what is added in our podfile.lock

  - sqflite (0.0.2):
    - Flutter
    - FMDB (>= 2.7.5)
  - SQLCipher (4.5.1):
    - SQLCipher/standard (= 4.5.1)
  - SQLCipher/common (4.5.1)
  - SQLCipher/standard (4.5.1):
    - SQLCipher/common
  - sqlcipher_flutter_libs (0.0.1):
    - Flutter
    - SQLCipher (~> 4.5.0)

dependencies:
  device_info_plus: ^3.2.2
  dio: ^4.0.6
  bot_toast: ^4.0.1
  devicelocale: ^0.5.0
  drift: ^1.5.0
  file_local_storage_inspector: ^0.2.3
  firebase_analytics: ^9.1.5
  firebase_core: ^1.14.1
  firebase_crashlytics: ^2.6.2
  firebase_messaging: ^11.2.13
  flutter:
    sdk: flutter
  flutter_cache_manager: ^3.3.0
  flutter_html: ^3.0.0-alpha.2
  flutter_inappwebview: ^5.4.0+1
  flutter_localizations:
    sdk: flutter
  flutter_multi_formatter: ^2.5.1
  flutter_secure_file_storage: ^0.0.7
  flutter_secure_storage: ^5.0.2
  flutter_svg: ^1.0.3
  get_it: ^7.2.0
  icapps_architecture: ^0.6.4
  in_app_update: ^3.0.0
  injectable: ^1.5.3
  jose: ^0.3.2
  json_annotation: ^4.4.0
  lottie: ^1.3.0
  modal_bottom_sheet: ^2.0.1
  niddler_dart: ^1.4.1
  package_info_plus: ^1.4.2
  path: ^1.8.0
  path_provider: ^2.0.9
  preferences_local_storage_inspector: ^0.2.3
  provider: ^6.0.2
  open_settings: 2.0.2
  retrofit: ^3.0.1+1
  scroll_when_needed: ^2.0.0
  secure_storage_local_storage_inspector: ^0.2.3
  shared_preferences: ^2.0.13
  share_plus: ^4.0.4
  signed_json: ^0.0.2
  sms_autofill: ^2.2.0
  sqlcipher_flutter_libs: ^0.5.1
  storage_inspector: ^0.2.3
  synchronized: ^3.0.0+2
  url_launcher: ^6.0.20
  uuid: ^3.0.6

dev_dependencies:
  build_runner: ^2.1.10
  drift_dev: ^1.5.2
  flutter_launcher_icons: ^0.9.2
  flutter_lints: ^1.0.4
  flutter_test:
    sdk: flutter
  icapps_translations: ^5.2.1
  injectable_generator: ^1.5.3
  json_serializable: ^6.1.6
  license_generator: ^1.0.3
  mockito: ^5.1.0
  model_generator: ^5.8.1
  retrofit_generator: ^4.0.1

@vanlooverenkoen
Copy link
Author

We found that flutter_cache_manager is using sqflite we will create new builds and retest

@vanlooverenkoen vanlooverenkoen changed the title PRAGMA key for encryption max length not documented flutter_cache_manager conflicting with encrypted drift Apr 22, 2022
@vanlooverenkoen
Copy link
Author

Hmm, still can't open the db.sqlite file in tableplus keeps asking for my password

@vanlooverenkoen
Copy link
Author

image

As you can see in the table plus it gives the same issue.

@vanlooverenkoen
Copy link
Author

Still having no cipher version

@vanlooverenkoen
Copy link
Author

Should our simulator databases be encrypted as well? no cipher version is also shown on simulators

@vanlooverenkoen
Copy link
Author

On device I get: {cipher_version: 4.5.1 community}

@ikbendewilliam
Copy link

We did some more testing and found a possible issue that the file is not released correctly.
If we start the app, the database is created, we close the app. If we start the app again, the database is not working (see errors above).
If we then restart the device, we can use the database again, so it might be that the file is not released correctly, but by restarting the device, the file is release and we can use it once more.

@vanlooverenkoen
Copy link
Author

vanlooverenkoen commented Apr 22, 2022

Afected devices:

  • iphone 12 (ios 15.2)
  • iphone se (ios 15.0.2)
  • iPhone 11 pro (ios 15.4.1)

@vanlooverenkoen
Copy link
Author

Not affected:

  • iPhone x (ios 14.2)

@vanlooverenkoen
Copy link
Author

Just disabled our encryption. (removing the pragma key)
This way our app keeps working. even when killing the app.

@vanlooverenkoen
Copy link
Author

vanlooverenkoen commented Apr 22, 2022

We will now try:

  final dbFolder = await getApplicationDocumentsDirectory();
  final db1Name = '${name}1.sqlite';
  final db2Name = '${name}2.sqlite';
  final file1 = File(join(dbFolder.path, db1Name));
  final file2 = File(join(dbFolder.path, db2Name));
  File file;
  if (file1.existsSync()) {
    file1.copySync(file2.path);
    staticLogger.debug('Copy ${file1.path.replaceAll(dbFolder.path, '')} to ${file2.path.replaceAll(dbFolder.path, '')}');
    file1.deleteSync();
    file = file2;
  } else if (file2.existsSync()) {
    staticLogger.debug('Copy ${file2.path.replaceAll(dbFolder.path, '')} to ${file1.path.replaceAll(dbFolder.path, '')}');
    file2.copySync(file1.path);
    file2.deleteSync();
    file = file1;
  } else {
    file = file1;
  }

To see if we can unlock the file by creating a copy of that file. and using the copied one.

@Mike278
Copy link
Contributor

Mike278 commented May 5, 2022

That made me think of https://sqlite.org/howtocorrupt.html#multiple_copies_of_sqlite_linked_into_the_same_application

I'm not sure how applicable that is here since (presumably..) there's only one thread/process involved, but just pointing it out as something to keep in mind while debugging.

@vanlooverenkoen
Copy link
Author

vanlooverenkoen commented May 6, 2022

Started removing each dependency one by one. Found out that it is firebase_messaging & flutter_cache_manager that it causing this issue.

  • With firebase messaging && with flutter_cache_manager -> startup ✅ after killing & restarting the app ❌
  • With firebase messaging && without flutter_cache_manager** -> startup ✅ after killing & restarting the app ❌
  • Without firebase messaging && with flutter_cache_manager -> startup ✅ after killing & restarting the app ❌
  • Without firebase messaging && without flutter_cache_manager-> startup ✅ after killing & restarting the app ✅

It can only be reproduced on release builds. because on iOS it is not possible to restart the app after killing it.

@vanlooverenkoen
Copy link
Author

@NicolaVerbeeck gave me this snippet to removesqlite3

post_install do |installer|
  installer.pods_project.targets.each do |target|
    flutter_additional_ios_build_settings(target)
    target.build_configurations.each do |config|
      xcconfig_path = config.base_configuration_reference.real_path
      xcconfig = File.read(xcconfig_path)
      new_xcconfig = xcconfig.sub(' -l"sqlite3"', '')
      File.open(xcconfig_path, "w") { |file| file << new_xcconfig }
    end
  end
end
  • Without firebase messaging && with flutter_cache_manager -> startup ✅ after killing & restarting the app ✅

I will be testing more the coming hours.

@vanlooverenkoen
Copy link
Author

@simolus3 @Mike278 could this be the sollution?

@davidmartos96
Copy link
Contributor

davidmartos96 commented May 6, 2022

@vanlooverenkoen Have you tried the solution from here? https://github.com/simolus3/sqlite3.dart/blob/master/sqlcipher_flutter_libs/README.md
In the "Incompatibilities with sqlite3" section.

@vanlooverenkoen
Copy link
Author

vanlooverenkoen commented May 6, 2022

Incompatibilities with sqlite3 on iOS and macOS

yes -framework SQLCipher was already added. but -l"sqlite3" was also added and that is now removed by this podfile change

@vanlooverenkoen
Copy link
Author

Testing opening the sqlite db in TablePlus:

  • Right now I see the correct tables
  • DB Key: 3d80ac84-6dff-40bb-b803-abeed6c5e297
    Tested with:
    - 3d80ac84-6dff-40bb-b803-abeed6c5e297 -> ✅
    - 3d80ac84-6dff-40bb-b803-abeed6c5e297 + test -> ❌
    - test + 3d80ac84-6dff-40bb-b803-abeed6c5e297 + test -> ❌
    - test + 3d80ac84-6dff-40bb-b803-abeed6c5e297 -> ❌
    - 3d80ac84-6dff-4 -> ❌
    - 3d80ac84-6dff-40 -> ❌

@vanlooverenkoen
Copy link
Author

@simolus3 is it possible to run a unit test with an encrypted database? or does that always have to be a driver test?

@simolus3
Copy link
Owner

simolus3 commented May 9, 2022

Sure! It is possible by instructing the sqlite3 package to load SQLCipher as a native library instead of the regular sqlite3. But you'd have to ensure that it is available on the system that you're testing on (perhaps manually download a .dylib/.so/.dll of SQLCipher). Then you can open it like this:

import 'package:sqlite3/open.dart';
import 'package:test/test.dart';

void main() {
  setupAll(() {
    open.overrideFor(OperatingSystem.linux, () => DynamicLibrary.open('libSQLCipher.so'));
    // ...
  });
}

But note that these tests wouldn't have discovered this issue since that's about how SQLCipher is linked into an application, something that you'd only catch with a driver test. For smaller unit tests that should work though.

@vanlooverenkoen
Copy link
Author

Yes indeed. We are writing tests for our migration process To make sure we move everything correctly.

@vanlooverenkoen
Copy link
Author

But it seems that this issue is fixed with:
#1810 (comment)

@simolus3
Copy link
Owner

The post_install callback looks like a very neat way to fix this issue. Thank a lot @NicolaVerbeeck for the snippet and @vanlooverenkoen for testing everything, I'm sure this must have been incredibly stressful to resolve.

To help others, I'm thinking about recommending that snippet in the readme of sqlcipher_flutter_libs (and probably in sqlite3_flutter_libs as well to avoid linking multiple sqlite3 versions?). I'm not very familiar with iOS development, so I wonder if we can stop recommending changing linker args manually if users add that snippet? Also, do you foresee any linker issues if those pods are compiled on their own (however that might happen)?

@vanlooverenkoen
Copy link
Author

Because we found this issue a couple of days before the release, it took some time to pinpoint where the issue was located.
Sorry for documenting every little step here 😂 For future reference (probably me 🙈)

Regarding the documentation. I think it is good to add the snippet as well.

  • For us SQLCipher was added automatically, So maybe for other people it was not the case.
  • If it will cause other linker issues, I am not 100% sure. But for us everything works perfectly.

We also wrote a migration (at file level, not db level.) from the non-encrypted to the encrypted database. So that is possible as well. Because when using SQLCipher can also read non encrypted databases

@NicolaVerbeeck
Copy link
Contributor

@simolus3 I don't think pods being compiled on their own will be an issue since the interface provided by both sqlite and sqlcipher is the same and linking only happens in the final executable creation step.

Until someone decides to statically include the sqlite.c file in their pod, in which case everything breaks.

@apoleo88
Copy link

I had this problem time ago and fixed it with your solution.

However, it came back and it seems not to work anymore:

[VERBOSE-2:dart_vm_initializer.cc(41)] Unhandled Exception: SqliteException(26): file is not a database, file is not a database (code 26)
  Causing statement: PRAGMA user_version;
#0      throwException (package:sqlite3/src/ffi/impl/exception.dart:34:3)
#1      PreparedStatementImpl._selectResults (package:sqlite3/src/ffi/impl/statement.dart:152:7)
#2      PreparedStatementImpl.select (package:sqlite3/src/ffi/impl/statement.dart:126:12)
#3      CommonDatabase.userVersion (package:sqlite3/src/common/database.dart:12:25)
#4      _VmVersionDelegate.schemaVersion (package:drift/src/sqlite3/database.dart:169:58)
#5      DelegatedDatabase._runMigrations (package:drift/src/runtime/executor/helpers/engines.dart:354:42)
#6      DelegatedDatabase.ensureOpen.<anonymous closure> (package:drift/src/runtime/executor/helpers/engines.dart:336:13)

Using Flutter 3.3.1, Dart 2.18.0 (I had this problem with 3.0.* too) and

  firebase_messaging: ^13.0.1
  drift: ^1.7.1
  sqlcipher_flutter_libs: ^0.5.0

@apoleo88
Copy link

apoleo88 commented Sep 15, 2022

The above solution was inserted after another piece of code:

      config.build_settings['GCC_PREPROCESSOR_DEFINITIONS'] ||= [
          '$(inherited)',

          ## dart: PermissionGroup.calendar
          'PERMISSION_EVENTS=0',

          ..

        ]

And apparently, it was not executed. I move it back on top and now it works as before.

@itsJoKr
Copy link

itsJoKr commented Jun 4, 2024

My problem was having SQLCipher and MLKit for some reason. This Podfile snippet helped, but I also had to remove conflicting package that had MLKit -> run -> and then return the package. I have no idea why this helped, but it solved it for me.

I also had a test project and verified that this is indeed making a difference.

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

No branches or pull requests

9 participants