Skip to content

Commit aec456f

Browse files
TheBlueMattclaude
andcommitted
Commit to payment_metadata in inbound payment HMAC
When payment_metadata is set in a BOLT 11 invoice, users expect to receive it back as-is in the payment onion. In order to ensure it isn't tampered with, they presumably will add an HMAC, or worse, not add one and forget that it can be tampered with. Instead, here we include it in the HMAC computation for the payment secret. This ensures that the sender must relay the correct metadata for the payment to be accepted by the receiver, binding the metadata to the payment cryptographically. The metadata is only included in the HMAC when present, so existing payments without metadata continue to verify correctly. However, this does break receiving payments with metadata today. On an upgrade this seems acceptable to me given we have seen almost no use of payment metadata in practice. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9f233e8 commit aec456f

12 files changed

Lines changed: 185 additions & 65 deletions

fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1370,7 +1370,7 @@ impl PaymentTracker {
13701370
payment_preimage.0[0..8].copy_from_slice(&self.payment_ctr.to_be_bytes());
13711371
let hash = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array());
13721372
let secret = dest
1373-
.create_inbound_payment_for_hash(hash, None, 3600, None)
1373+
.create_inbound_payment_for_hash(hash, None, 3600, None, None)
13741374
.expect("create_inbound_payment_for_hash failed");
13751375
assert!(self.payment_preimages.insert(hash, payment_preimage).is_none());
13761376
let mut id = PaymentId([0; 32]);

fuzz/src/full_stack.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -837,11 +837,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger + MaybeSend + MaybeSync>
837837
},
838838
16 => {
839839
let payment_preimage = PaymentPreimage(keys_manager.get_secure_random_bytes());
840-
let payment_hash =
841-
PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
840+
let hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
842841
// Note that this may fail - our hashes may collide and we'll end up trying to
843842
// double-register the same payment_hash.
844-
let _ = channelmanager.create_inbound_payment_for_hash(payment_hash, None, 1, None);
843+
let _ = channelmanager.create_inbound_payment_for_hash(hash, None, 1, None, None);
845844
},
846845
9 => {
847846
for payment in payments_received.drain(..) {

lightning-liquidity/tests/lsps2_integration_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ fn create_jit_invoice(
122122
let min_final_cltv_expiry_delta = MIN_FINAL_CLTV_EXPIRY_DELTA + 2;
123123
let (payment_hash, payment_secret) = node
124124
.node
125-
.create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta))
125+
.create_inbound_payment(None, expiry_secs, Some(min_final_cltv_expiry_delta), None)
126126
.map_err(|e| {
127127
log_error!(node.logger, "Failed to register inbound payment: {:?}", e);
128128
})?;

lightning/src/ln/bolt11_payment_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ fn payment_metadata_end_to_end_for_invoice_with_amount() {
3131
let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
3232

3333
let (payment_hash, payment_secret) =
34-
nodes[1].node.create_inbound_payment(None, 7200, None).unwrap();
34+
nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
3535

3636
let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
3737
let invoice = InvoiceBuilder::new(Currency::Bitcoin)
@@ -98,7 +98,7 @@ fn payment_metadata_end_to_end_for_invoice_with_no_amount() {
9898
let payment_metadata = vec![42, 43, 44, 45, 46, 47, 48, 49, 42];
9999

100100
let (payment_hash, payment_secret) =
101-
nodes[1].node.create_inbound_payment(None, 7200, None).unwrap();
101+
nodes[1].node.create_inbound_payment(None, 7200, None, Some(&payment_metadata)).unwrap();
102102

103103
let timestamp = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap();
104104
let invoice = InvoiceBuilder::new(Currency::Bitcoin)

lightning/src/ln/channelmanager.rs

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8595,6 +8595,7 @@ impl<
85958595
let verify_res = inbound_payment::verify(
85968596
payment_hash,
85978597
&payment_data,
8598+
onion_fields.payment_metadata.as_deref(),
85988599
self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
85998600
&self.inbound_payment_key,
86008601
&self.logger,
@@ -14261,7 +14262,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1426114262
) -> Result<Bolt11Invoice, SignOrCreationError<()>> {
1426214263
let Bolt11InvoiceParameters {
1426314264
amount_msats, description, invoice_expiry_delta_secs, min_final_cltv_expiry_delta,
14264-
payment_hash,
14265+
payment_hash, payment_metadata,
1426514266
} = params;
1426614267

1426714268
let currency =
@@ -14294,6 +14295,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1429414295
payment_hash, amount_msats,
1429514296
invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
1429614297
min_final_cltv_expiry_delta,
14298+
payment_metadata.as_deref(),
1429714299
)
1429814300
.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?;
1429914301
(payment_hash, payment_secret)
@@ -14303,6 +14305,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1430314305
.create_inbound_payment(
1430414306
amount_msats, invoice_expiry_delta_secs.unwrap_or(DEFAULT_EXPIRY_TIME as u32),
1430514307
min_final_cltv_expiry_delta,
14308+
payment_metadata.as_deref(),
1430614309
)
1430714310
.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?
1430814311
},
@@ -14341,7 +14344,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
1434114344
invoice = invoice.private_route(hint);
1434214345
}
1434314346

14344-
let raw_invoice = invoice.build_raw().map_err(|e| SignOrCreationError::CreationError(e))?;
14347+
let raw_invoice = if let Some(payment_metadata) = payment_metadata {
14348+
invoice.payment_metadata(payment_metadata).build_raw()
14349+
} else {
14350+
invoice.build_raw()
14351+
}.map_err(|e| SignOrCreationError::CreationError(e))?;
1434514352
let signature = self.node_signer.sign_invoice(&raw_invoice, Recipient::Node);
1434614353

1434714354
raw_invoice
@@ -14420,6 +14427,14 @@ pub struct Bolt11InvoiceParameters {
1442014427
/// involving another protocol where the payment hash is also involved outside the scope of
1442114428
/// lightning.
1442214429
pub payment_hash: Option<PaymentHash>,
14430+
14431+
/// The `payment_metadata` to include in the invoice. This is provided back to us in the payment
14432+
/// onion by the sender, available as [`RecipientOnionFields::payment_metadata`] via
14433+
/// [`Event::PaymentClaimable::onion_fields`].
14434+
///
14435+
/// Note that because it is exposed to the sender in the invoice you should consider encrypting
14436+
/// it. It is committed to, however, so cannot be modified by the sender.
14437+
pub payment_metadata: Option<Vec<u8>>,
1442314438
}
1442414439

1442514440
impl Default for Bolt11InvoiceParameters {
@@ -14430,6 +14445,7 @@ impl Default for Bolt11InvoiceParameters {
1443014445
invoice_expiry_delta_secs: None,
1443114446
min_final_cltv_expiry_delta: None,
1443214447
payment_hash: None,
14448+
payment_metadata: None,
1443314449
}
1443414450
}
1443514451
}
@@ -14921,7 +14937,7 @@ impl<
1492114937
refund,
1492214938
self.list_usable_channels(),
1492314939
|amount_msats, relative_expiry| {
14924-
self.create_inbound_payment(Some(amount_msats), relative_expiry, None)
14940+
self.create_inbound_payment(Some(amount_msats), relative_expiry, None, None)
1492514941
.map_err(|()| Bolt12SemanticError::InvalidAmount)
1492614942
},
1492714943
)?;
@@ -14964,7 +14980,7 @@ impl<
1496414980
/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
1496514981
pub fn create_inbound_payment(
1496614982
&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32,
14967-
min_final_cltv_expiry_delta: Option<u16>,
14983+
min_final_cltv_expiry_delta: Option<u16>, payment_metadata: Option<&[u8]>,
1496814984
) -> Result<(PaymentHash, PaymentSecret), ()> {
1496914985
inbound_payment::create(
1497014986
&self.inbound_payment_key,
@@ -14973,6 +14989,7 @@ impl<
1497314989
&self.entropy_source,
1497414990
self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
1497514991
min_final_cltv_expiry_delta,
14992+
payment_metadata,
1497614993
)
1497714994
}
1497814995

