From 28fe345b0bcf708c06abce8382d00de30d210b75 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 12 Jul 2022 15:31:03 +0200 Subject: [PATCH] keystore.check_password: raise better exc if called on pwless ks If keystore.check_password is called with some pw on a keystore that does not have a password set, it now raises better exceptions: it should now always raise InvalidPassword, and with a nicer msg. Previously the exc type would depend on the ks type. Examples before change: ``` >>> wallet.keystore.check_password("asd") Traceback (most recent call last): File "/home/user/wspace/electrum/electrum/keystore.py", line 580, in check_password xprv = pw_decode(self.xprv, password, version=self.pw_hash_version) File "/home/user/wspace/electrum/electrum/crypto.py", line 311, in pw_decode plaintext_bytes = pw_decode_bytes(data, password, version=version) File "/home/user/wspace/electrum/electrum/crypto.py", line 270, in pw_decode_bytes data_bytes = bytes(base64.b64decode(data)) File "/usr/lib/python3.10/base64.py", line 87, in b64decode return binascii.a2b_base64(s) binascii.Error: Incorrect padding ``` ``` >>> wallet.keystore.check_password("asd") Traceback (most recent call last): s = aes_decrypt_with_iv(secret, iv, e) File "/home/user/wspace/electrum/electrum/crypto.py", line 157, in aes_decrypt_with_iv data = decryptor.update(data) + decryptor.finalize() File "/usr/lib/python3/dist-packages/cryptography/hazmat/primitives/ciphers/base.py", line 148, in finalize data = self._ctx.finalize() File "/usr/lib/python3/dist-packages/cryptography/hazmat/backends/openssl/ciphers.py", line 193, in finalize raise ValueError( ValueError: The length of the provided data is not a multiple of the block length. The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/home/user/wspace/electrum/electrum/gui/qt/console.py", line 254, in exec_command result = eval(command, self.namespace, self.namespace) File "", line 1, in File "/home/user/wspace/electrum/electrum/keystore.py", line 248, in check_password self.get_private_key(pubkey, password) File "/home/user/wspace/electrum/electrum/keystore.py", line 267, in get_private_key sec = pw_decode(self.keypairs[pubkey], password, version=self.pw_hash_version) File "/home/user/wspace/electrum/electrum/crypto.py", line 311, in pw_decode plaintext_bytes = pw_decode_bytes(data, password, version=version) File "/home/user/wspace/electrum/electrum/crypto.py", line 271, in pw_decode_bytes return _pw_decode_raw(data_bytes, password, version=version) File "/home/user/wspace/electrum/electrum/crypto.py", line 255, in _pw_decode_raw raise InvalidPassword() from e electrum.util.InvalidPassword: Incorrect password ``` ----- Examples after change: ``` >>> wallet.keystore.check_password("asd") Traceback (most recent call last): return binascii.a2b_base64(s) binascii.Error: Incorrect padding The above exception was the direct cause of the following exception: Traceback (most recent call last): File "...\electrum\keystore.py", line 68, in wrapper return check_password_fn(self, password) File "...\electrum\keystore.py", line 605, in check_password xprv = pw_decode(self.xprv, password, version=self.pw_hash_version) File "...\electrum\crypto.py", line 311, in pw_decode plaintext_bytes = pw_decode_bytes(data, password, version=version) File "...\electrum\crypto.py", line 267, in pw_decode_bytes raise CiphertextFormatError("ciphertext not valid base64") from e electrum.crypto.CiphertextFormatError: ciphertext not valid base64 The above exception was the direct cause of the following exception: Traceback (most recent call last): File "...\electrum\gui\qt\console.py", line 254, in exec_command result = eval(command, self.namespace, self.namespace) File "", line 1, in File "...\electrum\keystore.py", line 76, in wrapper raise InvalidPassword("password given but keystore has no password") from e electrum.util.InvalidPassword: password given but keystore has no password ``` ``` >>> wallet.keystore.check_password("asd") Traceback (most recent call last): s = aes_decrypt_with_iv(secret, iv, e) File "...\electrum\crypto.py", line 158, in aes_decrypt_with_iv data = cipher.decrypt(data) File "...\Python310\site-packages\Cryptodome\Cipher\_mode_cbc.py", line 246, in decrypt raise ValueError("Data must be padded to %d byte boundary in CBC mode" % self.block_size) ValueError: Data must be padded to 16 byte boundary in CBC mode The above exception was the direct cause of the following exception: Traceback (most recent call last): File "...\electrum\keystore.py", line 68, in wrapper return check_password_fn(self, password) File "...\electrum\keystore.py", line 272, in check_password self.get_private_key(pubkey, password) File "...\electrum\keystore.py", line 291, in get_private_key sec = pw_decode(self.keypairs[pubkey], password, version=self.pw_hash_version) File "...\electrum\crypto.py", line 311, in pw_decode plaintext_bytes = pw_decode_bytes(data, password, version=version) File "...\electrum\crypto.py", line 268, in pw_decode_bytes return _pw_decode_raw(data_bytes, password, version=version) File "...\electrum\crypto.py", line 249, in _pw_decode_raw raise InvalidPassword() from e electrum.util.InvalidPassword: Incorrect password The above exception was the direct cause of the following exception: Traceback (most recent call last): File "...\electrum\gui\qt\console.py", line 254, in exec_command result = eval(command, self.namespace, self.namespace) File "", line 1, in File "...\electrum\keystore.py", line 76, in wrapper raise InvalidPassword("password given but keystore has no password") from e electrum.util.InvalidPassword: password given but keystore has no password ``` --- electrum/crypto.py | 15 +++++++++++++-- electrum/keystore.py | 32 +++++++++++++++++++++++++++++--- electrum/util.py | 8 +++++++- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/electrum/crypto.py b/electrum/crypto.py index f948736f1..4b50d6825 100644 --- a/electrum/crypto.py +++ b/electrum/crypto.py @@ -24,6 +24,7 @@ # SOFTWARE. import base64 +import binascii import os import sys import hashlib @@ -109,6 +110,10 @@ class InvalidPadding(Exception): pass +class CiphertextFormatError(Exception): + pass + + def append_PKCS7_padding(data: bytes) -> bytes: assert_bytes(data) padlen = 16 - (len(data) % 16) @@ -256,7 +261,10 @@ def pw_decode_bytes(data: str, password: Union[bytes, str], *, version:int) -> b """base64 ciphertext -> plaintext bytes""" if version not in KNOWN_PW_HASH_VERSIONS: raise UnexpectedPasswordHashVersion(version) - data_bytes = bytes(base64.b64decode(data)) + try: + data_bytes = bytes(base64.b64decode(data, validate=True)) + except binascii.Error as e: + raise CiphertextFormatError("ciphertext not valid base64") from e return _pw_decode_raw(data_bytes, password, version=version) @@ -273,7 +281,10 @@ def pw_encode_with_version_and_mac(data: bytes, password: Union[bytes, str]) -> def pw_decode_with_version_and_mac(data: str, password: Union[bytes, str]) -> bytes: """base64 ciphertext -> plaintext bytes""" - data_bytes = bytes(base64.b64decode(data)) + try: + data_bytes = bytes(base64.b64decode(data, validate=True)) + except binascii.Error as e: + raise CiphertextFormatError("ciphertext not valid base64") from e version = int(data_bytes[0]) encrypted = data_bytes[1:-4] mac = data_bytes[-4:] diff --git a/electrum/keystore.py b/electrum/keystore.py index 578e198de..353f33022 100644 --- a/electrum/keystore.py +++ b/electrum/keystore.py @@ -28,7 +28,7 @@ from unicodedata import normalize import hashlib import re from typing import Tuple, TYPE_CHECKING, Union, Sequence, Optional, Dict, List, NamedTuple -from functools import lru_cache +from functools import lru_cache, wraps from abc import ABC, abstractmethod from . import bitcoin, ecc, constants, bip32 @@ -39,7 +39,8 @@ from .bip32 import (convert_bip32_path_to_list_of_uint32, BIP32_PRIME, convert_bip32_intpath_to_strpath, is_xkey_consistent_with_key_origin_info) from .ecc import string_to_number from .crypto import (pw_decode, pw_encode, sha256, sha256d, PW_HASH_VERSION_LATEST, - SUPPORTED_PW_HASH_VERSIONS, UnsupportedPasswordHashVersion, hash_160) + SUPPORTED_PW_HASH_VERSIONS, UnsupportedPasswordHashVersion, hash_160, + CiphertextFormatError) from .util import (InvalidPassword, WalletFileException, BitcoinException, bh2u, bfh, inv_dict, is_hex_str) from .mnemonic import Mnemonic, Wordlist, seed_type, is_seed @@ -56,6 +57,27 @@ if TYPE_CHECKING: class CannotDerivePubkey(Exception): pass +def also_test_none_password(check_password_fn): + """Decorator for check_password, simply to give a friendlier exception if + check_password(x) is called on a keystore that does not have a password set. + """ + @wraps(check_password_fn) + def wrapper(self: 'Software_KeyStore', *args): + password = args[0] + try: + return check_password_fn(self, password) + except (CiphertextFormatError, InvalidPassword) as e: + if password is not None: + try: + check_password_fn(self, None) + except Exception: + pass + else: + raise InvalidPassword("password given but keystore has no password") from e + raise + return wrapper + + class KeyStore(Logger, ABC): type: str @@ -212,7 +234,8 @@ class Software_KeyStore(KeyStore): pass @abstractmethod - def check_password(self, password): + def check_password(self, password: Optional[str]) -> None: + """Raises InvalidPassword if password is not correct""" pass @abstractmethod @@ -243,6 +266,7 @@ class Imported_KeyStore(Software_KeyStore): def can_import(self): return True + @also_test_none_password def check_password(self, password): pubkey = list(self.keypairs.keys())[0] self.get_private_key(pubkey, password) @@ -576,6 +600,7 @@ class BIP32_KeyStore(Xpub, Deterministic_KeyStore): def get_master_private_key(self, password): return pw_decode(self.xprv, password, version=self.pw_hash_version) + @also_test_none_password def check_password(self, password): xprv = pw_decode(self.xprv, password, version=self.pw_hash_version) try: @@ -742,6 +767,7 @@ class Old_KeyStore(MasterPublicKeyMixin, Deterministic_KeyStore): if master_public_key != bfh(self.mpk): raise InvalidPassword() + @also_test_none_password def check_password(self, password): seed = self.get_hex_seed(password) self._check_seed(seed) diff --git a/electrum/util.py b/electrum/util.py index 157106097..b82a650bb 100644 --- a/electrum/util.py +++ b/electrum/util.py @@ -144,8 +144,14 @@ class NoDynamicFeeEstimates(Exception): class InvalidPassword(Exception): + def __init__(self, message: Optional[str] = None): + self.message = message + def __str__(self): - return _("Incorrect password") + if self.message is None: + return _("Incorrect password") + else: + return str(self.message) class AddTransactionException(Exception):