diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index 6c18e600b55..3826adc0e3f 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -1478,7 +1478,7 @@ impl Bolt11Invoice { unreachable!("ensured by constructor"); } - /// Get the payee's public key if one was included in the invoice + /// Get the payee's public key if one was explicitly included in the invoice's `n` field. pub fn payee_pub_key(&self) -> Option<&PublicKey> { self.signed_invoice.payee_pub_key().map(|x| &x.0) } @@ -1498,17 +1498,21 @@ impl Bolt11Invoice { self.signed_invoice.features() } - /// Recover the payee's public key (only to be used if none was included in the invoice) - pub fn recover_payee_pub_key(&self) -> PublicKey { - self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0 + /// Recover the payee's public key from the invoice signature. + /// + /// This attempts signature recovery regardless of whether a payee public key was explicitly + /// included in the invoice's `n` field. Recovery can fail for a valid invoice with an included + /// `n` field, so [`Self::get_payee_pub_key`] should be used to obtain the invoice's payee key. + pub fn recover_payee_pub_key(&self) -> Option { + self.signed_invoice.recover_payee_pub_key().ok().map(|p| p.0) } - /// Recover the payee's public key if one was included in the invoice, otherwise return the - /// recovered public key from the signature + /// Get the invoice's payee public key, preferring an explicitly included payee public key and + /// falling back to recovering the key from the signature. pub fn get_payee_pub_key(&self) -> PublicKey { match self.payee_pub_key() { Some(pk) => *pk, - None => self.recover_payee_pub_key(), + None => self.recover_payee_pub_key().expect("was checked by constructor"), } } @@ -2057,6 +2061,58 @@ mod test { assert!(new_signed.check_signature()); } + #[test] + fn recover_payee_pub_key_returns_signature_recovery_result() { + use crate::{ + Bolt11Invoice, Bolt11InvoiceSignature, Currency, InvoiceBuilder, PaymentHash, + PaymentSecret, SignedRawBolt11Invoice, + }; + use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId}; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + use core::time::Duration; + + let secp_ctx = Secp256k1::new(); + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key); + + let invoice_without_payee_pub_key = InvoiceBuilder::new(Currency::Bitcoin) + .description("Test".to_string()) + .payment_hash(PaymentHash([0; 32])) + .payment_secret(PaymentSecret([21; 32])) + .min_final_cltv_expiry_delta(144) + .duration_since_epoch(Duration::from_secs(1234567)) + .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key)) + .unwrap(); + assert_eq!(invoice_without_payee_pub_key.recover_payee_pub_key(), Some(public_key)); + assert_eq!(invoice_without_payee_pub_key.get_payee_pub_key(), public_key); + + let invoice_with_payee_pub_key = InvoiceBuilder::new(Currency::Bitcoin) + .description("Test".to_string()) + .payment_hash(PaymentHash([1; 32])) + .payment_secret(PaymentSecret([21; 32])) + .payee_pub_key(public_key) + .min_final_cltv_expiry_delta(144) + .duration_since_epoch(Duration::from_secs(1234567)) + .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key)) + .unwrap(); + + let signed_raw = invoice_with_payee_pub_key.into_signed_raw(); + let (raw_invoice, hash, signature) = signed_raw.into_parts(); + let (_orig_rid, sig_bytes) = signature.0.serialize_compact(); + let bad_rid = RecoveryId::from_i32(2).unwrap(); + let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap(); + let bad_signed_raw = SignedRawBolt11Invoice { + raw_invoice, + hash, + signature: Bolt11InvoiceSignature(bad_sig), + }; + let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap(); + + assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key)); + assert_eq!(bad_invoice.recover_payee_pub_key(), None); + assert_eq!(bad_invoice.get_payee_pub_key(), public_key); + } + #[test] fn test_check_feature_bits() { use crate::TaggedField::*; diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs index 98996fa28bb..10cda068b68 100644 --- a/lightning/src/ln/invoice_utils.rs +++ b/lightning/src/ln/invoice_utils.rs @@ -1281,7 +1281,7 @@ mod test { assert!(!invoice.features().unwrap().supports_basic_mpp()); let payment_params = PaymentParameters::from_node_id( - invoice.recover_payee_pub_key(), + invoice.get_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32, ) .with_bolt11_features(invoice.features().unwrap().clone()) @@ -1347,7 +1347,7 @@ mod test { payment_secret, payment_amt, payment_preimage_opt, - invoice.recover_payee_pub_key(), + invoice.get_payee_pub_key(), ); do_claim_payment_along_route(ClaimAlongRouteArgs::new( &nodes[0], diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 364bd86704e..99d9623cefb 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -1177,7 +1177,7 @@ impl PaymentParameters { /// [`PaymentParameters::expiry_time`]. pub fn from_bolt11_invoice(invoice: &Bolt11Invoice) -> Self { let mut payment_params = Self::from_node_id( - invoice.recover_payee_pub_key(), + invoice.get_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32, ) .with_route_hints(invoice.route_hints()) @@ -4094,7 +4094,7 @@ mod tests { use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId, P2PGossipSync}; use crate::routing::router::{ add_random_cltv_offset, build_route_from_hops_internal, default_node_features, get_route, - BlindedPathCandidate, BlindedTail, CandidateRouteHop, InFlightHtlcs, Path, + BlindedPathCandidate, BlindedTail, CandidateRouteHop, InFlightHtlcs, Path, Payee, PaymentParameters, PublicHopCandidate, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters, RoutingFees, ScorerAccountingForInFlightHtlcs, DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE, @@ -4113,6 +4113,8 @@ mod tests { use crate::util::test_utils as ln_test_utils; use bitcoin::amount::Amount; + use bitcoin::bech32::primitives::decode::CheckedHrpstring; + use bitcoin::bech32::{ByteIterExt, Fe32IterExt}; use bitcoin::constants::ChainHash; use bitcoin::hashes::Hash; use bitcoin::hex::FromHex; @@ -4124,10 +4126,58 @@ mod tests { use bitcoin::transaction::TxOut; use chacha20_poly1305::chacha20::ChaCha20; use chacha20_poly1305::{Key, Nonce}; + use lightning_invoice::{Bolt11Bech32, Bolt11Invoice, Currency, InvoiceBuilder}; use crate::io::Cursor; use crate::prelude::*; use crate::sync::{Arc, Mutex}; + use crate::types::payment::{PaymentHash, PaymentSecret}; + + fn invoice_with_included_payee_pub_key_and_bad_recovery_id() -> (Bolt11Invoice, PublicKey) { + let secp_ctx = Secp256k1::new(); + let private_key = SecretKey::from_slice(&[42; 32]).unwrap(); + let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key); + + let invoice = InvoiceBuilder::new(Currency::Bitcoin) + .description("Test".to_string()) + .amount_milli_satoshis(1000) + .payment_hash(PaymentHash([0; 32])) + .payment_secret(PaymentSecret([21; 32])) + .payee_pub_key(public_key) + .min_final_cltv_expiry_delta(144) + .duration_since_epoch(core::time::Duration::from_secs(1234567)) + .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key)) + .unwrap(); + + let invoice_string = invoice.to_string(); + let parsed = CheckedHrpstring::new::(&invoice_string).unwrap(); + let hrp = parsed.hrp(); + let mut data: Vec<_> = parsed.fe32_iter::<&mut dyn Iterator>().collect(); + let signature_start = data.len() - 104; + let mut signature_bytes: Vec = + data[signature_start..].iter().copied().fes_to_bytes().collect(); + signature_bytes[64] = 2; + let signature_data: Vec<_> = signature_bytes.into_iter().bytes_to_fes().collect(); + data.splice(signature_start.., signature_data); + + let bad_invoice_string = data + .into_iter() + .with_checksum::(&hrp) + .chars() + .collect::(); + (bad_invoice_string.parse().unwrap(), public_key) + } + + #[test] + fn payment_params_from_bolt11_invoice_uses_included_payee_pub_key() { + let (invoice, public_key) = invoice_with_included_payee_pub_key_and_bad_recovery_id(); + let payment_params = PaymentParameters::from_bolt11_invoice(&invoice); + + match payment_params.payee { + Payee::Clear { node_id, .. } => assert_eq!(node_id, public_key), + Payee::Blinded { .. } => panic!("BOLT11 invoice should create a clear payee"), + } + } #[rustfmt::skip] fn get_channel_details(short_channel_id: Option, node_id: PublicKey,