Add PaymentId authentication to public API
#3242
Conversation
|
CI is very sad, also feel free to rebase. |
jkczyz
force-pushed
the
2024-08-payment-id-auth-api
branch
from
4a1450c to
877f115
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3242 +/- ##
==========================================
+ Coverage 89.78% 90.14% +0.36%
==========================================
Files 123 124 +1
Lines 102330 106664 +4334
Branches 102330 106664 +4334
==========================================
+ Hits 91876 96152 +4276
- Misses 7763 7858 +95
+ Partials 2691 2654 -37 ☔ View full report in Codecov by Sentry. |
jkczyz
force-pushed
the
2024-08-payment-id-auth-api
branch
from
877f115 to
aba273a
Compare
lightning/src/ln/channelmanager.rs
Outdated
|
|
||
| /// Constructs an HMAC to include in [`OffersContext::OutboundPayment`] for the payment id | ||
| /// along with the given [`Nonce`]. | ||
| pub fn hmac_for_offer( |
There was a problem hiding this comment.
re: *_for_offer -- this may also be used for refunds, right? May be easier to name it in relation to invoice errors since that's what it's used for?
There was a problem hiding this comment.
We don't set the HMAC in refunds because (1) it would increase the QR code size and (2) we don't necessarily want someone to send an InvoiceError to one in response -- rather the Refund creator can set a short expiry or abandon it manually.
There was a problem hiding this comment.
Re: naming -- how about hmac_using_nonce?
There was a problem hiding this comment.
I think I do prefer that name... hmac_for_offer_payment also works for me.
There was a problem hiding this comment.
hmac_for_offer_payment SGTM
jkczyz
force-pushed
the
2024-08-payment-id-auth-api
branch
from
aba273a to
c6c9c29
Compare
| @@ -410,6 +410,6 @@ pub(crate) fn hmac_for_payment_id( | |||
|
|
|||
There was a problem hiding this comment.
Re: hmac_for_offer call above -- it looks like ExpandedKey::hmac_for_offer docs need an update since it is only supposed to be used to construct offer metadata atm. It almost looks like we should have used a new key for InvoiceError verification, though I might be missing a prior discussion.
There was a problem hiding this comment.
Right, we are using it for both offer metadata and payer metadata. I think we were fine with this because we add different inputs for each use case. @TheBlueMatt Is that correct?
There was a problem hiding this comment.
Yea, we use a different IV (which is always in the same position in the data being hashed) so its fine.
When receiving an InvoiceError message, it should be authenticated before using it to abandon the payment. Add methods to PaymentId's public API for constructing and verifying an HMAC for use in OffersContext::OutboundPayment. This allows other implementations of OffersMessageHandler to construct the HMAC and authenticate the message.
jkczyz
force-pushed
the
2024-08-payment-id-auth-api
branch
from
c6c9c29 to
5bee20e
Compare
valentinewallace
merged commit ccce9d9
into
lightningdevkit:main
When receiving an
InvoiceErrormessage, it should be authenticated before using it to abandon the payment. Add methods toPaymentId's public API for constructing and verifying an HMAC for use inOffersContext::OutboundPayment. This allows other implementations ofOffersMessageHandlerto construct the HMAC and authenticate the message.