search

ACINQ / phoenix

Phoenix is a self-custodial Bitcoin wallet using Lightning to send/receive payments.
https://phoenix.acinq.co
Apache License 2.0
644 stars 97 forks source link

"Top private" domain shouldn't be used for lnurls #176

Closed akovalenko closed 3 years ago

akovalenko commented 3 years ago

When confirming payments and withdraws in lnurl, phoenix shows top private domain instead of a full domain name validated by a TLS certificate.

I can imagine an example where it's more beautiful and convenient than a full name, but such beauty has no place in payment handling and validation. For a random LN project with a domain outside public domain list, you have no idea who controls subdomains and whether they're semantically the same with respect to payment. There's a vast spectrum between "anyone can register" and "top domain owner controls all the content", and in no way it can be solved by any predefined lists in software.

My own service, https://lnurl-pay.me, has recently started using subdomains to support lightning address for fiat payments (like sending to 4111111111111111@uacard.lnurl-pay.me). Even as I control everything there, I want the subdomain to be visible, and it would be much more important if some payout rails are outsourced to my partners (which may happen).

If there are any doubts concerning this, consider discussing the issue with lnurl-rfc contributors in https://t.me/lnurl.

Notice also that lnurl-pay rfc requires the host to be the same for the first and the second stage. I was unable to find this check in phoenix lnurl-pay handling code, so I have no idea if it exists and whether it compares full hostnames, or just "top private domains". Consider ensuring that it is present and compares full hostnames.

dpad85 commented 3 years ago

That's a good point, we should probably display the full host in the final lnurl-pay/withdraw screen. My worry is that subdomains can get a bit arcane and confuse users, which would kinda defeat the purpose of displaying this information, but being explicit is probably better.

akovalenko commented 3 years ago

Sorry, I was wrong about second-stage lnurlpay domain required to be the same asnthe first-stage one.

Somehow this restriction ended up in two popular lnurl implementations (lntxbot and BLW/SBW) without ever being in the spec.

Today, as many new services started to provide lightning address (basically non-bech32-encoded lnurlpay with user@domain syntax), some of them violate this extra-spec requirement (and some went even further by expecting a wallet to follow htrp redirect). Following the discussion in t.me/lnurl, I see that http redirects are out, but there's some disagreement about mismatching domains. Either the spec will be amended or lntxbot/blw/sbw will back off.

Perhaps to be on the safe side, it's better for phoenix to accept mismatching domains, at least until this restriction is in the spec.

If you decide to do it, please don't revert to showing "top private" domain — this part of my report was correct and important enough.

dpad85 commented 3 years ago

Perhaps to be on the safe side, it's better for phoenix to accept mismatching domains, at least until this restriction is in the spec.

Sounds good, thanks for the follow up.