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

Question: Encryption #40

Closed
rops opened this issue Mar 31, 2016 · 33 comments
Closed

Question: Encryption #40

rops opened this issue Mar 31, 2016 · 33 comments

Comments

@rops
Copy link

rops commented Mar 31, 2016

Is there any way to support encryption?

@andpor
Copy link
Owner

andpor commented Apr 1, 2016

in short yes. does it support it now? Obviously not. Would it be easy to add encryption? I have to think about a proper x-platform approach considering native libs on Android etc...

@brodycj
Copy link
Contributor

brodycj commented Apr 1, 2016

A relatively easy way would be to use SQLCipher. I support a SQLCipher version of the Cordova plugin at: https://github.com/litehelpers/Cordova-sqlcipher-adapter

@dryganets
Copy link
Contributor

dryganets commented Sep 15, 2016

Hey Andpor,

I'm working on encryption implementation for android and ios in our internal repository.
I would be eager to contribute back to your project.

I have following questions for you:

  1. which project would better suit for integration of android sqlitecipher? android or android-native. I'm not exactly sure what would be purpose of native proejct with the sqlitecypher.
    Internally I've made it in android as it's way easier to do - just replace the import and it works.
    I'm not sure if that make sense to do in android-native as sqlitecipher for android is absolutely different native implementation of sqltie with encryption enabled on the page level.
  2. for iOS what would be the best way to integrate sqlite cyper? It looks like I would need to have an access to the library sources at least according to this tutorial:
    https://www.zetetic.net/sqlcipher/ios-tutorial/
    Would you be ok with git submodule?
  3. Also we would need to have a different api as for migration purposes we would need to be able to open some databases in encrypted mode and some in unencrypted.
    That would require some API changes.
    Would you prefer to have an additional method to open db in encryped mode or
    to have an additional password argument to the existing open method?

@brodycj
Copy link
Contributor

brodycj commented Jan 5, 2017

To be honest I would favor supporting sqlcipher in a separate project. SQLCipher adds some things that the rest of the community would not want for a couple major reasons:

This is what I did in the Cordova sqlite/sqlcipher plugins that I maintain. Yes it would probably be a pain to keep merging changes but this may be the best solution for now.

@maxpowa
Copy link

maxpowa commented Feb 3, 2017

@andpor do you plan to create a sqlcipher version of this? Looking to use this library in a react app but need sqlcipher support - If you don't plan to create one, we might do it ourselves.

cc @itsrifat @ericsvmx

@brodycj
Copy link
Contributor

brodycj commented Feb 3, 2017

@maxpowa you may have to do it yourself. I don't think the owner @andpor will do this and @dryganets seems to have dropped off. I hope someone will share a version with sqlcipher with the user community.

To answer some questions raised by @dryganets:

which project would better suit for integration of android sqlitecipher? android or android-native. I'm not exactly sure what would be purpose of native proejct with the sqlitecypher.
Internally I've made it in android as it's way easier to do - just replace the import and it works.

