-
Notifications
You must be signed in to change notification settings - Fork 568
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
Comments
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 |
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. |
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:
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. |
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:
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! :-) |
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 ofSQLiteException
would keep all existing client code unaffected, while allowing us to distinguish the bad-passphrase scenario better.The text was updated successfully, but these errors were encountered: