Use anyhow's with_context

Otherwise you do not know which account needs unlocking if the
selected account is locked.

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Changes
 - refactorings #3026
 - move messages in batches #3058
+- working with encrypted storage drastically changed to be RAII #3063
 
 ### Fixes
 - avoid archived, fresh chats #3053

deltachat-ffi/deltachat.h

@@ -179,10 +179,12 @@ typedef struct _dc_accounts_event_emitter dc_accounts_event_emitter_t;
 // create/open/config/information
 
 /**
- * Create a new context object and try to open it without passphrase. If
- * database is encrypted, the result is the same as using
- * dc_context_new_closed() and the database should be opened with
- * dc_context_open() before using.
+ * Create a new context object and try to open it without passphrase.
+ *
+ * If database is encrypted NULL is returned, encrypted databases should be
+ * opened using dc_context_new_encrypted().
+ *
+ * If the database does not yet exist a new one will be created.
  *
  * @memberof dc_context_t
  * @param os_name Deprecated, pass NULL or empty string here.
@@ -192,54 +194,34 @@ typedef struct _dc_accounts_event_emitter dc_accounts_event_emitter_t;
  * @return A context object with some public members.
  *     The object must be passed to the other context functions
  *     and must be freed using dc_context_unref() after usage.
+ *     On failure NULL is returned.
+ *
+ * If you want to use multiple context objects at the same time,
+ * this can be managed using dc_accounts_t.
  */
 dc_context_t*   dc_context_new               (const char* os_name, const char* dbfile, const char* blobdir);
 
 
 /**
- * Create a new context object.  After creation it is usually opened with
- * dc_context_open() and started with dc_start_io() so it is connected and
- * mails are fetched.
+ * Create a new context object and try to open it with a passphrase.
+ *
+ * If the database does not yet exist a new one will be created.  If the
+ * database is not encrypted this will fail.
  *
  * @memberof dc_context_t
  * @param dbfile The file to use to store the database,
  *     something like `~/file` won't work, use absolute paths.
+ * @param passphrase The passphrase to use.  This MUST be non-NULL and MUST be valid
+ *     UTF-8, if either of this is not true this fails and NULL is retruned.
  * @return A context object with some public members.
  *     The object must be passed to the other context functions
  *     and must be freed using dc_context_unref() after usage.
+ *     On failure NULL is returned.
  *
  * If you want to use multiple context objects at the same time,
  * this can be managed using dc_accounts_t.
  */
-dc_context_t*   dc_context_new_closed        (const char* dbfile);
-
-
-/**
- * Opens the database with the given passphrase. This can only be used on
- * closed context, such as created by dc_context_new_closed(). If the database
- * is new, this operation sets the database passphrase. For existing databases
- * the passphrase should be the one used to encrypt the database the first
- * time.
- *
- * @memberof dc_context_t
- * @param context The context object.
- * @param passphrase The passphrase to use with the database. Pass NULL or
- * empty string to use no passphrase and no encryption.
- * @return 1 if the database is opened with this passphrase, 0 if the
- * passphrase is incorrect and on error.
- */
-int             dc_context_open              (dc_context_t *context, const char* passphrase);
-
-
-/**
- * Returns 1 if database is open.
- *
- * @memberof dc_context_t
- * @param context The context object.
- * @return 1 if database is open, 0 if database is closed
- */
-int             dc_context_is_open           (dc_context_t *context);
-
+dc_context_t*   dc_context_new_encrypted   (const char* dbfile, const char* passphrase);
 
 /**
  * Free a context object.
@@ -2637,21 +2619,22 @@ void           dc_accounts_unref                (dc_accounts_t* accounts);
 uint32_t       dc_accounts_add_account          (dc_accounts_t* accounts);
 
 /**
- * Add a new closed account to the account manager.
- * Internally, dc_context_new_closed() is called using a unique database-name
- * in the directory specified at dc_accounts_new().
+ * Add a new account to the account manager, encrypted with a passphrase.
  *
- * If the function succeeds,
- * dc_accounts_get_all() will return one more account
- * and you can access the newly created account using dc_accounts_get_account().
- * Moreover, the newly created account will be the selected one.
+ * The account's database will be encrypted with a key derived from the
+ * passphrase and will need to be opened with the passphrase to be usable.
+ *
+ * Encrypted accounts are not automatically opened when the account
+ * manager is created.  They need to be opened using dc_accounts_load_encrypted(),
+ * to find unloaded encrypted accounts use dc_accounts_get_encrypted().
  *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
- * @return Account-id, use dc_accounts_get_account() to get the context object.
+ * @return Account-id, use dc_accounts_get_encrypted_account() to get the
+ *     context object.
  *     On errors, 0 is returned.
  */
-uint32_t       dc_accounts_add_closed_account   (dc_accounts_t* accounts);
+uint32_t       dc_accounts_add_encrypted_account(dc_accounts_t* accounts, char* passphrase);
 
 /**
  * Migrate independent accounts into accounts managed by the account manager.
@@ -2687,6 +2670,10 @@ int            dc_accounts_remove_account       (dc_accounts_t* accounts, uint32
 /**
  * List all accounts.
  *
+ * Only lists loaded accounts, some accounts might be encrypted and need to be
+ * explicitly loaded using dc_accounts_get_encrypted() and
+ * dc_accounts_load_encrypted().
+ *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
  * @return An array containing all account-ids,
@@ -2694,10 +2681,27 @@ int            dc_accounts_remove_account       (dc_accounts_t* accounts, uint32
  */
 dc_array_t*    dc_accounts_get_all              (dc_accounts_t* accounts);
 
+/**
+ * Lists encrypted accounts.
+ *
+ * This returns all *locked* encrypted accounts which must be activated using
+ * dc_accounts_load_encrypted().  Once an encrypted account has been loaded it will
+ * show up in dc_accounts_get_all() and can be retrieved using
+ * dc_accounts_get_account().
+ *
+ * @memberof dc_accounts_t
+ * @param accounts Account manager as created by dc_accounts_new().
+ * @return An array containing account-ids, use dc_array_get_id() to get the ids.
+ */
+dc_array_t*    dc_accounts_get_encrypted        (dc_accounts_t* accounts);
 
 /**
  * Get an account-context from an account-id.
  *
+ * Only accounts returned by dc_accounts_get_all() can be retrieved with this
+ * function, encrypted accounts first need to be loaded using
+ * dc_accounts_load_encrypted().
+ *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
  * @param account_id The account-id as returned e.g. by dc_accounts_get_all() or dc_accounts_add_account().
@@ -2708,9 +2712,30 @@ dc_array_t*    dc_accounts_get_all              (dc_accounts_t* accounts);
  */
 dc_context_t*  dc_accounts_get_account          (dc_accounts_t* accounts, uint32_t account_id);
 
+/**
+ * Loads an encrypted account into the Account manager.
+ *
+ * Once the account has been loaded you can also access it using
+ * dc_accounts_get_account() like any other account.  Use *dc_accounts_get_encrypted()
+ * to find accounts which need to be loaded like this.
+ *
+ * @memberof dc_accounts_t
+ * @param accounts Account manager as created by dc_accounts_new().
+ * @param account_id The account-id as returned e.g. by dc_accounts_get_encrypted(),
+ *     dc_accounts_add_encrypted_account().
+ * @param passphrase The passphrase to decrypt the account with.
+ * @return The account-context, this can be used most similar as a normal,
+ *     unmanaged account-context as created by dc_context_new().
+ *     Once you do no longer need the context-object, you have to call dc_context_unref() on it,
+ *     which, however, will not close the account but only decrease a reference counter.
+ *     On failure NULL is returned.
+ */
+dc_context_t*  dc_accounts_load_encrypted (dc_accounts_t* accounts, uint32_t account_id, char* passphrase);
+
 
 /**
- * Get the currently selected account.
+ * Get the currently selected account ID.
+ *
  * If there is at least one account in the account-manager,
  * there is always a selected one.
  * To change the selected account, use dc_accounts_select_account();
@@ -2718,13 +2743,15 @@ dc_context_t*  dc_accounts_get_account          (dc_accounts_t* accounts, uint32
  *
  * @memberof dc_accounts_t
  * @param accounts Account manager as created by dc_accounts_new().
- * @return The account-context, this can be used most similar as a normal,
- *     unmanaged account-context as created by dc_context_new().
- *     Once you do no longer need the context-object, you have to call dc_context_unref() on it,
- *     which, however, will not close the account but only decrease a reference counter.
- *     If there is no selected account, NULL is returned.
+ * @return The account ID of the selected account.  The context of
+ *     the selected account can then be retrieved using dc_accounts_get_account().
+ *     Note that if the selected account is encrypted you may first have to
+ *     load it using dc_accounts_load_encrypted(), you can verify if this is needed
+ *     using dc_accounts_get_encrypted().
+ *     If no account is selected 0 is returned.  However this is only possible if
+ *     there is no single account in the account manager.
  */
