Skip to content

Making message size limit an exportable constant #623

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 6 commits into from
Jul 22, 2020
Merged

Making message size limit an exportable constant #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eb8bcaa
Making message size limit an exportable constant
dr-orlovsky May 17, 2020
bcb50a6
Fixing panic messages for decryption procedures
dr-orlovsky Jul 21, 2020
caed31a
Test cases for message encryption/decryption size limits
dr-orlovsky Jul 21, 2020
eae748a
Removing duplicated code in message decryption test cases
dr-orlovsky Jul 21, 2020
0e5dfad
Removing spec quotation from LN_MAX_MSG_LEN definition
dr-orlovsky Jul 21, 2020
4bb5955
Moving LN_MAX_MSG_LEN const to the actual use place
dr-orlovsky Jul 21, 2020
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
2 changes: 2 additions & 0 deletions lightning/src/ln/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,5 @@ mod functional_tests;
mod chanmon_update_fail_tests;
#[cfg(test)]
mod reorg_tests;

pub use self::peer_channel_encryptor::LN_MAX_MSG_LEN;
124 changes: 68 additions & 56 deletions lightning/src/ln/peer_channel_encryptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ use bitcoin::secp256k1;
use util::chacha20poly1305rfc::ChaCha20Poly1305RFC;
use util::byte_utils;

/// Maximum Lightning message data length according to
/// [BOLT-8](https://github.com/lightningnetwork/lightning-rfc/blob/v1.0/08-transport.md#lightning-message-specification)
/// and [BOLT-1](https://github.com/lightningnetwork/lightning-rfc/blob/master/01-messaging.md#lightning-message-format):
pub const LN_MAX_MSG_LEN: usize = ::std::u16::MAX as usize; // Must be equal to 65535

