Add support for auto-unloading of the master password

6 files changed, 62 insertions, 47 deletions

diff --git a/libsylph/masterpassword.c b/libsylph/masterpassword.c
index 8174def..bac49c4 100644
--- a/libsylph/masterpassword.c
+++ b/libsylph/masterpassword.c
@@ -9,7 +9,7 @@
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
  * Lesser General Public License for more details.
  *
  * You should have received a copy of the GNU Lesser General Public
@@ -30,6 +30,7 @@
 
 
 gchar *master_password;
+gboolean master_password_enabled_on_init;
 
 void set_master_password(const char *password) {
         master_password = password;
@@ -91,7 +92,7 @@ gchar *decrypt_with_master_password(const gchar *str) {
         if (master_password == NULL) {
                 /* we have empty or auto unloaded master password */
                 if ((!prefs_common.auto_unload_master_password) ||
-                        (check_master_password_interactively(3) != RC_OK)) {
+                        (check_master_password_interactively(3) != MP_RC_OK)) {
                         return g_strdup(str);
                 }
                 debug_print("Reloaded master password\n");
@@ -104,7 +105,7 @@ gchar *decrypt_with_master_password(const gchar *str) {
         if (decrypt_data(&new_str,
                                          str + str_prefix,
                                          master_password,
-                                         strlen(str) - str_prefix) != RC_OK) {
+                                         strlen(str) - str_prefix) != MP_RC_OK) {
                 OPENSSL_cleanse(new_str, strlen(new_str));
                 g_free(new_str);
                 return g_strdup(str);
@@ -123,17 +124,13 @@ gchar *encrypt_with_master_password(const gchar *str) {
         gchar *new_str, *mpes1_str;
         gint length_encrypted;
 
-        if ((!str) || (!prefs_common.use_master_password))
-                return g_strdup(str);
+        if ((!str) || (!master_password_active()))
+                return NULL;
 
-        if (master_password == NULL) {
-                /* we have empty or auto unloaded master password */
-                if ((!prefs_common.auto_unload_master_password) ||
-                        (check_master_password_interactively(3) != RC_OK)) {
-                        return g_strdup(str);
-                }
-                debug_print("Reloaded master password\n");
-        }
+        /*
+         * unlike the decrypt function, here it is up to the caller
+         * to make sure that auto unloaded master password is handled properly
+         */
 
         if (encrypt_data(&new_str,
                                          &length_encrypted,
@@ -141,19 +138,17 @@ gchar *encrypt_with_master_password(const gchar *str) {
                                          master_password,
                                          strlen(str),
                                          prefs_common.encrypted_password_min_length,
-                                         TRUE) != RC_OK) {
-                OPENSSL_cleanse(new_str, strlen(new_str));
+                                         TRUE) != MP_RC_OK) {
                 g_free(new_str);
-                return g_strdup(str);
+                return NULL;
         }
 
         mpes1_str = g_strdup_printf("mpes1:%s", new_str);
-        OPENSSL_cleanse(new_str, strlen(new_str));
         g_free(new_str);
 
         return mpes1_str;
 #else
-        return g_strdup(str);
+        return NULL;
 #endif
 
 }
@@ -170,7 +165,7 @@ gint set_master_password_interactively(guint max_attempts) {
         if (generate_password_hash(
                         &prefs_common.master_password_hash,
                         master_password,
-                        NULL) != RC_OK) {
+                        NULL) != MP_RC_OK) {
                 /* should not really happen unless buggy code / library */
                 g_free(prefs_common.master_password_hash);
                 prefs_common.master_password_hash = NULL;
@@ -204,8 +199,8 @@ gint check_master_password_interactively(guint max_attempts) {
                         continue;
                 }
                 if (check_password(master_password,
-                                                   prefs_common.master_password_hash) == RC_OK) {
-                        return RC_OK; /* match */
+                                                   prefs_common.master_password_hash) == MP_RC_OK) {
+                        return MP_RC_OK; /* match */
                 }
                 debug_print(_("Wrong master password entered (%d)\n"), cnt);
                 OPENSSL_cleanse(master_password, strlen(master_password));
diff --git a/libsylph/masterpassword.h b/libsylph/masterpassword.h
index cc32358..7254643 100644
--- a/libsylph/masterpassword.h
+++ b/libsylph/masterpassword.h
@@ -24,7 +24,12 @@
 #include "config.h"
 #endif
 
+#define MP_RC_OK 0
+#define MP_RC_WRONG_HASH_OR_KEY 1
+#define MP_RC_INVALID_FORMAT 2 /* invalid digest format */
+
 extern gchar *master_password;
+extern gboolean master_password_enabled_on_init; /* m.p. enabled on init? */
 void set_master_password(const char *password);
 gchar *get_master_password(void);
 void cleanse_buffer(void *buf, size_t len);
diff --git a/libsylph/ssl.c b/libsylph/ssl.c
index 44b2935..e782b0e 100644
--- a/libsylph/ssl.c
+++ b/libsylph/ssl.c
@@ -446,7 +446,7 @@ static gint secure_derive_key(guchar *key,
         OPENSSL_free(buffer);
         EVP_MD_CTX_destroy(mdctx);
 
-        return RC_OK;
+        return SSL_RC_OK;
 
 }
 
@@ -472,7 +472,7 @@ gint encrypt_data(gchar **encrypted,
         EVP_CIPHER_CTX *ctx;
         EVP_MD_CTX *mdctx;
 
-        rc = RC_ERROR;
+        rc = SSL_RC_ERROR;
 
         if (length_data < 1) {
                 return -1;
@@ -492,7 +492,7 @@ gint encrypt_data(gchar **encrypted,
         if (secure_derive_key(key,
                                           key_size,
                                           passphrase,
-                                                  salt) != RC_OK) {
+                                                  salt) != SSL_RC_OK) {
                 OPENSSL_cleanse(key, key_size);
                 debug_print("Could not generate secure key\n");
                 goto cleanup;
@@ -594,7 +594,7 @@ gint encrypt_data(gchar **encrypted,
         *encrypted = g_base64_encode(total_buffer, length_total);
         *length_encrypted = strlen(*encrypted);
 
-        rc = RC_OK;
+        rc = SSL_RC_OK;
 
 cleanup:
         /* key */
@@ -740,7 +740,7 @@ gint decrypt_data(gchar **decrypted,
                                 (const gchar*) cleartext_buffer,
                                 length_hash) != 0) {
                 debug_print("Invalid hash\n");
-                rc = RC_WRONG_HASH_OR_KEY;
+                rc = SSL_RC_WRONG_HASH_OR_KEY;
                 goto cleanup;
         }
 
@@ -755,7 +755,7 @@ gint decrypt_data(gchar **decrypted,
         *decrypted = OPENSSL_malloc(length_decrypted);
         memcpy(*decrypted, data_payload_buffer + 2, length_decrypted);
 
-        rc = RC_OK;
+        rc = SSL_RC_OK;
 
 cleanup:
 
@@ -793,7 +793,7 @@ gint generate_password_hash(gchar **password_hash,
         if (salt == NULL) {
                 if (RAND_bytes(lsalt, SALT_SIZE) != 1) {
                         debug_print("Random problems...\n");
-                        return RC_ERROR;
+                        return SSL_RC_ERROR;
                 }
         } else {
                 memcpy(lsalt, salt, SALT_SIZE);
@@ -827,7 +827,7 @@ gint generate_password_hash(gchar **password_hash,
         OPENSSL_cleanse(salt_b64, strlen(salt_b64));
         OPENSSL_free(salt_b64);
 
-        return RC_OK;
+        return SSL_RC_OK;
 
 }
 
@@ -838,7 +838,7 @@ gint check_password(const gchar *password, const gchar *password_hash) {
         gchar **tokens, *new_hash;
         gsize salt_length;
 
-        rc = RC_ERROR;
+        rc = SSL_RC_ERROR;
         tokens = g_strsplit(password_hash,
                                                 "$",
                                                 -1);
@@ -858,7 +858,7 @@ gint check_password(const gchar *password, const gchar *password_hash) {
                 goto cleanup;
         }
 
-        if (generate_password_hash(&new_hash, password, salt) != RC_OK) {
+        if (generate_password_hash(&new_hash, password, salt) != SSL_RC_OK) {
                 debug_print("Password hash generation failed\n");
                 goto cleanup;
         }
diff --git a/libsylph/ssl.h b/libsylph/ssl.h
index 6338911..4c0ccd1 100644
--- a/libsylph/ssl.h
+++ b/libsylph/ssl.h
@@ -37,9 +37,9 @@
 
 #include "socket.h"
 
-#define RC_OK 0
-#define RC_ERROR -1
-#define RC_WRONG_HASH_OR_KEY 1
+#define SSL_RC_OK 0
+#define SSL_RC_ERROR -1
+#define SSL_RC_WRONG_HASH_OR_KEY 1
 
 typedef enum {
         SSL_METHOD_SSLv23,
diff --git a/src/main.c b/src/main.c
index 3c87c70..97a4bd0 100644
--- a/src/main.c
+++ b/src/main.c
@@ -280,7 +280,7 @@ int main(int argc, char *argv[])
 #if USE_SSL
         if (prefs_common.use_master_password) {
                 if (prefs_common.master_password_hash != NULL) {
-                        if (check_master_password_interactively(3) != RC_OK) {
+                        if (check_master_password_interactively(3) != MP_RC_OK) {
                                 if (alertpanel(_("Master password"),
                                                            _("Invalid master password"),
                                                            GTK_STOCK_DISCARD,
@@ -292,7 +292,7 @@ int main(int argc, char *argv[])
                 } else {
                         alertpanel_notice(
                                 _("Master password enabled but not set. Setting one now"));
-                        if (set_master_password_interactively(3) != RC_OK) {
+                        if (set_master_password_interactively(3) != MP_RC_OK) {
                                 if (alertpanel(_("Master password"),
                                                            _("Unable to set master password"),
                                                            GTK_STOCK_DISCARD,
@@ -303,6 +303,16 @@ int main(int argc, char *argv[])
                         }
                 }
         }
+        /* security goal: 
+         * if the master password is enabled and loaded on init,
+         * an attacker can potentially set use_master_password - disabled
+         * afterwards and then force Sylpheed to save account data - the result
+         * being passwords saved to accountrc in plain-text.
+         * possible solution:
+         * Do not allow the passwords to be stored in plain-text if Sylpheed
+         * was started with use_master_password - enabled.
+         */
+        master_password_enabled_on_init = prefs_common.use_master_password;
 #endif
         filter_set_addressbook_func(addressbook_has_address);
         filter_read_config();
diff --git a/src/prefs_ui.c b/src/prefs_ui.c
index 051a058..2ab1d45 100644
--- a/src/prefs_ui.c
+++ b/src/prefs_ui.c
@@ -279,23 +279,28 @@ void prefs_set_data_from_epass_entry(PrefParam *pparam)
 
         /* This is where decrypted passwords are encrypted and stored again */
 
-        /* TODO: A potential exploit is disabling master password and then forcing
-         * Sylpheed to store the password
-         */
-
         /* master_password == NULL for any of the following reasons:
          * - use_master_password is not enabled (we just save without encrypting)
-         * - master_password is auto unloaded (let the encrypt function handle it)
+         * - master_password is auto unloaded (prompt for the master password)
          * - undefined reason (refure to store the password)
          */
         if (!prefs_common.use_master_password) {
-                prefs_set_data_from_entry(pparam);
-                return;
-        } else if ((master_password == NULL) &&
-                           (!prefs_common.auto_unload_master_password)) {
-                debug_print("Master password enabled, but not loaded for no "
-                                        "apparent reason. Not storing\n");
+                /* check if use_master_password was enabled when Sylpheed started */
+                if (!master_password_enabled_on_init) {
+                        prefs_set_data_from_entry(pparam);
+                }
                 return;
+        } else if (master_password == NULL) {
+        if (!prefs_common.auto_unload_master_password) {
+            debug_print("Master password enabled, but not loaded for no "
+                        "apparent reason. Not storing\n");
+            return;
+        } else {
+            if (check_master_password_interactively(3) != MP_RC_OK) {
+                debug_print("Failed to reload the master password\n");
+                return;
+            }
+        }
         }
 
         ui_data = (PrefsUIData *)pparam->ui_data;