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

Error encrypting in 3.11.0 #1732

Closed
StevenMagdy opened this Issue Feb 8, 2019 · 39 comments

Comments

Projects
None yet
7 participants
@StevenMagdy
Copy link

StevenMagdy commented Feb 8, 2019

Details for the issue

What did you do?

tried to encrypt using SQLCipher 4 defaults

What did you expect to see?

encrypt

What did you see instead?

SQL logic error (SELECT sqlcipher_export('sqlitebrowser_edit_encryption');)

Useful extra information

The info below often helps, please fill it out if you're able to. :)

What operating system are you using?

  • Windows: ( version: ___ )
  • Linux: ( distro: ___ )
  • Mac OS: ( _version: 10.14.3 )
  • Other: ___

What is your DB4S version?

  • 3.11.0
  • 3.11.0-alpha1 or 3.11.0-beta*
  • 3.10.1
  • Other: ___

Did you also

@chrisjlocke

This comment has been minimized.

Copy link
Contributor

chrisjlocke commented Feb 8, 2019

Just an obvious check, but you're running 'DB Browser for SQLCipher' and not 'DB Browser for SQLite' ? Have to ask, as its not clear whether you are.

@StevenMagdy

This comment has been minimized.

Copy link
Author

StevenMagdy commented Feb 8, 2019

I'm running DB Browser for SQLite 3.11.0

@chrisjlocke

This comment has been minimized.

Copy link
Contributor

chrisjlocke commented Feb 8, 2019

Aah, I might be talking nonsense. Hopefully someone else can advise, as I don't run MacOS. I know on the Windows version, there are two executables - with and without encryption. I was thinking this was similar on the MacOS...

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 9, 2019

On macOS there's just the SQLCipher one. It only occurred to me well after we did the packaging on Windows... that for this release both the SQLite and SQLCipher executables bundle the exact same version of SQLite. We might as well have shipped just the SQLCipher one there too. 😉

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Feb 9, 2019

This seems to be the same issue as in #1690. In my comment there I mention the only explanation I have found so far for this: According to the SQLCipher guys a mismatch in the OpenSSL versions can cause this problem - so it would be a build thing.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 9, 2019

k, I'll double check later on today. I manually tested our macOS binary being able to open/save SQLCipher 3 created databases. Don't think I checked for SQLCipher 4 ones though, rather assuming that would work.

And yeah, in hindsight that could be better. 🙀

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 10, 2019

Uh oh. Just tested the SQLCipher 4 encryption here on macOS... and yep, same problem. 😱

db4s-issue_1732-screenshot1

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAArrrggggggggggggh. 🤦‍♂️

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 10, 2019

Hmm, I can't get the SQLCipher encryption (using any settings of 3/4/custom) to encrypt a previously not-encrypted database. eg completely borked encryption functionality.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 10, 2019

It will open a previously encrypted (SQLCipher 3) database though, without any issue.

Damn. The one thing I didn't test. 😦

@justinclift justinclift added this to the 3.11.1 milestone Feb 10, 2019

@justinclift justinclift referenced this issue Feb 11, 2019

Closed

3.11.0 release - outstanding pieces? #1656

12 of 23 tasks complete
@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 11, 2019

Same problem exists with a newly setup Win7 x64 VM too, so it's probably not a packaging error.

At a guess, we'll probably just need to change the SQL statements we send to SQLCipher, and voila... it then works. That's my hope anyway. 😉

@JReally

This comment has been minimized.

Copy link

JReally commented Feb 11, 2019

Although it may have nothing to do with this situation... when I drop the SQLcipher from the 2019-02-11 build into my Lazarus application (replacing the SQLCipher version from December 2018), it fails to open encrypted databases.

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Feb 11, 2019

@justinclift Maybe you're right. But I'm not so optimistic yet 😉 Take example number 4 from here which is closest to what we do in the dialog:

PRAGMA key = 'testkey';
ATTACH DATABASE 'newdb.db' AS newdb KEY 'newkey';
PRAGMA newdb.cipher_page_size = 4096;
PRAGMA newdb.cipher = 'aes-256-cfb';
PRAGMA newdb.kdf_iter = 10000;
SELECT sqlcipher_export('newdb');
DETACH DATABASE newdb;

Here's what we do:

PRAGMA key = 'testkey'; -- While opening the database

ATTACH DATABASE 'filename.db.enctemp' AS sqlitebrowser_edit_encryption KEY 'newkey'; -- When changing the encryption
PRAGMA sqlitebrowser_edit_encryption.cipher_page_size = 4096;
PRAGMA sqlitebrowser_edit_encryption.kdf_iter = 10000;
PRAGMA sqlitebrowser_edit_encryption.cipher_hmac_algorithm =  SHA512;
PRAGMA sqlitebrowser_edit_encryption.cipher_kdf_algorithm = SHA512;
SELECT sqlcipher_export('sqlitebrowser_edit_encryption');

-- Extra PRAGMA here - but we don't even get this far according to the error message
PRAGMA sqlitebrowser_edit_encryption.user_version = 1;

