From 9664924096fce33cdd28f45fe9c1b9aa38c302e9 Mon Sep 17 00:00:00 2001 From: Peter Schmidbauer Date: Sat, 2 Nov 2019 02:06:05 +0100 Subject: [PATCH] Move all password handling to EncryptedJournal This commit should greatly simplify all password handling logic. No passwords are stored in the config dict anymore. Only the Encrypted Journals have any password related logic. I also had to remove some password fields from the test files, which shows how dangerous the previous approach was. A slight code change could've leaked passwords to the config file. However, I had to change the install progress a little bit to make this work. It will now not ask you for a password right away but rather if you want to encrypt or not. Only if you reply 'y' will it ask you for the password later on. --- features/data/configs/bug153.yaml | 1 - features/data/configs/dayone.yaml | 1 - features/data/configs/empty_folder.yaml | 1 - features/data/configs/encrypted.yaml | 1 - .../data/configs/markdown-headings-335.yaml | 1 - features/data/configs/multiple.yaml | 1 - features/data/configs/tags-216.yaml | 1 - features/data/configs/tags-237.yaml | 1 - features/data/configs/tags.yaml | 1 - features/multiple_journals.feature | 2 +- jrnl/EncryptedJournal.py | 68 +++++++++---------- jrnl/Journal.py | 19 +++--- jrnl/cli.py | 25 +++---- jrnl/install.py | 26 +++---- jrnl/util.py | 27 ++++++-- 15 files changed, 79 insertions(+), 97 deletions(-) diff --git a/features/data/configs/bug153.yaml b/features/data/configs/bug153.yaml index 765185c3..5bbcbf4a 100644 --- a/features/data/configs/bug153.yaml +++ b/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' diff --git a/features/data/configs/dayone.yaml b/features/data/configs/dayone.yaml index e7331733..8807e66b 100644 --- a/features/data/configs/dayone.yaml +++ b/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: "|" diff --git a/features/data/configs/empty_folder.yaml b/features/data/configs/empty_folder.yaml index ee401ada..52a21854 100644 --- a/features/data/configs/empty_folder.yaml +++ b/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: "|" diff --git a/features/data/configs/encrypted.yaml b/features/data/configs/encrypted.yaml index 254b81d9..4d50b607 100644 --- a/features/data/configs/encrypted.yaml +++ b/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: "|" diff --git a/features/data/configs/markdown-headings-335.yaml b/features/data/configs/markdown-headings-335.yaml index dafed64b..5c33c53e 100644 --- a/features/data/configs/markdown-headings-335.yaml +++ b/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: "|" diff --git a/features/data/configs/multiple.yaml b/features/data/configs/multiple.yaml index 2e282232..65f2c256 100644 --- a/features/data/configs/multiple.yaml +++ b/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: "|" diff --git a/features/data/configs/tags-216.yaml b/features/data/configs/tags-216.yaml index 73523c50..b71abea5 100644 --- a/features/data/configs/tags-216.yaml +++ b/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: "|" diff --git a/features/data/configs/tags-237.yaml b/features/data/configs/tags-237.yaml index 2b360f6f..a5a70d99 100644 --- a/features/data/configs/tags-237.yaml +++ b/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: "|" diff --git a/features/data/configs/tags.yaml b/features/data/configs/tags.yaml index 51a2b8b2..28ef5f69 100644 --- a/features/data/configs/tags.yaml +++ b/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: "|" diff --git a/features/multiple_journals.feature b/features/multiple_journals.feature index 56c4b3a7..0f41b0d9 100644 --- a/features/multiple_journals.feature +++ b/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" diff --git a/jrnl/EncryptedJournal.py b/jrnl/EncryptedJournal.py index f3ff63c6..c7a66838 100644 --- a/jrnl/EncryptedJournal.py +++ b/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) diff --git a/jrnl/Journal.py b/jrnl/Journal.py index b3f8f47e..808084b4 100644 --- a/jrnl/Journal.py +++ b/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() diff --git a/jrnl/cli.py b/jrnl/cli.py index 139c7eb5..2aa45567 100644 --- a/jrnl/cli.py +++ b/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) diff --git a/jrnl/install.py b/jrnl/install.py index 8eb73c76..f989e0a0 100644 --- a/jrnl/install.py +++ b/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)) + '*') diff --git a/jrnl/util.py b/jrnl/util.py index f70b2df9..07140ff5 100644 --- a/jrnl/util.py +++ b/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