Skip to content

Improve KeysInterface::sign_invoice API #1272

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ use std::collections::{HashSet, hash_map, HashMap};
use std::sync::{Arc,Mutex};
use std::sync::atomic;
use std::io::Cursor;
use bitcoin::bech32::u5;

const MAX_FEE: u32 = 10_000;
struct FuzzEstimator {
Expand Down Expand Up @@ -220,7 +221,7 @@ impl KeysInterface for KeyProvider {
})
}

fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
unreachable!()
}
}
Expand Down
3 changes: 2 additions & 1 deletion fuzz/src/full_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use std::convert::TryInto;
use std::cmp;
use std::sync::{Arc, Mutex};
use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering};
use bitcoin::bech32::u5;

#[inline]
pub fn slice_to_be16(v: &[u8]) -> u16 {
Expand Down Expand Up @@ -333,7 +334,7 @@ impl KeysInterface for KeyProvider {
))
}

fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
unreachable!()
}
}
Expand Down
26 changes: 2 additions & 24 deletions lightning-invoice/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use lightning::ln::features::InvoiceFeatures;
#[cfg(any(doc, test))]
use lightning::routing::network_graph::RoutingFees;
use lightning::routing::router::RouteHint;
use lightning::util::invoice::construct_invoice_preimage;

use secp256k1::key::PublicKey;
use secp256k1::{Message, Secp256k1};
Expand Down Expand Up @@ -869,32 +870,9 @@ macro_rules! find_all_extract {

#[allow(missing_docs)]
impl RawInvoice {
/// Construct the invoice's HRP and signatureless data into a preimage to be hashed.
pub(crate) fn construct_invoice_preimage(hrp_bytes: &[u8], data_without_signature: &[u5]) -> Vec<u8> {
use bech32::FromBase32;

let mut preimage = Vec::<u8>::from(hrp_bytes);

let mut data_part = Vec::from(data_without_signature);
let overhang = (data_part.len() * 5) % 8;
if overhang > 0 {
// add padding if data does not end at a byte boundary
data_part.push(u5::try_from_u8(0).unwrap());

// if overhang is in (1..3) we need to add u5(0) padding two times
if overhang < 3 {
data_part.push(u5::try_from_u8(0).unwrap());
}
}

preimage.extend_from_slice(&Vec::<u8>::from_base32(&data_part)
.expect("No padding error may occur due to appended zero above."));
preimage
}

/// Hash the HRP as bytes and signatureless data part.
fn hash_from_parts(hrp_bytes: &[u8], data_without_signature: &[u5]) -> [u8; 32] {
let preimage = RawInvoice::construct_invoice_preimage(hrp_bytes, data_without_signature);
let preimage = construct_invoice_preimage(hrp_bytes, data_without_signature);
let mut hash: [u8; 32] = Default::default();
hash.copy_from_slice(&sha256::Hash::hash(&preimage)[..]);
hash
Expand Down
5 changes: 2 additions & 3 deletions lightning-invoice/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Convenient utilities to create an invoice.

use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice};
use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError};
use payment::{Payer, Router};

use bech32::ToBase32;
Expand Down Expand Up @@ -118,8 +118,7 @@ where
let hrp_str = raw_invoice.hrp.to_string();
let hrp_bytes = hrp_str.as_bytes();
let data_without_signature = raw_invoice.data.to_base32();
let invoice_preimage = RawInvoice::construct_invoice_preimage(hrp_bytes, &data_without_signature);
let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(invoice_preimage));
let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(hrp_bytes, &data_without_signature));
match signed_raw_invoice {
Ok(inv) => Ok(Invoice::from_signed(inv).unwrap()),
Err(e) => Err(SignOrCreationError::SignError(e))
Expand Down
14 changes: 9 additions & 5 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use bitcoin::network::constants::Network;
use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey, ChildNumber};
use bitcoin::util::bip143;

use bitcoin::bech32::u5;
use bitcoin::hashes::{Hash, HashEngine};
use bitcoin::hashes::sha256::HashEngine as Sha256State;
use bitcoin::hashes::sha256::Hash as Sha256;
Expand All @@ -42,6 +43,7 @@ use prelude::*;
use core::sync::atomic::{AtomicUsize, Ordering};
use io::{self, Error};
use ln::msgs::{DecodeError, MAX_VALUE_MSAT};
use util::invoice::construct_invoice_preimage;