-- DETACH DATABASE sqlitebrowser_edit_encryption;
-- No detaching for us. We just close the entire connection.

So three things seem to be different here (in a way which might matter):

  1. kdf_iter comes last in the example but second in our case. I don't think that's important.
  2. We use cipher_hmac_algorithm and cipher_kdf_algorithm instead of the cipher pragma. I'm not sure what to think about this. The cipher pragma isn't even documented.
  3. We say SHA512 instead of HMAC_SHA512.

So let's try them out 😄 Can you maybe test again with this patch applied:

diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index b234caf4..a85ac1cd 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -3178,11 +3178,11 @@ void MainWindow::editEncryption()
         if(ok)
             ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.cipher_page_size = %1").arg(cipherSettings.getPageSize()), false, false);
         if(ok)
-            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.kdf_iter = %1").arg(cipherSettings.getKdfIterations()), false, false);
+            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.cipher_hmac_algorithm = HMAC_%1").arg(cipherSettings.getHmacAlgorithm()), false, false);
         if(ok)
-            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.cipher_hmac_algorithm = %1").arg(cipherSettings.getHmacAlgorithm()), false, false);
+            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.cipher_kdf_algorithm = PBKDF2_HMAC_%1").arg(cipherSettings.getKdfAlgorithm()), false, false);
         if(ok)
-            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.cipher_kdf_algorithm = %1").arg(cipherSettings.getKdfAlgorithm()), false, false);
+            ok = db.executeSQL(QString("PRAGMA sqlitebrowser_edit_encryption.kdf_iter = %1").arg(cipherSettings.getKdfIterations()), false, false);
 
         // Export the current database to the new one
         qApp->processEvents();

This one is addressing points 1) and 3).

justinclift added a commit that referenced this issue Feb 12, 2019

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

With that patch applied... encryption seems to fully work!

Tested it here just now in my new local Win 7 build VM. With this patch, I'm able to encrypt using:

  • SQLCipher 4 default settings
  • SQLCipher 3 default settings
  • SQLCipher 4 default settings, but with 4k page size

The SQLCipher build is then able to correctly open the given encrypted database - but only if matching settings are given (good).

Our previous release - 3.10.1 - is also able to open the two databases done with SQLCipher 3 settings, and is unable to open the SQLCipher 4 one.

Looks like a win. I'll create proper macOS and Win32/64 one-off test builds in a bit, and put them online for people to test as well. 😄

@StevenMagdy

This comment has been minimized.

Copy link
Author

StevenMagdy commented Feb 12, 2019

I've Tested it and it's working perfectly.
The UI is weird in this build though.
screen shot 2019-02-12 at 11 02 57 am

MKleusberg added a commit that referenced this issue Feb 12, 2019

sqlcipher: Fix editing the encryption for SQLCipher4
With SQLCipher4 the encryption was not working as expected because the
KDF and HMAC algorithms were not set properly. This is fixed in this
commit so it should work now with SQLCipher4 as well as SQLCipher3.

See issues #1690 and #1732.

MKleusberg added a commit that referenced this issue Feb 12, 2019

sqlcipher: Fix editing the encryption for SQLCipher4
With SQLCipher4 the encryption was not working as expected because the
KDF and HMAC algorithms were not set properly. This is fixed in this
commit so it should work now with SQLCipher4 as well as SQLCipher3.

See issues #1690 and #1732.

MKleusberg added a commit that referenced this issue Feb 12, 2019

sqlcipher: Fix editing the encryption for SQLCipher4
With SQLCipher4 the encryption was not working as expected because the
KDF and HMAC algorithms were not set properly. This is fixed in this
commit so it should work now with SQLCipher4 as well as SQLCipher3.

See issues #1690 and #1732.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Feb 12, 2019

Awesome! Thanks for testing, everyone 😄

@justinclift I have just pushed a modified version of that patch. In theory it should do exactly the same but in a more proper way which is less error prone in case of future changes 😉

@StevenMagdy The weird UI might be because of PR #1684. Not sure though. @mgrojo, what do you think?

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

The UI is weird in this build though.

Yeah, I made the build using our development branch (master) code base, instead of doing it with our 3.11.x code base. So, you're seeing the current state of the development branch... can be a bit buggy at times. 😉

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

@MKleusberg So, best to make another build with the new code (eg directly from master) for people to test, just to make sure it's 100% ok yeah? 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

@StevenMagdy Looking at that screenshot... it kind of looks nifty. "Zebra theme" maybe. 😉

Not sure how it'd go on the eyes though, trying to use it for an extended amount of time. 😁

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

@MKleusberg Thinking it over, the next build should probably be v3.11.x branch + the updated patch, just to make 100% sure it works ok with the 3.11.x code base too. 😄

justinclift added a commit that referenced this issue Feb 12, 2019

sqlcipher: Fix editing the encryption for SQLCipher4
With SQLCipher4 the encryption was not working as expected because the
KDF and HMAC algorithms were not set properly. This is fixed in this
commit so it should work now with SQLCipher4 as well as SQLCipher3.

