Merge pull request #708 from pspeter/simplify-password-logic

Refactor password logic to prevent accidental password leakage

features/data/configs/bug153.yaml

@@ -6,7 +6,6 @@ highlight: true
 journals:
   default: features/journals/bug153.dayone
 linewrap: 80
-password: ''
 tagsymbols: '@'
 template: false
 timeformat: '%Y-%m-%d %H:%M'

features/data/configs/dayone.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/dayone.dayone
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/empty_folder.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/empty_folder
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/encrypted.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/encrypted.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/markdown-headings-335.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/markdown-headings-335.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/multiple.yaml

@@ -13,7 +13,6 @@ journals:
     encrypt: true
     journal: features/journals/new_encrypted.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags-216.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags-216.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags-237.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags-237.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/multiple_journals.feature

@@ -48,4 +48,4 @@ Feature: Multiple journals
         these three eyes
         n
         """
-	Then we should see the message "Journal 'new_encrypted' created"
+	Then we should see the message "Encrypted journal 'new_encrypted' created"

jrnl/EncryptedJournal.py

@@ -1,4 +1,5 @@
-from . import Journal, util
+from . import util
+from .Journal import Journal, LegacyJournal
 from cryptography.fernet import Fernet, InvalidToken
 from cryptography.hazmat.primitives import hashes, padding
 from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
@@ -9,6 +10,8 @@ import sys
 import os
 import base64
 import logging
+from typing import Optional
+
 
 log = logging.getLogger()
 
@@ -27,10 +30,11 @@ def make_key(password):
     return base64.urlsafe_b64encode(key)
 
 
-class EncryptedJournal(Journal.Journal):
+class EncryptedJournal(Journal):
     def __init__(self, name='default', **kwargs):
         super().__init__(name, **kwargs)
         self.config['encrypt'] = True
+        self.password = None
 
     def open(self, filename=None):
         """Opens the journal file defined in the config and parses it into a list of Entries.
@@ -38,27 +42,17 @@ class EncryptedJournal(Journal.Journal):
         filename = filename or self.config['journal']
 
         if not os.path.exists(filename):
-            password = util.create_password()
-            if password:
-                if util.yesno("Do you want to store the password in your keychain?", default=True):
-                    util.set_keychain(self.name, password)
-                else:
-                    util.set_keychain(self.name, None)
-                self.config['password'] = password
-                text = ""
-                self._store(filename, text)
-                print(f"[Journal '{self.name}' created at {filename}]", file=sys.stderr)
-            else:
-                print("No password supplied for encrypted journal", file=sys.stderr)
-                sys.exit(1)
-        else:
-            text = self._load(filename)
+            self.create_file(filename)
+            self.password = util.create_password(self.name)
+            print(f"Encrypted journal '{self.name}' created at {filename}", file=sys.stderr)
+
+        text = self._load(filename)
         self.entries = self._parse(text)
         self.sort()
         log.debug("opened %s with %d entries", self.__class__.__name__, len(self))
         return self
 
-    def _load(self, filename, password=None):
+    def _load(self, filename):
         """Loads an encrypted journal from a file and tries to decrypt it.
         If password is not provided, will look for password in the keychain
         and otherwise ask the user to enter a password up to three times.
@@ -67,50 +61,52 @@ class EncryptedJournal(Journal.Journal):
         with open(filename, 'rb') as f:
             journal_encrypted = f.read()
 
-        def validate_password(password):
+        def decrypt_journal(password):
             key = make_key(password)
             try:
                 plain = Fernet(key).decrypt(journal_encrypted).decode('utf-8')
-                self.config['password'] = password
+                self.password = password
                 return plain
             except (InvalidToken, IndexError):
                 return None
-        if password:
-            return validate_password(password)
-        return util.get_password(keychain=self.name, validator=validate_password)
+
+        if self.password:
+            return decrypt_journal(self.password)
+
+        return util.decrypt_content(keychain=self.name, decrypt_func=decrypt_journal)
 
     def _store(self, filename, text):
-        key = make_key(self.config['password'])
+        key = make_key(self.password)
         journal = Fernet(key).encrypt(text.encode('utf-8'))
         with open(filename, 'wb') as f:
             f.write(journal)
 
     @classmethod
-    def _create(cls, filename, password):
-        key = make_key(password)
-        dummy = Fernet(key).encrypt(b"")
-        with open(filename, 'wb') as f:
-            f.write(dummy)
+    def from_journal(cls, other: Journal):
+        new_journal = super().from_journal(other)
+        new_journal.password = other.password if hasattr(other, "password") else util.create_password(other.name)
+        return new_journal
 
 
-class LegacyEncryptedJournal(Journal.LegacyJournal):
+class LegacyEncryptedJournal(LegacyJournal):
     """Legacy class to support opening journals encrypted with the jrnl 1.x
     standard. You'll not be able to save these journals anymore."""
     def __init__(self, name='default', **kwargs):
         super().__init__(name, **kwargs)
         self.config['encrypt'] = True