@@ -14992,6 +15009,9 @@ impl<
1499215009
/// before a [`PaymentClaimable`] event will be generated, ensuring that we do not provide the
1499315010
/// sender "proof-of-payment" unless they have paid the required amount.
1499415011
///
15012+
/// The returned secret commits to the `payment_metadata` and thus the invoice's metadata must
15013+
/// match what is provided here.
15014+
///
1499515015
/// `invoice_expiry_delta_secs` describes the number of seconds that the invoice is valid for
1499615016
/// in excess of the current time. This should roughly match the expiry time set in the invoice.
1499715017
/// After this many seconds, we will remove the inbound payment, resulting in any attempts to
@@ -15025,6 +15045,7 @@ impl<
1502515045
pub fn create_inbound_payment_for_hash(
1502615046
&self, payment_hash: PaymentHash, min_value_msat: Option<u64>,
1502715047
invoice_expiry_delta_secs: u32, min_final_cltv_expiry: Option<u16>,
15048+
payment_metadata: Option<&[u8]>,
1502815049
) -> Result<PaymentSecret, ()> {
1502915050
inbound_payment::create_from_hash(
1503015051
&self.inbound_payment_key,
@@ -15033,18 +15054,25 @@ impl<
1503315054
invoice_expiry_delta_secs,
1503415055
self.highest_seen_timestamp.load(Ordering::Acquire) as u64,
1503515056
min_final_cltv_expiry,
15057+
payment_metadata,
1503615058
)
1503715059
}
1503815060

15039-
/// Gets an LDK-generated payment preimage from a payment hash and payment secret that were
15061+
/// Gets an LDK-generated payment preimage from a payment hash, metadata and secret that were
1504015062
/// previously returned from [`create_inbound_payment`].
1504115063
///
1504215064
/// [`create_inbound_payment`]: Self::create_inbound_payment
1504315065
pub fn get_payment_preimage(
1504415066
&self, payment_hash: PaymentHash, payment_secret: PaymentSecret,
15067+
payment_metadata: Option<&[u8]>,
1504515068
) -> Result<PaymentPreimage, APIError> {
1504615069
let expanded_key = &self.inbound_payment_key;
15047-
inbound_payment::get_payment_preimage(payment_hash, payment_secret, expanded_key)
15070+
inbound_payment::get_payment_preimage(
15071+
payment_hash,
15072+
payment_secret,
15073+
payment_metadata,
15074+
expanded_key,
15075+
)
1504815076
}
1504915077

1505015078
/// [`BlindedMessagePath`]s for an async recipient to communicate with this node and interactively
@@ -17113,7 +17141,8 @@ impl<
1711317141
self.create_inbound_payment(
1711417142
Some(amount_msats),
1711517143
relative_expiry,
17116-
None
17144+
None,
17145+
None,
1711717146
).map_err(|_| Bolt12SemanticError::InvalidAmount)
1711817147
};
1711917148

