Efcore: Microsoft.Data.Sqlite: Adding support for providing an encryption key in SqliteConnectionString class

[brunzefb]

I would like to discuss adding the key in the SqliteConnectionString class of the Microsoft.Data.Sqlite project, but am not 100% sure this can be made to work throughout the stack- If Password were a property of SqliteConnectionString (and passed to the lower dll) and you had a SQLitePCLRaw.core with a provider that supported encryption, and a native dll that also supported encryption, you would get the encryption if you provided the “Password” in the connection string, otherwise it would be unencrypted. If you used a SQLitePCLRaw.core with a provider that did not support encryption, or if the low-level dll did not support encryption, usage of the “Password” in the connection string would basically be a No-Op or throw an exception. Effectively - the user of Microsoft.Data.Sqlite could specify encryption without recompiling Microsoft.Data.Sqlite, if the rest of the stack supported it. From what I can see, we only need to support the ‘sqlite3_key()’ API.

I know that it is possible to use a #PRAGMA in an SQLCommand to specify the encryption key, but that is a pretty un-intuitive way of providing the key. Adding the Password property makes more sense to me.

[bricelam]

Note for triage: We should also consider the HexPassword keyword along with ChangePassword() and SetPassword() on SqliteConnection when discussing this.

[bricelam]

Notes for triage: I investigated, and sending PRAGMA key on databases that don't support encryption is a no-op. This will hide the fact that the database is not actually encrypted when you use the Password keyword with builds that don't support it.

[bricelam]

We decided that given the behavior, this is too risky for us to implement. If someone wants to investigate ways of making it throw, we would reconsider.

[brunzefb]

can someone be a bit more specific about what the risk is of implementing this? Feels like there were some discussions that I was not part of. Is it because it possibly requires changes in the SQLitePCLRaw.core? People do use encrypted SQLite database -- are we saying "Not Supported" on .net Core?. The security of the encrypted db's is questionable if the code is available -- the key or pieces to get the key can usually be retrieved from disassembling the code.

I might be willing to submit a PR to implement changes to support encryption that we can then discuss -- but if the community is against this, it is not worth spending my time.

Let me know,

Best,
Friedrich

[ericsink]

I can't speak for the maintainers on this particular issue, but I can clarify that encryption (with sqlcipher) is still fully supported by SQLitePCL.raw and Microsoft.Data.Sqlite (using PRAGMA key).

[divega]

The concern with this feature is what @bricelam explained in https://github.com/aspnet/Microsoft.Data.Sqlite/issues/401#issuecomment-327273600: that customers won’t notice that although a key was supplied in the connection string, the database is not encrypted, i.e. there is no error if encryption is not supported.

[brunzefb]

@ericsink You're right that encryption is supported - the thing that I don't like is that usage of PRAGMA key is pretty obscure. And if the back-end does not support encryption, you have the same problem. Is there any way we can detect the presence of encryption support by calling the C sqlite3_key() API, and catching the error if not there? Would that allow detection of encryption support at the back end? If that's the case the Microsoft.Data.Sqlite could throw an exception if Password is used and the back end does not support it.

[ericsink]

I understand the concern you have. I was just trying to clarify something I thought might be confusing.

[ericsink]

If there is a simple and robust way to determine at runtime whether a SQLite-ish library supports encryption, I am not aware of it.

Interesting question though. I wonder why it doesn't come up more often. I'm guessing in general, app developers know whether they build with encryption or not, so the need to answer that question at runtime is uncommon.

[jonreis]

Hello @bricelam, thank you for the reply.

Encryption is definitely the most obvious one. Say I have an existing encrypted database that I would like to migrate from v1 to v2 using RoundHousE. There is currently no way of operating on the encrypted database because there is no mechanism to provide a password in the connection string. One could first unencrypt the database (in the application), perform the migration (using the library), and then re-encrypt (using the application) but this could be time-consuming if the database is large, and adds complexity to the migration logic and interruption recovery logic.
I read through the #401 and I do not agree with the justification for not doing it. I cannot think of another database where it is not possible to access the data contained within after providing a valid connection string. Tools like RoundHousE expect this.

There are other parameters that it would be nice to tweak in the connection string in the database migration scenario. For example, perhaps during a big migration I am less concerned with a computer failure and I would like to disable transaction logging to speed up the migration.

What about being able to include the PRAGMAs directly in the connection string and make it the responsibility of the SqliteConnection to execute them when Open is first called on the connection?

[bricelam]

I've been thinking more about ways to prevent people from shooting themselves in the foot, and I think using code like this (don't shoot me @ericsink) would cover most cases:

var providerName = typeof(SQLitePCL.raw)
    .GetField("_imp", BindingFlags.Static | BindingFlags.NonPublic)?.GetValue(null)
    .GetType().Name;
if (providerName == "SQLite3Provider_e_sqlite3")
{
    throw new InvalidOperationException(Resources.EncryptionNotSupported);
}

We could consider blocking more providers (e.g. winsqlite3), but chances are, if they're using an non-default provider, they know what they're doing.

[ericsink]

Oh my.

Dislike.

[bricelam]

lol, I don’t like it either. Just brainstorming about possibilities

[bricelam]

WIP branch: bricelam:keywords