Feature/constant length packet payloads (#268)

* Reducing ACK packet size to exactly what we need

* Made fragmentidentifier into an array

* Padding all sent messages so they'd be split into constant length packets

clients/native/src/client/real_messages_control/acknowlegement_control/acknowledgement_listener.rs

@@ -49,7 +49,7 @@ impl AcknowledgementListener {
                 warn!("Received invalid ACK!"); // should we do anything else about that?
                 return;
             }
-            Some(frag_id_bytes) => match FragmentIdentifier::try_from_bytes(&frag_id_bytes) {
+            Some(frag_id_bytes) => match FragmentIdentifier::try_from_bytes(frag_id_bytes) {
                 Ok(frag_id) => frag_id,
                 Err(err) => {
                     warn!("Received invalid ACK! - {:?}", err); // should we do anything else about that?

clients/native/src/client/real_messages_control/acknowlegement_control/mod.rs

@@ -132,6 +132,7 @@ where
         let message_chunker = MessageChunker::new_with_rng(
             rng,
             ack_recipient.clone(),
+            true,
             average_packet_delay_duration,
             average_ack_delay_duration,
         );

clients/native/src/client/received_buffer.rs

@@ -56,7 +56,7 @@ impl ReceivedMessagesBuffer {
         ReceivedMessagesBuffer {
             inner: Arc::new(Mutex::new(ReceivedMessagesBufferInner {
                 messages: Vec::new(),
-                message_reconstructor: MessageReconstructor::new(),
+                message_reconstructor: MessageReconstructor::new(true),
                 message_sender: None,
                 recently_reconstructed: HashSet::new(),
             })),
@@ -111,7 +111,7 @@ impl ReceivedMessagesBuffer {
         mutex_guard: &mut MutexGuard<ReceivedMessagesBufferInner>,
         raw_fragment: Vec<u8>,
     ) -> Option<Vec<u8>> {
-        if raw_fragment == LOOP_COVER_MESSAGE_PAYLOAD {
+        if nymsphinx::cover::is_cover(&raw_fragment) {
             trace!("The message was a loop cover message! Skipping it");
             return None;
         }

common/client-libs/gateway-client/src/packet_router.rs

@@ -17,6 +17,7 @@
 
 use futures::channel::mpsc;
 use log::*;
+use nymsphinx::addressing::nodes::MAX_NODE_ADDRESS_UNPADDED_LEN;
 use nymsphinx::params::packet_sizes::PacketSize;
 
 pub type MixnetMessageSender = mpsc::UnboundedSender<Vec<Vec<u8>>>;
@@ -46,21 +47,33 @@ impl PacketRouter {
         let mut received_messages = Vec::new();
         let mut received_acks = Vec::new();
 
-        for received_packet in unwrapped_packets {
-            // TODO: currently this is not true because gateways are removing padding from the packets
-            // but will be fixed soon enough by all other changes in the pipeline
-            // the question is, however, what exactly will gateways be returning instead. payloads?
-            // 'plaintext'?. To be determined later on.
-            // if received_packet.len() == PacketSize::ACKPacket.payload_size() {
+        // remember: gateway removes final layer of sphinx encryption and from the unwrapped
+        // data he takes the SURB-ACK and first hop address.
+        // currently SURB-ACKs are attached in EVERY packet, even cover, so this is always true
+        let ack_overhead = PacketSize::ACKPacket.size() + MAX_NODE_ADDRESS_UNPADDED_LEN;
 
-            // this is an extremely ugly if statement, but will be improved once things are actually
-            // constant length everywhere
-            if received_packet.len() == 21 {
+        for received_packet in unwrapped_packets {
+            // NOTE TO FUTURE-SELF:
+            // Right now we're kinda cheating to achieve constant length packets
+            // by basically including padding in the message itself
+            // this will eventually be removed in favour of proper encryption.
+            // and I guess some changes in gateways to maybe not remove padding from sphinx packets
+            // themselves? to be determined.
+
+            if received_packet.len() == PacketSize::ACKPacket.plaintext_size() {
                 received_acks.push(received_packet);
+            } else if received_packet.len()
+                == PacketSize::RegularPacket.plaintext_size() - ack_overhead
+            {
+                received_messages.push(received_packet);
+            } else if received_packet.len()
+                == PacketSize::ExtendedPacket.plaintext_size() - ack_overhead
+            {
+                warn!("received extended packet? Did not expect this...");
+                received_messages.push(received_packet);
             } else {
-                // well, technically all 21 bytes packets will be considered acks which is not
-                // entirely true, but for time being let's stick with it until other changes are
-                // introduced
+                // this can happen if other clients are not padding their messages
+                warn!("Received message of unexpected size. Probably from an outdated client... len: {}", received_packet.len());
                 received_messages.push(received_packet);
             }
         }

common/nymsphinx/acknowledgements/src/identifier.rs

@@ -47,10 +47,8 @@ fn random_iv<R: RngCore + CryptoRng>(rng: &mut R) -> AckAes128IV {
 pub fn prepare_identifier<R: RngCore + CryptoRng>(
     rng: &mut R,
     key: &AckAes128Key,
-    marshaled_id: &[u8],
+    marshaled_id: [u8; 5],
 ) -> Vec<u8> {
-    // TODO: should we have some length checks on the id?
-
     let iv = random_iv(rng);
     let mut cipher = Aes128Ctr::new(key, &iv);
     let mut output = marshaled_id.to_vec();
@@ -60,13 +58,11 @@ pub fn prepare_identifier<R: RngCore + CryptoRng>(
     iv.into_iter().chain(output.into_iter()).collect()
 }
 
-pub fn recover_identifier(key: &AckAes128Key, iv_ciphertext: &[u8]) -> Option<Vec<u8>> {
+pub fn recover_identifier(key: &AckAes128Key, iv_ciphertext: &[u8]) -> Option<[u8; 5]> {
     // first few bytes are expected to be the concatenated IV. It must be followed by at least 1 more
     // byte that we wish to recover, but it can be no longer from what we can physically store inside
     // an ack
-    if iv_ciphertext.len() <= Aes128NonceSize::to_usize()
-        || iv_ciphertext.len() > PacketSize::ACKPacket.plaintext_size()
-    {
+    if iv_ciphertext.len() != PacketSize::ACKPacket.plaintext_size() {
         return None;
     }
 
@@ -75,7 +71,9 @@ pub fn recover_identifier(key: &AckAes128Key, iv_ciphertext: &[u8]) -> Option<Ve
     let mut output = iv_ciphertext[Aes128NonceSize::to_usize()..].to_vec();
     cipher.apply_keystream(&mut output);
 
-    Some(output)
+    let mut output_arr = [0u8; 5];
+    output_arr.copy_from_slice(&output);
+    Some(output_arr)
 }
 
 #[cfg(test)]
@@ -88,16 +86,11 @@ mod tests {
         let mut rng = OsRng;
         let key = generate_key(&mut rng);
 
-        let id1 = vec![42]; // single byte case
-        let id2 = vec![1, 2, 3, 4, 5]; // 5byte we expect to use
-        let id3 = vec![42; 8]; // some reasonable upper bound id size we could use later on
-
-        let iv_ciphertext1 = prepare_identifier(&mut rng, &key, &id1);
-        let iv_ciphertext2 = prepare_identifier(&mut rng, &key, &id2);
-        let iv_ciphertext3 = prepare_identifier(&mut rng, &key, &id3);
-
-        assert_eq!(id1, recover_identifier(&key, &iv_ciphertext1).unwrap());
-        assert_eq!(id2, recover_identifier(&key, &iv_ciphertext2).unwrap());
-        assert_eq!(id3, recover_identifier(&key, &iv_ciphertext3).unwrap());
+        let id = [1, 2, 3, 4, 5];
+        let iv_ciphertext = prepare_identifier(&mut rng, &key, id);
+        assert_eq!(
+            id.to_vec(),
+            recover_identifier(&key, &iv_ciphertext).unwrap()
+        );
     }
 }

common/nymsphinx/acknowledgements/src/surb_ack.rs

@@ -45,7 +45,7 @@ impl SURBAck {
         rng: &mut R,
         recipient: &Recipient,
         ack_key: &AckAes128Key,
-        marshaled_fragment_id: &[u8],
+        marshaled_fragment_id: [u8; 5],
         average_delay: time::Duration,
         topology: &T,
     ) -> Result<Self, NymTopologyError>

common/nymsphinx/chunking/src/fragment.rs

@@ -72,20 +72,18 @@ pub struct FragmentIdentifier {
 }
 
 impl FragmentIdentifier {
-    pub fn to_bytes(&self) -> Vec<u8> {
-        self.set_id
-            .to_be_bytes()
-            .iter()
-            .cloned()
-            .chain(std::iter::once(self.fragment_position))
-            .collect()
+    pub fn to_bytes(&self) -> [u8; 5] {
+        let set_id_bytes = self.set_id.to_be_bytes();
+        [
+            set_id_bytes[0],
+            set_id_bytes[1],
+            set_id_bytes[2],
+            set_id_bytes[3],
+            self.fragment_position,
+        ]
     }
 
-    pub fn try_from_bytes(b: &[u8]) -> Result<Self, ChunkingError> {
-        if b.len() != 5 {
-            return Err(ChunkingError::MalformedFragmentIdentifier);
-        }
-
+    pub fn try_from_bytes(b: [u8; 5]) -> Result<Self, ChunkingError> {
         let set_id = i32::from_be_bytes([b[0], b[1], b[2], b[3]]);
         // set_id == 0 is valid for, and only for, COVER_FRAG_ID
         if set_id < 0 {

common/nymsphinx/chunking/src/lib.rs

@@ -12,7 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::fragment::{Fragment, FragmentIdentifier};
+use crate::fragment::{
+    linked_fragment_payload_max_len, unlinked_fragment_payload_max_len, Fragment,
+    FragmentIdentifier,
+};
 use crate::set::split_into_sets;
 use nymsphinx_acknowledgements::identifier::AckAes128Key;
 use nymsphinx_acknowledgements::surb_ack::SURBAck;
@@ -34,6 +37,10 @@ use topology::{NymTopology, NymTopologyError};
 
 // perhaps it might be useful down the line for interaction testing between client,mixes,etc?
 
+// TODO: this module has evolved significantly since the tests were first written
+// they should definitely be revisited.
+// For instance there are not tests for the cases when we are padding the message
+
 pub mod fragment;
 pub mod reconstruction;
 pub mod set;
@@ -88,6 +95,7 @@ pub struct MessageChunker<R: CryptoRng + Rng> {
     ack_recipient: Recipient,
     packet_size: PacketSize,
     reply_surbs: bool,
+    should_pad: bool,
     average_packet_delay_duration: Duration,
     average_ack_delay_duration: Duration,
 }
@@ -95,12 +103,14 @@ pub struct MessageChunker<R: CryptoRng + Rng> {
 impl MessageChunker<DefaultRng> {
     pub fn new(
         ack_recipient: Recipient,
+        should_pad: bool,
         average_packet_delay_duration: Duration,
         average_ack_delay_duration: Duration,
     ) -> Self {
         Self::new_with_rng(
             DEFAULT_RNG,
             ack_recipient,
+            should_pad,
             average_packet_delay_duration,
             average_ack_delay_duration,
         )
@@ -115,7 +125,12 @@ impl MessageChunker<DefaultRng> {
             DestinationAddressBytes::from_bytes(empty_address),
             NodeAddressBytes::from_bytes(empty_address),
         );
-        Self::new(empty_recipient, Default::default(), Default::default())
+        Self::new(
+            empty_recipient,
+            false,
+            Default::default(),
+            Default::default(),
+        )
     }
 }
 
@@ -123,12 +138,14 @@ impl<R: CryptoRng + Rng> MessageChunker<R> {
     pub fn new_with_rng(
         rng: R,
         ack_recipient: Recipient,
+        should_pad: bool,
         average_packet_delay_duration: Duration,
         average_ack_delay_duration: Duration,
     ) -> Self {
         MessageChunker {
             rng,
             ack_recipient,
+            should_pad,
             packet_size: Default::default(),
             reply_surbs: false,
             average_packet_delay_duration,
@@ -219,22 +236,259 @@ impl<R: CryptoRng + Rng> MessageChunker<R> {
             &mut self.rng,
             &self.ack_recipient,
             ack_key,
-            &fragment_id.to_bytes(),
+            fragment_id.to_bytes(),
             self.average_ack_delay_duration,
             topology,
         )
     }
 
+    /// Returns number of fragments the message will be split to as well as number of available
+    /// bytes in the final fragment
+    pub fn number_of_required_fragments(
+        message_len: usize,
+        plaintext_per_fragment: usize,
+    ) -> (usize, usize) {
+        let max_unlinked = unlinked_fragment_payload_max_len(plaintext_per_fragment);
+        let max_linked = linked_fragment_payload_max_len(plaintext_per_fragment);
+
+        match set::total_number_of_sets(message_len, plaintext_per_fragment) {
+            n if n == 1 => {
+                // is if it's a single fragment message
+                if message_len < max_unlinked {
+                    return (1, max_unlinked - message_len);
+                }
+
+                // all fragments will be 'unlinked'
+                let quot = message_len / max_unlinked;
+                let rem = message_len % max_unlinked;
+
+                if rem == 0 {
+                    (quot, 0)
+                } else {
+                    (quot + 1, max_unlinked - rem)
+                }
+            }
+
+            n => {
+                // in first and last set there will be one 'linked' fragment
+                // and two 'linked' fragment in every other set, meaning
+                // there will be 2 * (n - 2) + 2 = 2n - 2 'linked' fragments total
+                // rest will be 'unlinked'
+
+                // we know for sure that all fragments in all but last set are definitely full
+                // (last one has single 'linked' fragment)
+                let without_last = (n - 1) * (u8::max_value() as usize);
+                let linked_fragments_without_last = (2 * n - 2) - 1;
+                let unlinked_fragments_without_last = without_last - linked_fragments_without_last;
+
+                let final_set_message_len = message_len
+                    - linked_fragments_without_last * max_linked
+                    - unlinked_fragments_without_last * max_unlinked;
+
+                // we must be careful with the last set as it might be the case that it only
+                // consists of a single, linked, non-full fragment
+                if final_set_message_len < max_linked {
+                    return (without_last + 1, max_linked - final_set_message_len);
+                } else if final_set_message_len == max_linked {
+                    return (without_last + 1, 0);
+                }
+
+                let remaining_len = final_set_message_len - max_linked;
+
+                let quot = remaining_len / max_unlinked;
+                let rem = remaining_len % max_unlinked;
+
+                if rem == 0 {
+                    (without_last + quot + 1, 0)
+                } else {
+                    (without_last + quot + 2, max_unlinked - rem)
+                }
+            }
+        }
+    }
+
+    /// Takes the entire message and splits it into bytes chunks that will fit into sphinx packets
+    /// after attaching SURB-ACK, such that the payload of the sphinx packet will be fully
+    /// used up.
+    /// After receiving they can be combined using `reconstruction::MessageReconstructor`
+    /// to obtain the original message back.
+    pub fn split_message_to_constant_length_chunks(&mut self, message: &[u8]) -> Vec<Fragment> {
+        let available_plaintext_per_fragment = self.available_plaintext_size();
+
+        // 1 is added as there will always have to be at least a single byte of padding (1) added
+        // to be able to later remove the padding
+        let (_, space_left) =
+            Self::number_of_required_fragments(message.len() + 1, available_plaintext_per_fragment);
+
+        // TODO: this makes copy of all data and so will a fragment chunker,
+        // so a tiny optimization would be to make all
+        // methods using this value, i.e. take Vec<u8> rather than &[u8]
+        let message: Vec<_> = message
+            .iter()
+            .cloned()
+            .chain(std::iter::once(1u8))
+            .chain(std::iter::repeat(0u8).take(space_left))
+            .collect();
+
+        split_into_sets(&mut self.rng, &message, available_plaintext_per_fragment)
+            .into_iter()
+            .flat_map(|fragment_set| fragment_set.into_iter())
+            .collect()
+    }
+
     /// Takes the entire message and splits it into bytes chunks that will fit into sphinx packets
     /// after attaching SURB-ACK.
     /// After receiving they can be combined using `reconstruction::MessageReconstructor`
     /// to obtain the original message back.
     pub fn split_message(&mut self, message: &[u8]) -> Vec<Fragment> {
-        let available_plaintext_per_fragment = self.available_plaintext_size();
+        if self.should_pad {
+            self.split_message_to_constant_length_chunks(message)
+        } else {
+            let available_plaintext_per_fragment = self.available_plaintext_size();
 
-        split_into_sets(&mut self.rng, message, available_plaintext_per_fragment)
-            .into_iter()
-            .flat_map(|fragment_set| fragment_set.into_iter())
-            .collect()
+            split_into_sets(&mut self.rng, &message, available_plaintext_per_fragment)
+                .into_iter()
+                .flat_map(|fragment_set| fragment_set.into_iter())
+                .collect()
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::set::{max_one_way_linked_set_payload_length, two_way_linked_set_payload_length};
+
+    #[test]
+    fn calculating_number_of_required_fragments() {
+        // plaintext len should not affect this at all, but let's test it with something tiny
+        // and reasonable
+        let used_plaintext_len = PacketSize::default().plaintext_size()
+            - PacketSize::ACKPacket.size()
+            - MAX_NODE_ADDRESS_UNPADDED_LEN;
+
+        let plaintext_lens = vec![17, used_plaintext_len, 20, 42, 10000];
+        const SET_LEN: usize = u8::max_value() as usize;
+
+        for plaintext_len in plaintext_lens {
+            let unlinked_len = unlinked_fragment_payload_max_len(plaintext_len);
+            let linked_len = linked_fragment_payload_max_len(plaintext_len);
+            let full_edge_set = max_one_way_linked_set_payload_length(plaintext_len);
+            let full_middle_set = two_way_linked_set_payload_length(plaintext_len);
+
+            let single_non_full_frag_message_len = unlinked_len - 5;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                single_non_full_frag_message_len,
+                plaintext_len,
+            );
+            assert_eq!(frags, 1);
+            assert_eq!(space_left, unlinked_len - single_non_full_frag_message_len);
+
+            let single_full_frag_message_len = unlinked_len;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                single_full_frag_message_len,
+                plaintext_len,
+            );
+            assert_eq!(frags, 1);
+            assert_eq!(space_left, 0);
+
+            let two_non_full_frags_len = unlinked_len + 1;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_non_full_frags_len,
+                plaintext_len,
+            );
+            assert_eq!(frags, 2);
+            assert_eq!(space_left, unlinked_len - 1);
+
+            let two_full_frags_len = 2 * unlinked_len;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_full_frags_len,
+                plaintext_len,
+            );
+            assert_eq!(frags, 2);
+            assert_eq!(space_left, 0);
+
+            let multi_single_set_frags_non_full = unlinked_len * 42 - 5;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                multi_single_set_frags_non_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, 42);
+            assert_eq!(space_left, 5);
+
+            let multi_single_set_frags_full = unlinked_len * 42;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                multi_single_set_frags_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, 42);
+            assert_eq!(space_left, 0);
+
+            let two_set_one_non_full_frag = full_edge_set + linked_len - 1;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_set_one_non_full_frag,
+                plaintext_len,
+            );
+            assert_eq!(frags, SET_LEN + 1);
+            assert_eq!(space_left, 1);
+
+            let two_set_one_full_frag = full_edge_set + linked_len;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_set_one_full_frag,
+                plaintext_len,
+            );
+            assert_eq!(frags, SET_LEN + 1);
+            assert_eq!(space_left, 0);
+
+            let two_set_multi_frags_non_full = full_edge_set + linked_len + unlinked_len * 41 - 5;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_set_multi_frags_non_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, SET_LEN + 42);
+            assert_eq!(space_left, 5);
+
+            let two_set_multi_frags_full = full_edge_set + linked_len + unlinked_len * 41;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                two_set_multi_frags_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, SET_LEN + 42);
+            assert_eq!(space_left, 0);
+
+            let ten_set_one_non_full_frag = full_edge_set + 8 * full_middle_set + linked_len - 1;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                ten_set_one_non_full_frag,
+                plaintext_len,
+            );
+            assert_eq!(frags, 9 * SET_LEN + 1);
+            assert_eq!(space_left, 1);
+
+            let ten_set_one_full_frag = full_edge_set + 8 * full_middle_set + linked_len;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                ten_set_one_full_frag,
+                plaintext_len,
+            );
+            assert_eq!(frags, 9 * SET_LEN + 1);
+            assert_eq!(space_left, 0);
+
+            let ten_set_multi_frags_non_full =
+                full_edge_set + 8 * full_middle_set + linked_len + 41 * unlinked_len - 5;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                ten_set_multi_frags_non_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, 9 * SET_LEN + 42);
+            assert_eq!(space_left, 5);
+
+            let ten_set_multi_frags_full =
+                full_edge_set + 8 * full_middle_set + linked_len + 41 * unlinked_len;
+            let (frags, space_left) = MessageChunker::<DefaultRng>::number_of_required_fragments(
+                ten_set_multi_frags_full,
+                plaintext_len,
+            );
+            assert_eq!(frags, 9 * SET_LEN + 42);
+            assert_eq!(space_left, 0);
+        }
     }
 }

common/nymsphinx/chunking/src/reconstruction.rs

@@ -150,6 +150,7 @@ impl ReconstructionBuffer {
 /// returning original messages that they encapsulate.
 #[derive(Default, PartialEq, Debug, Clone)]
 pub struct MessageReconstructor {
+    expects_padding: bool,
     // TODO: some cleaner thread/routine that if message is incomplete and
     // we haven't received any fragments in X time, we assume they
     // were lost and message can't be restored.
@@ -163,8 +164,11 @@ pub struct MessageReconstructor {
 
 impl MessageReconstructor {
     /// Creates an empty `MessageReconstructor`.
-    pub fn new() -> Self {
-        Default::default()
+    pub fn new(expects_padding: bool) -> Self {
+        MessageReconstructor {
+            expects_padding,
+            reconstructed_sets: HashMap::new(),
+        }
     }
 
     /// Given fully received set of given `id`, if it has any post-linked sets, recursively
@@ -266,13 +270,26 @@ impl MessageReconstructor {
         let set_id_sequence: Vec<_> =
             std::iter::successors(Some(starting_id), |&id| self.next_linked_set_id(id)).collect();
 
-        let message_content = set_id_sequence
+        let message_content: Vec<_> = set_id_sequence
             .iter()
             .map(|&id| self.extract_set_payload(id))
             .flat_map(|payload| payload.into_iter())
             .collect();
 
-        (message_content, set_id_sequence)
+        if self.expects_padding {
+            // remove padding
+            // we are looking for first occurrence of 1 in the tail and we get its index
+            if let Some(i) = message_content.iter().rposition(|b| *b == 1) {
+                // and now we only take bytes until that point (but not including it)
+                let unpadded_content = message_content.into_iter().take(i).collect();
+                (unpadded_content, set_id_sequence)
+            } else {
+                error!("received unpadded message!");
+                (message_content, set_id_sequence)
+            }
+        } else {
+            (message_content, set_id_sequence)
+        }
     }
 
     /// Given recovered `Fragment`, tries to insert it into an appropriate `ReconstructionBuffer`.
@@ -539,7 +556,7 @@ mod message_reconstructor {
     #[should_panic]
     fn checking_front_chain_is_not_allowed_for_incomplete_sets() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -580,7 +597,7 @@ mod message_reconstructor {
     #[should_panic]
     fn checking_back_chain_is_not_allowed_for_incomplete_sets() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message = vec![
             42u8;
@@ -625,7 +642,7 @@ mod message_reconstructor {
     #[test]
     fn checking_front_chain_returns_false_for_complete_set_but_incomplete_message() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -673,7 +690,7 @@ mod message_reconstructor {
     #[test]
     fn checking_back_chain_returns_false_for_complete_set_but_incomplete_message() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -715,7 +732,7 @@ mod message_reconstructor {
     fn checking_front_chain_returns_true_for_if_there_are_no_more_front_sets() {
         // case of 2 sets: [id1 -- id2], where id1 is completed and being checked
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -757,7 +774,7 @@ mod message_reconstructor {
     fn checking_back_chain_returns_true_for_if_there_are_no_more_back_sets() {
         // case of 2 sets: [id1 -- id2], where id2 is completed and being checked
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message = vec![
             42u8;
@@ -798,7 +815,7 @@ mod message_reconstructor {
     fn checking_front_chain_returns_true_for_complete_front_chain() {
         // case of 3 sets: [id1 -- id2 -- id3], where id1 and id2 are completed and id2 is being checked
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -841,7 +858,7 @@ mod message_reconstructor {
     fn checking_back_chain_returns_true_for_complete_back_chain() {
         // case of 3 sets: [id1 -- id2 -- id3], where id2 and id3 are completed and id2 is being checked
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -881,13 +898,13 @@ mod message_reconstructor {
 
     #[test]
     fn checking_if_set_is_fully_received_returns_false_if_no_fragments_were_ever_received() {
-        let reconstructor = MessageReconstructor::new();
+        let reconstructor = MessageReconstructor::default();
         assert!(!reconstructor.is_set_fully_received(12345));
     }
 
     #[test]
     fn checking_if_set_is_fully_received_if_exists_returns_whatever_is_complete_flag_is_set_to() {
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         reconstructor.reconstructed_sets.insert(
             12345,
             ReconstructionBuffer {
@@ -915,7 +932,7 @@ mod message_reconstructor {
     #[test]
     fn finding_starting_set_id_returns_none_if_message_was_not_fully_received() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message1 = vec![
             42u8;
@@ -990,7 +1007,7 @@ mod message_reconstructor {
     #[test]
     fn finding_starting_set_id_returns_expected_starting_id() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -1076,7 +1093,7 @@ mod message_reconstructor {
     #[should_panic]
     fn getting_previous_linked_set_id_is_not_allowed_for_incomplete_sets() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -1109,7 +1126,7 @@ mod message_reconstructor {
 
     #[test]
     fn getting_previous_linked_set_id_returns_id_of_previous_set() {
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         reconstructor.reconstructed_sets.insert(
             12345,
             ReconstructionBuffer {
@@ -1136,7 +1153,7 @@ mod message_reconstructor {
     #[should_panic]
     fn getting_next_linked_set_id_is_not_allowed_for_incomplete_sets() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -1169,7 +1186,7 @@ mod message_reconstructor {
 
     #[test]
     fn getting_next_linked_set_id_returns_id_of_next_set() {
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         reconstructor.reconstructed_sets.insert(
             12345,
             ReconstructionBuffer {
@@ -1196,7 +1213,7 @@ mod message_reconstructor {
     #[should_panic]
     fn extracting_set_payload_is_not_allowed_for_incomplete_sets() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
 
         let message =
             vec![
@@ -1230,7 +1247,7 @@ mod message_reconstructor {
     #[test]
     fn extracting_set_payload_is_returns_entire_set_data() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         let mut set_buf = ReconstructionBuffer::new(3);
         let mut rng = thread_rng();
 
@@ -1265,7 +1282,7 @@ mod message_reconstructor {
     fn reconstructing_message_for_single_set_is_equivalent_to_extracting_set_payload() {
         // we're inserting this via the buffer approach as not to trigger immediate re-assembly
         let mut message_chunker = MessageChunker::test_fixture();
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         let mut set_buf = ReconstructionBuffer::new(3);
         let mut rng = thread_rng();
 
@@ -1304,7 +1321,7 @@ mod message_reconstructor {
         let mut message_chunker = MessageChunker::test_fixture();
 
         // we're inserting this via the buffer approach as not to trigger immediate re-assembly
-        let mut reconstructor = MessageReconstructor::new();
+        let mut reconstructor = MessageReconstructor::default();
         let mut set_buf1 = ReconstructionBuffer::new(u8::max_value());
         let mut set_buf2 = ReconstructionBuffer::new(1);
 
@@ -1358,13 +1375,13 @@ mod message_reconstructor {
     #[test]
     fn adding_invalid_fragment_does_not_change_reconstructor_state() {
         let mut message_chunker = MessageChunker::test_fixture();
-        let empty_reconstructor = MessageReconstructor::new();
+        let empty_reconstructor = MessageReconstructor::default();
         assert!(empty_reconstructor
             .recover_fragment([24u8; 43].to_vec())
             .is_err());
-        assert_eq!(empty_reconstructor, MessageReconstructor::new());
+        assert_eq!(empty_reconstructor, MessageReconstructor::default());
 
-        let mut reconstructor_with_data = MessageReconstructor::new();
+        let mut reconstructor_with_data = MessageReconstructor::default();
         let dummy_message =
             vec![
                 24u8;
@@ -1422,7 +1439,7 @@ mod message_reconstruction {
                 .collect();
             assert_eq!(fragment.len(), 1);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             let reconstructed_message = message_reconstructor
                 .insert_new_fragment(
                     message_reconstructor
@@ -1454,7 +1471,7 @@ mod message_reconstruction {
                 .collect();
             assert_eq!(fragment.len(), 1);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             let reconstructed_message = message_reconstructor
                 .insert_new_fragment(
                     message_reconstructor
@@ -1487,7 +1504,7 @@ mod message_reconstruction {
                 .collect();
             assert_eq!(fragments.len(), 2);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             assert!(message_reconstructor
                 .insert_new_fragment(
                     message_reconstructor
@@ -1528,7 +1545,7 @@ mod message_reconstruction {
                 .collect();
             assert_eq!(fragments.len(), 2);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             assert!(message_reconstructor
                 .insert_new_fragment(
                     message_reconstructor
@@ -1569,7 +1586,7 @@ mod message_reconstruction {
                 .collect();
             assert_eq!(fragments.len(), 30);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             for i in 0..29 {
                 assert!(message_reconstructor
                     .insert_new_fragment(
@@ -1615,7 +1632,7 @@ mod message_reconstruction {
             // shuffle the fragments
             fragments.shuffle(&mut rng);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             for i in 0..29 {
                 assert!(message_reconstructor
                     .insert_new_fragment(
@@ -1672,7 +1689,7 @@ mod message_reconstruction {
             let fragments = fragments1;
             assert_eq!(fragments.len(), 60);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             for fragment in fragments {
                 if let Some(reconstructed_msg) = message_reconstructor.insert_new_fragment(
                     message_reconstructor
@@ -1721,7 +1738,7 @@ mod message_reconstruction {
             let fragments = fragments1;
             assert_eq!(fragments.len(), (u8::max_value() as usize) * 2);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             for fragment in fragments.into_iter() {
                 if let Some(reconstructed_msg) = message_reconstructor.insert_new_fragment(
                     message_reconstructor
@@ -1767,7 +1784,7 @@ mod message_reconstruction {
             // shuffle the fragments
             fragments.shuffle(&mut rng);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             let mut finished_reconstruction = false;
             for fragment in fragments.into_iter() {
                 if finished_reconstruction {
@@ -1808,7 +1825,7 @@ mod message_reconstruction {
             // shuffle the fragments
             fragments.shuffle(&mut rng);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             let mut finished_reconstruction = false;
             for fragment in fragments.into_iter() {
                 if finished_reconstruction {
@@ -1850,7 +1867,7 @@ mod message_reconstruction {
             // shuffle the fragments
             fragments.shuffle(&mut rng);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             let mut finished_reconstruction = false;
             for fragment in fragments.into_iter() {
                 if finished_reconstruction {
@@ -1906,7 +1923,7 @@ mod message_reconstruction {
             let fragments = fragments1;
             assert_eq!(fragments.len(), (u8::max_value() as usize) * 8);
 
-            let mut message_reconstructor = MessageReconstructor::new();
+            let mut message_reconstructor = MessageReconstructor::default();
             for fragment in fragments.into_iter() {
                 if let Some(msg) = message_reconstructor.insert_new_fragment(
                     message_reconstructor

common/nymsphinx/chunking/src/set.rs

@@ -200,7 +200,7 @@ fn prepare_linked_fragment_set(
 }
 
 /// Based on total message length, determines the number of sets into which it is going to be split.
-fn total_number_of_sets(message_len: usize, max_plaintext_size: usize) -> usize {
+pub(crate) fn total_number_of_sets(message_len: usize, max_plaintext_size: usize) -> usize {
     if message_len <= max_unlinked_set_payload_length(max_plaintext_size) {
         1
     } else if message_len > max_unlinked_set_payload_length(max_plaintext_size)

common/nymsphinx/cover/src/lib.rs

@@ -70,7 +70,7 @@ where
         rng,
         full_address,
         ack_key,
-        &COVER_FRAG_ID.to_bytes(),
+        COVER_FRAG_ID.to_bytes(),
         average_ack_delay,
         topology,
     )?)
@@ -93,9 +93,16 @@ where
         generate_loop_cover_surb_ack(rng, topology, ack_key, full_address, average_ack_delay)?
             .prepare_for_sending();
 
+    let plaintext_size = PacketSize::default().plaintext_size();
+
     let cover_payload: Vec<_> = ack_bytes
         .into_iter()
         .chain(LOOP_COVER_MESSAGE_PAYLOAD.into_iter().cloned())
+        // let's be lazy about it (temporarily! because cover messages will need to be encrypted)
+        // TODO: to remember: encrypt cover messages
+        .chain(std::iter::once(1))
+        .chain(std::iter::repeat(0))
+        .take(plaintext_size)
         .collect();
 
     let route = topology.random_route_to_gateway(&full_address.gateway())?;
@@ -114,3 +121,64 @@ where
 
     Ok((first_hop_address.into(), packet))
 }
+
+/// Helper function used to determine if given message represents a loop cover message.
+// It kinda seems like there must exist "prefix" or "starts_with" method for bytes
+// or something, but I couldn't find anything
+pub fn is_cover(data: &[u8]) -> bool {
+    if data.len() < LOOP_COVER_MESSAGE_PAYLOAD.len() {
+        return false;
+    }
+
+    for i in 0..LOOP_COVER_MESSAGE_PAYLOAD.len() {
+        if data[i] != LOOP_COVER_MESSAGE_PAYLOAD[i] {
+            return false;
+        }
+    }
+
+    true
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn is_cover_works_for_identical_input() {
+        assert!(is_cover(&LOOP_COVER_MESSAGE_PAYLOAD))
+    }
+
+    #[test]
+    fn is_cover_works_for_longer_input() {
+        let input: Vec<_> = LOOP_COVER_MESSAGE_PAYLOAD
+            .iter()
+            .cloned()
+            .chain(std::iter::repeat(42).take(100))
+            .collect();
+        assert!(is_cover(&input))
+    }
+
+    #[test]
+    fn is_cover_returns_false_for_unrelated_input() {
+        // make sure the length checks out
+        let input: Vec<_> = LOOP_COVER_MESSAGE_PAYLOAD.iter().map(|_| 42).collect();
+        assert!(!is_cover(&input))
+    }
+
+    #[test]
+    fn is_cover_returns_false_for_part_of_correct_input() {
+        let input: Vec<_> = LOOP_COVER_MESSAGE_PAYLOAD
+            .iter()
+            .cloned()
+            .take(LOOP_COVER_MESSAGE_PAYLOAD.len() - 1)
+            .chain(std::iter::once(42))
+            .collect();
+        assert!(!is_cover(&input))
+    }
+
+    #[test]
+    fn is_cover_returns_false_for_empty_input() {
+        let empty = Vec::new();
+        assert!(!is_cover(&empty))
+    }
+}

common/nymsphinx/params/src/packet_sizes.rs

@@ -18,7 +18,7 @@ use std::convert::TryFrom;
 
 // it's up to the smart people to figure those values out : )
 const REGULAR_PACKET_SIZE: usize = HEADER_SIZE + PAYLOAD_OVERHEAD_SIZE + 2 * 1024;
-const ACK_PACKET_SIZE: usize = HEADER_SIZE + PAYLOAD_OVERHEAD_SIZE + 24;
+const ACK_PACKET_SIZE: usize = HEADER_SIZE + PAYLOAD_OVERHEAD_SIZE + 21; // 16B IV + 5B ID
 const EXTENDED_PACKET_SIZE: usize = HEADER_SIZE + PAYLOAD_OVERHEAD_SIZE + 32 * 1024;
 
 pub struct InvalidPacketSize;

gateway/src/node/mixnet_handling/receiver/packet_processing.rs

@@ -171,7 +171,7 @@ impl PacketProcessor {
 
         // JS: I think this would never get called anyway, because if loop cover messages are sent
         // it means client is online and hence all his messages should be pushed directly to him?
-        if message == LOOP_COVER_MESSAGE_PAYLOAD {
+        if nymsphinx::cover::is_cover(&message) {
             debug!("Received a loop cover message - not going to store it");
             return Ok(());
         }