Skip to content

Commit 07b5963

Browse files
authored
Merge pull request #10349 from SomberNight/202512_openalias
openalias: always require DNSSEC validation, and fix a qml regression
2 parents 23b6465 + cf8c243 commit 07b5963

File tree

7 files changed

+25
-32
lines changed

7 files changed

+25
-32
lines changed

electrum/commands.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -870,12 +870,10 @@ async def _resolver(self, x, wallet: Abstract_Wallet):
870870
if x is None:
871871
return None
872872
out = await wallet.contacts.resolve(x)
873-
if out.get('type') == 'openalias' and self.nocheck is False and out.get('validated') is False:
874-
raise UserFacingException(f"cannot verify alias: {x}")
875873
return out['address']
876874

877875
@command('n')
878-
async def sweep(self, privkey, destination, fee=None, feerate=None, nocheck=False, imax=100):
876+
async def sweep(self, privkey, destination, fee=None, feerate=None, imax=100):
879877
"""
880878
Sweep private keys. Returns a transaction that spends UTXOs from
881879
privkey to a destination address. The transaction will not be broadcast.
@@ -885,12 +883,10 @@ async def sweep(self, privkey, destination, fee=None, feerate=None, nocheck=Fals
885883
arg:decimal:fee:Transaction fee (absolute, in BTC)
886884
arg:decimal:feerate:Transaction fee rate (in sat/vbyte)
887885
arg:int:imax:Maximum number of inputs
888-
arg:bool:nocheck:Do not verify aliases
889886
"""
890887
from .wallet import sweep
891888
fee_policy = self._get_fee_policy(fee, feerate)
892889
privkeys = privkey.split()
893-
self.nocheck = nocheck
894890
#dest = self._resolver(destination)
895891
tx = await sweep(
896892
privkeys,
@@ -942,7 +938,7 @@ def _get_fee_policy(self, fee: str, feerate: str):
942938

943939
@command('wp')
944940
async def payto(self, destination, amount, fee=None, feerate=None, from_addr=None, from_coins=None, change_addr=None,
945-
nocheck=False, unsigned=False, rbf=True, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None):
941+
unsigned=False, rbf=True, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None):
946942
"""Create an on-chain transaction.
947943
948944
arg:str:destination:Bitcoin address, contact or alias
@@ -955,7 +951,6 @@ async def payto(self, destination, amount, fee=None, feerate=None, from_addr=Non
955951
arg:bool:addtransaction:Whether transaction is to be used for broadcasting afterwards. Adds transaction to the wallet
956952
arg:int:locktime:Set locktime block number
957953
arg:bool:unsigned:Do not sign transaction
958-
arg:bool:nocheck:Do not verify aliases
959954
arg:json:from_coins:Source coins (must be in wallet; use sweep to spend from non-wallet address)
960955
"""
961956
return await self.paytomany(
@@ -965,7 +960,6 @@ async def payto(self, destination, amount, fee=None, feerate=None, from_addr=Non
965960
from_addr=from_addr,
966961
from_coins=from_coins,
967962
change_addr=change_addr,
968-
nocheck=nocheck,
969963
unsigned=unsigned,
970964
rbf=rbf,
971965
password=password,
@@ -976,7 +970,7 @@ async def payto(self, destination, amount, fee=None, feerate=None, from_addr=Non
976970

977971
@command('wp')
978972
async def paytomany(self, outputs, fee=None, feerate=None, from_addr=None, from_coins=None, change_addr=None,
979-
nocheck=False, unsigned=False, rbf=True, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None):
973+
unsigned=False, rbf=True, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None):
980974
"""Create a multi-output transaction.
981975
982976
arg:json:outputs:json list of ["address", "amount in BTC"]
@@ -988,10 +982,8 @@ async def paytomany(self, outputs, fee=None, feerate=None, from_addr=None, from_
988982
arg:bool:addtransaction:Whether transaction is to be used for broadcasting afterwards. Adds transaction to the wallet
989983
arg:int:locktime:Set locktime block number
990984
arg:bool:unsigned:Do not sign transaction
991-
arg:bool:nocheck:Do not verify aliases
992985
arg:json:from_coins:Source coins (must be in wallet; use sweep to spend from non-wallet address)
993986
"""
994-
self.nocheck = nocheck
995987
fee_policy = self._get_fee_policy(fee, feerate)
996988
domain_addr = from_addr.split(',') if from_addr else None
997989
domain_coins = from_coins.split(',') if from_coins else None
@@ -1150,7 +1142,11 @@ async def getopenalias(self, key, wallet: Abstract_Wallet = None):
11501142
11511143
arg:str:key:the alias to be retrieved
11521144
"""
1153-
return await wallet.contacts.resolve(key)
1145+
d = await wallet.contacts.resolve(key)
1146+
if d.get("type") == "openalias":
1147+
# we always validate DNSSEC now
1148+
d["validated"] = True
1149+
return d
11541150

11551151
@command('w')
11561152
async def searchcontacts(self, query, wallet: Abstract_Wallet = None):

electrum/contacts.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,11 @@ async def resolve(self, k) -> dict:
107107
async def resolve_openalias(cls, url: str) -> Dict[str, Any]:
108108
out = await cls._resolve_openalias(url)
109109
if out:
110-
address, name, validated = out
110+
address, name = out
111111
return {
112112
'address': address,
113113
'name': name,
114114
'type': 'openalias',
115-
'validated': validated
116115
}
117116
return {}
118117

@@ -138,14 +137,17 @@ async def f():
138137
asyncio.run_coroutine_threadsafe(f(), get_asyncio_loop())
139138

140139
@classmethod
141-
async def _resolve_openalias(cls, url: str) -> Optional[Tuple[str, str, bool]]:
140+
async def _resolve_openalias(cls, url: str) -> Optional[Tuple[str, str]]:
142141
# support email-style addresses, per the OA standard
143142
url = url.replace('@', '.')
144143
try:
145144
records, validated = await dnssec.query(url, dns.rdatatype.TXT)
146145
except DNSException as e:
147146
_logger.info(f'Error resolving openalias: {repr(e)}')
148147
return None
148+
if not validated: # enforce DNSSEC validation. without it, DNS is completely insecure
149+
_logger.info(f"DNSSEC validation failed for {url=!r}, or maybe dependencies are missing and could not even try.")
150+
return None
149151
prefix = 'btc'
150152
for record in records:
151153
if record.rdtype != dns.rdatatype.TXT:
@@ -158,7 +160,7 @@ async def _resolve_openalias(cls, url: str) -> Optional[Tuple[str, str, bool]]:
158160
name = address
159161
if not address:
160162
continue
161-
return address, name, validated
163+
return address, name
162164
return None
163165

164166
@staticmethod

electrum/dnssec.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
# http://backreference.org/2010/11/17/dnssec-verification-with-dig/
3131
# https://github.com/rthalley/dnspython/blob/master/tests/test_dnssec.py
3232

33+
import logging
3334

3435
import dns
3536
import dns.name
@@ -137,7 +138,11 @@ async def _get_and_validate(ns, url, _type) -> dns.rrset.RRset:
137138
return rrset
138139

139140

140-
async def query(url, rtype) -> Tuple[dns.rrset.RRset, bool]:
141+
async def query(url: str, rtype: dns.rdatatype.RdataType) -> Tuple[dns.rrset.RRset, bool]:
142+
"""Try to do DNS resolution, including DNSSEC.
143+
'validated' shows whether the DNSSEC checks passed. DNS is completely INSECURE without DNSSEC,
144+
so the caller must carefully consider whether the response can be used for anything if validated=False.
145+
"""
141146
# FIXME this method is not using the network proxy. (although the proxy might not support UDP?)
142147
# 8.8.8.8 is Google's public DNS server
143148
nameservers = ['8.8.8.8']
@@ -146,7 +151,8 @@ async def query(url, rtype) -> Tuple[dns.rrset.RRset, bool]:
146151
out = await _get_and_validate(ns, url, rtype)
147152
validated = True
148153
except Exception as e:
149-
_logger.info(f"DNSSEC error: {repr(e)}")
154+
log_level = logging.WARNING if isinstance(e, ImportError) else logging.INFO
155+
_logger.log(log_level, f"DNSSEC error: {repr(e)}")
150156
out = await dns.asyncresolver.resolve(url, rtype)
151157
validated = False
152158
return out, validated

electrum/gui/qml/qeinvoice.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,8 @@ def validateRecipient(self, pi: PaymentIdentifier):
529529
PaymentIdentifierType.SPK, PaymentIdentifierType.BIP21,
530530
PaymentIdentifierType.BIP70, PaymentIdentifierType.BOLT11,
531531
PaymentIdentifierType.LNADDR, PaymentIdentifierType.LNURLP,
532-
PaymentIdentifierType.EMAILLIKE, PaymentIdentifierType.DOMAINLIKE
532+
PaymentIdentifierType.EMAILLIKE, PaymentIdentifierType.DOMAINLIKE,
533+
PaymentIdentifierType.OPENALIAS,
533534
]:
534535
self.validationError.emit('unknown', _('Unknown invoice'))
535536
return

electrum/gui/qt/settings_dialog.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,8 +437,7 @@ def set_alias_color(self):
437437
self.alias_e.setStyleSheet("")
438438
return
439439
if self.wallet.contacts.alias_info:
440-
alias_addr, alias_name, validated = self.wallet.contacts.alias_info
441-
self.alias_e.setStyleSheet((ColorScheme.GREEN if validated else ColorScheme.RED).as_stylesheet(True))
440+
self.alias_e.setStyleSheet(ColorScheme.GREEN.as_stylesheet(True))
442441
else:
443442
self.alias_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True))
444443

electrum/payment_identifier.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,10 +330,6 @@ async def _do_resolve(self, *, on_finished: Callable[['PaymentIdentifier'], None
330330
elif openalias_result := await openalias_task:
331331
self.openalias_data = openalias_result
332332
address = openalias_result.get('address')
333-
if not openalias_result.get('validated'):
334-
self.warning = _(
335-
'WARNING: the alias "{}" could not be validated via an additional '
336-
'security check, DNSSEC, and thus may not be correct.').format(openalias_key)
337333
try:
338334
# this assertion error message is shown in the GUI
339335
assert bitcoin.is_address(address), f"{_('Openalias address invalid')}: {address[:100]}"
@@ -594,10 +590,6 @@ def get_fields_for_GUI(self) -> FieldsForGUI:
594590
name = self.openalias_data.get('name')
595591
description = name
596592
recipient = key + ' <' + address + '>'
597-
validated = self.openalias_data.get('validated')
598-
if not validated:
599-
self.warning = _('WARNING: the alias "{}" could not be validated via an additional '
600-
'security check, DNSSEC, and thus may not be correct.').format(key)
601593

602594
elif self.bolt11:
603595
recipient, amount, description = self._get_bolt11_fields()

electrum/paymentrequest.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,9 +225,6 @@ async def verify_dnssec(self, pr):
225225
sig = pr.signature
226226
alias = pr.pki_data
227227
info: dict = await Contacts.resolve_openalias(alias)
228-
if info.get('validated') is not True:
229-
self.error = "Alias verification failed (DNSSEC)"
230-
return False
231228
if pr.pki_type == "dnssec+btc":
232229
self.requestor = alias
233230
address = info.get('address')

0 commit comments

Comments
 (0)