/// Used as initial key material, to be expanded into multiple secret keys (but not to be used
/// directly). This is used within LDK to encrypt/decrypt inbound payment data.
Expand Down Expand Up @@ -398,11 +400,12 @@ pub trait KeysInterface {
/// you've read all of the provided bytes to ensure no corruption occurred.
fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError>;

/// Sign an invoice's preimage (note that this is the preimage of the invoice, not the HTLC's
/// preimage). By parameterizing by the preimage instead of the hash, we allow implementors of
/// Sign an invoice.
/// By parameterizing by the raw invoice bytes instead of the hash, we allow implementors of
/// this trait to parse the invoice and make sure they're signing what they expect, rather than
/// blindly signing the hash.
fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()>;
/// The hrp is ascii bytes, while the invoice data is base32.
fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()>;

/// Get secret key material as bytes for use in encrypting and decrypting inbound payment data.
///
Expand Down Expand Up @@ -1092,8 +1095,9 @@ impl KeysInterface for KeysManager {
InMemorySigner::read(&mut io::Cursor::new(reader))
}

fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&invoice_preimage)), &self.get_node_secret()))
fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
let preimage = construct_invoice_preimage(&hrp_bytes, &invoice_data);
Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&preimage)), &self.get_node_secret()))
}
}

Expand Down
3 changes: 2 additions & 1 deletion lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5840,6 +5840,7 @@ mod tests {
use bitcoin::hashes::Hash;
use bitcoin::hash_types::{Txid, WPubkeyHash};
use core::num::NonZeroU8;
use bitcoin::bech32::u5;
use sync::Arc;
use prelude::*;

Expand Down Expand Up @@ -5884,7 +5885,7 @@ mod tests {
}
fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> { panic!(); }
fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { panic!(); }
}

fn public_from_secret_hex(secp_ctx: &Secp256k1<All>, hex: &str) -> PublicKey {
Expand Down
26 changes: 26 additions & 0 deletions lightning/src/util/invoice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//! Low level invoice utilities.

use bitcoin::bech32::{u5, FromBase32};
use prelude::*;

/// Construct the invoice's HRP and signatureless data into a preimage to be hashed.
pub fn construct_invoice_preimage(hrp_bytes: &[u8], data_without_signature: &[u5]) -> Vec<u8> {
let mut preimage = Vec::<u8>::from(hrp_bytes);

let mut data_part = Vec::from(data_without_signature);
let overhang = (data_part.len() * 5) % 8;
if overhang > 0 {
// add padding if data does not end at a byte boundary
data_part.push(u5::try_from_u8(0).unwrap());

// if overhang is in (1..3) we need to add u5(0) padding two times
if overhang < 3 {
data_part.push(u5::try_from_u8(0).unwrap());
}
}

preimage.extend_from_slice(&Vec::<u8>::from_base32(&data_part)
.expect("No padding error may occur due to appended zero above."));
preimage
}

1 change: 1 addition & 0 deletions lightning/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod events;
pub mod errors;
pub mod ser;
pub mod message_signing;
pub mod invoice;

pub(crate) mod atomic_counter;
pub(crate) mod byte_utils;
Expand Down
7 changes: 4 additions & 3 deletions lightning/src/util/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use core::time::Duration;
use sync::{Mutex, Arc};
use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use core::{cmp, mem};
use bitcoin::bech32::u5;
use chain::keysinterface::{InMemorySigner, KeyMaterial};

pub struct TestVecWriter(pub Vec<u8>);
Expand Down Expand Up @@ -87,7 +88,7 @@ impl keysinterface::KeysInterface for OnlyReadsKeysInterface {
false
))
}
fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> { unreachable!(); }
fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { unreachable!(); }
}

pub struct TestChainMonitor<'a> {
Expand Down Expand Up @@ -528,8 +529,8 @@ impl keysinterface::KeysInterface for TestKeysInterface {
))
}

fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
self.backing.sign_invoice(invoice_preimage)
fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
self.backing.sign_invoice(hrp_bytes, invoice_data)
}
}

Expand Down