+        self.password = None
 
-    def _load(self, filename, password=None):
+    def _load(self, filename):
         with open(filename, 'rb') as f:
             journal_encrypted = f.read()
         iv, cipher = journal_encrypted[:16], journal_encrypted[16:]
 
-        def validate_password(password):
+        def decrypt_journal(password):
             decryption_key = hashlib.sha256(password.encode('utf-8')).digest()
             decryptor = Cipher(algorithms.AES(decryption_key), modes.CBC(iv), default_backend()).decryptor()
             try:
                 plain_padded = decryptor.update(cipher) + decryptor.finalize()
-                self.config['password'] = password
+                self.password = password
                 if plain_padded[-1] in (" ", 32):
                     # Ancient versions of jrnl. Do not judge me.
                     return plain_padded.decode('utf-8').rstrip(" ")
@@ -120,6 +116,6 @@ class LegacyEncryptedJournal(Journal.LegacyJournal):
                     return plain.decode('utf-8')
             except ValueError:
                 return None
-        if password:
-            return validate_password(password)
-        return util.get_password(keychain=self.name, validator=validate_password)
+        if self.password:
+            return decrypt_journal(self.password)
+        return util.decrypt_content(keychain=self.name, decrypt_func=decrypt_journal)

jrnl/Journal.py

@@ -41,6 +41,7 @@ class Journal:
         # Set up date parser
         self.search_tags = None  # Store tags we're highlighting
         self.name = name
+        self.entries = []
 
     def __len__(self):
         """Returns the number of entries"""
@@ -69,9 +70,9 @@ class Journal:
         filename = filename or self.config['journal']
 
         if not os.path.exists(filename):
+            self.create_file(filename)
             print(f"[Journal '{self.name}' created at {filename}]", file=sys.stderr)
-            self._create(filename)
-
+        
         text = self._load(filename)
         self.entries = self._parse(text)
         self.sort()
@@ -92,6 +93,11 @@ class Journal:
                 return False
         return True
 
+    @staticmethod
+    def create_file(filename):
+        with open(filename, "w"):
+            pass
+
     def _to_text(self):
         return "\n".join([str(e) for e in self.entries])
 
@@ -101,10 +107,6 @@ class Journal:
     def _store(self, filename, text):
         raise NotImplementedError
 
-    @classmethod
-    def _create(cls, filename):
-        raise NotImplementedError
-
     def _parse(self, journal_txt):
         """Parses a journal that's stored in a string and returns a list of entries"""
 
@@ -274,11 +276,6 @@ class Journal:
 
 
 class PlainJournal(Journal):
-    @classmethod
-    def _create(cls, filename):
-        with open(filename, "a", encoding="utf-8"):
-            pass
-
     def _load(self, filename):
         with open(filename, "r", encoding="utf-8") as f:
             return f.read()

jrnl/cli.py

