Skip to content
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
1 change: 1 addition & 0 deletions fuzz/src/bin/gen_target.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ GEN_TEST chanmon_consistency
GEN_TEST full_stack
GEN_TEST peer_crypt
GEN_TEST router
GEN_TEST zbase32

GEN_TEST msg_accept_channel msg_targets::
GEN_TEST msg_announcement_signatures msg_targets::
Expand Down
102 changes: 102 additions & 0 deletions fuzz/src/bin/zbase32_target.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// This file is Copyright its original authors, visible in version control
// history.
//
// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
// You may not use this file except in accordance with one or both of these
// licenses.

// This file is auto-generated by gen_target.sh based on target_template.txt
// To modify it, modify target_template.txt and run gen_target.sh instead.

#![cfg_attr(feature = "libfuzzer_fuzz", no_main)]

extern crate lightning_fuzz;
use lightning_fuzz::zbase32::*;

#[cfg(feature = "afl")]
#[macro_use] extern crate afl;
#[cfg(feature = "afl")]
fn main() {
fuzz!(|data| {
zbase32_run(data.as_ptr(), data.len());
});
}

#[cfg(feature = "honggfuzz")]
#[macro_use] extern crate honggfuzz;
#[cfg(feature = "honggfuzz")]
fn main() {
loop {
fuzz!(|data| {
zbase32_run(data.as_ptr(), data.len());
});
}
}

#[cfg(feature = "libfuzzer_fuzz")]
#[macro_use] extern crate libfuzzer_sys;
#[cfg(feature = "libfuzzer_fuzz")]
fuzz_target!(|data: &[u8]| {
zbase32_run(data.as_ptr(), data.len());
});

#[cfg(feature = "stdin_fuzz")]
fn main() {
use std::io::Read;

let mut data = Vec::with_capacity(8192);
std::io::stdin().read_to_end(&mut data).unwrap();
zbase32_run(data.as_ptr(), data.len());
}

#[test]
fn run_test_cases() {
use std::fs;
use std::io::Read;
use lightning_fuzz::utils::test_logger::StringBuffer;

use std::sync::{atomic, Arc};
{
let data: Vec<u8> = vec![0];
zbase32_run(data.as_ptr(), data.len());
}
let mut threads = Vec::new();
let threads_running = Arc::new(atomic::AtomicUsize::new(0));
if let Ok(tests) = fs::read_dir("test_cases/zbase32") {
for test in tests {
let mut data: Vec<u8> = Vec::new();
let path = test.unwrap().path();
fs::File::open(&path).unwrap().read_to_end(&mut data).unwrap();
threads_running.fetch_add(1, atomic::Ordering::AcqRel);

let thread_count_ref = Arc::clone(&threads_running);
let main_thread_ref = std::thread::current();
threads.push((path.file_name().unwrap().to_str().unwrap().to_string(),
std::thread::spawn(move || {
let string_logger = StringBuffer::new();

let panic_logger = string_logger.clone();
let res = if ::std::panic::catch_unwind(move || {
zbase32_test(&data, panic_logger);
}).is_err() {
Some(string_logger.into_string())
} else { None };
thread_count_ref.fetch_sub(1, atomic::Ordering::AcqRel);
main_thread_ref.unpark();
res
})
));
while threads_running.load(atomic::Ordering::Acquire) > 32 {
std::thread::park();
}
}
}
for (test, thread) in threads.drain(..) {
if let Some(output) = thread.join().unwrap() {
println!("Output of {}:\n{}", test, output);
panic!();
}
}
}
1 change: 1 addition & 0 deletions fuzz/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ pub mod chanmon_consistency;
pub mod full_stack;
pub mod peer_crypt;
pub mod router;
pub mod zbase32;

pub mod msg_targets;
33 changes: 33 additions & 0 deletions fuzz/src/zbase32.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// This file is Copyright its original authors, visible in version control
// history.
//
// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
// You may not use this file except in accordance with one or both of these
// licenses.

use lightning::util::zbase32;

use utils::test_logger;

#[inline]
pub fn do_test(data: &[u8]) {
let res = zbase32::encode(data);
assert_eq!(&zbase32::decode(&res).unwrap()[..], data);

if let Ok(s) = std::str::from_utf8(data) {
if let Ok(decoded) = zbase32::decode(s) {
assert_eq!(&zbase32::encode(&decoded), &s.to_ascii_lowercase());
}
}
}

pub fn zbase32_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
do_test(data);
}

#[no_mangle]
pub extern "C" fn zbase32_run(data: *const u8, datalen: usize) {
do_test(unsafe { std::slice::from_raw_parts(data, datalen) });
}
1 change: 1 addition & 0 deletions fuzz/targets.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ void chanmon_consistency_run(const unsigned char* data, size_t data_len);
void full_stack_run(const unsigned char* data, size_t data_len);
void peer_crypt_run(const unsigned char* data, size_t data_len);
void router_run(const unsigned char* data, size_t data_len);
void zbase32_run(const unsigned char* data, size_t data_len);
void msg_accept_channel_run(const unsigned char* data, size_t data_len);
void msg_announcement_signatures_run(const unsigned char* data, size_t data_len);
void msg_channel_reestablish_run(const unsigned char* data, size_t data_len);
Expand Down
140 changes: 140 additions & 0 deletions lightning/src/util/message_signing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
// You may not use this file except in accordance with one or both of these
// licenses.

//! Lightning message signing and verification lives here. These tools can be used to sign messages using the node's
//! secret so receivers are sure that they come from you. You can also use this to verify that a given message comes
//! from a specific node.
//! Furthermore, these tools can be used to sign / verify messages using ephemeral keys not tied to node's identities.
//!
//! Note this is not part of the specs, but follows lnd's signing and verifying protocol, which can is defined as follows:
//!
//! signature = zbase32(SigRec(sha256d(("Lightning Signed Message:" + msg)))
//! zbase32 from https://philzimmermann.com/docs/human-oriented-base-32-encoding.txt
//! SigRec has first byte 31 + recovery id, followed by 64 byte sig.
//!
//! This implementation is compatible with both lnd's and c-lightning's
//!
//! https://lightning.readthedocs.io/lightning-signmessage.7.html
//! https://api.lightning.community/#signmessage
use crate::util::zbase32;
use bitcoin::hashes::{sha256d, Hash};
use bitcoin::secp256k1::recovery::{RecoverableSignature, RecoveryId};
use bitcoin::secp256k1::{Error, Message, PublicKey, Secp256k1, SecretKey};

static LN_MESSAGE_PREFIX: &[u8] = b"Lightning Signed Message:";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bind message to a lightning signing context to avoid replay of signed data in another context with same key material ? Though w.r.t to this concern our message API signing is safe as this is always committed by the implementation and not an argument caller, not sure if it's worthy to document.

Do we have risks of harmful API misuses we should document ? I think we're good, can't foresee real ones.

Copy link
Contributor Author

@sr-gi sr-gi Apr 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the purpose of the prefix AFAIK. I can write a comment there stating the purpose of the prefix if you feel it's necessary.

Do we have risks of harmful API misuses we should document ? I think we're good, can't foresee real ones.

I cannot see any either.


fn sigrec_encode(sig_rec: RecoverableSignature) -> Vec<u8> {
let (rid, rsig) = sig_rec.serialize_compact();
let prefix = rid.to_i32() as u8 + 31;

[&[prefix], &rsig[..]].concat()
}

fn sigrec_decode(sig_rec: Vec<u8>) -> Result<RecoverableSignature, Error> {
let rsig = &sig_rec[1..];
let rid = sig_rec[0] as i32 - 31;

match RecoveryId::from_i32(rid) {
Ok(x) => RecoverableSignature::from_compact(rsig, x),
Err(e) => Err(e)
}
}