See issues #1690 and #1732.
@chrisjlocke

This comment has been minimized.

Copy link
Contributor

chrisjlocke commented Feb 12, 2019

Using v2, I'm able to open an encrypted database created with v1. Also able to create/open a database created with v2.
Seems to work OK for me.
Neither version had a weird GUI for me (on Windows).

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

Excellent, thanks @chrisjlocke. 😄

@StevenMagdy

This comment has been minimized.

Copy link
Author

StevenMagdy commented Feb 12, 2019

Tested V2, the encryption is working perfectly however, I'm still seeing the zebra theme on macOS. 😜

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

Ok, that's weird. 😦

Oh... I bet the previous v1 build saved it's colour scheme when it exited. Oops.

You may need to hit the "Reset Preferences" (or similar wording) button, in the Preferences dialog.

Sorry about that, didn't even think of it. 🙄

@StevenMagdy

This comment has been minimized.

Copy link
Author

StevenMagdy commented Feb 12, 2019

Nope, still there after I tried to remove the app with all its data from macOS.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 12, 2019

Okaaaaaaaaayyyyyy. Some weird voodoo level thing is happening now. 👿

k, "phone a friend time". @mgrojo Please help. 😄

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Feb 13, 2019

@justinclift Definitely makes sense to test the actual patch I committed again with the v3.11.x branch 😄 Unfortunately I don't have an idea either regarding the zebra look...

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Feb 13, 2019

The pretty zebra look 😉 was already seen in this screenshot by @wmertens in this comment. The dark mode is broken in Qt 3.11 and macOS Mojave. see QTBUG-68891. I suppose the workaround is to disable the dark mode in macOS.

Given that it is said to be solved in 5.12.0 Alpha, we should try to apply the workaround described in this comment in QTBUG-73721. I tried that, but don't know how to apply it to all the cells. Another option is disabling word wrapping altogether until the bug is solved (hopefully, because there is a bit of debate about it being a bug or not). The situation for mac users with dark mode is worst than loosing word wrapping, in my opinion.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 14, 2019

Thinking it over, if disabling dark mode support on macOS isn't too hard, we should do that for our 3.11.1 release, along with using Qt 5.11.3 for it. That's probably the safest bet, which is the right kind of thing for our release code.

We can experiment with the word wrapping, dark mode (etc) + Qt 5.12 in the nightlies, as we're not under pressure to get an immediate, stable result there.

Is disabling dark mode support on macOS fairly simple to do?

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Feb 14, 2019

I don't know if we can disable dark mode from our code. Indeed I was refering it as a possible workaround for the users to do. I suppose they have the dark mode enabled at the OS level. (I know, it's a shame they'd had to disable only because of DB4S).

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 14, 2019

Ok. I'm just not yet sure how we're supposed to proceed.

Our mac mini is too old to run macOS Mojave, and it seems like we need to be running that macOS release to get non-broken dark mode support for any macOS version user.

Hmm... maybe Mojave will work in a VM on the mac mini? It wouldn't be efficient, but the thing is slow anyway, so no major difference there... 😉

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 15, 2019

Our mac mini is too old to run macOS Mojave

macOS Mojave does seem to load and run in a VM, even though the host machine itself is too old. Will see if the rest of the needed bits for doing builds also work. If so, I'll make a test build for people to try the Dark Mode support with.

MKleusberg added a commit that referenced this issue Feb 15, 2019

sqlcipher: Fix editing the encryption for SQLCipher4
With SQLCipher4 the encryption was not working as expected because the
KDF and HMAC algorithms were not set properly. This is fixed in this
commit so it should work now with SQLCipher4 as well as SQLCipher3.

See issues #1690 and #1732.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Feb 15, 2019

I have just cherry-picked the encryption fix over to the 3.11.x branch. So at least the original issue here should be resolved now 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 15, 2019

Excellent. Thanks @MKleusberg. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 15, 2019

Well, this is a bit weird. I'm able to compile DB4S on macOS Mojave... but the node based program we use for creating the pretty looking .dmg is crashing.

Attempting to build node on Mojave, specifically so it can be customised for our purposes, is failing on what seems like a bug in Homebrew. Filed an issue for it just now: Homebrew/brew#5741

Might be able to get around that with a manual install of node. Not sure, will experiment.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 15, 2019

Fixed the node crashing problem, by using a manual install of Nodejs rather than the Homebrew provided one.

Created an initial "Dark Mode support" build of DB4S (from our master branch), and put it online for testing: #1493 (comment)

Going to close this (encryption) issue for now as it's been solved. The Dark mode problem is being worked on in #1493. 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Feb 15, 2019

@StevenMagdy If you have a minute to try that new test build (for the Dark Mode problem), that'd be super useful: #1493 (comment) 😄

@justinclift justinclift referenced this issue Feb 16, 2019

Closed

3.11.1 - outstanding pieces #1747

12 of 13 tasks complete

@justinclift justinclift changed the title error encrypting in 3.11.0 Error encrypting in 3.11.0 Feb 16, 2019

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