Skip to content

Impl some message deserialization, initial fuzzing-found bug fixes #14

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.

Sign up for GitHub

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
merged 4 commits into from
Mar 23, 2018
Merged

Impl some message deserialization, initial fuzzing-found bug fixes #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d14e31f
Remove useless Sha256 copies
TheBlueMatt Mar 21, 2018
298557c
impl a bunch of additional messages, remove some useless mut
TheBlueMatt Mar 22, 2018
1bb4f44
Fix reserve check on HTLC receive/send
TheBlueMatt Mar 21, 2018
e9b1af2
Check validity of dust_limit_satoshis in remote messages
TheBlueMatt Mar 22, 2018
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
35 changes: 20 additions & 15 deletions src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,34 +43,33 @@ pub struct ChannelKeys {

impl ChannelKeys {
pub fn new_from_seed(seed: &[u8; 32]) -> Result<ChannelKeys, secp256k1::Error> {
let sha = Sha256::new();
let mut prk = [0; 32];
hkdf_extract(sha, b"rust-lightning key gen salt", seed, &mut prk);
hkdf_extract(Sha256::new(), b"rust-lightning key gen salt", seed, &mut prk);
let secp_ctx = Secp256k1::new();

let mut okm = [0; 32];
hkdf_expand(sha, &prk, b"rust-lightning funding key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning funding key info", &mut okm);
let funding_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning revocation base key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning revocation base key info", &mut okm);
let revocation_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning payment base key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning payment base key info", &mut okm);
let payment_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning delayed payment base key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning delayed payment base key info", &mut okm);
let delayed_payment_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning htlc base key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning htlc base key info", &mut okm);
let htlc_base_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning channel close key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning channel close key info", &mut okm);
let channel_close_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning channel monitor claim key info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning channel monitor claim key info", &mut okm);
let channel_monitor_claim_key = SecretKey::from_slice(&secp_ctx, &okm)?;

hkdf_expand(sha, &prk, b"rust-lightning local commitment seed info", &mut okm);
hkdf_expand(Sha256::new(), &prk, b"rust-lightning local commitment seed info", &mut okm);

Ok(ChannelKeys {
funding_key: funding_key,
Expand Down Expand Up @@ -367,7 +366,9 @@ impl Channel {
if msg.push_msat > (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000 {
return Err(HandleError{err: "push_msat more than highest possible value", msg: None});
}
//TODO Check if dust_limit is sane?
if msg.dust_limit_satoshis > 21000000 * 100000000 {
return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
}
if msg.max_htlc_value_in_flight_msat > msg.funding_satoshis * 1000 {
return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
}
Expand Down Expand Up @@ -827,13 +828,15 @@ impl Channel {

pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
// Check sanity of message fields:
//TODO Check if dust_limit is sane?
if !self.channel_outbound {
return Err(HandleError{err: "Got an accept_channel message from an inbound peer", msg: None});
}
if self.channel_state != ChannelState::OurInitSent as u32 {
return Err(HandleError{err: "Got an accept_channel message at a strange time", msg: None});
}
if msg.dust_limit_satoshis > 21000000 * 100000000 {
return Err(HandleError{err: "Peer never wants payout outputs?", msg: None});
}
if msg.max_htlc_value_in_flight_msat > self.channel_value_satoshis * 1000 {
return Err(HandleError{err: "Bogus max_htlc_value_in_flight_satoshis", msg: None});
}
Expand Down Expand Up @@ -1013,8 +1016,10 @@ impl Channel {
if htlc_inbound_value_msat + msg.amount_msat > Channel::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis) {
return Err(HandleError{err: "Remote HTLC add would put them over their max HTLC value in flight", msg: None});
}
// Check our_channel_reserve_satoshis:
if htlc_inbound_value_msat + htlc_outbound_value_msat + msg.amount_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
// Check our_channel_reserve_satoshis (we're getting paid, so they have to at least meet
// the reserve_satoshis we told them to always have as direct payment so that they lose
// something if we punish them for broadcasting an old state).
if htlc_inbound_value_msat + htlc_outbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 {
return Err(HandleError{err: "Remote HTLC add would put them over their reserve value", msg: None});
}
if self.next_remote_htlc_id != msg.htlc_id {
Expand Down Expand Up @@ -1592,7 +1597,7 @@ impl Channel {
return Err(HandleError{err: "Cannot send value that would put us over our max HTLC value in flight", msg: None});
}
// Check their_channel_reserve_satoshis:
if htlc_outbound_value_msat + amount_msat > (self.channel_value_satoshis - self.their_channel_reserve_satoshis) * 1000 - htlc_inbound_value_msat {
if htlc_inbound_value_msat + htlc_outbound_value_msat + amount_msat + (self.channel_value_satoshis * 1000 - self.value_to_self_msat) > (self.channel_value_satoshis - self.their_channel_reserve_satoshis) * 1000 {
return Err(HandleError{err: "Cannot send value that would put us over our reserve value", msg: None});
}

Expand Down
128 changes: 110 additions & 18 deletions src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,20 @@ impl MsgEncodable for ClosingSigned {
}

impl MsgDecodable for UpdateAddHTLC {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+8+8+32+4+1+33+20*65+32 {
return Err(DecodeError::WrongLength);
}
let mut payment_hash = [0; 32];
payment_hash.copy_from_slice(&v[48..80]);
Ok(Self{
channel_id: deserialize(&v[0..32]).unwrap(),
htlc_id: byte_utils::slice_to_be64(&v[32..40]),
amount_msat: byte_utils::slice_to_be64(&v[40..48]),
payment_hash,
cltv_expiry: byte_utils::slice_to_be32(&v[80..84]),
onion_routing_packet: OnionPacket::decode(&v[84..])?,
})
}
}
impl MsgEncodable for UpdateAddHTLC {
Expand All @@ -695,8 +707,17 @@ impl MsgEncodable for UpdateAddHTLC {
}

impl MsgDecodable for UpdateFulfillHTLC {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+8+32 {
return Err(DecodeError::WrongLength);
}
let mut payment_preimage = [0; 32];
payment_preimage.copy_from_slice(&v[40..72]);
Ok(Self{
channel_id: deserialize(&v[0..32]).unwrap(),
htlc_id: byte_utils::slice_to_be64(&v[32..40]),
payment_preimage,
})
}
}
impl MsgEncodable for UpdateFulfillHTLC {
Expand All @@ -706,8 +727,15 @@ impl MsgEncodable for UpdateFulfillHTLC {
}

impl MsgDecodable for UpdateFailHTLC {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+8 {
return Err(DecodeError::WrongLength);
}
Ok(Self{
channel_id: deserialize(&v[0..32]).unwrap(),
htlc_id: byte_utils::slice_to_be64(&v[32..40]),
reason: OnionErrorPacket::decode(&v[40..])?,
})
}
}
impl MsgEncodable for UpdateFailHTLC {
Expand All @@ -717,8 +745,18 @@ impl MsgEncodable for UpdateFailHTLC {
}

impl MsgDecodable for UpdateFailMalformedHTLC {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+8+32+2 {
return Err(DecodeError::WrongLength);
}
let mut sha256_of_onion = [0; 32];
sha256_of_onion.copy_from_slice(&v[40..72]);
Ok(Self{
channel_id: deserialize(&v[0..32]).unwrap(),
htlc_id: byte_utils::slice_to_be64(&v[32..40]),
sha256_of_onion,
failure_code: byte_utils::slice_to_be16(&v[72..74]),
})
}
}
impl MsgEncodable for UpdateFailMalformedHTLC {
Expand All @@ -728,8 +766,24 @@ impl MsgEncodable for UpdateFailMalformedHTLC {
}

impl MsgDecodable for CommitmentSigned {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+64+2 {
return Err(DecodeError::WrongLength);
}
let htlcs = byte_utils::slice_to_be16(&v[96..98]) as usize;
if v.len() < 32+64+2+htlcs*64 {
return Err(DecodeError::WrongLength);
}
let mut htlc_signatures = Vec::with_capacity(htlcs);
let secp_ctx = Secp256k1::without_caps();
for i in 0..htlcs {
htlc_signatures.push(secp_signature!(&secp_ctx, &v[98+i*64..98+(i+1)*64]));
}
Ok(Self {
channel_id: deserialize(&v[0..32]).unwrap(),
signature: secp_signature!(&secp_ctx, &v[32..96]),
htlc_signatures,
})
}
}
impl MsgEncodable for CommitmentSigned {
Expand All @@ -739,8 +793,18 @@ impl MsgEncodable for CommitmentSigned {
}

impl MsgDecodable for RevokeAndACK {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+32+33 {
return Err(DecodeError::WrongLength);
}
let mut per_commitment_secret = [0; 32];
per_commitment_secret.copy_from_slice(&v[32..64]);
let secp_ctx = Secp256k1::without_caps();
Ok(Self {
channel_id: deserialize(&v[0..32]).unwrap(),
per_commitment_secret,
next_per_commitment_point: secp_pubkey!(&secp_ctx, &v[64..97]),
})
}
}
impl MsgEncodable for RevokeAndACK {
Expand All @@ -750,8 +814,14 @@ impl MsgEncodable for RevokeAndACK {
}

impl MsgDecodable for UpdateFee {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 32+4 {
return Err(DecodeError::WrongLength);
}
Ok(Self {
channel_id: deserialize(&v[0..32]).unwrap(),
feerate_per_kw: byte_utils::slice_to_be32(&v[32..36]),
})
}
}
impl MsgEncodable for UpdateFee {
Expand Down Expand Up @@ -954,8 +1024,21 @@ impl MsgEncodable for OnionHopData {
}

impl MsgDecodable for OnionPacket {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 1+33+20*65+32 {
return Err(DecodeError::WrongLength);
}
let mut hop_data = [0; 20*65];
hop_data.copy_from_slice(&v[34..1334]);
let mut hmac = [0; 32];
hmac.copy_from_slice(&v[1334..1366]);
let secp_ctx = Secp256k1::without_caps();
Ok(Self {
version: v[0],
public_key: secp_pubkey!(&secp_ctx, &v[1..34]),
hop_data,
hmac,
})
}
}
impl MsgEncodable for OnionPacket {
Expand Down Expand Up @@ -987,8 +1070,17 @@ impl MsgEncodable for DecodedOnionErrorPacket {
}

impl MsgDecodable for OnionErrorPacket {
fn decode(_v: &[u8]) -> Result<Self, DecodeError> {
unimplemented!();
fn decode(v: &[u8]) -> Result<Self, DecodeError> {
if v.len() < 2 {
return Err(DecodeError::WrongLength);
}
let len = byte_utils::slice_to_be16(&v[0..2]) as usize;
if v.len() < 2 + len {
return Err(DecodeError::WrongLength);
}
Ok(Self {
data: v[2..len+2].to_vec(),
})
}
}
impl MsgEncodable for OnionErrorPacket {
Expand Down
27 changes: 11 additions & 16 deletions src/ln/peer_channel_encryptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,11 @@ impl PeerChannelEncryptor {

#[inline]
fn hkdf(state: &mut BidirectionalNoiseState, ss: SharedSecret) -> [u8; 32] {
let sha = Sha256::new();
let mut hkdf = [0; 64];
{
let mut prk = [0; 32];
hkdf_extract(sha, &state.ck, &ss[..], &mut prk);
hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
hkdf_extract(Sha256::new(), &state.ck, &ss[..], &mut prk);
hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);
}
state.ck.copy_from_slice(&hkdf[0..32]);
let mut res = [0; 32];
Expand Down Expand Up @@ -313,10 +312,9 @@ impl PeerChannelEncryptor {

PeerChannelEncryptor::encrypt_with_ad(&mut res[50..], 0, &temp_k, &bidirectional_state.h, &[0; 0]);

sha.reset();
let mut prk = [0; 32];
hkdf_extract(sha, &bidirectional_state.ck, &[0; 0], &mut prk);
hkdf_expand(sha, &prk, &[0;0], &mut final_hkdf);
hkdf_extract(Sha256::new(), &bidirectional_state.ck, &[0; 0], &mut prk);
hkdf_expand(Sha256::new(), &prk, &[0;0], &mut final_hkdf);
ck = bidirectional_state.ck.clone();
res
},
Expand Down Expand Up @@ -375,10 +373,9 @@ impl PeerChannelEncryptor {

PeerChannelEncryptor::decrypt_with_ad(&mut [0; 0], 0, &temp_k, &bidirectional_state.h, &act_three[50..])?;

sha.reset();
let mut prk = [0; 32];
hkdf_extract(sha, &bidirectional_state.ck, &[0; 0], &mut prk);
hkdf_expand(sha, &prk, &[0;0], &mut final_hkdf);
hkdf_extract(Sha256::new(), &bidirectional_state.ck, &[0; 0], &mut prk);
hkdf_expand(Sha256::new(), &prk, &[0;0], &mut final_hkdf);
ck = bidirectional_state.ck.clone();
},
_ => panic!("Wrong direction for act"),
Expand Down Expand Up @@ -416,11 +413,10 @@ impl PeerChannelEncryptor {
match self.noise_state {
NoiseState::Finished { ref mut sk, ref mut sn, ref mut sck, rk: _, rn: _, rck: _ } => {
if *sn >= 1000 {
let mut sha = Sha256::new();
let mut prk = [0; 32];
hkdf_extract(sha, sck, sk, &mut prk);
hkdf_extract(Sha256::new(), sck, sk, &mut prk);
let mut hkdf = [0; 64];
hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);

sck[..].copy_from_slice(&hkdf[0..32]);
sk[..].copy_from_slice(&hkdf[32..]);
Expand All @@ -447,11 +443,10 @@ impl PeerChannelEncryptor {
match self.noise_state {
NoiseState::Finished { sk: _, sn: _, sck: _, ref mut rk, ref mut rn, ref mut rck } => {
if *rn >= 1000 {
let mut sha = Sha256::new();
let mut prk = [0; 32];
hkdf_extract(sha, rck, rk, &mut prk);
hkdf_extract(Sha256::new(), rck, rk, &mut prk);
let mut hkdf = [0; 64];
hkdf_expand(sha, &prk, &[0;0], &mut hkdf);
hkdf_expand(Sha256::new(), &prk, &[0;0], &mut hkdf);

rck[..].copy_from_slice(&hkdf[0..32]);
rk[..].copy_from_slice(&hkdf[32..]);
Expand Down Expand Up @@ -752,7 +747,7 @@ mod tests {
let res = outbound_peer.encrypt_message(&msg);
assert_eq!(res.len(), 5 + 2*16 + 2);

let mut len_header = res[0..2+16].to_vec();
let len_header = res[0..2+16].to_vec();
assert_eq!(inbound_peer.decrypt_length_header(&len_header[..]).unwrap() as usize, msg.len());
assert_eq!(inbound_peer.decrypt_message(&res[2+16..]).unwrap()[..], msg[..]);

Expand Down