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

The first query after "pragma key" returns error #36

Closed
little-brother opened this issue Apr 14, 2021 · 6 comments
Closed

The first query after "pragma key" returns error #36

little-brother opened this issue Apr 14, 2021 · 6 comments

Comments

@little-brother
Copy link

#include <stdio.h>
#include <stdlib.h>
#include "sqlite3.h"

int main() {
	sqlite3* db;
	sqlite3_open_v2("D:/crypt_a.sqlite", &db, SQLITE_OPEN_READWRITE, NULL); // db with key = a

	sqlite3_exec(db, "select * from sqlite_master", 0, 0, 0);
	sqlite3_exec(db, "pragma cipher=sqlcipher", 0, 0, 0);
	sqlite3_exec(db, "pragma legacy=4", 0, 0, 0);
	sqlite3_exec(db, "pragma key=b", 0, 0, 0);
	printf("With the incorrect key: %i\n", SQLITE_OK == sqlite3_exec(db, "select * from sqlite_master", 0, 0, 0)); // 0
	sqlite3_exec(db, "pragma key=a", 0, 0, 0);
	printf("With the correct key: %i\n", SQLITE_OK == sqlite3_exec(db, "select * from sqlite_master", 0, 0, 0)); // 0
	printf("With the correct key: %i\n", SQLITE_OK == sqlite3_exec(db, "select * from sqlite_master", 0, 0, 0)); // 1

	sqlite3_close_v2(db);
	return 0;
}

MinGW32, Win7x64, Release page dll.

@utelle
Copy link
Owner

utelle commented Apr 14, 2021

I can reproduce the phenomenon, but at the moment I have no explanation.

However, if you remove the line sqlite3_exec(db, "select * from sqlite_master", 0, 0, 0); after opening the database and before issuing the pragma commands, then it works as expected.

It also works without removing the first select command, if the correct passphrase is given immediately.

I will look into this issue within the next couple of days and will try to find out why it happens and how it can be possibly solved.

@little-brother
Copy link
Author

little-brother commented Apr 14, 2021

if you remove the line

With multi cipher sqlite3_open_v2 always returns SQLITE_OK. So, select * from sqlite_master is a just verification that the database is encrypted, or not. Depends on select-result I load meta-data from the database or show a cipher dialog.

@utelle
Copy link
Owner

utelle commented Apr 15, 2021

As a workaround you could close (sqlite3_close_v2) and then reopen (sqlite3_open_v2) the database connection after a failed attempt.

Nevertheless I will check as said what's causing the observed behavior.

utelle added a commit that referenced this issue Apr 15, 2021
Clear pager cache after setting a new passphrase to force a reread of the database header.
@utelle
Copy link
Owner

utelle commented Apr 15, 2021

Well, the observed behavior is caused by the fact that SQLite uses a cache to avoid disk I/O. In this case the cache still contains data that were decrypted with the wrong key.

I applied a fix in commit ab5df84 to clear the cache after setting a new passphrase. This solves the issue.

However, in my humble opinion a database connection should be closed after getting the error SQLITE_NOTADB (as is the case here) and then reopened thereafter with new connection settings.

@little-brother
Copy link
Author

Thanks!

a database connection should be closed after getting the error SQLITE_NOTADB

This command sequence is near by how my GUI app intends to use a cipher. An user selects a database via Open dialog control, the database is opened and checked on encryption (first select). If the database is encrypted then the user inputs cipher parameters with the wrong key b. Then the user tries again with the correct key a and gets SQLITE_NOTADB error. The database closing is an inappropriate behaviour in this case.

P.S. I assumed that is a cache error, but I didn't find any suitable sqlite3 API function myself.

@utelle
Copy link
Owner

utelle commented Apr 15, 2021

This command sequence is near by how my GUI app intends to use a cipher. An user selects a database via Open dialog control, the database is opened and checked on encryption (first select).

If the database is in fact encrypted, you get the error SQLITE_NOTADB already here.

If the database is encrypted then the user inputs cipher parameters with the wrong key b. Then the user tries again with the correct key a and gets SQLITE_NOTADB error. The database closing is an inappropriate behaviour in this case.

Of course, I understand how you intend to proceed. However, I disagree that closing the connection is inappropriate. If SQLite tells you that the database file seems not to be a database file - either because it is indeed no database file or because it is an encrypted database file, it is a natural thing to do to close the connection.

The procedure could be similar to the following pseudo code:

bool openSuccess = false;
bool cancelled = false;
do
{
  if (!(cancelled = showDialogWithOptionalCipherConfigAndPassphrase()))
  {
    openDatabaseConnectionWithGivenParameters(); /* open + pragmas */
    if (!(openSuccess = testConnection())) /* select from sqlite_master */
    {
      closeDatabaseConnection();
    }
  }
}
while (!openSuccess && !cancelled);

P.S. I assumed that is a cache error, but I didn't find any suitable sqlite3 API function myself.

Well, there isn't a public function for clearing the cache. And it wouldn't be a good idea to interfere with SQLite's cache management anyway - at least in general.

In our case clearing the cache (with an internal function) does no harm (i.e. to performance), because the passphrase has to be specified right after opening the database connection. That is, usually you will have just the first database page in the cache at most.

@utelle utelle closed this as completed Apr 20, 2021
utelle added a commit that referenced this issue Apr 23, 2021
- Fix issue #37: Allow concurrent access from legacy applications by establishing WAL journal mode compatibility
- Fix issue #36: Clear pager cache after setting a new passphrase to force a reread of the database header
- Adjust build files for MinGW
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

2 participants