/// Creates a digital signature of a message given a SecretKey, like the node's secret.
/// A receiver knowing the PublicKey (e.g. the node's id) and the message can be sure that the signature was generated by the caller.
/// Signatures are EC recoverable, meaning that given the message and the signature the PublicKey of the signer can be extracted.
pub fn sign(msg: &[u8], sk: SecretKey) -> Result<String, Error> {
let secp_ctx = Secp256k1::signing_only();
let msg_hash = sha256d::Hash::hash(&[LN_MESSAGE_PREFIX, msg].concat());

let sig = secp_ctx.sign_recoverable(&Message::from_slice(&msg_hash)?, &sk);
Ok(zbase32::encode(&sigrec_encode(sig)))
}

/// Recovers the PublicKey of the signer of the message given the message and the signature.
pub fn recover_pk(msg: &[u8], sig: &str) -> Result<PublicKey, Error> {
let secp_ctx = Secp256k1::verification_only();
let msg_hash = sha256d::Hash::hash(&[LN_MESSAGE_PREFIX, msg].concat());

match zbase32::decode(&sig) {
Ok(sig_rec) => {
match sigrec_decode(sig_rec) {
Ok(sig) => secp_ctx.recover(&Message::from_slice(&msg_hash)?, &sig),
Err(e) => Err(e)
}
},
Err(_) => Err(Error::InvalidSignature)
}
}

/// Verifies a message was signed by a PrivateKey that derives to a given PublicKey, given a message, a signature,
/// and the PublicKey.
pub fn verify(msg: &[u8], sig: &str, pk: PublicKey) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with the sample and lnd's telling me the message is invalid (the pubkey's correct): https://i.imgur.com/lizoFib.png

Haven't tested further.
Instructions to reproduce/test further (I used Polar but feel free to use whatever you want):

Also added verifymessage to the sample: https://github.com/valentinewallace/ldk-sample/blob/for-testing-signing/src/cli.rs#L390 if you wanna test that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with the sample and lnd's telling me the message is invalid (the pubkey's correct): https://i.imgur.com/lizoFib.png

Wouldn't this mean the two nodes are not properly connected (or the node that originated the signature is not on the verifiers database?). I'm wondering since you point that the recovered public key is correct, so the signature is actually valid.

From lnd docs:

VerifyMessage verifies a signature over a msg. The signature must be zbase32 encoded and signed by an active node in the resident node's channel database. In addition to returning the validity of the signature, VerifyMessage also returns the recovered pubkey from the signature.

Copy link
Contributor Author

@sr-gi sr-gi Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valentinewallace I double checked this against lnd and c-lightning. Both return:

{
   "pubkey": our_public_key,
   "verified": false
}

However, in c-lightning, if our public key is passed as optional parameter then the message is verified.

Checking c-lightning's docs, we can find the following:

As a special case, if pubkey is not specified, we will try every known node key (as per listnodes), and verification succeeds if it matches for any one of them.

And, If I check listnodes, the list is empty (even though the node is one of our peers), so I'm guessing that's the root of the issue. The pubkey is correctly recovered in both cases, but the node seems not to be part of neither of the peers known nodes, so it fails validation against known nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Furthermore, querying describegraph in lnd's node does not show our testing node, which I'd say backs the aforementioned claim.

I'm not sure why that's happening though. Any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asked around #c-lightning on IRC, looks like a channel announced is required to show-up in listnodes, and I'm assuming the same may apply to describegraph in lnd:

20:12 <sr_gi> Hey guys, what is needed for a node to show in listnodes? I was doing some testing using Polar and
connecting a c-lightning node to a rust-lightning node and even though they were connected the peer did no show up
in the list. Is a channel required or something on those lines?
20:14 <@cdecker> An announced channel is required, yes

Copy link
Contributor

@valentinewallace valentinewallace Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, this seems like a reasonable explanation for why lnd's giving verified: false :) Though it might be reasonable to test at some point with a public channel using the sample.

match recover_pk(msg, sig) {
Ok(x) => x == pk,
Err(_) => false
}
}