@@ -6,7 +6,8 @@
     license: MIT, see LICENSE for more details.
 """
 
-from . import Journal
+from .Journal import PlainJournal, open_journal
+from .EncryptedJournal import EncryptedJournal
 from . import util
 from . import install
 from . import plugins
@@ -77,28 +78,19 @@ def guess_mode(args, config):
 
 def encrypt(journal, filename=None):
     """ Encrypt into new file. If filename is not set, we encrypt the journal file itself. """
-    from . import EncryptedJournal
-
-    journal.config['password'] = util.create_password()
     journal.config['encrypt'] = True
 
-    new_journal = EncryptedJournal.EncryptedJournal(None, **journal.config)
-    new_journal.entries = journal.entries
+    new_journal = EncryptedJournal.from_journal(journal)
     new_journal.write(filename)
 
-    if util.yesno("Do you want to store the password in your keychain?", default=True):
-        util.set_keychain(journal.name, journal.config['password'])
-
     print("Journal encrypted to {}.".format(filename or new_journal.config['journal']), file=sys.stderr)
 
 
 def decrypt(journal, filename=None):
     """ Decrypts into new file. If filename is not set, we encrypt the journal file itself. """
     journal.config['encrypt'] = False
-    journal.config['password'] = ""
 
-    new_journal = Journal.PlainJournal(filename, **journal.config)
-    new_journal.entries = journal.entries
+    new_journal = PlainJournal.from_journal(journal)
     new_journal.write(filename)
     print("Journal decrypted to {}.".format(filename or new_journal.config['journal']), file=sys.stderr)
 
@@ -156,11 +148,12 @@ def run(manual_args=None):
 
     # If the first textual argument points to a journal file,
     # use this!
-    journal_name = args.text[0] if (args.text and args.text[0] in config['journals']) else 'default'
 
-    if journal_name != 'default':
+    journal_name = install.DEFAULT_JOURNAL_KEY
+    if args.text and args.text[0] in config['journals']:
+        journal_name = args.text[0]
         args.text = args.text[1:]
-    elif "default" not in config['journals']:
+    elif install.DEFAULT_JOURNAL_KEY not in config['journals']:
         print("No default journal configured.", file=sys.stderr)
         print(list_journals(config), file=sys.stderr)
         sys.exit(1)
@@ -211,7 +204,7 @@ def run(manual_args=None):
 
     # This is where we finally open the journal!
     try:
-        journal = Journal.open_journal(journal_name, config)
+        journal = open_journal(journal_name, config)
     except KeyboardInterrupt:
         print(f"[Interrupted while opening journal]", file=sys.stderr)
         sys.exit(1)

jrnl/install.py

@@ -19,6 +19,7 @@ if "win32" not in sys.platform:
 
 DEFAULT_CONFIG_NAME = 'jrnl.yaml'
 DEFAULT_JOURNAL_NAME = 'journal.txt'
+DEFAULT_JOURNAL_KEY = 'default'
 XDG_RESOURCE = 'jrnl'
 
 USER_HOME = os.path.expanduser('~')
@@ -45,7 +46,7 @@ def module_exists(module_name):
 default_config = {
     'version': __version__,
     'journals': {
-        "default": JOURNAL_FILE_PATH
+        DEFAULT_JOURNAL_KEY: JOURNAL_FILE_PATH
     },
     'editor': os.getenv('VISUAL') or os.getenv('EDITOR') or "",
     'encrypt': False,
@@ -118,32 +119,23 @@ def install():
     # Where to create the journal?
     path_query = f'Path to your journal file (leave blank for {JOURNAL_FILE_PATH}): '
     journal_path = input(path_query).strip() or JOURNAL_FILE_PATH
-    default_config['journals']['default'] = os.path.expanduser(os.path.expandvars(journal_path))
+    default_config['journals'][DEFAULT_JOURNAL_KEY] = os.path.expanduser(os.path.expandvars(journal_path))
 
-    path = os.path.split(default_config['journals']['default'])[0]  # If the folder doesn't exist, create it
+    path = os.path.split(default_config['journals'][DEFAULT_JOURNAL_KEY])[0]  # If the folder doesn't exist, create it
     try:
         os.makedirs(path)
     except OSError:
         pass
 
     # Encrypt it?
-    password = getpass.getpass("Enter password for journal (leave blank for no encryption): ")
-    if password:
+    encrypt = util.yesno("Do you want to encrypt your journal? You can always change this later", default=False)
+    if encrypt:
         default_config['encrypt'] = True
-        if util.yesno("Do you want to store the password in your keychain?", default=True):
-            util.set_keychain("default", password)
-        else:
-            util.set_keychain("default", None)
-        EncryptedJournal._create(default_config['journals']['default'], password)
         print("Journal will be encrypted.", file=sys.stderr)
-    else:
-        PlainJournal._create(default_config['journals']['default'])
 
-    config = default_config
-    save_config(config)
-    if password:
-        config['password'] = password
-    return config
+    save_config(default_config)
+    return default_config
+
 
 def autocomplete(text, state):
     expansions = glob.glob(os.path.expanduser(os.path.expandvars(text)) + '*')

jrnl/util.py

@@ -4,8 +4,10 @@ import sys
 import os
 import getpass as gp
 import yaml
+
 if "win32" in sys.platform:
     import colorama
+
     colorama.init()
 import re
 import tempfile
@@ -13,6 +15,7 @@ import subprocess
 import unicodedata
 import shlex
 import logging
+from typing import Optional, Callable
 
 log = logging.getLogger(__name__)
 
@@ -37,19 +40,29 @@ class UserAbort(Exception):
     pass
 
 
-def create_password():
+def create_password(journal_name: str, prompt: str = "Enter password for new journal: ") -> str:
     while True:
-        pw = gp.getpass("Enter password for new journal: ")
-        if pw == gp.getpass("Enter password again: "):
-            return pw
+        pw = gp.getpass(prompt)
+        if not pw:
+            print("Password can't be an empty string!", file=sys.stderr)
+            continue
+        elif pw == gp.getpass("Enter password again: "):
+            break
 
         print("Passwords did not match, please try again", file=sys.stderr)
 
+    if yesno("Do you want to store the password in your keychain?", default=True):
+        set_keychain(journal_name, pw)
+    else:
+        set_keychain(journal_name, None)
 
-def get_password(validator, keychain=None, max_attempts=3):
+    return pw
+
+
+def decrypt_content(decrypt_func: Callable[[str], Optional[str]], keychain: str = None, max_attempts: int = 3) -> str:
     pwd_from_keychain = keychain and get_keychain(keychain)
     password = pwd_from_keychain or gp.getpass()
-    result = validator(password)
+    result = decrypt_func(password)
     # Password is bad:
     if result is None and pwd_from_keychain:
         set_keychain(keychain, None)
@@ -57,7 +70,7 @@ def get_password(validator, keychain=None, max_attempts=3):
     while result is None and attempt < max_attempts:
         print("Wrong password, try again.", file=sys.stderr)
         password = gp.getpass()
-        result = validator(password)
+        result = decrypt_func(password)
         attempt += 1
     if result is not None:
         return result