@@ -21325,15 +21354,15 @@ mod tests {
2132521354
// payment verification fails as expected.
2132621355
let mut bad_payment_hash = payment_hash.clone();
2132721356
bad_payment_hash.0[0] += 1;
21328-
match inbound_payment::verify(bad_payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
21357+
match inbound_payment::verify(bad_payment_hash, &payment_data, None, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
2132921358
Ok(_) => panic!("Unexpected ok"),
2133021359
Err(()) => {
2133121360
nodes[0].logger.assert_log_contains("lightning::ln::inbound_payment", "Failing HTLC with user-generated payment_hash", 1);
2133221361
}
2133321362
}
2133421363

2133521364
// Check that using the original payment hash succeeds.
21336-
assert!(inbound_payment::verify(payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
21365+
assert!(inbound_payment::verify(payment_hash, &payment_data, None, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
2133721366
}
2133821367

2133921368
fn check_not_connected_to_peer_error<T>(
@@ -22006,7 +22035,7 @@ pub mod bench {
2200622035
payment_preimage.0[0..8].copy_from_slice(&payment_count.to_le_bytes());
2200722036
payment_count += 1;
2200822037
let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
22009-
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None).unwrap();
22038+
let payment_secret = $node_b.create_inbound_payment_for_hash(payment_hash, None, 7200, None, None).unwrap();
2201022039

2201122040
$node_a.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret, 10_000),
2201222041
PaymentId(payment_hash.0),

lightning/src/ln/functional_test_utils.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2807,6 +2807,7 @@ pub fn get_payment_preimage_hash(
28072807
min_value_msat,
28082808
7200,
28092809
min_final_cltv_expiry_delta,
2810+
None,
28102811
)
28112812
.unwrap();
28122813
(payment_preimage, payment_hash, payment_secret)

lightning/src/ln/functional_tests.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,10 @@ pub fn test_duplicate_htlc_different_direction_onchain() {
293293
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 900_000);
294294

295295
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[0], payment_value_msats);
296-
let node_a_payment_secret =
297-
nodes[0].node.create_inbound_payment_for_hash(payment_hash, None, 7200, None).unwrap();
296+
let node_a_payment_secret = nodes[0]
297+
.node
298+
.create_inbound_payment_for_hash(payment_hash, None, 7200, None, None)
299+
.unwrap();
298300
send_along_route_with_secret(
299301
&nodes[1],
300302
route,
@@ -4157,8 +4159,10 @@ pub fn test_duplicate_payment_hash_one_failure_one_success() {
41574159
let (our_payment_preimage, dup_payment_hash, ..) =
41584160
route_payment(&nodes[0], &[&nodes[1], &nodes[2], &nodes[3]], 900_000);
41594161

4160-
let payment_secret =
4161-
nodes[4].node.create_inbound_payment_for_hash(dup_payment_hash, None, 7200, None).unwrap();
4162+
let payment_secret = nodes[4]
4163+
.node
4164+
.create_inbound_payment_for_hash(dup_payment_hash, None, 7200, None, None)
4165+
.unwrap();
41624166
let payment_params = PaymentParameters::from_node_id(node_e_id, TEST_FINAL_CLTV)
41634167
.with_bolt11_features(nodes[4].node.bolt11_invoice_features())
41644168
.unwrap();
@@ -4425,13 +4429,13 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
44254429
// 2nd HTLC (not added - smaller than dust limit + HTLC tx fee):
44264430
let path_5: &[&[_]] = &[&[&nodes[2], &nodes[3], &nodes[5]]];
44274431
let payment_secret =
4428-
nodes[5].node.create_inbound_payment_for_hash(hash_1, None, 7200, None).unwrap();
4432+
nodes[5].node.create_inbound_payment_for_hash(hash_1, None, 7200, None, None).unwrap();
44294433
let route = route_to_5.clone();
44304434
send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_1, payment_secret);
44314435

44324436
// 3rd HTLC (not added - smaller than dust limit + HTLC tx fee):
44334437
let payment_secret =
4434-
nodes[5].node.create_inbound_payment_for_hash(hash_2, None, 7200, None).unwrap();
4438+
nodes[5].node.create_inbound_payment_for_hash(hash_2, None, 7200, None, None).unwrap();
44354439
let route = route_to_5;
44364440
send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_2, payment_secret);
44374441

@@ -4444,12 +4448,12 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
44444448

44454449
// 6th HTLC:
44464450
let payment_secret =
4447-
nodes[5].node.create_inbound_payment_for_hash(hash_3, None, 7200, None).unwrap();
4451+
nodes[5].node.create_inbound_payment_for_hash(hash_3, None, 7200, None, None).unwrap();
44484452
send_along_route_with_secret(&nodes[1], route.clone(), path_5, 1000000, hash_3, payment_secret);
44494453

44504454
// 7th HTLC:
44514455
let payment_secret =
4452-
nodes[5].node.create_inbound_payment_for_hash(hash_4, None, 7200, None).unwrap();
4456+
nodes[5].node.create_inbound_payment_for_hash(hash_4, None, 7200, None, None).unwrap();
44534457
send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_4, payment_secret);
44544458

44554459
// 8th HTLC:
@@ -4458,7 +4462,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
44584462
// 9th HTLC (not added - smaller than dust limit + HTLC tx fee):
44594463
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], dust_limit_msat);
44604464
let payment_secret =
4461-
nodes[5].node.create_inbound_payment_for_hash(hash_5, None, 7200, None).unwrap();
4465+
nodes[5].node.create_inbound_payment_for_hash(hash_5, None, 7200, None, None).unwrap();
44624466
send_along_route_with_secret(&nodes[1], route, path_5, dust_limit_msat, hash_5, payment_secret);
44634467

44644468
// 10th HTLC (not added - smaller than dust limit + HTLC tx fee):
@@ -4467,7 +4471,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
44674471
// 11th HTLC:
44684472
let (route, _, _, _) = get_route_and_payment_hash!(nodes[1], nodes[5], 1000000);
44694473
let payment_secret =
4470-
nodes[5].node.create_inbound_payment_for_hash(hash_6, None, 7200, None).unwrap();
4474+
nodes[5].node.create_inbound_payment_for_hash(hash_6, None, 7200, None, None).unwrap();
44714475
send_along_route_with_secret(&nodes[1], route, path_5, 1000000, hash_6, payment_secret);
44724476

44734477
// Double-check that six of the new HTLC were added
@@ -6062,7 +6066,7 @@ pub fn test_check_htlc_underpaying() {
60626066
let (_, our_payment_hash, _) = get_payment_preimage_hash(&nodes[0], None, None);
60636067
let our_payment_secret = nodes[1]
60646068
.node
6065-
.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None)
6069+
.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None, None)
60666070
.unwrap();
60676071
let onion = RecipientOnionFields::secret_only(our_payment_secret, route.get_total_amount());
60686072
let id = PaymentId(our_payment_hash.0);
@@ -7230,7 +7234,7 @@ pub fn test_preimage_storage() {
72307234

72317235
{
72327236
let (payment_hash, payment_secret) =
7233-
nodes[1].node.create_inbound_payment(Some(100_000), 7200, None).unwrap();
7237+
nodes[1].node.create_inbound_payment(Some(100_000), 7200, None, None).unwrap();
72347238
let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
72357239
let onion = RecipientOnionFields::secret_only(payment_secret, 100_000);
72367240
let id = PaymentId(payment_hash.0);
@@ -7275,7 +7279,7 @@ pub fn test_bad_secret_hash() {
72757279
let random_hash = PaymentHash([42; 32]);
72767280
let random_secret = PaymentSecret([43; 32]);
72777281
let (our_payment_hash, our_payment_secret) =
7278-
nodes[1].node.create_inbound_payment(Some(100_000), 2, None).unwrap();
7282+
nodes[1].node.create_inbound_payment(Some(100_000), 2, None, None).unwrap();
72797283
let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
72807284

72817285
// All the below cases should end up being handled exactly identically, so we macro the
@@ -9494,9 +9498,13 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash
94949498
} else {
94959499
let (hash, payment_secret) = nodes[1]
94969500
.node
9497-
.create_inbound_payment(Some(recv_value), 7200, Some(min_cltv_expiry_delta))
9501+
.create_inbound_payment(Some(recv_value), 7200, Some(min_cltv_expiry_delta), None)
94989502
.unwrap();
9499-
(hash, nodes[1].node.get_payment_preimage(hash, payment_secret).unwrap(), payment_secret)
9503+
(
9504+
hash,
9505+
nodes[1].node.get_payment_preimage(hash, payment_secret, None).unwrap(),
9506+
payment_secret,
9507+
)
95009508
};
95019509
let route = get_route!(nodes[0], payment_parameters, recv_value).unwrap();
95029510
let onion = RecipientOnionFields::secret_only(payment_secret, recv_value);

0 commit comments

Comments
 (0)