Skip to content

Add dedicated SQLCipherDecryptionException #76

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

Closed
commonsguy opened this issue Nov 19, 2012 · 4 comments
Closed

Add dedicated SQLCipherDecryptionException #76

commonsguy opened this issue Nov 19, 2012 · 4 comments

Comments

@commonsguy
Copy link
Collaborator

Right now, trying to decrypt with an invalid passphrase results in a generic SQLiteException, making it difficult to distinguish between a bad passphase and some other type of error. We'd have to read into the text of the exception itself, which is not especially reliable.

Ideally, the "file is encrypted or is not a database" scenario would throw a dedicated exception that we could catch and handle differently. For example, a SQLCipherDecryptionException subclass of SQLiteException would keep all existing client code unaffected, while allowing us to distinguish the bad-passphrase scenario better.

@developernotes
Copy link
Member

Hi Mark,

This posses the risk of disguising other error conditions that in fact are not invalid passwords, but use the same error message as to not leak too much information. For example, a corrupt data file, a failed HMAC check, or opening a non database file could result in this same error message. The exception itself could be misleading with regard to the true error condition.

Nick Parker

@commonsguy
Copy link
Collaborator Author

I am not asking you to distinguish invalid passphrases from "a corrupt data file, a failed HMAC check, or opening a non database file". I am asking you to distinguish those cases from anything else. SQLiteException is used all over the place.

@developernotes
Copy link
Member

Hi Mark,

Thanks again for your feedback on the library, we do appreciate open communication and the more constructive edits and re-wording of your last comment.

We definitely understand the reasoning behind your request and appreciate the importance of making it easy to track down the source of an issue. However, addressing this request reliably is not as trivial as it may initially seem. I'd like to explain a few of the considerations, with the hope that it helps you understand our position.

While it would be easy to add specific exception classes to the Android library, it would be difficult to detect specific error conditions because the underlying SQLCipher library does not use any custom error codes internally. We have deliberately favored returning a single, standard, SQLite error code in the past in order to:

  1. Minimize the differences between SQLite core and SQLCipher and
  2. Avoid "leaking" information that might be exploited in a side channel attack on the software

For this reason all internal failures in the SQLCipher library are reported under the single SQLite error code, manifesting the "file is encrypted or is not a database" error you mentioned. Because differing error states don't exist in the underlying SQLCipher code, it would be very difficult to reliably produce custom exceptions int the wrapping java code. Furthermore, even if additional information was available from the underlying library, there are a multitude of situations where we could not accurately determine the source of an issue to report it. Note that we can't even reliably tell that a file is a valid database, only that given the various inputs, it could not be opened.

While custom errors aren't supported today by the underlying library, we'd concede that it may be possible to introduce a custom exception in the wrapper code opens or creates the database that would force a database operation immediately after setting the key, catch any error that occurs, and rethrow as a generic "SQLCipherException". However, we very much dislike this option because it would introduce inconsistent behavior. For instance, an HMAC failure in the first one or two pages might trigger a SQLCipherException when a database is opened, whereas an HMAC error elsewhere in the database would throw an SQLiteException. Likewise, a failure to open a main database would throw one exception, while a failure to attach a database would throw a completely different exception for the same exact error condition. These inconsistencies could lead to confusion or unexpected results.

Please let us know what you think.

@commonsguy
Copy link
Collaborator Author

I surrender.

However, as you contemplate changes to this project, or future projects, please take usability into account.

Your point #2 is perfect for cases where there are no ordinary people anywhere around. Once we start talking about real honest-to-goodness users, we have to consider the impacts that API decisions have on usability. If users get frustrated because developers treat invalid password attempts as meaning that the database is corrupt and the user has lost all their data, those same users will be less likely to use encryption in the future, seeing it as a source of difficulty. Securing some data against side-channel attacks, while causing more data to simply be left unencrypted to avoid perceived problems, is not an improvement in overall security.

If nothing else, I encourage you to augment the documentation to provide recommended practices for common user scenarios that developers are going to encounter. For example:

  • "If the user enters a bad passphrase, you will likely get exception X when calling Y() or Y2(), and our recommended response is to display error message Z and allow the user to retry their passphrase -- BTW, here is where you can contribute translations for Z to other languages"
  • "If you are upgrading an existing deployed app to use SQLCipher for Android, your user may already have a database with data in it that you and the user want to retain. Here is the set of recommended practices for how you determine that you have an unencrypted database (rather than one you encrypted previously, or a missing database, since our SQLiteOpenHelper is a literal translation of Google's and is not really designed for this situation). Here is the set of recommended practices for how you then encrypt that database in situ so the user does not lose their data."

I, and perhaps others, will wind up attempting to document these practices, but we are not SQLCipher for Android experts, and therefore are likely to screw this up. Obi-Nick, you're our only hope! :-)

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

No branches or pull requests

2 participants