-dc_context_t*  dc_accounts_get_selected_account (dc_accounts_t* accounts);
+uint32_t*  dc_accounts_get_selected_account_id (dc_accounts_t* accounts);
 
 
 /**

deltachat-ffi/src/lib.rs

@@ -16,6 +16,7 @@ extern crate serde_json;
 
 use std::collections::BTreeMap;
 use std::convert::TryFrom;
+use std::ffi::CStr;
 use std::fmt::Write;
 use std::ops::Deref;
 use std::ptr;
@@ -94,20 +95,33 @@ pub unsafe extern "C" fn dc_context_new(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_context_new_closed(dbfile: *const libc::c_char) -> *mut dc_context_t {
+pub unsafe extern "C" fn dc_context_new_encrypted(
+    dbfile: *const libc::c_char,
+    passphrase: *const libc::c_char,
+) -> *mut dc_context_t {
     setup_panic!();
-
-    if dbfile.is_null() {
-        eprintln!("ignoring careless call to dc_context_new_closed()");
+    if dbfile.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_context_new_encrypted()");
         return ptr::null_mut();
     }
-
+    // Generate random ID if dc_accounts_t is not used.
     let id = rand::thread_rng().gen();
-    match block_on(Context::new_closed(
-        as_path(dbfile).to_path_buf().into(),
+    let dbfile = as_path(dbfile).to_path_buf();
+    let cstr = CStr::from_ptr(passphrase);
+    let passphrase = match cstr.to_str() {
+        Ok(s) => s,
+        Err(err) => {
+            eprintln!("passphrase was not UTF-8: {:#}", err);
+            return ptr::null_mut();
+        }
+    };
+    let ctx = block_on(Context::new_encrypted(
+        dbfile.into(),
         id,
-    )) {
-        Ok(context) => Box::into_raw(Box::new(context)),
+        passphrase.to_string(),
+    ));
+    match ctx {
+        Ok(ctx) => Box::into_raw(Box::new(ctx)),
         Err(err) => {
             eprintln!("failed to create context: {:#}", err);
             ptr::null_mut()
@@ -115,35 +129,6 @@ pub unsafe extern "C" fn dc_context_new_closed(dbfile: *const libc::c_char) -> *
     }
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn dc_context_open(
-    context: *mut dc_context_t,
-    passphrase: *const libc::c_char,
-) -> libc::c_int {
-    if context.is_null() {
-        eprintln!("ignoring careless call to dc_context_open()");
-        return 0;
-    }
-
-    let ctx = &*context;
-    let passphrase = to_string_lossy(passphrase);
-    block_on(ctx.open(passphrase))
-        .log_err(ctx, "dc_context_open() failed")
-        .map(|b| b as libc::c_int)
-        .unwrap_or(0)
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn dc_context_is_open(context: *mut dc_context_t) -> libc::c_int {
-    if context.is_null() {
-        eprintln!("ignoring careless call to dc_context_is_open()");
-        return 0;
-    }
-
-    let ctx = &*context;
-    block_on(ctx.is_open()) as libc::c_int
-}
-
 /// Release the context structure.
 ///
 /// This function releases the memory of the `dc_context_t` structure.
@@ -4083,18 +4068,17 @@ pub unsafe extern "C" fn dc_accounts_get_account(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_accounts_get_selected_account(
+pub unsafe extern "C" fn dc_accounts_get_selected_account_id(
     accounts: *mut dc_accounts_t,
-) -> *mut dc_context_t {
+) -> u32 {
     if accounts.is_null() {
         eprintln!("ignoring careless call to dc_accounts_get_selected_account()");
-        return ptr::null_mut();
+        return 0;
     }
 
     let accounts = &*accounts;
-    block_on(async move { accounts.read().await.get_selected_account().await })
-        .map(|ctx| Box::into_raw(Box::new(ctx)))
-        .unwrap_or_else(std::ptr::null_mut)
+    block_on(async move { accounts.read().await.get_selected_account_id().await })
+        .unwrap_or(0)
 }
 
 #[no_mangle]
@@ -4148,17 +4132,29 @@ pub unsafe extern "C" fn dc_accounts_add_account(accounts: *mut dc_accounts_t) -
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn dc_accounts_add_closed_account(accounts: *mut dc_accounts_t) -> u32 {
-    if accounts.is_null() {
-        eprintln!("ignoring careless call to dc_accounts_add_closed_account()");
+pub unsafe extern "C" fn dc_accounts_add_encrypted_account(
+    accounts: *mut dc_accounts_t,
+    passphrase: *const libc::c_char,
+) -> u32 {
+    if accounts.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_accounts_add_with_password()");
         return 0;
     }
-
-    let accounts = &mut *accounts;
-
+    let accounts: &AccountsWrapper = &mut *accounts;
+    let cstr = CStr::from_ptr(passphrase);
     block_on(async move {
         let mut accounts = accounts.write().await;
-        match accounts.add_closed_account().await {
+        let passphrase = match cstr.to_str() {
+            Ok(s) => s,
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Passphrase was not UTF-8: {:#}",
+                    err
+                )));
+                return 0;
+            }
+        };
+        match accounts.add_encrypted_account(passphrase.to_string()).await {
             Ok(id) => id,
             Err(err) => {
                 accounts.emit_event(EventType::Error(format!(
@@ -4243,6 +4239,60 @@ pub unsafe extern "C" fn dc_accounts_get_all(accounts: *mut dc_accounts_t) -> *m
     Box::into_raw(Box::new(array))
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn dc_accounts_get_encrypted(
+    accounts: *mut dc_accounts_t,
+) -> *mut dc_array_t {
+    if accounts.is_null() {
+        eprintln!("ignoring careless call to dc_accounts_get_all()");
+        return ptr::null_mut();
+    }
+    let accounts: &AccountsWrapper = &*accounts;
+    let list = block_on(async move { accounts.read().await.get_encrypted() });
+    let array: dc_array_t = list.into();
+    Box::into_raw(Box::new(array))
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn dc_accounts_load_encrypted(
+    accounts: *mut dc_accounts_t,
+    account_id: u32,
+    passphrase: *const libc::c_char,
+) -> *mut dc_context_t {
+    if accounts.is_null() || passphrase.is_null() {
+        eprintln!("ignoring careless call to dc_context_new_encrypted()");
+        return ptr::null_mut();
+    }
+    let accounts: &AccountsWrapper = &*accounts;
+    let cstr = CStr::from_ptr(passphrase);
+    block_on(async move {
+        let mut accounts = accounts.write().await;
+        let passphrase = match cstr.to_str() {
+            Ok(s) => s,
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Passphrase was not UTF-8: {:#}",
+                    err
+                )));
+                return ptr::null_mut();
+            }
+        };
+        match accounts
+            .load_encrypted_account(account_id, passphrase.to_string())
+            .await
+        {
+            Ok(ctx) => Box::into_raw(Box::new(ctx)),
+            Err(err) => {
+                accounts.emit_event(EventType::Error(format!(
+                    "Failed to load encrypted account: {:#}",
+                    err
+                )));
+                ptr::null_mut()
+            }
+        }
+    })
+}
+
 #[no_mangle]
 pub unsafe extern "C" fn dc_accounts_all_work_done(accounts: *mut dc_accounts_t) -> libc::c_int {
     if accounts.is_null() {

src/accounts.rs

@@ -1,6 +1,6 @@
 //! # Account manager module.
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 use async_std::channel::{self, Receiver, Sender};
 use async_std::fs;
@@ -12,7 +12,7 @@ use uuid::Uuid;
 use anyhow::{ensure, Context as _, Result};
 use serde::{Deserialize, Serialize};
 
-use crate::context::Context;
+use crate::context::{Context, ContextError};
 use crate::events::{Event, EventType, Events};
 
 /// Account manager, that can handle multiple accounts in a single place.
@@ -50,7 +50,7 @@ impl Accounts {
 
     /// Opens an existing accounts structure. Will error if the folder doesn't exist,
     /// no account exists and no config exists.
-    pub async fn open(dir: PathBuf) -> Result<Self> {
+    async fn open(dir: PathBuf) -> Result<Self> {
         ensure!(dir.exists().await, "directory does not exist");
 
         let config_file = dir.join(CONFIG_NAME);
@@ -91,6 +91,9 @@ impl Accounts {
     }
 
     /// Get the currently selected account.
+    ///
+    /// If the selected account is encrypted and not yet loaded using
+    /// [`Accounts::load_encrypted_account`] `None` will be returned.
     pub async fn get_selected_account(&self) -> Option<Context> {
         let id = self.config.get_selected_account().await;
         self.accounts.get(&id).cloned()
@@ -124,17 +127,41 @@ impl Accounts {
         Ok(account_config.id)
     }
 
-    /// Adds a new closed account.
-    pub async fn add_closed_account(&mut self) -> Result<u32> {
+    /// Adds an new encrypted account and opens it.
+    ///
+    /// Creates a new account with encrypted database using the provided password.  Returns
+    /// the account ID of the opened account.
+    pub async fn add_encrypted_account(&mut self, passphrase: String) -> Result<u32> {
         let account_config = self.config.new_account(&self.dir).await?;
-
-        let ctx = Context::new_closed(account_config.dbfile().into(), account_config.id).await?;
+        let ctx = Context::new_encrypted(
+            account_config.dbfile().into(),
+            account_config.id,
+            passphrase,
+        )
+        .await?;
         self.emitter.add_account(&ctx).await?;
         self.accounts.insert(account_config.id, ctx);
-
         Ok(account_config.id)
     }
 
+    /// Decrypts and open an existing account.
+    pub async fn load_encrypted_account(&mut self, id: u32, passphrase: String) -> Result<Context> {
+        let account_config = self
+            .config
+            .get_account(id)
+            .await
+            .with_context(|| format!("No such account with id {}", id))?;
+        let ctx = Context::new_encrypted(
+            account_config.dbfile().into(),
+            account_config.id,
+            passphrase,
+        )
+        .await?;
+        self.emitter.add_account(&ctx).await?;
+        self.accounts.insert(id, ctx.clone());
+        Ok(ctx)
+    }
+
     /// Remove an account.
     pub async fn remove_account(&mut self, id: u32) -> Result<()> {
         let ctx = self.accounts.remove(&id);
@@ -228,6 +255,21 @@ impl Accounts {
         self.accounts.keys().copied().collect()
     }
 
+    /// Returns all encrypted accounts.
+    ///
+    /// Note that we can't really distinguish between unreadable/corrupted accounts and
+    /// encrypted accounts.  We consider all known accounts which failed to load encrypted,
+    /// they can be loaded using [`Accounts::load_encrypted_account`].
+    pub fn get_encrypted(&self) -> Vec<u32> {
+        let configured_ids: BTreeSet<u32> = self
+            .config
+            .all_configured_accounts()
+            .map(|cfg| cfg.id)
+            .collect();
+        let loaded_ids: BTreeSet<u32> = self.accounts.keys().copied().collect();
+        configured_ids.difference(&loaded_ids).copied().collect()
+    }
+
     /// This is meant especially for iOS, because iOS needs to tell the system when its background work is done.
     ///
     /// Returns whether all accounts finished their background work.
@@ -400,19 +442,29 @@ impl Config {
         Ok(Config { file, inner })
     }
 
+    /// Returns all account configurations.
+    fn all_configured_accounts(&self) -> impl Iterator<Item = &AccountConfig> {
+        self.inner.accounts.iter()
+    }
+
+    /// Loads all unencrypted accounts.
     pub async fn load_accounts(&self) -> Result<BTreeMap<u32, Context>> {
         let mut accounts = BTreeMap::new();
         for account_config in &self.inner.accounts {
-            let ctx = Context::new(account_config.dbfile().into(), account_config.id)
-                .await
-                .with_context(|| {
-                    format!(
-                        "failed to create context from file {:?}",
-                        account_config.dbfile()
-                    )
-                })?;
-
-            accounts.insert(account_config.id, ctx);
+            match Context::new(account_config.dbfile().into(), account_config.id).await {
+                Ok(ctx) => {
+                    accounts.insert(account_config.id, ctx);
+                }
+                Err(ContextError::WrongKey) => {
+                    continue;
+                }
+                Err(ContextError::Other(err)) => {
+                    return Err(err.context(format!(
+                        "failed to create context from file {}",
+                        account_config.dbfile().display()
+                    )));
+                }
+            }
         }
 
         Ok(accounts)
@@ -746,36 +798,73 @@ mod tests {
 
         assert_eq!(accounts.accounts.len(), 0);
         let account_id = accounts
-            .add_closed_account()
+            .add_encrypted_account("foobar".to_string())
             .await
-            .context("failed to add closed account")?;
+            .context("failed to add encrypted account")?;
         let account = accounts
             .get_selected_account()
             .await
             .context("failed to get account")?;
         assert_eq!(account.id, account_id);
-        let passphrase_set_success = account
-            .open("foobar".to_string())
-            .await
-            .context("failed to set passphrase")?;
-        assert!(passphrase_set_success);
         drop(accounts);
 
-        let accounts = Accounts::new(p.clone())
+        let mut accounts = Accounts::new(p.clone())
             .await
             .context("failed to create second accounts manager")?;
+        assert!(accounts.get_selected_account().await.is_none());
+        let id = accounts
+            .get_selected_account_id()
+            .await
+            .context("failed to get selected account id")?;
+
+        // Try wrong passphrase
+        assert!(accounts
+            .load_encrypted_account(id, "barfoo".to_string())
+            .await
+            .is_err());
+
+        let loaded_account = accounts
+            .load_encrypted_account(id, "foobar".to_string())
+            .await
+            .context("failed to load encrypted account")?;
+
         let account = accounts
             .get_selected_account()
             .await
             .context("failed to get account")?;
-        assert_eq!(account.is_open().await, false);
+        assert_eq!(loaded_account.id, account.id);
 
-        // Try wrong passphrase.
-        assert_eq!(account.open("barfoo".to_string()).await?, false);
-        assert_eq!(account.open("".to_string()).await?, false);
+        Ok(())
+    }
 
-        assert_eq!(account.open("foobar".to_string()).await?, true);
-        assert_eq!(account.is_open().await, true);
+    #[async_std::test]
+    async fn test_get_encrypted() -> Result<()> {
+        let dir = tempfile::tempdir().context("failed to create tempdir")?;
+        let p: PathBuf = dir.path().join("accounts").into();
+        let mut accounts = Accounts::new(p.clone())
+            .await
+            .context("failed to create accounts manager")?;
+        let account_id = accounts
+            .add_encrypted_account("secret".to_string())
+            .await
+            .context("failed to add encrypted account")?;
+        drop(accounts);
+
+        let mut accounts = Accounts::new(p.clone())
+            .await
+            .context("failed to create second accounts manager")?;
+        let encrypted_ids = accounts.get_encrypted();
+        assert_eq!(vec![account_id], encrypted_ids);
+
+        for id in encrypted_ids {
+            let res = accounts
+                .load_encrypted_account(id, "secret".to_string())
+                .await;
+            assert!(res.is_ok());
+        }
+
+        let encrypted_ids = accounts.get_encrypted();
+        assert!(encrypted_ids.is_empty());
 
         Ok(())
     }

src/context.rs

@@ -5,7 +5,7 @@ use std::ffi::OsString;
 use std::ops::Deref;
 use std::time::{Instant, SystemTime};
 
-use anyhow::{bail, ensure, Result};
+use anyhow::{bail, Context as _, Result};
 use async_std::{
     channel::{self, Receiver, Sender},
     path::{Path, PathBuf},
@@ -25,7 +25,7 @@ use crate::message::{self, MessageState, MsgId};
 use crate::quota::QuotaInfo;
 use crate::scheduler::Scheduler;
 use crate::securejoin::Bob;
-use crate::sql::Sql;
+use crate::sql::{Sql, SqlOpenError};
 
 #[derive(Clone, Debug)]
 pub struct Context {
@@ -103,74 +103,62 @@ pub fn get_info() -> BTreeMap<&'static str, String> {
     res
 }
 
+#[derive(Debug, thiserror::Error)]
+pub enum ContextError {
+    #[error("wrong passphrase or unencrypted context")]
+    WrongKey,
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
+impl From<SqlOpenError> for ContextError {
+    fn from(source: SqlOpenError) -> Self {
+        match source {
+            SqlOpenError::WrongKey => Self::WrongKey,
+            SqlOpenError::Other(err) => Self::Other(err),
+        }
+    }
+}
+
 impl Context {
     /// Creates new context and opens the database.
-    pub async fn new(dbfile: PathBuf, id: u32) -> Result<Context> {
-        let context = Self::new_closed(dbfile, id).await?;
-
-        // Open the database if is not encrypted.
-        if context.check_passphrase("".to_string()).await? {
-            context.sql.open(&context, "".to_string()).await?;
-        }
-        Ok(context)
+    pub async fn new(dbfile: PathBuf, id: u32) -> Result<Context, ContextError> {
+        Context::new_common(dbfile, id, None).await
     }
 
-    /// Creates new context without opening the database.
-    pub async fn new_closed(dbfile: PathBuf, id: u32) -> Result<Context> {
+    pub async fn new_encrypted(
+        dbfile: PathBuf,
+        id: u32,
+        passphrase: String,
+    ) -> Result<Context, ContextError> {
+        Context::new_common(dbfile, id, Some(passphrase)).await
+    }
+
+    async fn new_common(
+        dbfile: PathBuf,
+        id: u32,
+        passphrase: Option<String>,
+    ) -> Result<Context, ContextError> {
         let mut blob_fname = OsString::new();
         blob_fname.push(dbfile.file_name().unwrap_or_default());
         blob_fname.push("-blobs");
         let blobdir = dbfile.with_file_name(blob_fname);
         if !blobdir.exists().await {
-            async_std::fs::create_dir_all(&blobdir).await?;
+            async_std::fs::create_dir_all(&blobdir)
+                .await
+                .context("Failed to create blobdir")?;
         }
-        let context = Context::with_blobdir(dbfile, blobdir, id).await?;
-        Ok(context)
-    }
 
-    /// Opens the database with the given passphrase.
-    ///
-    /// Returns true if passphrase is correct, false is passphrase is not correct. Fails on other
-    /// errors.
-    pub async fn open(&self, passphrase: String) -> Result<bool> {
-        if self.sql.check_passphrase(passphrase.clone()).await? {
-            self.sql.open(self, passphrase).await?;
-            Ok(true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    /// Returns true if database is open.
-    pub async fn is_open(&self) -> bool {
-        self.sql.is_open().await
-    }
-
-    /// Tests the database passphrase.
-    ///
-    /// Returns true if passphrase is correct.
-    ///
-    /// Fails if database is already open.
-    pub(crate) async fn check_passphrase(&self, passphrase: String) -> Result<bool> {
-        self.sql.check_passphrase(passphrase).await
-    }
-
-    pub(crate) async fn with_blobdir(
-        dbfile: PathBuf,
-        blobdir: PathBuf,
-        id: u32,
-    ) -> Result<Context> {
-        ensure!(
-            blobdir.is_dir().await,
-            "Blobdir does not exist: {}",
-            blobdir.display()
-        );
+        let sql = match passphrase {
+            Some(passphrase) => Sql::new_encrypted(dbfile, passphrase).await?,
+            None => Sql::new(dbfile).await?,
+        };
 
         let inner = InnerContext {
             id,
             blobdir,
             running_state: RwLock::new(Default::default()),
-            sql: Sql::new(dbfile),
+            sql,
             bob: Default::default(),
             last_smeared_timestamp: RwLock::new(0),
             generating_key_mutex: Mutex::new(()),
@@ -189,6 +177,7 @@ impl Context {
         let ctx = Context {
             inner: Arc::new(inner),
         };
+        ctx.sql.run_migrations(&ctx).await?;
 
         Ok(ctx)
     }
@@ -680,21 +669,16 @@ mod tests {
     use crate::dc_tools::dc_create_outgoing_rfc724_mid;
     use crate::message::Message;
     use crate::test_utils::TestContext;
-    use anyhow::Context as _;
     use std::time::Duration;
     use strum::IntoEnumIterator;
-    use tempfile::tempdir;
 
     #[async_std::test]
-    async fn test_wrong_db() -> Result<()> {
-        let tmp = tempfile::tempdir()?;
+    async fn test_wrong_db() {
+        let tmp = tempfile::tempdir().unwrap();
         let dbfile = tmp.path().join("db.sqlite");
-        std::fs::write(&dbfile, b"123")?;
-        let res = Context::new(dbfile.into(), 1).await?;
-
-        // Broken database is indistinguishable from encrypted one.
-        assert_eq!(res.is_open().await, false);
-        Ok(())
+        std::fs::write(&dbfile, b"123").unwrap();
+        let res = Context::new(dbfile.into(), 1).await;
+        assert!(res.is_err());
     }
 
     #[async_std::test]
@@ -849,16 +833,6 @@ mod tests {
         assert!(blobdir.is_dir());
     }
 
-    #[async_std::test]
-    async fn test_wrong_blogdir() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = tmp.path().join("db.sqlite-blobs");
-        std::fs::write(&blobdir, b"123").unwrap();
-        let res = Context::new(dbfile.into(), 1).await;
-        assert!(res.is_err());
-    }
-
     #[async_std::test]
     async fn test_sqlite_parent_not_exists() {
         let tmp = tempfile::tempdir().unwrap();
@@ -870,24 +844,6 @@ mod tests {
         assert!(dbfile2.is_file());
     }
 
-    #[async_std::test]
-    async fn test_with_empty_blobdir() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = PathBuf::new();
-        let res = Context::with_blobdir(dbfile.into(), blobdir, 1).await;
-        assert!(res.is_err());
-    }
-
-    #[async_std::test]
-    async fn test_with_blobdir_not_exists() {
-        let tmp = tempfile::tempdir().unwrap();
-        let dbfile = tmp.path().join("db.sqlite");
-        let blobdir = tmp.path().join("blobs");
-        let res = Context::with_blobdir(dbfile.into(), blobdir.into(), 1).await;
-        assert!(res.is_err());
-    }
-
     #[async_std::test]
     async fn no_crashes_on_context_deref() {
         let t = TestContext::new().await;
@@ -1048,27 +1004,42 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn test_check_passphrase() -> Result<()> {
-        let dir = tempdir()?;
+    async fn test_reopen_unencrypted() {
+        let dir = tempfile::tempdir().unwrap();
         let dbfile = dir.path().join("db.sqlite");
 
-        let id = 1;
-        let context = Context::new_closed(dbfile.clone().into(), id)
-            .await
-            .context("failed to create context")?;
-        assert_eq!(context.open("foo".to_string()).await?, true);
-        assert_eq!(context.is_open().await, true);
-        drop(context);
+        // First time: creates new db.
+        Context::new(dbfile.clone().into(), 1).await.unwrap();
 
-        let id = 2;
-        let context = Context::new(dbfile.into(), id)
-            .await
-            .context("failed to create context")?;
-        assert_eq!(context.is_open().await, false);
-        assert_eq!(context.check_passphrase("bar".to_string()).await?, false);
-        assert_eq!(context.open("false".to_string()).await?, false);
-        assert_eq!(context.open("foo".to_string()).await?, true);
+        // Opening it encrypted should now fail.
+        let res = Context::new_encrypted(dbfile.clone().into(), 1, "secret".to_string()).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
 
-        Ok(())
+        // But opening normally still works.
+        let res = Context::new(dbfile.into(), 1).await;
+        assert!(res.is_ok());
+    }
+
+    #[async_std::test]
+    async fn test_reopen_ecrypted() {
+        let dir = tempfile::tempdir().unwrap();
+        let dbfile = dir.path().join("db.sqlite");
+
+        // First time: creates new db.
+        Context::new_encrypted(dbfile.clone().into(), 1, "secret".to_string())
+            .await
+            .unwrap();
+
+        // Opening it unencrypted should now fail.
+        let res = Context::new(dbfile.clone().into(), 1).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
+
+        // Wrong password also fails.
+        let res = Context::new_encrypted(dbfile.clone().into(), 1, "oops".to_string()).await;
+        assert!(matches!(res, Err(ContextError::WrongKey)));
+
+        // Finally using the right password still works.
+        let res = Context::new_encrypted(dbfile.into(), 1, "secret".to_string()).await;
+        assert!(res.is_ok());
     }
 }

src/sql.rs

@@ -7,10 +7,10 @@ use std::collections::HashSet;
 use std::convert::TryFrom;
 use std::time::Duration;
 
-use anyhow::{bail, Context as _, Result};
+use anyhow::{anyhow, Context as _, Result};
 use async_std::path::PathBuf;
 use async_std::prelude::*;
-use rusqlite::{config::DbConfig, Connection, OpenFlags};
+use rusqlite::{config::DbConfig, OpenFlags};
 
 use crate::blob::BlobObject;
 use crate::chat::{add_device_msg, update_device_icon, update_saved_messages_icon};
@@ -49,40 +49,47 @@ pub struct Sql {
     is_encrypted: RwLock<Option<bool>>,
 }
 
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum SqlOpenError {
+    #[error("wrong passphrase or unencrypted context")]
+    WrongKey,
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
 impl Sql {
-    pub fn new(dbfile: PathBuf) -> Sql {
-        Self {
+    /// Opens or creates a new (unencrypted) database.
+    ///
+    /// Note after creating this you **MUST** call [`Sql::run_migrations`].
+    pub(crate) async fn new(dbfile: PathBuf) -> Result<Sql, SqlOpenError> {
+        let sql = Self {
             dbfile,
             pool: Default::default(),
             is_encrypted: Default::default(),
-        }
+        };
+        sql.open("".into()).await.map(|_| sql)
     }
 
-    /// Tests SQLCipher passphrase.
+    /// Opens or creates an encrypted database.
     ///
-    /// Returns true if passphrase is correct, i.e. the database is new or can be unlocked with
-    /// this passphrase, and false if the database is already encrypted with another passphrase or
-    /// corrupted.
+    /// If the database does not exist this creates a new encrypted database with the
+    /// provided passphrase.  If the database does exist attempts to open it with the
+    /// provided passphrase.
     ///
-    /// Fails if database is already open.
-    pub async fn check_passphrase(&self, passphrase: String) -> Result<bool> {
-        if self.is_open().await {
-            bail!("Database is already opened.");
+    /// Note after creating this you **MUST** call [`Sql::run_migrations`].
+    pub(crate) async fn new_encrypted(
+        dbfile: PathBuf,
+        passphrase: String,
+    ) -> Result<Sql, SqlOpenError> {
+        if passphrase.is_empty() {
+            return Err(anyhow!("Empty passphrase").into());
         }
-
-        // Hold the lock to prevent other thread from opening the database.
-        let _lock = self.pool.write().await;
-
-        // Test that the key is correct using a single connection.
-        let connection = Connection::open(&self.dbfile)?;
-        connection
-            .pragma_update(None, "key", &passphrase)
-            .context("failed to set PRAGMA key")?;
-        let key_is_correct = connection
-            .query_row("SELECT count(*) FROM sqlite_master", [], |_row| Ok(()))
-            .is_ok();
-
-        Ok(key_is_correct)
+        let sql = Self {
+            dbfile,
+            pool: Default::default(),
+            is_encrypted: Default::default(),
+        };
+        sql.open(passphrase).await.map(|_| sql)
     }
 
     /// Checks if there is currently a connection to the underlying Sqlite database.
@@ -194,11 +201,18 @@ impl Sql {
         Ok(pool)
     }
 
-    async fn try_open(&self, context: &Context, dbfile: &Path, passphrase: String) -> Result<()> {
-        *self.pool.write().await = Some(Self::new_pool(dbfile, passphrase.to_string())?);
+    async fn try_open(&self, dbfile: &Path, passphrase: String) -> Result<(), SqlOpenError> {
+        *self.pool.write().await = Some(Self::new_pool(dbfile, passphrase)?);
 
         {
             let conn = self.get_conn().await?;
+            let res = conn.query_row("SELECT count(*) FROM sqlite_master", params![], |_row| {
+                Ok(())
+            });
+            if res.is_err() {
+                // This hides SqliteFailure "NotADatabase"
+                return Err(SqlOpenError::WrongKey);
+            }
 
             // Try to enable auto_vacuum. This will only be
             // applied if the database is new or after successful
@@ -206,21 +220,22 @@ impl Sql {
             // When auto_vacuum is INCREMENTAL, it is possible to
             // use PRAGMA incremental_vacuum to return unused
             // database pages to the filesystem.
-            conn.pragma_update(None, "auto_vacuum", &"INCREMENTAL".to_string())?;
+            conn.pragma_update(None, "auto_vacuum", &"INCREMENTAL".to_string())
+                .context("auto_vacuum pragma failed")?;
 
             // journal_mode is persisted, it is sufficient to change it only for one handle.
-            conn.pragma_update(None, "journal_mode", &"WAL".to_string())?;
+            conn.pragma_update(None, "journal_mode", &"WAL".to_string())
+                .context("journal_mode pragma failed")?;
 
             // Default synchronous=FULL is much slower. NORMAL is sufficient for WAL mode.
-            conn.pragma_update(None, "synchronous", &"NORMAL".to_string())?;
+            conn.pragma_update(None, "synchronous", &"NORMAL".to_string())
+                .context("synchronous pragma failed")?;
         }
 
-        self.run_migrations(context).await?;
-
         Ok(())
     }
 
-    pub async fn run_migrations(&self, context: &Context) -> Result<()> {
+    pub(crate) async fn run_migrations(&self, context: &Context) -> Result<()> {
         // (1) update low-level database structure.
         // this should be done before updates that use high-level objects that
         // rely themselves on the low-level structure.
@@ -290,28 +305,31 @@ impl Sql {
             }
         }
 
+        info!(context, "Migrations finished");
         Ok(())
     }
 
-    /// Opens the provided database and runs any necessary migrations.
+    /// Opens the provided database.
+    ///
+    /// To open an unencrypted database provide an empty passphrase, if the passphrase is
+    /// wrong an error is returned.
+    ///
     /// If a database is already open, this will return an error.
-    pub async fn open(&self, context: &Context, passphrase: String) -> Result<()> {
+    async fn open(&self, passphrase: String) -> Result<(), SqlOpenError> {
         if self.is_open().await {
-            error!(
-                context,
-                "Cannot open, database \"{:?}\" already opened.", self.dbfile,
-            );
-            bail!("SQL database is already opened.");
+            return Err(anyhow!("SQL database is already opened.").into());
         }
 
         let passphrase_nonempty = !passphrase.is_empty();
-        if let Err(err) = self.try_open(context, &self.dbfile, passphrase).await {
-            self.close().await;
-            Err(err)
-        } else {
-            info!(context, "Opened database {:?}.", self.dbfile);
-            *self.is_encrypted.write().await = Some(passphrase_nonempty);
-            Ok(())
+        match self.try_open(&self.dbfile, passphrase).await {
+            Ok(()) => {
+                *self.is_encrypted.write().await = Some(passphrase_nonempty);
+                Ok(())
+            }
+            Err(err) => {
+                self.close().await;
+                Err(err)
+            }
         }
     }
 
@@ -888,7 +906,7 @@ mod tests {
 
         t.sql.close().await;
         housekeeping(&t).await.unwrap_err(); // housekeeping should fail as the db is closed
-        t.sql.open(&t, "".to_string()).await.unwrap();
+        t.sql.open("".to_string()).await.unwrap();
 
         let a = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
         assert_eq!(avatar_bytes, &async_std::fs::read(&a).await.unwrap()[..]);
@@ -906,54 +924,10 @@ mod tests {
         }
     }
 
-    /// Regression test.
-    ///
-    /// Previously the code checking for existence of `config` table
-    /// checked it with `PRAGMA table_info("config")` but did not
-    /// drain `SqlitePool.fetch` result, only using the first row
-    /// returned. As a result, prepared statement for `PRAGMA` was not
-    /// finalized early enough, leaving reader connection in a broken
-    /// state after reopening the database, when `config` table
-    /// existed and `PRAGMA` returned non-empty result.
-    ///
-    /// Statements were not finalized due to a bug in sqlx:
-    /// <https://github.com/launchbadge/sqlx/issues/1147>
-    #[async_std::test]
-    async fn test_db_reopen() -> Result<()> {
-        use tempfile::tempdir;
-
-        // The context is used only for logging.
-        let t = TestContext::new().await;
-
-        // Create a separate empty database for testing.
-        let dir = tempdir()?;
-        let dbfile = dir.path().join("testdb.sqlite");
-        let sql = Sql::new(dbfile.into());
-
-        // Create database with all the tables.
-        sql.open(&t, "".to_string()).await.unwrap();
-        sql.close().await;
-
-        // Reopen the database
-        sql.open(&t, "".to_string()).await?;
-        sql.execute(
-            "INSERT INTO config (keyname, value) VALUES (?, ?);",
-            paramsv!("foo", "bar"),
-        )
-        .await?;
-
-        let value: Option<String> = sql
-            .query_get_value("SELECT value FROM config WHERE keyname=?;", paramsv!("foo"))
-            .await?;
-        assert_eq!(value.unwrap(), "bar");
-
-        Ok(())
-    }
-
     #[async_std::test]
     async fn test_migration_flags() -> Result<()> {
         let t = TestContext::new().await;
-        t.evtracker.get_info_contains("Opened database").await;
+        t.evtracker.get_info_contains("Migrations finished").await;
 
         // as migrations::run() was already executed on context creation,
         // another call should not result in any action needed.
@@ -988,36 +962,4 @@ mod tests {
 
         Ok(())
     }
-
-    #[async_std::test]
-    async fn test_check_passphrase() -> Result<()> {
-        use tempfile::tempdir;
-
-        // The context is used only for logging.
-        let t = TestContext::new().await;
-
-        // Create a separate empty database for testing.
-        let dir = tempdir()?;
-        let dbfile = dir.path().join("testdb.sqlite");
-        let sql = Sql::new(dbfile.clone().into());
-
-        sql.check_passphrase("foo".to_string()).await?;
-        sql.open(&t, "foo".to_string())
-            .await
-            .context("failed to open the database first time")?;
-        sql.close().await;
-
-        // Reopen the database
-        let sql = Sql::new(dbfile.into());
-
-        // Test that we can't open encrypted database without a passphrase.
-        assert!(sql.open(&t, "".to_string()).await.is_err());
-
-        // Now open the database with passpharse, it should succeed.
-        sql.check_passphrase("foo".to_string()).await?;
-        sql.open(&t, "foo".to_string())
-            .await
-            .context("failed to open the database second time")?;
-        Ok(())
-    }
 }