#[cfg(test)]
mod test {
use std::str::FromStr;
use util::message_signing::{sign, recover_pk, verify};
use bitcoin::secp256k1::key::ONE_KEY;
use bitcoin::secp256k1::{PublicKey, Secp256k1};

#[test]
fn test_sign() {
let message = "test message";
let zbase32_sig = sign(message.as_bytes(), ONE_KEY);

assert_eq!(zbase32_sig.unwrap(), "d9tibmnic9t5y41hg7hkakdcra94akas9ku3rmmj4ag9mritc8ok4p5qzefs78c9pqfhpuftqqzhydbdwfg7u6w6wdxcqpqn4sj4e73e")
}

#[test]
fn test_recover_pk() {
let message = "test message";
let sig = "d9tibmnic9t5y41hg7hkakdcra94akas9ku3rmmj4ag9mritc8ok4p5qzefs78c9pqfhpuftqqzhydbdwfg7u6w6wdxcqpqn4sj4e73e";
let pk = recover_pk(message.as_bytes(), sig);

assert_eq!(pk.unwrap(), PublicKey::from_secret_key(&Secp256k1::signing_only(), &ONE_KEY))
}

#[test]
fn test_verify() {
let message = "another message";
let sig = sign(message.as_bytes(), ONE_KEY).unwrap();
let pk = PublicKey::from_secret_key(&Secp256k1::signing_only(), &ONE_KEY);

assert!(verify(message.as_bytes(), &sig, pk))
}

#[test]
fn test_verify_ground_truth_ish() {
// There are no standard tests vectors for Sign/Verify, using the same tests vectors as c-lightning to see if they are compatible.
// Taken from https://github.com/ElementsProject/lightning/blob/1275af6fbb02460c8eb2f00990bb0ef9179ce8f3/tests/test_misc.py#L1925-L1938

let corpus = [
["@bitconner",
"is this compatible?",
"rbgfioj114mh48d8egqx8o9qxqw4fmhe8jbeeabdioxnjk8z3t1ma1hu1fiswpakgucwwzwo6ofycffbsqusqdimugbh41n1g698hr9t",
"02b80cabdf82638aac86948e4c06e82064f547768dcef977677b9ea931ea75bab5"],
["@duck1123",
"hi",
"rnrphcjswusbacjnmmmrynh9pqip7sy5cx695h6mfu64iac6qmcmsd8xnsyczwmpqp9shqkth3h4jmkgyqu5z47jfn1q7gpxtaqpx4xg",
"02de60d194e1ca5947b59fe8e2efd6aadeabfb67f2e89e13ae1a799c1e08e4a43b"],
["@jochemin",
"hi",
"ry8bbsopmduhxy3dr5d9ekfeabdpimfx95kagdem7914wtca79jwamtbw4rxh69hg7n6x9ty8cqk33knbxaqftgxsfsaeprxkn1k48p3",
"022b8ece90ee891cbcdac0c1cc6af46b73c47212d8defbce80265ac81a6b794931"],
];

for c in &corpus {
assert!(verify(c[1].as_bytes(), c[2], PublicKey::from_str(c[3]).unwrap()))
}
}
}
5 changes: 5 additions & 0 deletions lightning/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ pub(crate) mod fuzz_wrappers;
pub mod events;
pub mod errors;
pub mod ser;
pub mod message_signing;

pub(crate) mod byte_utils;
pub(crate) mod chacha20;
#[cfg(feature = "fuzztarget")]
pub mod zbase32;
#[cfg(not(feature = "fuzztarget"))]
pub(crate) mod zbase32;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the fuzztarget mode needs it, see CI.

Suggested change
pub(crate) mod zbase32;
#[cfg(feature = "fuzztarget")]
pub mod zbase32;
#[cfg(not(feature = "fuzztarget"))]
pub(crate) mod zbase32;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it in cf384a3 and changed zbase32::{encode, decode} to pub so they can be used in fuzzing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compile error in CI is that poly1305 is no longer #[cfg(not(feature = "fuzztarget"))] (ie not compiled in fuzzing). You need to duplicate the cfg line, above.

#[cfg(not(feature = "fuzztarget"))]
pub(crate) mod poly1305;
pub(crate) mod chacha20poly1305rfc;
Expand Down
Loading