Changing key doesn't change key and says ok

[rogerbinns]

If I do the key pragma on an already open database with a different key, I still get back ok. But that new passphrase is wrong and had no effect. I don't think ok is the right thing to return.

>>> print(apsw.mc_version)
SQLite3 Multiple Ciphers 1.8.5
>>> con=apsw.Connection("database.sqlite3")
>>> con.pragma("key", "my secret passphrase")
'ok'
>>> con.execute("create table foo(x);")
>>> con.pragma("key", "a different passphrase")
'ok'

[utelle]

If I do the key pragma on an already open database with a different key, I still get back ok. But that new passphrase is wrong and had no effect. I don't think ok is the right thing to return.

I agree that returning ok seems a bit odd at first glance, but this is how things work even in the official SEE encryption extension. PRAGMA key just sets up the key material, it does not check whether the provided key is correct or not. An error message is only issued after actually accessing the database file. Quote from the official SEE documentation:

Note that the "ok" string is returned when any key is loaded, not necessarily the correct key. The only way to determine if the key is correct is to try to read from the database file. An incorrect key will result in a read error.

That is, the behaviour you are seeing is expected behaviour.

[rogerbinns]

Note that I set the key, modified the database, and then set a different key. ok the first time is fine. By the second time the extension knows better and the database has been accessed.

[utelle]

Note that I set the key, modified the database, and then set a different key. ok the first time is fine. By the second time the extension knows better and the database has been accessed.

Unfortunately, the extension does not know the state of the database. This would require to keep track of the state of the database - whether a read/write after issuing PRAGMA key was successful or not. This is not done. SEE doesn't do it, SQLCipher doesn't do it, ... and SQLite3 Multiple Ciphers doesn't do it either.

If PRAGMA key is used to set the correct key, all will work as expected. If it is used to set an incorrect key, the next database read/write operation will fail.

[rogerbinns]

You would only need to track whether there have been any reads or writes since open - a single bit of information :-)

As you described this also affects unencrypted databases where key pragma says ok`` and then the next read/write gets the file is not a database error`.

I was surprised at how easy it is to make this mistake. I suggest closing as wontfix, or implementing it as a feature request making it easier to catch programming errors.

[utelle]

You would only need to track whether there have been any reads or writes since open - a single bit of information :-)

Sounds easy to accomplish, but isn't. For example, let's assume we have an existing unencrypted database. We open the database and perform some read/write operations successfully. Where do we store the information that the database file was accessed successfully? There is no attached codec where we could add this information. The codec is only created when PRAGMA key is called.

As you described this also affects unencrypted databases where key pragma says ok and then the next read/write gets the file is not a database error.

Well, if the user is able to access the database without a key, why on earth should he/she issue a PRAGMA key command?

I was surprised at how easy it is to make this mistake. I suggest closing as wontfix, or implementing it as a feature request making it easier to catch programming errors.

Typically, an application should issue a PRAGMA key command just once immediately after opening a database connection, if the user says that the database is encrypted. Only if accessing the database fails after issuing a PRAGMA key command, it would make sense to repeat asking the user for the password and issuing a PRAGMA key command again.

[rogerbinns]

I'm guessing that you have no VFS hooks hence no idea about reads/writes that have happened. There isn't even a file control with I/O counters. There are some DBSTATUS counters but that would be fragile.

Note that this issue is not about correct usage! It is about developers making mistakes. The very first code I wrote did this. Python developers expect exceptions when they make mistakes as soon as possible, and it does lead to more robust code including detecting problems where and when they happen, rather than later. That is why I was surprised and created the issue,

I suggest closing as cantfix.

[utelle]

I'm guessing that you have no VFS hooks hence no idea about reads/writes that have happened.

SQlite3 Multiple Ciphers's encryption extension is implemented as a VFS shim. So detecting that reads/writes have happened is possible. However, The VFS layer has no means to detect whether the data read are valid or not. Only SQLite's pager can tell whether the data are valid or corrupt.

I'm not aware of any SQLite hook that allows to detect whether SQLite identifies read data as valid or not.

Note that this issue is not about correct usage! It is about developers making mistakes.

Since I started with providing a SQLite encryption extension in 2005, this topic came up several times. In principal, the behaviour of the function sqlite3_key could be changed to force a database read internally, and thus allowing to detect whether the read worked or not. (Years ago I had a discussion with the developer of sqleet who had implemented such a validation check. And AFAICR performing the validation check could have unwanted side effects.)

I decided to keep the current behaviour for 2 reasons:

1. It is the documented behaviour for the official SQLite Encryption Extension (SEE),
2. It would violate SQLite's principle to actually access the database file at the latest possible moment.

I suggest closing as cantfix.

Well, as said in principal it could be fixed, but because of the reasons mentioned I don't intend to change anything at the moment.

[rogerbinns]

If the apply_key approach documented in the README is used then this issue is negated. The answer then becomes - use best practise