// Sha256("Noise_XK_secp256k1_ChaChaPoly_SHA256")
const NOISE_CK: [u8; 32] = [0x26, 0x40, 0xf5, 0x2e, 0xeb, 0xcd, 0x9e, 0x88, 0x29, 0x58, 0x95, 0x1c, 0x79, 0x42, 0x50, 0xee, 0xdb, 0x28, 0x00, 0x2c, 0x05, 0xd7, 0xdc, 0x2e, 0xa0, 0xf1, 0x95, 0x40, 0x60, 0x42, 0xca, 0xf1];
// Sha256(NOISE_CK || "lightning")
Expand Down Expand Up @@ -373,7 +378,7 @@ impl PeerChannelEncryptor {
/// Encrypts the given message, returning the encrypted version
/// panics if msg.len() > 65535 or Noise handshake has not finished.
Copy link
Contributor

Choose a reason for hiding this comment

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

You deliberately leave "65535" in the comments or you just missed it? I actually don't know what is better. Probably you're right and the number works fine for a doc reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deliberately. Constant is unreadable in text, you have to look up for the value; while integer literal allow to check what value the constant must have and understand on which conditions the code will fail.

pub fn encrypt_message(&mut self, msg: &[u8]) -> Vec<u8> {
if msg.len() > 65535 {
if msg.len() > LN_MAX_MSG_LEN {
panic!("Attempted to encrypt message longer than 65535 bytes!");
}

Expand Down Expand Up @@ -420,15 +425,15 @@ impl PeerChannelEncryptor {
*rn += 1;
Ok(byte_utils::slice_to_be16(&res))
},
_ => panic!("Tried to encrypt a message prior to noise handshake completion"),
_ => panic!("Tried to decrypt a message prior to noise handshake completion"),
}
}

/// Decrypts the given message.
/// panics if msg.len() > 65535 + 16
pub fn decrypt_message(&mut self, msg: &[u8]) -> Result<Vec<u8>, LightningError> {
if msg.len() > 65535 + 16 {
panic!("Attempted to encrypt message longer than 65535 bytes!");
if msg.len() > LN_MAX_MSG_LEN + 16 {
panic!("Attempted to decrypt message longer than 65535 + 16 bytes!");
}

match self.noise_state {
Expand All @@ -440,7 +445,7 @@ impl PeerChannelEncryptor {

Ok(res)
},
_ => panic!("Tried to encrypt a message prior to noise handshake completion"),
_ => panic!("Tried to decrypt a message prior to noise handshake completion"),
}
}

Expand All @@ -467,6 +472,8 @@ impl PeerChannelEncryptor {

#[cfg(test)]
mod tests {
use super::LN_MAX_MSG_LEN;

use bitcoin::secp256k1::key::{PublicKey,SecretKey};

use hex;
Expand All @@ -481,6 +488,36 @@ mod tests {
outbound_peer
}

fn get_inbound_peer_for_test_vectors() -> PeerChannelEncryptor {
// transport-responder successful handshake
let our_node_id = SecretKey::from_slice(&hex::decode("2121212121212121212121212121212121212121212121212121212121212121").unwrap()[..]).unwrap();
let our_ephemeral = SecretKey::from_slice(&hex::decode("2222222222222222222222222222222222222222222222222222222222222222").unwrap()[..]).unwrap();

let mut inbound_peer = PeerChannelEncryptor::new_inbound(&our_node_id);

let act_one = hex::decode("00036360e856310ce5d294e8be33fc807077dc56ac80d95d9cd4ddbd21325eff73f70df6086551151f58b8afe6c195782c6a").unwrap().to_vec();
assert_eq!(inbound_peer.process_act_one_with_keys(&act_one[..], &our_node_id, our_ephemeral.clone()).unwrap()[..], hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap()[..]);

let act_three = hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap().to_vec();
// test vector doesn't specify the initiator static key, but it's the same as the one
// from transport-initiator successful handshake
assert_eq!(inbound_peer.process_act_three(&act_three[..]).unwrap().serialize()[..], hex::decode("034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa").unwrap()[..]);

match inbound_peer.noise_state {
NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {
assert_eq!(sk, hex::decode("bb9020b8965f4df047e07f955f3c4b88418984aadc5cdb35096b9ea8fa5c3442").unwrap()[..]);
assert_eq!(sn, 0);
assert_eq!(sck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
assert_eq!(rk, hex::decode("969ab31b4d288cedf6218839b27a3e2140827047f2c0f01bf5c04435d43511a9").unwrap()[..]);
assert_eq!(rn, 0);
assert_eq!(rck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
},
_ => panic!()
}

inbound_peer
}

#[test]
fn noise_initiator_test_vectors() {
let our_node_id = SecretKey::from_slice(&hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap();
Expand Down Expand Up @@ -539,28 +576,7 @@ mod tests {
let our_ephemeral = SecretKey::from_slice(&hex::decode("2222222222222222222222222222222222222222222222222222222222222222").unwrap()[..]).unwrap();

{
// transport-responder successful handshake
let mut inbound_peer = PeerChannelEncryptor::new_inbound(&our_node_id);

let act_one = hex::decode("00036360e856310ce5d294e8be33fc807077dc56ac80d95d9cd4ddbd21325eff73f70df6086551151f58b8afe6c195782c6a").unwrap().to_vec();
assert_eq!(inbound_peer.process_act_one_with_keys(&act_one[..], &our_node_id, our_ephemeral.clone()).unwrap()[..], hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap()[..]);

let act_three = hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap().to_vec();
// test vector doesn't specify the initiator static key, but it's the same as the one
// from transport-initiator successful handshake
assert_eq!(inbound_peer.process_act_three(&act_three[..]).unwrap().serialize()[..], hex::decode("034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa").unwrap()[..]);

match inbound_peer.noise_state {
NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {
assert_eq!(sk, hex::decode("bb9020b8965f4df047e07f955f3c4b88418984aadc5cdb35096b9ea8fa5c3442").unwrap()[..]);
assert_eq!(sn, 0);
assert_eq!(sck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
assert_eq!(rk, hex::decode("969ab31b4d288cedf6218839b27a3e2140827047f2c0f01bf5c04435d43511a9").unwrap()[..]);
assert_eq!(rn, 0);
assert_eq!(rck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
},
_ => panic!()
}
let _ = get_inbound_peer_for_test_vectors();
}
{
// transport-responder act1 short read test
Expand Down Expand Up @@ -659,35 +675,7 @@ mod tests {
}
}

let mut inbound_peer;

{
// transport-responder successful handshake
let our_node_id = SecretKey::from_slice(&hex::decode("2121212121212121212121212121212121212121212121212121212121212121").unwrap()[..]).unwrap();
let our_ephemeral = SecretKey::from_slice(&hex::decode("2222222222222222222222222222222222222222222222222222222222222222").unwrap()[..]).unwrap();

inbound_peer = PeerChannelEncryptor::new_inbound(&our_node_id);

let act_one = hex::decode("00036360e856310ce5d294e8be33fc807077dc56ac80d95d9cd4ddbd21325eff73f70df6086551151f58b8afe6c195782c6a").unwrap().to_vec();
assert_eq!(inbound_peer.process_act_one_with_keys(&act_one[..], &our_node_id, our_ephemeral.clone()).unwrap()[..], hex::decode("0002466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f276e2470b93aac583c9ef6eafca3f730ae").unwrap()[..]);

let act_three = hex::decode("00b9e3a702e93e3a9948c2ed6e5fd7590a6e1c3a0344cfc9d5b57357049aa22355361aa02e55a8fc28fef5bd6d71ad0c38228dc68b1c466263b47fdf31e560e139ba").unwrap().to_vec();
// test vector doesn't specify the initiator static key, but it's the same as the one
// from transport-initiator successful handshake
assert_eq!(inbound_peer.process_act_three(&act_three[..]).unwrap().serialize()[..], hex::decode("034f355bdcb7cc0af728ef3cceb9615d90684bb5b2ca5f859ab0f0b704075871aa").unwrap()[..]);

match inbound_peer.noise_state {
NoiseState::Finished { sk, sn, sck, rk, rn, rck } => {
assert_eq!(sk, hex::decode("bb9020b8965f4df047e07f955f3c4b88418984aadc5cdb35096b9ea8fa5c3442").unwrap()[..]);
assert_eq!(sn, 0);
assert_eq!(sck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
assert_eq!(rk, hex::decode("969ab31b4d288cedf6218839b27a3e2140827047f2c0f01bf5c04435d43511a9").unwrap()[..]);
assert_eq!(rn, 0);
assert_eq!(rck, hex::decode("919219dbb2920afa8db80f9a51787a840bcf111ed8d588caf9ab4be716e42b01").unwrap()[..]);
},
_ => panic!()
}
}
let mut inbound_peer = get_inbound_peer_for_test_vectors();

for i in 0..1005 {
let msg = [0x68, 0x65, 0x6c, 0x6c, 0x6f];
Expand All @@ -713,4 +701,28 @@ mod tests {
}
}
}

#[test]
fn max_msg_len_limit_value() {
assert_eq!(LN_MAX_MSG_LEN, 65535);
assert_eq!(LN_MAX_MSG_LEN, ::std::u16::MAX as usize);
}

#[test]
#[should_panic(expected = "Attempted to encrypt message longer than 65535 bytes!")]
fn max_message_len_encryption() {
let mut outbound_peer = get_outbound_peer_for_initiator_test_vectors();
let msg = [4u8; LN_MAX_MSG_LEN + 1];
outbound_peer.encrypt_message(&msg);
}

#[test]
#[should_panic(expected = "Attempted to decrypt message longer than 65535 + 16 bytes!")]
fn max_message_len_decryption() {
let mut inbound_peer = get_inbound_peer_for_test_vectors();

// MSG should not exceed LN_MAX_MSG_LEN + 16
let msg = [4u8; LN_MAX_MSG_LEN + 17];
inbound_peer.decrypt_message(&msg).unwrap();
}
}