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

Signal-Desktop doesn't notice if db.sqlite is unencrypted #5245

Closed
primeos opened this issue May 11, 2021 · 11 comments
Closed

Signal-Desktop doesn't notice if db.sqlite is unencrypted #5245

primeos opened this issue May 11, 2021 · 11 comments

Comments

@primeos
Copy link

primeos commented May 11, 2021

This is one of the causes for #4513 (likely the main one) and was reported in #5097. I'm reopening this because the underlying issue is still not fixed. Signal-Desktop needs a way to determine if the DB is encrypted or not and refuse to launch if sqlite is loaded instead of sqlcipher. IIRC sqlcipher doesn't allow this yet but this clearly requires a solution.
(Or just drop the unnecessary encryption on Linux - I'd be happy with that as well.)

There are currently two issues:

  • When transitioning between an encrypted and unencrypted DB users will use all of their data (Database startup error: Error: SQLITE_NOTADB: file is not a database #4513).
    • This is super annoying for users!
  • Signal-Desktop wants the DB to be encrypted using sqlcipher but doesn't notice if the DB is unencrypted.
    • While I don't mind that this should be considered a security issue until Signal-Desktop switches from sqlcipher to sqlite.

Feedback from Linux users whether their DB is encrypted or not might also be nice.
In my case it's still unenecrypted:

$ grep PRETTY_NAME /etc/os-release
PRETTY_NAME="NixOS 21.05 (Okapi)"
$ file ~/.config/Signal/sql/db.sqlite
/home/michael/.config/Signal/sql/db.sqlite: SQLite 3.x database, user version 24, last written using SQLite version 3035002

PS: Sorry for the brevity but I need to cool down first as this caused a lot of unnecessary frustration and work for me as a downstream package maintainer for a Linux distribution.

@primeos
Copy link
Author

primeos commented May 12, 2021

I've just updated to Signal-Desktop and noticed the following console output:

updateSchema:
  Current user_version: 24;
  Most recent db schema: 27;
  SQLite version: 3.35.2;
  SQLCipher version: undefined;
  (deprecated) schema_version: 114;

So it seems like Signal-Desktop already has enough information to solve this problem.

After the update:

updateSchema:
  Current user_version: 27;
  Most recent db schema: 27;
  SQLite version: 3.35.2;
  SQLCipher version: undefined;
  (deprecated) schema_version: 160;

@EvanHahn-Signal
Copy link
Contributor

Sorry you're running into this.

To make sure I understand everything: you want Signal Desktop to verify that it's using SQLCipher and not SQLite. Is that right?

I'm also confused about why some databases would be unencrypted—ideally, all of the databases would be encrypted and would be unreadable by "vanilla" SQLite. Do some unofficial Linux packages swap SQLCipher for plain SQLite?

Let me know if there's some discussion I'm missing here.

@primeos
Copy link
Author

primeos commented May 12, 2021

To make sure I understand everything: you want Signal Desktop to verify that it's using SQLCipher and not SQLite. Is that right?

Yes. However, if I think more about it that adds a new issue as this might be difficult to avoid on (some) Linux setups/distributions. I guess it would be nice if there was also an environment variable (e.g. FORCE_UNENCRYPTED to reverse the behaviour - at least for a few releases until everyone has a properly working setup).

ideally, all of the databases would be encrypted

Yes, that should be the case. But e.g. in my case it's still unencrypted and Signal-Desktop doesn't detect that or at least doesn't complain about it.

and would be unreadable by "vanilla" SQLite.

This is the case and causes the very misleading Database startup error: Error: SQLITE_NOTADB: file is not a database error when transition from SQLCipher to SQLite and vice versa. The only choice a normal user has is to "Delete all data and restart" and then the new DB will be unencrypted (or encrypted in the reverse case).

This is the biggest issue as it causes data loss. Unfortunately it is tricky to avoid. So it might be best if Signal-Desktop simply detects this case and shows a more appropriate error message.

Do some unofficial Linux packages swap SQLCipher for plain SQLite?

The issue is that Signal-Desktop uses Electron and as a result it depends on a lot of system libraries. IIRC the issue was that Electron depends on GTK3 which depends on tracker3 which recently added a dependency on sqlite: https://gitlab.gnome.org/GNOME/tracker/-/issues/289

The result is that "enabling tracker3 in gtk3 [which is the default AFAIK] causes sqlite to be loaded in pretty much all gui processes"

https://gitlab.gnome.org/GNOME/tracker/-/issues/289 should point to all of the relevant information (sqlcipher/sqlcipher#385 might be most relevant as this is currently a SQLCipher limitation/issue).

The alternative (which I'd also be fine with) would be that Signal-Desktop stops using SQLCipher on Linux (not sure about other platforms but at least on Linux the encryption key (~/.config/Signal/config.json) is stored with the same permissions as the encrypted DB (~/.config/Signal/sql/db.sqlite) so I see little reason to encrypt the DB in the first case).

@primeos
Copy link
Author

primeos commented May 14, 2021

@EvanHahn-Signal this comment from a core SQLCipher developer is also relevant for Signal-Desktop: sqlcipher/sqlcipher#385 (comment)

That said, this issue can and should be handled downstream at the application level:

  1. While it may feel like a workaround, using LD_PRELOAD is a legitimate approach here because it will substitute the system SQLite with SQLCipher which is the intended usage model;
  2. Signal could statically link SQLCipher so that it cannot be substituted at runtime by another library; or
  3. The Signal application build could include its own ld version script when it builds SQLCipher from source as you proposed

[...]

We usually advise that applications using SQLCipher via dynamic linking perform a runtime check to verify that the expected libsqlcipher.so library is being used. Executing the PRAGMA cipher_version statement provides a simple way for an application to do this. SQLCipher will report its version and build type, which can be checked before use of the library. If a non-SQLCipher library is loaded instead the statement will return an empty result set, and the application can fail safely. This might be a good addition to Signal to help prevent unexpected behavior like this.

@ghost
Copy link

ghost commented May 17, 2021

@EvanHahn-Signal Just want to chime in that it would make sense to be able to force an unencrypted Signal database (using standard Sqlite) for many users such as myself. I already have my GNU/Linux computer configured for full disk encryption, so encrypting the database on top of that is unnecessary. The database only needs proper file permissions set so other programs can't access it. This would also solve all the issues related to SQLCipher causing crashes and unrecoverable corruption.

@EvanHahn-Signal
Copy link
Contributor

We use SQLCipher for two reasons:

  1. To encrypt the data at rest. Some users don't have full disk encryption, and this can (theoretically) help them.
  2. SQLCipher, unlike "vanilla" SQLite, deletes data from disk. If, for example, a message expires, we can ensure it's actually deleted instead of marking it deleted but not actually deleting it.

We'd really rather not maintain two separate database implementations (even if they are similar) as this increases our maintenance burden; in addition, we believe SQLite to be less secure.

Instead, we should figure out how to make SQLCipher work for everyone.

@ryannathans
Copy link

What's the point of encrypting the data at rest if the key is stored with the db?

@stale
Copy link

stale bot commented Oct 25, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 25, 2021
@primeos
Copy link
Author

primeos commented Oct 25, 2021

Not stale.

Also related: signalapp/better-sqlite3#3

@stale stale bot removed the stale label Oct 25, 2021
@stale
Copy link

stale bot commented Jan 23, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 23, 2022
@primeos
Copy link
Author

primeos commented Jan 23, 2022

Not stale and still an issue but with signalapp/better-sqlite3#3 merged an (accidentally) unencrypted DB shouldn't be possible anymore so a check is much less important now.

I guess it's best to simply close this issue (otherwise it'll remain being ignored anyway).

@primeos primeos closed this as completed Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants