Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 480
Sending to Offer without signing_pubkey#3017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -210,10 +210,9 @@ macro_rules! invoice_explicit_signing_pubkey_builder_methods { ($self: ident, $s | ||
| #[cfg_attr(c_bindings, allow(dead_code))] | ||
| pub(super) fn for_offer( | ||
| invoice_request: &'a InvoiceRequest, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>, | ||
| created_at: Duration, payment_hash: PaymentHash | ||
| created_at: Duration, payment_hash: PaymentHash, signing_pubkey: PublicKey | ||
| ) -> Result<Self, Bolt12SemanticError> { | ||
| let amount_msats = Self::amount_msats(invoice_request)?; | ||
| let signing_pubkey = invoice_request.contents.inner.offer.signing_pubkey(); | ||
| let contents = InvoiceContents::ForOffer { | ||
| invoice_request: invoice_request.contents.clone(), | ||
| fields: Self::fields( | ||
| @@ -272,7 +271,7 @@ macro_rules! invoice_derived_signing_pubkey_builder_methods { ($self: ident, $se | ||
| created_at: Duration, payment_hash: PaymentHash, keys: KeyPair | ||
| ) -> Result<Self, Bolt12SemanticError> { | ||
| let amount_msats = Self::amount_msats(invoice_request)?; | ||
| let signing_pubkey = invoice_request.contents.inner.offer.signing_pubkey(); | ||
| let signing_pubkey = keys.public_key(); | ||
| let contents = InvoiceContents::ForOffer { | ||
| invoice_request: invoice_request.contents.clone(), | ||
| fields: Self::fields( | ||
| @@ -1435,8 +1434,8 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents { | ||
| features, signing_pubkey, | ||
| }; | ||
| match offer_tlv_stream.node_id { | ||
| Some(expected_signing_pubkey) => { | ||
| match (offer_tlv_stream.node_id, &offer_tlv_stream.paths) { | ||
| (Some(expected_signing_pubkey), _) => { | ||
| if fields.signing_pubkey != expected_signing_pubkey { | ||
| return Err(Bolt12SemanticError::InvalidSigningPubkey); | ||
| } | ||
| @@ -1446,7 +1445,21 @@ impl TryFrom<PartialInvoiceTlvStream> for InvoiceContents { | ||
| )?; | ||
| Ok(InvoiceContents::ForOffer { invoice_request, fields }) | ||
| }, | ||
| None => { | ||
| (None, Some(paths)) => { | ||
| if !paths | ||
| .iter() | ||
| .filter_map(|path| path.blinded_hops.last()) | ||
| .any(|last_hop| fields.signing_pubkey == last_hop.blinded_node_id) | ||
Comment on lines
+1450
to
+1452
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the spec, it looks like we MUST verify that the signing pubkey matches a path we sent an invoice request to. I'm not certain it matters or if the spec should change but it looks like we don't specifically check that at the moment. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is a slight deviation as we don't have the blinded node id for the path that the invoice was sent over when parsing. We could do some sort of check at the handler level, but even that would require piping that data through. | ||
| { | ||
| return Err(Bolt12SemanticError::InvalidSigningPubkey); | ||
| } | ||
| let invoice_request = InvoiceRequestContents::try_from( | ||
| (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) | ||
| )?; | ||
| Ok(InvoiceContents::ForOffer { invoice_request, fields }) | ||
| }, | ||
| (None, None) => { | ||
| let refund = RefundContents::try_from( | ||
| (payer_tlv_stream, offer_tlv_stream, invoice_request_tlv_stream) | ||
| )?; | ||
| @@ -1464,7 +1477,7 @@ mod tests { | ||
| use bitcoin::blockdata::script::ScriptBuf; | ||
| use bitcoin::hashes::Hash; | ||
| use bitcoin::network::constants::Network; | ||
| use bitcoin::secp256k1::{Message, Secp256k1, XOnlyPublicKey, self}; | ||
| use bitcoin::secp256k1::{KeyPair, Message, Secp256k1, SecretKey, XOnlyPublicKey, self}; | ||
| use bitcoin::address::{Address, Payload, WitnessProgram, WitnessVersion}; | ||
| use bitcoin::key::TweakedPublicKey; | ||
| @@ -1633,6 +1646,7 @@ mod tests { | ||
| quantity: None, | ||
| payer_id: Some(&payer_pubkey()), | ||
| payer_note: None, | ||
| paths: None, | ||
| }, | ||
| InvoiceTlvStreamRef { | ||
| paths: Some(Iterable(payment_paths.iter().map(|(_, path)| path))), | ||
| @@ -1725,6 +1739,7 @@ mod tests { | ||
| quantity: None, | ||
| payer_id: Some(&payer_pubkey()), | ||
| payer_note: None, | ||
| paths: None, | ||
| }, | ||
| InvoiceTlvStreamRef { | ||
| paths: Some(Iterable(payment_paths.iter().map(|(_, path)| path))), | ||
| @@ -2365,6 +2380,81 @@ mod tests { | ||
| } | ||
| } | ||
| #[test] | ||
| fn parses_invoice_with_node_id_from_blinded_path() { | ||
| let paths = vec![ | ||
| BlindedPath { | ||
| introduction_node: IntroductionNode::NodeId(pubkey(40)), | ||
| blinding_point: pubkey(41), | ||
| blinded_hops: vec![ | ||
| BlindedHop { blinded_node_id: pubkey(43), encrypted_payload: vec![0; 43] }, | ||
| BlindedHop { blinded_node_id: pubkey(44), encrypted_payload: vec![0; 44] }, | ||
| ], | ||
| }, | ||
| BlindedPath { | ||
| introduction_node: IntroductionNode::NodeId(pubkey(40)), | ||
| blinding_point: pubkey(41), | ||
| blinded_hops: vec![ | ||
| BlindedHop { blinded_node_id: pubkey(45), encrypted_payload: vec![0; 45] }, | ||
| BlindedHop { blinded_node_id: pubkey(46), encrypted_payload: vec![0; 46] }, | ||
| ], | ||
| }, | ||
| ]; | ||
| let blinded_node_id_sign = |message: &UnsignedBolt12Invoice| { | ||
| let secp_ctx = Secp256k1::new(); | ||
| let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()); | ||
| Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys)) | ||
| }; | ||
| let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) | ||
| .clear_signing_pubkey() | ||
| .amount_msats(1000) | ||
| .path(paths[0].clone()) | ||
| .path(paths[1].clone()) | ||
| .build().unwrap() | ||
| .request_invoice(vec![1; 32], payer_pubkey()).unwrap() | ||
| .build().unwrap() | ||
| .sign(payer_sign).unwrap() | ||
| .respond_with_no_std_using_signing_pubkey( | ||
| payment_paths(), payment_hash(), now(), pubkey(46) | ||
| ).unwrap() | ||
| .build().unwrap() | ||
| .sign(blinded_node_id_sign).unwrap(); | ||
| let mut buffer = Vec::new(); | ||
| invoice.write(&mut buffer).unwrap(); | ||
| if let Err(e) = Bolt12Invoice::try_from(buffer) { | ||
| panic!("error parsing invoice: {:?}", e); | ||
| } | ||
| let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) | ||
| .clear_signing_pubkey() | ||
| .amount_msats(1000) | ||
| .path(paths[0].clone()) | ||
| .path(paths[1].clone()) | ||
| .build().unwrap() | ||
| .request_invoice(vec![1; 32], payer_pubkey()).unwrap() | ||
| .build().unwrap() | ||
| .sign(payer_sign).unwrap() | ||
| .respond_with_no_std_using_signing_pubkey( | ||
| payment_paths(), payment_hash(), now(), recipient_pubkey() | ||
| ).unwrap() | ||
| .build().unwrap() | ||
| .sign(recipient_sign).unwrap(); | ||
| let mut buffer = Vec::new(); | ||
| invoice.write(&mut buffer).unwrap(); | ||
| match Bolt12Invoice::try_from(buffer) { | ||
| Ok(_) => panic!("expected error"), | ||
| Err(e) => { | ||
| assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidSigningPubkey)); | ||
| }, | ||
| } | ||
| } | ||
| #[test] | ||
| fn fails_parsing_invoice_without_signature() { | ||
| let mut buffer = Vec::new(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -747,7 +747,27 @@ macro_rules! invoice_request_respond_with_explicit_signing_pubkey_methods { ( | ||
| return Err(Bolt12SemanticError::UnknownRequiredFeatures); | ||
| } | ||
| <$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash) | ||
| let signing_pubkey = match $contents.contents.inner.offer.signing_pubkey() { | ||
| Some(signing_pubkey) => signing_pubkey, | ||
| None => return Err(Bolt12SemanticError::MissingSigningPubkey), | ||
| }; | ||
| <$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash, signing_pubkey) | ||
| } | ||
| #[cfg(test)] | ||
| #[allow(dead_code)] | ||
| pub(super) fn respond_with_no_std_using_signing_pubkey( | ||
| &$self, payment_paths: Vec<(BlindedPayInfo, BlindedPath)>, payment_hash: PaymentHash, | ||
| created_at: core::time::Duration, signing_pubkey: PublicKey | ||
| ) -> Result<$builder, Bolt12SemanticError> { | ||
| debug_assert!($contents.contents.inner.offer.signing_pubkey().is_none()); | ||
| if $contents.invoice_request_features().requires_unknown_bits() { | ||
| return Err(Bolt12SemanticError::UnknownRequiredFeatures); | ||
| } | ||
| <$builder>::for_offer(&$contents, payment_paths, created_at, payment_hash, signing_pubkey) | ||
| } | ||
| } } | ||
| @@ -855,6 +875,11 @@ macro_rules! invoice_request_respond_with_derived_signing_pubkey_methods { ( | ||
| Some(keys) => keys, | ||
| }; | ||
| match $contents.contents.inner.offer.signing_pubkey() { | ||
| Some(signing_pubkey) => debug_assert_eq!(signing_pubkey, keys.public_key()), | ||
| None => return Err(Bolt12SemanticError::MissingSigningPubkey), | ||
| } | ||
| <$builder>::for_offer_using_keys( | ||
| &$self.inner, payment_paths, created_at, payment_hash, keys | ||
| ) | ||
| @@ -961,6 +986,7 @@ impl InvoiceRequestContentsWithoutPayerId { | ||
| quantity: self.quantity, | ||
| payer_id: None, | ||
| payer_note: self.payer_note.as_ref(), | ||
| paths: None, | ||
| }; | ||
| (payer, offer, invoice_request) | ||
| @@ -993,13 +1019,17 @@ pub(super) const INVOICE_REQUEST_TYPES: core::ops::Range<u64> = 80..160; | ||
| /// [`Refund::payer_id`]: crate::offers::refund::Refund::payer_id | ||
| pub(super) const INVOICE_REQUEST_PAYER_ID_TYPE: u64 = 88; | ||
| // This TLV stream is used for both InvoiceRequest and Refund, but not all TLV records are valid for | ||
| // InvoiceRequest as noted below. | ||
| tlv_stream!(InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef, INVOICE_REQUEST_TYPES, { | ||
| (80, chain: ChainHash), | ||
| (82, amount: (u64, HighZeroBytesDroppedBigSize)), | ||
| (84, features: (InvoiceRequestFeatures, WithoutLength)), | ||
| (86, quantity: (u64, HighZeroBytesDroppedBigSize)), | ||
| (INVOICE_REQUEST_PAYER_ID_TYPE, payer_id: PublicKey), | ||
| (89, payer_note: (String, WithoutLength)), | ||
| // Only used for Refund since the onion message of an InvoiceRequest has a reply path. | ||
| (90, paths: (Vec<BlindedPath>, WithoutLength)), | ||
jkczyz marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }); | ||
| type FullInvoiceRequestTlvStream = | ||
| @@ -1082,7 +1112,9 @@ impl TryFrom<PartialInvoiceRequestTlvStream> for InvoiceRequestContents { | ||
| let ( | ||
| PayerTlvStream { metadata }, | ||
| offer_tlv_stream, | ||
| InvoiceRequestTlvStream { chain, amount, features, quantity, payer_id, payer_note }, | ||
| InvoiceRequestTlvStream { | ||
| chain, amount, features, quantity, payer_id, payer_note, paths, | ||
| }, | ||
| ) = tlv_stream; | ||
| let payer = match metadata { | ||
| @@ -1109,6 +1141,10 @@ impl TryFrom<PartialInvoiceRequestTlvStream> for InvoiceRequestContents { | ||
| Some(payer_id) => payer_id, | ||
| }; | ||
| if paths.is_some() { | ||
| return Err(Bolt12SemanticError::UnexpectedPaths); | ||
| } | ||
| Ok(InvoiceRequestContents { | ||
| inner: InvoiceRequestContentsWithoutPayerId { | ||
| payer, offer, chain, amount_msats: amount, features, quantity, payer_note, | ||
| @@ -1233,7 +1269,7 @@ mod tests { | ||
| assert_eq!(unsigned_invoice_request.paths(), &[]); | ||
| assert_eq!(unsigned_invoice_request.issuer(), None); | ||
| assert_eq!(unsigned_invoice_request.supported_quantity(), Quantity::One); | ||
| assert_eq!(unsigned_invoice_request.signing_pubkey(), recipient_pubkey()); | ||
| assert_eq!(unsigned_invoice_request.signing_pubkey(), Some(recipient_pubkey())); | ||
| assert_eq!(unsigned_invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin)); | ||
| assert_eq!(unsigned_invoice_request.amount_msats(), None); | ||
| assert_eq!(unsigned_invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty()); | ||
| @@ -1265,7 +1301,7 @@ mod tests { | ||
| assert_eq!(invoice_request.paths(), &[]); | ||
| assert_eq!(invoice_request.issuer(), None); | ||
| assert_eq!(invoice_request.supported_quantity(), Quantity::One); | ||
| assert_eq!(invoice_request.signing_pubkey(), recipient_pubkey()); | ||
| assert_eq!(invoice_request.signing_pubkey(), Some(recipient_pubkey())); | ||
| assert_eq!(invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin)); | ||
| assert_eq!(invoice_request.amount_msats(), None); | ||
| assert_eq!(invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty()); | ||
| @@ -1300,6 +1336,7 @@ mod tests { | ||
| quantity: None, | ||
| payer_id: Some(&payer_pubkey()), | ||
| payer_note: None, | ||
| paths: None, | ||
| }, | ||
| SignatureTlvStreamRef { signature: Some(&invoice_request.signature()) }, | ||
| ), | ||
| @@ -2260,7 +2297,7 @@ mod tests { | ||
| .amount_msats(1000) | ||
| .supported_quantity(Quantity::Unbounded) | ||
| .build().unwrap(); | ||
| assert_eq!(offer.signing_pubkey(), node_id); | ||
| assert_eq!(offer.signing_pubkey(), Some(node_id)); | ||
| let invoice_request = offer.request_invoice(vec![1; 32], payer_pubkey()).unwrap() | ||
| .chain(Network::Testnet).unwrap() | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its usually worth having a comment on these kinds of assertions as to why we think its not reachable.