From 612a8e64246865d4412e2909d61112a721d7dadd Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 10 Jul 2023 17:50:53 +0000 Subject: [PATCH] qt: fix: bip70 pay reqs need x509 verification regression from https://github.com/spesmilo/electrum/pull/8462 - pr.verify() was called in qml, but not in qt gui - we now call pr.verify() in get_payment_request(), to make the API less error-prone - it is now ok to call pr.verify() multiple times, the result is cached --- electrum/contacts.py | 31 ++++++++++++++++++--------- electrum/gui/kivy/main_window.py | 2 +- electrum/gui/qml/qeinvoice.py | 2 +- electrum/gui/qt/main_window.py | 2 +- electrum/payment_identifier.py | 2 +- electrum/paymentrequest.py | 36 +++++++++++++++++++++----------- 6 files changed, 49 insertions(+), 26 deletions(-) diff --git a/electrum/contacts.py b/electrum/contacts.py index febfcd57d..6630dd502 100644 --- a/electrum/contacts.py +++ b/electrum/contacts.py @@ -21,7 +21,7 @@ # CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. import re -from typing import Optional, Tuple +from typing import Optional, Tuple, Dict, Any import dns import threading @@ -30,10 +30,13 @@ from dns.exception import DNSException from . import bitcoin from . import dnssec from .util import read_json_file, write_json_file, to_string -from .logging import Logger +from .logging import Logger, get_logger from .util import trigger_callback +_logger = get_logger(__name__) + + class AliasNotFoundException(Exception): pass @@ -90,7 +93,13 @@ class Contacts(dict, Logger): 'address': addr, 'type': 'contact' } - out = self.resolve_openalias(k) + if openalias := self.resolve_openalias(k): + return openalias + raise AliasNotFoundException("Invalid Bitcoin address or alias", k) + + @classmethod + def resolve_openalias(cls, url: str) -> Dict[str, Any]: + out = cls._resolve_openalias(url) if out: address, name, validated = out return { @@ -99,7 +108,7 @@ class Contacts(dict, Logger): 'type': 'openalias', 'validated': validated } - raise AliasNotFoundException("Invalid Bitcoin address or alias", k) + return {} def by_name(self, name): for k in self.keys(): @@ -118,33 +127,35 @@ class Contacts(dict, Logger): if alias: alias = str(alias) def f(): - self.alias_info = self.resolve_openalias(alias) + self.alias_info = self._resolve_openalias(alias) trigger_callback('alias_received') t = threading.Thread(target=f) t.daemon = True t.start() - def resolve_openalias(self, url: str) -> Optional[Tuple[str, str, bool]]: + @classmethod + def _resolve_openalias(cls, url: str) -> Optional[Tuple[str, str, bool]]: # support email-style addresses, per the OA standard url = url.replace('@', '.') try: records, validated = dnssec.query(url, dns.rdatatype.TXT) except DNSException as e: - self.logger.info(f'Error resolving openalias: {repr(e)}') + _logger.info(f'Error resolving openalias: {repr(e)}') return None prefix = 'btc' for record in records: string = to_string(record.strings[0], 'utf8') if string.startswith('oa1:' + prefix): - address = self.find_regex(string, r'recipient_address=([A-Za-z0-9]+)') - name = self.find_regex(string, r'recipient_name=([^;]+)') + address = cls.find_regex(string, r'recipient_address=([A-Za-z0-9]+)') + name = cls.find_regex(string, r'recipient_name=([^;]+)') if not name: name = address if not address: continue return address, name, validated - def find_regex(self, haystack, needle): + @staticmethod + def find_regex(haystack, needle): regex = re.compile(needle) try: return regex.search(haystack).groups()[0] diff --git a/electrum/gui/kivy/main_window.py b/electrum/gui/kivy/main_window.py index 590e7b0fb..9697dcd04 100644 --- a/electrum/gui/kivy/main_window.py +++ b/electrum/gui/kivy/main_window.py @@ -459,7 +459,7 @@ class ElectrumWindow(App, Logger, EventListener): if not self.wallet: self.show_error(_('No wallet loaded.')) return - if pr.verify(self.wallet.contacts): + if pr.verify(): invoice = Invoice.from_bip70_payreq(pr, height=0) if invoice and self.wallet.get_invoice_status(invoice) == PR_PAID: self.show_error("invoice already paid") diff --git a/electrum/gui/qml/qeinvoice.py b/electrum/gui/qml/qeinvoice.py index 171959dbd..7a73ece05 100644 --- a/electrum/gui/qml/qeinvoice.py +++ b/electrum/gui/qml/qeinvoice.py @@ -473,7 +473,7 @@ class QEInvoiceParser(QEInvoice): def _bip70_payment_request_resolved(self, pr: 'PaymentRequest'): self._logger.debug('resolved payment request') - if pr.verify(self._wallet.wallet.contacts): + if pr.verify(): invoice = Invoice.from_bip70_payreq(pr, height=0) if self._wallet.wallet.get_invoice_status(invoice) == PR_PAID: self.validationError.emit('unknown', _('Invoice already paid')) diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index cf70c2f83..d79973bd4 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -1438,7 +1438,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): grid.addWidget(QLabel(format_time(invoice.exp + invoice.time)), 4, 1) if invoice.bip70: pr = paymentrequest.PaymentRequest(bytes.fromhex(invoice.bip70)) - pr.verify(self.contacts) + pr.verify() grid.addWidget(QLabel(_("Requestor") + ':'), 5, 0) grid.addWidget(QLabel(pr.get_requestor()), 5, 1) grid.addWidget(QLabel(_("Signature") + ':'), 6, 0) diff --git a/electrum/payment_identifier.py b/electrum/payment_identifier.py index c601889a2..93a9191a6 100644 --- a/electrum/payment_identifier.py +++ b/electrum/payment_identifier.py @@ -340,7 +340,7 @@ class PaymentIdentifier(Logger): elif self.bip70: from . import paymentrequest pr = await paymentrequest.get_payment_request(self.bip70) - if not pr.error: + if pr.verify(): self.bip70_data = pr self.set_state(PaymentIdentifierState.MERCHANT_NOTIFY) else: diff --git a/electrum/paymentrequest.py b/electrum/paymentrequest.py index 73273dd7e..49082f422 100644 --- a/electrum/paymentrequest.py +++ b/electrum/paymentrequest.py @@ -39,13 +39,14 @@ except ImportError: sys.exit("Error: could not find paymentrequest_pb2.py. Create it with 'contrib/generate_payreqpb2.sh'") from . import bitcoin, constants, ecc, util, transaction, x509, rsakey -from .util import bfh, make_aiohttp_session, error_text_bytes_to_safe_str +from .util import bfh, make_aiohttp_session, error_text_bytes_to_safe_str, get_running_loop from .invoices import Invoice, get_id_from_onchain_outputs from .crypto import sha256 from .bitcoin import address_to_script from .transaction import PartialTxOutput from .network import Network from .logging import get_logger, Logger +from .contacts import Contacts if TYPE_CHECKING: from .simple_config import SimpleConfig @@ -104,6 +105,10 @@ async def get_payment_request(url: str) -> 'PaymentRequest': data = None error = f"Unknown scheme for payment request. URL: {url}" pr = PaymentRequest(data, error=error) + loop = get_running_loop() + # do x509/dnssec verification now (in separate thread, to avoid blocking event loop). + # we still expect the caller to at least check pr.error! + await loop.run_in_executor(None, pr.verify) return pr @@ -111,15 +116,17 @@ class PaymentRequest: def __init__(self, data: bytes, *, error=None): self.raw = data - self.error = error # FIXME overloaded and also used when 'verify' succeeds - self.parse(data) + self.error = error # type: Optional[str] + self._verified_success = None # caches result of _verify + self._verified_success_msg = None # type: Optional[str] + self._parse(data) self.requestor = None # known after verify self.tx = None def __str__(self): return str(self.raw) - def parse(self, r: bytes): + def _parse(self, r: bytes): self.outputs = [] # type: List[PartialTxOutput] if self.error: return @@ -147,8 +154,11 @@ class PaymentRequest: self.memo = self.details.memo self.payment_url = self.details.payment_url - def verify(self, contacts): + def verify(self) -> bool: # FIXME: we should enforce that this method was called before we attempt payment + # note: this method might do network requests (at least for verify_dnssec) + if self._verified_success is True: + return True if self.error: return False if not self.raw: @@ -167,7 +177,7 @@ class PaymentRequest: if pr.pki_type in ["x509+sha256", "x509+sha1"]: return self.verify_x509(pr) elif pr.pki_type in ["dnssec+btc", "dnssec+ecdsa"]: - return self.verify_dnssec(pr, contacts) + return self.verify_dnssec(pr) else: self.error = "ERROR: Unsupported PKI Type for Message Signature" return False @@ -209,13 +219,14 @@ class PaymentRequest: self.error = "ERROR: Invalid Signature for Payment Request Data" return False ### SIG Verified - self.error = 'Signed by Trusted CA: ' + ca.get_common_name() + self._verified_success_msg = 'Signed by Trusted CA: ' + ca.get_common_name() + self._verified_success = True return True - def verify_dnssec(self, pr, contacts): + def verify_dnssec(self, pr): sig = pr.signature alias = pr.pki_data - info = contacts.resolve(alias) + info = Contacts.resolve_openalias(alias) if info.get('validated') is not True: self.error = "Alias verification failed (DNSSEC)" return False @@ -225,7 +236,8 @@ class PaymentRequest: pr.signature = b'' message = pr.SerializeToString() if ecc.verify_message_with_address(address, sig, message): - self.error = 'Verified with DNSSEC' + self._verified_success_msg = 'Verified with DNSSEC' + self._verified_success = True return True else: self.error = "verify failed" @@ -257,8 +269,8 @@ class PaymentRequest: def get_requestor(self): return self.requestor if self.requestor else self.get_address() - def get_verify_status(self): - return self.error if self.requestor else "No Signature" + def get_verify_status(self) -> str: + return (self.error or self._verified_success_msg) if self.requestor else "No Signature" def get_memo(self): return self.memo