I think that is the correct way, assuming you include the SQLCipher JAR and native libs. The android-native version uses my Android-sqlite-connector library which is currently not supported with SQLCipher. (I hope to do this someday ref: sqlcipher/sqlcipher#125)

for iOS what would be the best way to integrate sqlite cyper? It looks like I would need to have an access to the library sources at least according to this tutorial:
https://www.zetetic.net/sqlcipher/ios-tutorial/
Would you be ok with git submodule?

I tried using a git submodule in the past and was not so happy with it. It adds an extra build step which is not so great for plugins and I find it a bit inflexible in certain ways. For example: if I would remove an external module from git then add it again (from a different location for example) there is still an extra artifact in the local .git tree in the way.

I think your best bet is to just include the sqlcipher code, libs, and dependencies in your git repo. You should be able to use git to merge in updates from this git project moving forward. I can think of the following extra alternatives:

  • In the original cordova-sqlite-storage project I fetch the sqlite dependencies by npm through a Cordova hook script but this causes problems with some build systems.
  • It may be possible to use a npm prepublish script to include external module dependencies, like I see in https://github.com/nolanlawson/cordova-plugin-sqlite-2.

@dryganets
Copy link
Contributor

I could prepare a pull request with SQLite cipher for both iOS and android If there are enough people interested in it.

We are using this internally for a few months.

It's worth to mention that SQLite cipher supports only exclusive transactions out of the box so you might experience some performance degradation as result of usage of the plugin.

I have a couple of fixes to share with the community as well.

In the long term, we are going to move to our internal implementation as:

  1. Error handling is not production ready. We have some hard to explain crashes in production inside of the current plugin implementation. I was trying to inject the errors in different paths of the native code and I still can't repeat the same problem. It says that transaction has failed but in reality, hell know what happened inside.
  2. Transactions implementation is weird. I don't understand why I need artificial Select 1;. Read transaction should be a non-exclusive transaction. Write transaction should be an exclusive transaction. Now read is select 1 and write is a read transaction. We have a transaction which needs to live during multiple bridge round trips and be exclusive. Now it's true only because of SQLite cipher transaction implementation.
  3. Native threading model allows only single thread per DB. One thread for the DB is probably good enough for most of the cases but I want to experiment with it in order to improve application startup time.

If plans wouldn't change I will start on our internal implementation of the plugin in typescript. But it would take a time to release it's publically.

@dryganets
Copy link
Contributor

dryganets commented Feb 22, 2017

@brodybits,
As promised I send pull requests with our changes required for SQLiteCipher to work.
This is the first part not actually related to SQLiteCipher itself so it could be contributed to the main repository.

Android toLowerCase fix for Turkish locale. It's a root cause of insert and begin commands failures for Turkish locale. Probably specific to SQLiteCipher too, but the fix is safe anyway:
#128

Android correctly close statements and queries:
#132

You need to merge #131
in order to make android-native compile again.

iOS fix for crash on module dealloc:
#130

@andpor
Copy link
Owner

andpor commented Feb 23, 2017

@dryganets - this is awesome. Thanks a lot for these. I will review and merge tonight.

@dryganets
Copy link
Contributor

https://github.com/dryganets/react-native-sqlite-storage/tree/sergeyd/sqlite-cipher

has our implementation.

I would make sure that iOS builds correctly tomorrow.
See android module for android integration.
ios integration is done through cocoapods
just pass the key parameter in addition and it would encrypt your database.

@brodycj
Copy link
Contributor

brodycj commented Feb 24, 2017

From #132 (comment):

@dryganets Sergey - if it is a small diff maybe it would make sense to incorporate it somehow to this module...I would have no objections actually..can you suggest something?

@andpor I would recommend keeping encryption separate. When publishing apps with encryption people would have to follow the guide at https://discuss.zetetic.net/t/export-requirements-for-applications-using-sqlcipher/47 and it may be even worse in places like France.

@dryganets
Copy link
Contributor

Yep, It's better to make a plugin system.
That would give you control over the library you use and wouldn't require an additional dependency if you don't need it.

In addition to that we get rid of an extra project with code duplication.

@itinance
Copy link
Contributor

According to @brodybits arguments: can we leave this optional per #ifdef direction on objC-Side and something similar on android?
For those who need encryption they can activate it then easily and others don't have to ship one more MB with the app and aren't bothered by the export-requirements for apps using cipher

@dryganets
Copy link
Contributor

dryganets commented Feb 24, 2017

There is no #ifdef on android. I think I would take a time at some moment and refactor this to plugin model on android.

As for iOS - I updated the branch.
I don't know if there is a better way to integrate it honestly. Any ideas are welcome.
I'm not an iOS developer ;)

Basically, the problem is to have a relative path to SQLiteCipher customized.
As we have our module with SQLiteCipher integrated to the react-native-sqlite-storage.
And we have react-native-sqlite-storage integrated through npm into the main project.
Npm paths are different for the main project and SQLite plugin and both needs SQLiteCipher.

My intention was to keep plugin compile as a part of another project and on it's own.

On android, it's not a problem as we have a gradle dependency management and we could get any dependency we want not relying on npm.

On iOS I choose cocoa pods integration and this way requires react native sources modification.

They are using <React/Header> and by some reason pods sees only "Header".

maybe it's my bad, though and there is a better way to do it.

@itinance
Copy link
Contributor

@dryganets This changed with RN 0.40 from "header" to <React/Header>. That means you should just concentrate on the ne format <React/Header>

@dryganets
Copy link
Contributor

Probably, I just didn't figure out how to make it compile with pods @itinance :(
I see that cocoa pods users and swift developers are not happy with this change:
facebook/react-native@e1577df

For now, we have our own fork of react-native for Skype so we are free to revert this commit.

@itinance
Copy link
Contributor

@dryganets can you tell me a concrete issue? CocoaPods works well in my newest app (i am working on several ones using RN) and the thing with headers is that there was a huge change with RN 0.40 as you can see in the commit of you previous comment here

i'd like to help you on iOS-side

@dryganets
Copy link
Contributor

CocoaPods/CocoaPods#4605
I think this is the root cause as result <React/ ... > can't be found.

as a structure is flattened.

@andpor
Copy link
Owner

andpor commented Feb 25, 2017

i have run the latest with cocoapods setup and have not had any issues...i am not sure I understand what the problem is...

@half-shell
Copy link

Hi, and thanks for the awesome work guys.
Could we get some instructions down the line on how to set up react-native-sqlite-storage with encryption?

@andpor
Copy link
Owner

andpor commented Mar 16, 2017

@dryganets is the man behind the encryption...maybe he can share how to get this going.

@dryganets
Copy link
Contributor

dryganets commented Mar 16, 2017

@half-shell ,

Integration should be as easy as:

  1. add my fork to your package.json.
    "react-native-sqlite-storage": "git+https://github.com/dryganets/react-native-sqlite-storage #0060a69" (latest commit in sergeyd/sqlite-cipher branch)
  2. npm install
  3. link android (not android-native) to your application the same way it described in documentation for this package
  4. Link iOS to your application using cocoapods.
  5. Fix bugs on iOS if you have any. I had problems with imports though we are on 0.34.0 version of react-native still.

When I would have a time I'll add sample app to my fork and add the more accurate description.

All changes you need on js level - pass the key as a parameter to the open method.
Though now it doesn't re-encrypt the database if it wasn't initially encrypted, so if you have a database on the same path - it will throw an error.

It should be easy to fix, as I remember there is a function to re-encrypt the database in sqlitecipher though we don't have such a use-case so it's not implemented.

@mikaelfs
Copy link

Hi, with the latest release, we experienced this issue when compiling the iOS project:

implicit declaration of function sqlite3 key is invalid in c99

The library works fine until 3.2.2 so the cipher support is a breaking change. As @brodybits mentioned, would it be possible to keep the encryption separate?

@philographer
Copy link

@mikaelfs +1
same here like this
image

@andpor
Copy link
Owner

andpor commented Apr 22, 2017

this is fixed in 3.3.1

@henryphtang
Copy link

henryphtang commented Jul 21, 2017

@dryganets Your fork is awesome,

but I would like to know more on how Android can make use of SQLCipher? iOS side has no problem, yet not working on Android... But I'm quite sure I have linked it well. I have to manually import SQLCipher jars and native libs right?

See android module for android integration.

Can you tell me which files I have to go through (actually I am an iOS developer :P)
And I didn't see anywhere in Android to set the key for the database?

@dryganets
Copy link
Contributor

@henryphtang
https://github.com/dryganets/react-native-sqlite-storage/blob/sergeyd/sqlite-cipher/src/android/src/main/java/org/pgsqlite/SQLitePlugin.java

already uses SQLite cipher packages.

https://github.com/dryganets/react-native-sqlite-storage/blob/sergeyd/sqlite-cipher/src/android/build.gradle
has sqlitecipher as a dependency already, so it should work out of the box.

So if you are using https://github.com/dryganets/react-native-sqlite-storage/blob/sergeyd/sqlite-cipher/src/android everything should work.

https://github.com/dryganets/react-native-sqlite-storage/blob/sergeyd/sqlite-cipher/src/android-native doesn't have sqlitecipher integrated.

I'll try to take a time on this weekend and contribute the plugin system to this repository.
This way we could drop android-native support later on.
There is too much of duplicated code in there ...

@dryganets
Copy link
Contributor

https://github.com/dryganets/react-native-sqlite-storage/tree/sergeyd/plugin-system

It's work in progress already close to done, but I didn't finish few moments yet + needs to be tested.
Will see if I'll find time next weekend :)

@dryganets
Copy link
Contributor

It looks like it is pretty close:
#187

@andpor
Copy link
Owner

andpor commented Aug 2, 2017

btw @dryganets - I have added a #ifdef for SQLCIPHER to iOS codebase for SQL Cipher key retrieval. If you define this at your main project level the library will pick up the code automatically else it will just work as plain database.

@andpor andpor closed this as completed Aug 2, 2017
@brodycj
Copy link
Contributor

brodycj commented Oct 23, 2017

I have added a #ifdef for SQLCIPHER to iOS codebase for SQL Cipher key retrieval. If you define this at your main project level the library will pick up the code automatically else it will just work as plain database.

Quick question: SQLCipher would usually use CommonCrypto by default on iOS to do the actual encryption. Can you tell if this would already be linked by this plugin, the react-native framework, or something else?

In general a library should not link to such a framework, even dynamically, unless explicitly required for the application.

@defi-bear
Copy link

Could you update the component as cipher support?

@sashiksu
Copy link

Guys, does react-native-sqlite-storage supports encryption databases?

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

No branches or pull requests