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

Add support for SQLCipher #1109

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

jgiannuzzi
Copy link

@jgiannuzzi jgiannuzzi commented Nov 10, 2022

This PR adds support for SQLCipher whilst keeping the existing embedded sqlite3 implementation. The support is enabled via the sqlcipher and libsqlcipher build tags, which respectively use the embedded SQLCipher implementation or link directly to libsqlcipher.

An upgrade script has been added to automate the inclusion of new versions of SQLCipher.

I believe that adding this support would benefit the Go ecosystem, as multiple unmaintained repos have flourished that are forks of this repo bolting on support for older versions of SQLCipher. Having support here would enable gorm users to benefit from the encryption feature without having to patch anything.

@imkos
Copy link

imkos commented Nov 23, 2022

tried it, it works well.

@7fold
Copy link

7fold commented Nov 24, 2022

@jgiannuzzi I tried it as well, and it works flawlessly! Thank you for the tremendous job.
I was wondering, wouldn't it be more beneficial to use SQLite3MultipleCiphers?

@jgiannuzzi
Copy link
Author

@7fold I hadn't heard of SQLite3MultipleCiphers before. How well supported is it compared to SQLCipher?
As it's using an amalgamation too, it shouldn't be too much work to add it or replace SQLCipher with it in my branch.

@mattn what are your thoughts on this?

@7fold
Copy link

7fold commented Nov 24, 2022

@jgiannuzzi It is maintained pretty well, and updates are released almost every month. But honestly, I'm not sure how good it is compared to SQLCipher. It's also used by another Node.js DB library - better-sqlite3-multiple-ciphers.

@mattn
Copy link
Owner

mattn commented Jan 24, 2023

@rittneje I'm okay to add this feature into go-sqlite3. any thought?

@rittneje
Copy link
Collaborator

It seems not great to force people to download 2 gigantic C files when they only ever need (at most) one, especially in CI/CD cases.

I wonder if there is a way to treat this more like libsqlite3, where we can somehow link against an external C library? Then only people that care about SQLCipher (which seems pretty niche, given typical use cases for SQLite) have to download the second giant C file. (And that can even be in a totally separate repo.)

@damoclark
Copy link

Hi @rittneje and @mattn

As an alternate perspective, might 'perfect' be the enemy of 'good' here?

Perhaps @jgiannuzzi 's patch could be a stepping stone to something more optimal in the future, if the additional large C file does become problematic?

@rittneje
Copy link
Collaborator

@damoclark It would have to be supported forever because of backwards compatibility.

@damoclark
Copy link

@rittneje Supported yes, but that doesn't mean the implementation can't change. Like changing from the sqlite preferred amalgamation method for sqlcipher to linking with an external library as you suggested.

Or am I completely misunderstanding the build process proposed?

@afazzdev
Copy link

This is definitely what i was looking for. Hopefully it will soon to be merged. Thanks for your hardwork!

@renathoaz
Copy link

We really need encryption feature. will we have it any time soon?

@jgiannuzzi
Copy link
Author

I wonder if there is a way to treat this more like libsqlite3, where we can somehow link against an external C library? Then only people that care about SQLCipher (which seems pretty niche, given typical use cases for SQLite) have to download the second giant C file. (And that can even be in a totally separate repo.)

@rittneje This is already possible in my proposed change (libsqlcipher tag) — however the big disadvantage is that anyone needing this feature would then need to manually compile SQLCipher with the features they need, as opposed to having it all nicely integrated.

As @7fold suggested above, another way to go may be to use SQLite3MultipleCiphers which seems really well-maintained, supports more ciphers than SQLCipher (whilst being compatible with it), does not depend on a separate crypto library, and seems to be consistently keeping up with new SQLite3 releases. I would be happy to update this PR or create another one where the SQLite3MC amalgamation would replace the SQLite3 one.

Thoughts?

@jgiannuzzi
Copy link
Author

I have created a branch that supports SQLite3MultipleCiphers: jgiannuzzi:sqlite3mc

@gainskills
Copy link

gainskills commented Mar 30, 2023

We really need encryption feature. will we have it any time soon?

+1

thanks for jgiannuzzi for the branch.
in case in need to run your app with the branch, run

go mod edit -replace="github.com/mattn/go-sqlite3=github.com/jgiannuzzi/go-sqlite3@sqlite3mc"
if you want to browser the DB by DB Browser for SQLite with SQLCipher 4 default like me, add the following params: _cipher=sqlcipher&_legacy=4&_key=mydbkey

Reference:

https://github.com/jgiannuzzi/go-sqlite3/tree/sqlite3mc
utelle/SQLite3MultipleCiphers#47 (comment)
https://stackoverflow.com/a/56792766/2701959

@jgiannuzzi
Copy link
Author

I have created a branch that supports SQLite3MultipleCiphers: jgiannuzzi:sqlite3mc

Hey @rittneje and @mattn, what are your thoughts on this?

I would really like to add encryption support to go-sqlite3 and it looks like there is a bit of demand for it based on the various issues created over time and the reactions to this PR. How can we move forward with this?

@rittneje
Copy link
Collaborator

rittneje commented Apr 15, 2023

