Skip to content

Commit 69c57f9

Browse files
shaavanshaavan
committed
Cleanup: Remove redundant (hmac, nonce) from codebase
Now that we have introduced an alternate mechanism for authentication in the codebase, we can safely remove the now redundant (hmac, nonce) fields from the Payment ReceiveTlvs's while maintaining the security of the onion messages.
1 parent a0a527d commit 69c57f9

File tree

11 files changed

+76
-259
lines changed

11 files changed

+76
-259
lines changed

‎fuzz/src/invoice_request_deser.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@ use bitcoin::secp256k1::{self, Keypair, Parity, PublicKey, Secp256k1, SecretKey}
1212
use core::convert::TryFrom;
1313
use lightning::blinded_path::payment::{
1414
BlindedPaymentPath, Bolt12OfferContext, ForwardTlvs, PaymentConstraints, PaymentContext,
15-
PaymentForwardNode, PaymentRelay, UnauthenticatedReceiveTlvs,
15+
PaymentForwardNode, PaymentRelay, ReceiveTlvs,
1616
};
1717
use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
1818
use lightning::ln::inbound_payment::ExpandedKey;
1919
use lightning::offers::invoice::UnsignedBolt12Invoice;
2020
use lightning::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields};
21-
use lightning::offers::nonce::Nonce;
2221
use lightning::offers::offer::OfferId;
2322
use lightning::offers::parse::Bolt12SemanticError;
2423
use lightning::sign::{EntropySource, ReceiveAuthKey};
@@ -84,7 +83,6 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
8483
) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
8584
let expanded_key = ExpandedKey::new([42; 32]);
8685
let entropy_source = Randomness {};
87-
let nonce = Nonce::from_entropy_source(&entropy_source);
8886
let receive_auth_key = ReceiveAuthKey([41; 32]);
8987

9088
let invoice_request_fields =
@@ -107,15 +105,14 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
107105
offer_id: OfferId([42; 32]),
108106
invoice_request: invoice_request_fields,
109107
});
110-
let payee_tlvs = UnauthenticatedReceiveTlvs {
108+
let payee_tlvs = ReceiveTlvs {
111109
payment_secret: PaymentSecret([42; 32]),
112110
payment_constraints: PaymentConstraints {
113111
max_cltv_expiry: 1_000_000,
114112
htlc_minimum_msat: 1,
115113
},
116114
payment_context,
117115
};
118-
let payee_tlvs = payee_tlvs.authenticate(nonce, &expanded_key);
119116
let intermediate_nodes = [PaymentForwardNode {
120117
tlvs: ForwardTlvs {
121118
short_channel_id: 43,
@@ -125,7 +122,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
125122
fee_base_msat: 1,
126123
},
127124
payment_constraints: PaymentConstraints {
128-
max_cltv_expiry: payee_tlvs.tlvs().payment_constraints.max_cltv_expiry + 40,
125+
max_cltv_expiry: payee_tlvs.payment_constraints.max_cltv_expiry + 40,
129126
htlc_minimum_msat: 100,
130127
},
131128
features: BlindedHopFeatures::empty(),

‎fuzz/src/refund_deser.rs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,10 @@ use bitcoin::secp256k1::{self, Keypair, PublicKey, Secp256k1, SecretKey};
1212
use core::convert::TryFrom;
1313
use lightning::blinded_path::payment::{
1414
BlindedPaymentPath, Bolt12RefundContext, ForwardTlvs, PaymentConstraints, PaymentContext,
15-
PaymentForwardNode, PaymentRelay, UnauthenticatedReceiveTlvs,
15+
PaymentForwardNode, PaymentRelay, ReceiveTlvs,
1616
};
1717
use lightning::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA;
18-
use lightning::ln::inbound_payment::ExpandedKey;
1918
use lightning::offers::invoice::UnsignedBolt12Invoice;
20-
use lightning::offers::nonce::Nonce;
2119
use lightning::offers::parse::Bolt12SemanticError;
2220
use lightning::offers::refund::Refund;
2321
use lightning::sign::{EntropySource, ReceiveAuthKey};
@@ -69,20 +67,17 @@ fn privkey(byte: u8) -> SecretKey {
6967
fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
7068
refund: &Refund, signing_pubkey: PublicKey, secp_ctx: &Secp256k1<T>,
7169
) -> Result<UnsignedBolt12Invoice, Bolt12SemanticError> {
72-
let expanded_key = ExpandedKey::new([42; 32]);
7370
let entropy_source = Randomness {};
7471
let receive_auth_key = ReceiveAuthKey([41; 32]);
75-
let nonce = Nonce::from_entropy_source(&entropy_source);
7672
let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {});
77-
let payee_tlvs = UnauthenticatedReceiveTlvs {
73+
let payee_tlvs = ReceiveTlvs {
7874
payment_secret: PaymentSecret([42; 32]),
7975
payment_constraints: PaymentConstraints {
8076
max_cltv_expiry: 1_000_000,
8177
htlc_minimum_msat: 1,
8278
},
8379
payment_context,
8480
};
85-
let payee_tlvs = payee_tlvs.authenticate(nonce, &expanded_key);
8681
let intermediate_nodes = [PaymentForwardNode {
8782
tlvs: ForwardTlvs {
8883
short_channel_id: 43,
@@ -92,7 +87,7 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
9287
fee_base_msat: 1,
9388
},
9489
payment_constraints: PaymentConstraints {
95-
max_cltv_expiry: payee_tlvs.tlvs().payment_constraints.max_cltv_expiry + 40,
90+
max_cltv_expiry: payee_tlvs.payment_constraints.max_cltv_expiry + 40,
9691
htlc_minimum_msat: 100,
9792
},
9893
features: BlindedHopFeatures::empty(),

‎lightning/src/blinded_path/payment.rs‎

Lines changed: 19 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
//! Data structures and methods for constructing [`BlindedPaymentPath`]s to send a payment over.
1111
12-
use bitcoin::hashes::hmac::Hmac;
13-
use bitcoin::hashes::sha256::Hash as Sha256;
1412
use bitcoin::secp256k1::ecdh::SharedSecret;
1513
use bitcoin::secp256k1::{self, PublicKey, Secp256k1, SecretKey};
1614

@@ -20,8 +18,6 @@ use crate::crypto::streams::ChaChaDualPolyReadAdapter;
2018
use crate::io;
2119
use crate::io::Cursor;
2220
use crate::ln::channel_state::CounterpartyForwardingInfo;
23-
use crate::ln::channelmanager::Verification;
24-
use crate::ln::inbound_payment::ExpandedKey;
2521
use crate::ln::msgs::DecodeError;
2622
use crate::ln::onion_utils;
2723
use crate::offers::invoice_request::InvoiceRequestFields;
@@ -137,7 +133,7 @@ impl BlindedPaymentPath {
137133

138134
let blinded_payinfo = compute_payinfo(
139135
intermediate_nodes,
140-
&payee_tlvs.tlvs,
136+
&payee_tlvs,
141137
htlc_maximum_msat,
142138
min_final_cltv_expiry_delta,
143139
)?;
@@ -328,26 +324,8 @@ pub struct TrampolineForwardTlvs {
328324

329325
/// Data to construct a [`BlindedHop`] for receiving a payment. This payload is custom to LDK and
330326
/// may not be valid if received by another lightning implementation.
331-
///
332-
/// Can only be constructed by calling [`UnauthenticatedReceiveTlvs::authenticate`].
333327
#[derive(Clone, Debug)]
334328
pub struct ReceiveTlvs {
335-
/// The TLVs for which the HMAC in `authentication` is derived.
336-
pub(crate) tlvs: UnauthenticatedReceiveTlvs,
337-
/// An HMAC of `tlvs` along with a nonce used to construct it.
338-
pub(crate) authentication: (Hmac<Sha256>, Nonce),
339-
}
340-
341-
impl ReceiveTlvs {
342-
/// Returns the underlying TLVs.
343-
pub fn tlvs(&self) -> &UnauthenticatedReceiveTlvs {
344-
&self.tlvs
345-
}
346-
}
347-
348-
/// An unauthenticated [`ReceiveTlvs`].
349-
#[derive(Clone, Debug)]
350-
pub struct UnauthenticatedReceiveTlvs {
351329
/// Used to authenticate the sender of a payment to the receiver and tie MPP HTLCs together.
352330
pub payment_secret: PaymentSecret,
353331
/// Constraints for the receiver of this payment.
@@ -356,17 +334,6 @@ pub struct UnauthenticatedReceiveTlvs {
356334
pub payment_context: PaymentContext,
357335
}
358336

359-
impl UnauthenticatedReceiveTlvs {
360-
/// Creates an authenticated [`ReceiveTlvs`], which includes an HMAC and the provide [`Nonce`]
361-
/// that can be use later to verify it authenticity.
362-
pub fn authenticate(self, nonce: Nonce, expanded_key: &ExpandedKey) -> ReceiveTlvs {
363-
ReceiveTlvs {
364-
authentication: (self.hmac_for_offer_payment(nonce, expanded_key), nonce),
365-
tlvs: self,
366-
}
367-
}
368-
}
369-
370337
/// Data to construct a [`BlindedHop`] for sending a payment over.
371338
///
372339
/// [`BlindedHop`]: crate::blinded_path::BlindedHop
@@ -539,19 +506,12 @@ impl Writeable for TrampolineForwardTlvs {
539506
}
540507
}
541508

509+
// Note: Authentication TLV field was removed in LDK v0.2 following the
510+
// introduction of `ReceiveAuthKey`-based authentication for inbound
511+
// `BlindedPaymentPaths`s. Because we do not support receiving to those
512+
// contexts anymore (they will fail the `ReceiveAuthKey`-based
513+
// authentication checks), we can reuse those fields here.
542514
impl Writeable for ReceiveTlvs {
543-
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
544-
encode_tlv_stream!(w, {
545-
(12, self.tlvs.payment_constraints, required),
546-
(65536, self.tlvs.payment_secret, required),
547-
(65537, self.tlvs.payment_context, required),
548-
(65539, self.authentication, required),
549-
});
550-
Ok(())
551-
}
552-
}
553-
554-
impl Writeable for UnauthenticatedReceiveTlvs {
555515
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
556516
encode_tlv_stream!(w, {
557517
(12, self.payment_constraints, required),
@@ -586,7 +546,6 @@ impl Readable for BlindedPaymentTlvs {
586546
(14, features, (option, encoding: (BlindedHopFeatures, WithoutLength))),
587547
(65536, payment_secret, option),
588548
(65537, payment_context, option),
589-
(65539, authentication, option),
590549
});
591550

592551
if let Some(short_channel_id) = scid {
@@ -605,12 +564,9 @@ impl Readable for BlindedPaymentTlvs {
605564
return Err(DecodeError::InvalidValue);
606565
}
607566
Ok(BlindedPaymentTlvs::Receive(ReceiveTlvs {
608-
tlvs: UnauthenticatedReceiveTlvs {
609-
payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
610-
payment_constraints: payment_constraints.0.unwrap(),
611-
payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
612-
},
613-
authentication: authentication.ok_or(DecodeError::InvalidValue)?,
567+
payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
568+
payment_constraints: payment_constraints.0.unwrap(),
569+
payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
614570
}))
615571
}
616572
}
@@ -626,7 +582,6 @@ impl Readable for BlindedTrampolineTlvs {
626582
(14, features, (option, encoding: (BlindedHopFeatures, WithoutLength))),
627583
(65536, payment_secret, option),
628584
(65537, payment_context, option),
629-
(65539, authentication, option),
630585
});
631586

632587
if let Some(next_trampoline) = next_trampoline {
@@ -645,12 +600,9 @@ impl Readable for BlindedTrampolineTlvs {
645600
return Err(DecodeError::InvalidValue);
646601
}
647602
Ok(BlindedTrampolineTlvs::Receive(ReceiveTlvs {
648-
tlvs: UnauthenticatedReceiveTlvs {
649-
payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
650-
payment_constraints: payment_constraints.0.unwrap(),
651-
payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
652-
},
653-
authentication: authentication.ok_or(DecodeError::InvalidValue)?,
603+
payment_secret: payment_secret.ok_or(DecodeError::InvalidValue)?,
604+
payment_constraints: payment_constraints.0.unwrap(),
605+
payment_context: payment_context.ok_or(DecodeError::InvalidValue)?,
654606
}))
655607
}
656608
}
@@ -737,7 +689,7 @@ where
737689
}
738690

739691
pub(super) fn compute_payinfo(
740-
intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &UnauthenticatedReceiveTlvs,
692+
intermediate_nodes: &[PaymentForwardNode], payee_tlvs: &ReceiveTlvs,
741693
payee_htlc_maximum_msat: u64, min_final_cltv_expiry_delta: u16,
742694
) -> Result<BlindedPayInfo, ()> {
743695
let (aggregated_base_fee, aggregated_prop_fee) =
@@ -860,7 +812,7 @@ impl_writeable_tlv_based!(Bolt12RefundContext, {});
860812
mod tests {
861813
use crate::blinded_path::payment::{
862814
Bolt12RefundContext, ForwardTlvs, PaymentConstraints, PaymentContext, PaymentForwardNode,
863-
PaymentRelay, UnauthenticatedReceiveTlvs,
815+
PaymentRelay, ReceiveTlvs,
864816
};
865817
use crate::ln::functional_test_utils::TEST_FINAL_CLTV;
866818
use crate::types::features::BlindedHopFeatures;
@@ -910,7 +862,7 @@ mod tests {
910862
htlc_maximum_msat: u64::max_value(),
911863
},
912864
];
913-
let recv_tlvs = UnauthenticatedReceiveTlvs {
865+
let recv_tlvs = ReceiveTlvs {
914866
payment_secret: PaymentSecret([0; 32]),
915867
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
916868
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
@@ -928,7 +880,7 @@ mod tests {
928880

929881
#[test]
930882
fn compute_payinfo_1_hop() {
931-
let recv_tlvs = UnauthenticatedReceiveTlvs {
883+
let recv_tlvs = ReceiveTlvs {
932884
payment_secret: PaymentSecret([0; 32]),
933885
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
934886
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
@@ -985,7 +937,7 @@ mod tests {
985937
htlc_maximum_msat: u64::max_value(),
986938
},
987939
];
988-
let recv_tlvs = UnauthenticatedReceiveTlvs {
940+
let recv_tlvs = ReceiveTlvs {
989941
payment_secret: PaymentSecret([0; 32]),
990942
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 3 },
991943
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
@@ -1044,7 +996,7 @@ mod tests {
1044996
htlc_maximum_msat: u64::max_value(),
1045997
},
1046998
];
1047-
let recv_tlvs = UnauthenticatedReceiveTlvs {
999+
let recv_tlvs = ReceiveTlvs {
10481000
payment_secret: PaymentSecret([0; 32]),
10491001
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
10501002
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),
@@ -1113,7 +1065,7 @@ mod tests {
11131065
htlc_maximum_msat: 10_000,
11141066
},
11151067
];
1116-
let recv_tlvs = UnauthenticatedReceiveTlvs {
1068+
let recv_tlvs = ReceiveTlvs {
11171069
payment_secret: PaymentSecret([0; 32]),
11181070
payment_constraints: PaymentConstraints { max_cltv_expiry: 0, htlc_minimum_msat: 1 },
11191071
payment_context: PaymentContext::Bolt12Refund(Bolt12RefundContext {}),

‎lightning/src/ln/async_payments_tests.rs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,6 @@ fn pass_async_payments_oms(
272272
fn create_static_invoice_builder<'a>(
273273
recipient: &Node, offer: &'a Offer, offer_nonce: Nonce, relative_expiry: Option<Duration>,
274274
) -> StaticInvoiceBuilder<'a> {
275-
let entropy = recipient.keys_manager;
276275
let amount_msat = offer.amount().and_then(|amount| match amount {
277276
Amount::Bitcoin { amount_msats } => Some(amount_msats),
278277
Amount::Currency { .. } => None,
@@ -296,7 +295,6 @@ fn create_static_invoice_builder<'a>(
296295
.flow
297296
.create_static_invoice_builder(
298297
&recipient.router,
299-
entropy,
300298
offer,
301299
offer_nonce,
302300
payment_secret,
@@ -1860,7 +1858,7 @@ fn expired_static_invoice_payment_path() {
18601858
.advance_path_by_one(&nodes[1].keys_manager, &nodes[1].node, &secp_ctx)
18611859
.unwrap();
18621860
match blinded_path.decrypt_intro_payload(&nodes[2].keys_manager).unwrap().0 {
1863-
BlindedPaymentTlvs::Receive(tlvs) => tlvs.tlvs.payment_constraints.max_cltv_expiry,
1861+
BlindedPaymentTlvs::Receive(tlvs) => tlvs.payment_constraints.max_cltv_expiry,
18641862
_ => panic!(),
18651863
}
18661864
};
@@ -3091,7 +3089,6 @@ fn intercepted_hold_htlc() {
30913089
.flow
30923090
.test_create_blinded_payment_paths(
30933091
&recipient.router,
3094-
recipient.keys_manager,
30953092
first_hops,
30963093
None,
30973094
payment_secret,

0 commit comments

Comments
 (0)