Add support for using sqlcipher

[thomcc]

This adds a sqlcipher feature (which is incompatible with the bundled_sqlite3) support for opening a Store with a key, changing the key of an open Store, and support for it through the CLI and (in theory) the FFI. It's based some on @mhammond's branch, and on debugging we did on Wednesday.

The additional APIs aren't behind cfg(feature = "sqlcipher") mostly because it seems like that's a big pain to use and requires callers add a lot of similar cfg's. Instead, we emit a Result::Err when you try to do something that would require sqlcipher when we weren't build with support for it. I could easily be convinced we should do things the other way.

It's easy-ish to try on mac, since you can brew install sqlcipher --with-fts, then cargo build --no-default-features --features sqlcipher. See caveat number 1 about building/running the cli or building the ffi, though (needs to be done from the directory in question due to a cargo bug). On other platforms this is trickier.

Some caveats:

1. Until New behavior of --feature + --package combination rust-lang/cargo#5364 is fixed, building nested crates that depend on mentat itself (e.g. tools/cli and ffi) must be done inside the directory of those crates. Specifically cargo build --no-default-features --features sqlcipher -p mentat_cli will apply the --features and --no-default-features flags to the wrong package. cd tools/cli; cargo build --no-default-features --features sqlcipher; cd '-'; works instead. Ditto substituting tools/cli with ffi. Yes, this is annoying.

2. PRAGMA page_size doesn't seem to be valid in all cases when used in conjunction with PRAGMA cipher_page_size. I can't find official documentation on this. What I have found is:

   • Specifying page_size but not cipher_page_size seems never to be valid.
   • Specifying them both seems only to work if cipher_page_size is a positive integer multiple of page_size (I didn't check this very thoroughly though).
   • Someone on the freenode sqlite IRC channel suggested that page_size is not used with sqlcipher, however this seems to be false given the above (perhaps they meant it should not be used, though? IDK).
   • Most importantly, unlike page_size, the cipher_page_size is not simply an optimization detail! Having the correct cipher_page_size is required to read the contents of a database, e.g. if you open a DB with cipher_page_size at 8192 (for example) that's the value it must always have in order for reads/writes to be successful.

   I set it to the value we were using for page_size before, and made the code use both cipher_page_size and page_size when using sqlcipher. It's not at all clear to me this was the right choice, though.

3. There are no tests outside the CLI parsing changes. I'm open to suggestions about how best to test this, though. Maybe adding an encrypted db in fixtures and making sure that we can open it would be enough?

[rnewman]

I have relatively strong feelings that a misconfiguration — trying to build a key-using Mentat application against a non-crypto Mentat — should fail to build or panic at launch (the behavior that will result with bad run-time linking) rather than yielding a runtime error; after all, we have a wonderful compiler to help us, and the consequences of accidentally muffling the error (especially if someone writes expedient fallback code!) are potentially severe.

I don’t think it’s too onerous to specify a cipher feature when importing Mentat; indeed, we already expose a bundling feature that we pass through to rusqlite and that we expect real-world callers to have to think about. (Firefox will specify non-bundled because SQLite is already linked into libxul.)

An additional layer of runtime checks, as rusqlite does for some features, seems like a good option to handle the dynamic linking risk in a descriptive way.

[thomcc]

I have relatively strong feelings that a misconfiguration — trying to build a key-using Mentat application against a non-crypto Mentat — should fail to build...

I can get a patch up for this relatively easily because I did it this way first, before deciding that it was annoying. My concerns are basically:

1. We're exposing a different ABI depending on compile time options. This is often considered somewhat dubious in C code because it means anybody who wants to conditionally call those functions needs to do either require the feature be present, or get the symbol via dlopen.
2. The first usage code i wrote for it was in the CLI, where you'd like to still an error if a user tries to do the unsupported thing. That said, it's probably not a good representation of user code.

Neither of these may actually matter, though.

An additional layer of runtime checks, as rusqlite does for some features, seems like a good option to handle the dynamic linking risk in a descriptive way.

I'm not sure how to check this at runtime in a way that gives a descriptive result and isn't sort of disgusting. I can think of:

1. Making a dummy database, and decrypting it with the wrong key.
2. Using dlopen and dlsym equivalents: I don't think we know we'll always dynamically link against sqlcipher though. The common case is probably static (complete guess without any evidence).
3. #[cfg(feature = "sqlcipher")] extern "C" fn sqlite3_key(...) in libsqlite3_sys or something similar. The main difficulty here is integrating with their build system, but maybe we could just -DSQLITE_HAS_CODEC for bindgen or something? IDK.
4. If getting it into libsqlite3_sys is a problem, hacky solutions like https://gist.github.com/thomcc/95cb28cf3dd08f3e01a4cddc06205d61 exist, but. I suspect this needs some extra flags on #[link] in order to make it work in the dynamically linked case but don't know.

It's likely that I'm either unaware of or forgetting a substantially easier way to do this. (Ideally there would be a way that just requires running SQL!)

[ncalexan]

I prefer the feature gunk, even if it is onerous, so let's fold that in to the landing.

This looked exactly as I expected, so I just have a few nits and minor naming requests.

A test fixture to ensure we don't regress loading and a simple query would go a long way. Somewhere in the db crate is most appropriate. Can we create in-memory encrypted databases? (I assume so.) An additional test that runs over an in-memory DB would be appreciated too.

Please reference #118 (close it, if appropriate) in your landing.

I'll add you as a collaborator so you can commit to master.

Thanks, Thom!

[ncalexan]

it is a little ambiguous: maybe "database key provided, but sqlcipher not enabled in sqlite library"?

[ncalexan]

nit: can we always use encryption_key, for names and parameters? "key" can mean many things; let's make grep do work for ourselves in the future.

[ncalexan]

This block might be more pleasant as a match.

[ncalexan]

"builds" could refer to Mentat or to the underlying SQLite library. Perhaps, ".*_encrypted require the sqlcipher Mentat feature."

[thomcc]

Whelp, I have no clue how to get travis working with sqlcipher. AFAICT the version of ubuntu on travis means that apt-get install libsqlcipher-dev is installing a version from 2013, which means it's too old for the sqlite that mentat needs.

For now I've abandoned testing on travis, and will file a bug about that.

[ncalexan]

Just FYI, this doesn't build with stable Rust. I'll fix and push follow-up.

[ncalexan]

Just FYI, this doesn't build with stable Rust. I'll fix and push follow-up.

Sorry, with Rust 1.25:

rustc 1.25.0 (84203cac6 2018-03-25)

Our minimum version is set in

mentat/build/version.rs

Line 22 in 6a1a265

static MIN_VERSION: &'static str = "1.25.0";

[ncalexan]

Just FYI, this doesn't build with stable Rust. I'll fix and push follow-up.

I've addressed this in f041dfe.