@jgiannuzzi The debate over SQLCipher vs. SQLite3MultipleCiphers only solidifies my earlier point - I do not believe it belongs in this library. Really there is a generic desire to allow linking against a custom/external build of SQLite. (See also #1153 #1150.) At the moment I am not sure how to make that work well in the confines of cgo. Is that something you'd be able to look into? I assume with that in place there would be no need of forking anymore.

@jgiannuzzi
Copy link
Author

Hey @rittneje and @mattn, would you mind taking a second look at my SQLite3MultipleCiphers branch? Here is a comparison to help you see what I changed.

This should solve the issue you had with the 2 "gigantic" C files, as this essentially augments the original SQLite implementation. SQLite3MultipleCiphers has a great track record of keeping up-to-date with SQLite and has a very clean architecture that aims at minimising patching.

I would really like to avoid making users link to a custom build or having to use a fork of go-sqlite3, especially for a feature that seems quite popular!

@ellermister
Copy link

ellermister commented Jul 20, 2023

@jgiannuzzi @mattn
The branch of jgiannuzzi is very easy to use. It solves the problem that I use sqlite+sqlcipher.

I used the branch of github.com/mutecomm/go-sqlcipher before, but its new version does not support enough, and the database cannot be recognized.

That old version has the problem of memory overflow when executing large-volume DB files.

Those above problems were solved by @jgiannuzzi, thanks a lot.

I also think that sqlcipher should be part of the sqlite base library. Developers can easily read and write encrypted databases without looking for other solutions.

Using this branch, I successfully connected to the Wechat client database, and share the method with you:

go.mod

require (
	github.com/mattn/go-sqlite3 v1.14.17

replace github.com/mattn/go-sqlite3 => github.com/jgiannuzzi/go-sqlite3 v1.14.17-0.20230719111531-6e53453ccbd3

db.go

func ConnectDB(path string, key string) *sql.DB {
	key = url.QueryEscape(key)
	dbname := fmt.Sprintf("%s?_cipher=sqlcipher&_legacy=3&_hmac_use=off&_kdf_iter=4000&_legacy_page_size=1024&_key=%s", path, key)
	db, err := sql.Open("sqlite3", dbname)
	if err != nil {
		log.Fatalf("Open Error %v\n", err)
	}
	return db
}

Hope to merge as soon as possible, there is no need to replace package

@KiddoV
Copy link

KiddoV commented Oct 11, 2023

How is this haven't merge to the master brand?

AnalogJ added a commit to fastenhealth/fasten-onprem that referenced this pull request Oct 18, 2023
Migrated from github.com/glebarez/sqlite to gorm.io/driver/sqlite (which uses github.com/mattn/go-sqlite3)

Override github.com/mattn/go-sqlite3 with forked version from @jgiannuzzi which supports Encryption at rest.
See mattn/go-sqlite3#1109
See #236

Added documentation for how to open an encrypted sqlite database in IntelliJ - CONTRIBUTING.md update
@AnalogJ
Copy link

AnalogJ commented Oct 19, 2023

I'd like to thank @jgiannuzzi for his hard work, and confirm that his branch works as a drop-in replacement for mattn/go-sqlite3 when used with Gorm.

@gainskills and @ellermister's DSN string example was also incredibly helpful.


Baking encryption-at-rest into this library would be an incredibly powerful feature. As someone who's built a number of apps ontop of SQLite, 50% or more would have benefited from the additional security, and my most recent tool - Fasten Health requires it since its a open-source personal health record with access to all your private medical information.

Pointing to the sqlite.org source-code and saying "see, they don't bake in the SQLite Encryption Extension, so we shouldn't either" ignores the fact that they have artificially segmented their user base to sell an add-on in order to build a sustainable business -- which is completely within their rights.

It doesn't mean that we need to live under the same handicap with this port of SQLite.
There are a number of issues , across a variety of similar Golang SQLite libraries, asking for encryption-at-rest support. Also, the active communities around https://github.com/sqlcipher/sqlcipher (5.5k stars) and https://github.com/utelle/SQLite3MultipleCiphers (250+ stars) should give you a sense of how important this is to some SQLite users. Given that mattn/go-sqlite3 is the de-facto implementation for Golang, I think it would be a very well received addition.

Personally I migrated to mattn/go-sqlite3 only because of @jgiannuzzi -- and just the possibility of this PR getting merged.

AnalogJ added a commit to fastenhealth/fasten-onprem that referenced this pull request Nov 7, 2023
Migrated from github.com/glebarez/sqlite to gorm.io/driver/sqlite (which uses github.com/mattn/go-sqlite3)

Override github.com/mattn/go-sqlite3 with forked version from @jgiannuzzi which supports Encryption at rest.
See mattn/go-sqlite3#1109
See #236

Added documentation for how to open an encrypted sqlite database in IntelliJ - CONTRIBUTING.md update
@FabioRodrigues
Copy link

Thank you @jgiannuzzi for you effort. I really appreciate it. Gonna use your branch for my use-case 🚀

@jgiannuzzi
Copy link
Author

jgiannuzzi commented Jan 22, 2024

For anyone interested in my sqlite3mc branch, I have just updated it to SQLite3MultipleCipher 1.8.2 / SQLite3 3.45.0.
The replace directive now looks like this:

replace github.com/mattn/go-sqlite3 => github.com/jgiannuzzi/go-sqlite3 v1.14.17-0.20240122133042-fb824c8e339e

@javea7171
Copy link

Please merge, this is a big win for Go developers using sqlite

@jt-wang
Copy link

jt-wang commented Mar 8, 2024

this is one of the most important features that will definitely benefit so many apps, services, developers, and users forward. thanks to @jgiannuzzi for this! Encryption, though might not be considered that important in decades ago, has been indispensable now, especially for database on the cloud.

It would be undoubtedly great if this could be merged. Thanks! @mattn

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

Successfully merging this pull request may close these issues.

None yet