phoenix
phoenix copied to clipboard
ACINQ
Remove security warning when sending to zero amount invoices
The exploit that allowed intermediate routing nodes to potentially steal a user funds when using zero amount invoices appears to have been patched a long time ago but Phoenix still shows a security warning when attempting to send to a zero amount invoice the first time the user attempts to send to one.
This security warning should be removed as it creates unnecessary stress on the user.
No, this warning cannot be removed yet. We need recipients to implement trampoline to be completely safe. Before that, our node, if malicious, could in theory steal the payment, so users should be made aware.
It's true for all LSP that do the path-finding on behalf of the wallet (unless they compute the route, then send it to the wallet and let the wallet create the payment onion), whether they use trampoline or not.
For trampoline nodes, it's true whenever the recipient does not support trampoline. If the recipient supports trampoline (e.g. phoenix -> phoenix payments) then this warning doesn't apply.
Is that clearer? It's still a bit messy, the only way to really resolve this would be to have other wallets and implementations at least support receiving trampoline payments, even if they don't support sending or relaying them.
Can you describe why? Requiring payment secrets ubiquitously resolved the issues which had been previously described, as far as I'm aware.
This is because of trampoline (and its lack of support in wallets).
We have what I hoped would be a very temporary "hack" in Phoenix where if the recipient doesn't support trampoline, Phoenix lets our node transform the payment to a "normal" payment. That means it's really our node making the payment on behalf of Phoenix, so it needs all the information from the invoice, including the payment_secret. Since our node has the payment_secret, we could pay less than what the sender asked us to.
I was young and optimistic when we did that, I figured that implementing just the trampoline onion decryption part was so easy that all wallets would support it soon and we could deprecate that hack, but that didn't happen :sweat_smile:
So until Phoenix is able to do real trampoline payments to everyone, I'd like to keep this warning to show that paying amountless invoices to non-trampoline recipients requires trust in our node.
Huh, that sucks. Can you have the payer do the mpp split (randomly or just don't split if its a small payment) and just give the last hop pre-encrypted to the server? Not sure how much that would kill payment reliability.
That wouldn't use trampoline at all anymore, because the sender doesn't know yet what the last hop could be. We would need to compute routes, send them back to the sender and let them build the onion themselves. This is quite a big change, I don't think this issue is high enough on our priority list to implement: I'd rather spend time explaining trampoline and convincing wallets to implement at least the recipient part!
Maybe I'm missing something, the sender doesn't know the second-to-last-hop, but they know who is going to receive the payment. What the sender doesn't know is how much and how many MPP parts the recipient should receive, but you could restrict that somewhat, it just may have less payment reliability.
I agree that there are different protocols that could let users delegate path-finding without disclosing the payment_secret, but that means implementing a new protocol, whereas I believe having trampoline support everywhere is a clean solution, so there's no reason for us right now to change this (until the discussions on trampoline make more progress).
Right, yea, I get that'd be a big change, so not suggesting it be a priority, I was just kinda hoping 0 amount invoices would be more available sooner rather than later.