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

@@ -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]

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")

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'))

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)

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:

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