Re-implement most of what Monica originally did, but with some variations:

- Went back to a single session counter instead of two counter states
 - Went to a full 64-bit counter in the header as recommended by Noise, turns
   out there is a good reason. It simplifies everything.
 - Implemented Monica's simpler stateless counter window algorithm, but
   also only one on the whole session.
 - Simplified some counter logic generally.
 - Header check codes are temporarily gone, coming back in a different form.

This is being committed "on top" of what was there instead of reverting the old
commits to preserve the history.

zssp/src/applicationlayer.rs

@@ -64,19 +64,9 @@ pub trait ApplicationLayer: Sized {
 
     /// Check whether a new session should be accepted.
     ///
-    /// On success a tuple of local session ID, psk, and associated object is returned.
-    /// Set psk to all zeros if one is not in use with the remote party.
-    ///
-    /// When `accept_new_session` is called, `remote_static_public` and `remote_metadata` have not yet been
-    /// authenticated. As such avoid mutating state until OkNewSession(Session) is returned, as the connection
-    /// may be adversarial.
-    ///
-    /// When `remote_static_public` and `remote_metadata` are eventually authenticated, the zssp protocol cannot
-    /// guarantee that they are unique, i.e. `remote_static_public` and `remote_metadata` may be duplicates from
-    /// an old attempt to establish a session, and may even have been replayed by an adversary. If your use-case
-    /// needs uniqueness for reliability or security, consider either including a timestamp in the metadata, or
-    /// sending the metadata as an extra transport packet after the session is fully established.
-    /// It is guaranteed they will be unique for at least the lifetime of the returned session.
+    /// On success a tuple of local session ID, static secret, and associated object is returned. The
+    /// static secret is whatever results from agreement between the local and remote static public
+    /// keys.
     fn accept_new_session(
         &self,
         receive_context: &ReceiveContext<Self>,

zssp/src/constants.rs

@@ -27,20 +27,15 @@ pub(crate) const MAX_FRAGMENTS: usize = 48; // hard protocol max: 63
 pub(crate) const KEY_EXCHANGE_MAX_FRAGMENTS: usize = 2; // enough room for p384 + ZT identity + kyber1024 + tag/hmac/etc.
 
 /// Start attempting to rekey after a key has been used to send packets this many times.
-///
-/// This is 1/4 the NIST recommended maximum and 1/8 the absolute limit where u32 wraps.
-/// As such it should leave plenty of margin against nearing key reuse bounds w/AES-GCM.
+/// This is 1/4 the recommended NIST limit for AES-GCM key lifetimes under most conditions.
 pub(crate) const REKEY_AFTER_USES: u64 = 536870912;
 
-/// Maximum random jitter to add to rekey-after usage count.
-pub(crate) const REKEY_AFTER_USES_MAX_JITTER: u32 = 1048576;
-
 /// Hard expiration after this many uses.
 ///
 /// Use of the key beyond this point is prohibited. If we reach this number of key uses
 /// the key will be destroyed in memory and the session will cease to function. A hard
 /// error is also generated.
-pub(crate) const EXPIRE_AFTER_USES: u64 = (u32::MAX - 1024) as u64;
+pub(crate) const EXPIRE_AFTER_USES: u64 = REKEY_AFTER_USES * 2;
 
 /// Start attempting to rekey after a key has been in use for this many milliseconds.
 pub(crate) const REKEY_AFTER_TIME_MS: i64 = 1000 * 60 * 60; // 1 hour
@@ -75,12 +70,13 @@ pub(crate) const HMAC_SIZE: usize = 48;
 pub(crate) const SESSION_ID_SIZE: usize = 6;
 
 /// Maximum difference between out-of-order incoming packet counters, and size of deduplication buffer.
-pub(crate) const COUNTER_MAX_ALLOWED_OOO: usize = 16;
+pub(crate) const COUNTER_MAX_DELTA: usize = 16;
 
 // Packet types can range from 0 to 15 (4 bits) -- 0-3 are defined and 4-15 are reserved for future use
 pub(crate) const PACKET_TYPE_DATA: u8 = 0;
-pub(crate) const PACKET_TYPE_INITIAL_KEY_OFFER: u8 = 1; // "alice"
-pub(crate) const PACKET_TYPE_KEY_COUNTER_OFFER: u8 = 2; // "bob"
+pub(crate) const PACKET_TYPE_NOP: u8 = 1;
+pub(crate) const PACKET_TYPE_INITIAL_KEY_OFFER: u8 = 2; // "alice"
+pub(crate) const PACKET_TYPE_KEY_COUNTER_OFFER: u8 = 3; // "bob"
 
 // Key usage labels for sub-key derivation using NIST-style KBKDF (basically just HMAC KDF).
 pub(crate) const KBKDF_KEY_USAGE_LABEL_HMAC: u8 = b'M'; // HMAC-SHA384 authentication for key exchanges

zssp/src/counter.rs

@@ -1,88 +0,0 @@
-use std::sync::atomic::{Ordering, AtomicU32};
-
-use crate::constants::COUNTER_MAX_ALLOWED_OOO;
-
-/// Outgoing packet counter with strictly ordered atomic semantics.
-/// Count sequence always starts at 1u32, it must never be allowed to overflow
-///
-#[repr(transparent)]
-pub(crate) struct Counter(AtomicU32);
-
-impl Counter {
-    #[inline(always)]
-    pub fn new() -> Self {
-        // Using a random value has no security implication. Zero would be fine. This just
-        // helps randomize packet contents a bit.
-        Self(AtomicU32::new(1u32))
-    }
-
-    #[inline(always)]
-    pub fn reset_for_new_key_offer(&self) {
-        self.0.store(1u32, Ordering::SeqCst);
-    }
-
-    /// Get the value most recently used to send a packet.
-    #[inline(always)]
-    pub fn current(&self) -> CounterValue {
-        CounterValue(self.0.load(Ordering::SeqCst))
-    }
-
-    /// Get a counter value for the next packet being sent.
-    #[inline(always)]
-    pub fn next(&self) -> CounterValue {
-        CounterValue(self.0.fetch_add(1, Ordering::SeqCst))
-    }
-}
-
-/// A value of the outgoing packet counter.
-#[repr(transparent)]
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
-pub(crate) struct CounterValue(u32);
-
-impl CounterValue {
-    /// Get the 32-bit counter value used to build packets.
-    #[inline(always)]
-    pub fn to_u32(&self) -> u32 {
-        self.0 as u32
-    }
-}
-
-/// Incoming packet deduplication and replay protection window.
-pub(crate) struct CounterWindow([AtomicU32; COUNTER_MAX_ALLOWED_OOO]);
-
-impl CounterWindow {
-    #[inline(always)]
-    pub fn new() -> Self {
-        Self(std::array::from_fn(|_| AtomicU32::new(0)))
-    }
-    ///this creates a counter window that rejects everything
-    pub fn new_invalid() -> Self {
-            Self(std::array::from_fn(|_| AtomicU32::new(u32::MAX)))
-    }
-    pub fn reset_for_new_key_offer(&self) {
-        for i in 0..COUNTER_MAX_ALLOWED_OOO {
-            self.0[i].store(0, Ordering::SeqCst)
-        }
-    }
-    pub fn invalidate(&self) {
-        for i in 0..COUNTER_MAX_ALLOWED_OOO {
-            self.0[i].store(u32::MAX, Ordering::SeqCst)
-        }
-    }
-
-    #[inline(always)]
-    pub fn message_received(&self, received_counter_value: u32) -> bool {
-        let idx = (received_counter_value % COUNTER_MAX_ALLOWED_OOO as u32) as usize;
-        //it is highly likely this can be a Relaxed ordering, but I want someone else to confirm that is the case
-        let pre = self.0[idx].load(Ordering::SeqCst);
-        return pre < received_counter_value;
-    }
-
-    #[inline(always)]
-    pub fn message_authenticated(&self, received_counter_value: u32) -> bool {
-        //if a valid message is received but one of its fragments was lost, it can technically be replayed. However since the message is incomplete, we know it still exists in the gather array, so the gather array will deduplicate the replayed message. Even if the gather array gets flushed, that flush still effectively deduplicates the replayed message.
-        //eventually the counter of that kind of message will be too OOO to be accepted anymore so it can't be used to DOS.
-        let idx = (received_counter_value % COUNTER_MAX_ALLOWED_OOO as u32) as usize;
-        return self.0[idx].fetch_max(received_counter_value, Ordering::SeqCst) < received_counter_value;
-    }
-}

zssp/src/error.rs

@@ -1,43 +1,39 @@
 use crate::sessionid::SessionId;
 
 pub enum Error {
-    /// The packet was addressed to an unrecognized local session (should usually be ignored).
+    /// The packet was addressed to an unrecognized local session (should usually be ignored)
     UnknownLocalSessionId(SessionId),
 
-    /// Packet was not well formed.
+    /// Packet was not well formed
     InvalidPacket,
 
-    /// An invalid parameter was supplied to the function.
+    /// An invalid parameter was supplied to the function
     InvalidParameter,
 
-    /// Packet failed one or more authentication (MAC) checks.
-    ///
-    /// **IMPORTANT**: Do not reply to a peer who has sent a packet that has failed authentication.
-    /// Any response at all will leak to an attacker what authentication step their packet failed at
-    /// (timing attack), which lowers the total authentication entropy they have to brute force.
-    /// There is a safe way to reply if absolutely necessary; by sending the reply back after a constant
-    /// amount of time, but this is very difficult to get correct.
+    /// Packet failed one or more authentication (MAC) checks
+    /// IMPORTANT: Do not reply to a peer who has sent a packet that has failed authentication. Any response at all will leak to an attacker what authentication step their packet failed at (timing attack), which lowers the total authentication entropy they have to brute force.
+    /// There is a safe way to reply if absolutely necessary, by sending the reply back after a constant amount of time, but this is difficult to get correct.
     FailedAuthentication,
 
     /// New session was rejected by the application layer.
     NewSessionRejected,
 
-    /// Rekeying failed and session secret has reached its hard usage count limit.
+    /// Rekeying failed and session secret has reached its hard usage count limit
     MaxKeyLifetimeExceeded,
 
-    /// Attempt to send using session without established key.
+    /// Attempt to send using session without established key
     SessionNotEstablished,
 
     /// Packet ignored by rate limiter.
     RateLimited,
 
-    /// The other peer specified an unrecognized protocol version.
+    /// The other peer specified an unrecognized protocol version
     UnknownProtocolVersion,
 
-    /// Caller supplied data buffer is too small to receive data.
+    /// Caller supplied data buffer is too small to receive data
     DataBufferTooSmall,
 
-    /// Data object is too large to send, even with fragmentation.
+    /// Data object is too large to send, even with fragmentation
     DataTooLarge,
 
     /// An unexpected buffer overrun occured while attempting to encode or decode a packet.

zssp/src/lib.rs

@@ -1,5 +1,4 @@
 mod applicationlayer;
-mod counter;
 mod error;
 mod sessionid;
 mod tests;

zssp/src/tests.rs

@@ -9,7 +9,6 @@ mod tests {
     use zerotier_crypto::secret::Secret;
     use zerotier_utils::hex;
 
-    use crate::counter::CounterWindow;
     use crate::*;
     use constants::*;
 
@@ -17,7 +16,7 @@ mod tests {
         local_s: P384KeyPair,
         local_s_hash: [u8; 48],
         psk: Secret<64>,
-        session: Mutex<Option<Arc<Session<TestHost>>>>,
+        session: Mutex<Option<Arc<Session<Box<TestHost>>>>>,
         session_id_counter: Mutex<u64>,
         queue: Mutex<LinkedList<Vec<u8>>>,
         key_id: Mutex<[u8; 16]>,
@@ -43,9 +42,9 @@ mod tests {
         }
     }
 
-    impl ApplicationLayer for TestHost {
+    impl ApplicationLayer for Box<TestHost> {
         type Data = u32;
-        type SessionRef<'a> = Arc<Session<TestHost>>;
+        type SessionRef<'a> = Arc<Session<Box<TestHost>>>;
         type IncomingPacketBuffer = Vec<u8>;
         type RemoteAddress = u32;
 
@@ -98,10 +97,10 @@ mod tests {
         let mut psk: Secret<64> = Secret::default();
         random::fill_bytes_secure(&mut psk.0);
 
-        let alice_host = TestHost::new(psk.clone(), "alice", "bob");
-        let bob_host = TestHost::new(psk.clone(), "bob", "alice");
-        let alice_rc: ReceiveContext<TestHost> = ReceiveContext::new(&alice_host);
-        let bob_rc: ReceiveContext<TestHost> = ReceiveContext::new(&bob_host);
+        let alice_host = Box::new(TestHost::new(psk.clone(), "alice", "bob"));
+        let bob_host = Box::new(TestHost::new(psk.clone(), "bob", "alice"));
+        let alice_rc: Box<ReceiveContext<Box<TestHost>>> = Box::new(ReceiveContext::new(&alice_host));
+        let bob_rc: Box<ReceiveContext<Box<TestHost>>> = Box::new(ReceiveContext::new(&bob_host));
 
         //println!("zssp: size of session (bytes): {}", std::mem::size_of::<Session<Box<TestHost>>>());
 
@@ -195,8 +194,8 @@ mod tests {
                                     "zssp: new key at {}: fingerprint {} ratchet {} kyber {}",
                                     host.this_name,
                                     hex::to_string(key_id.as_ref()),
-                                    security_info.2,
-                                    security_info.3
+                                    security_info.1,
+                                    security_info.2
                                 );
                             }
                         }
@@ -209,7 +208,7 @@ mod tests {
                                 )
                                 .is_ok());
                         }
-                        if (test_loop % 8) == 0 && test_loop >= 8 {
+                        if (test_loop % 8) == 0 && test_loop >= 8 && host.this_name.eq("alice") {
                             session.service(host, send_to_other, &[], mtu_buffer.len(), test_loop as i64, true);
                         }
                     }
@@ -217,54 +216,4 @@ mod tests {
             }
         }
     }
-
-
-    #[inline(always)]
-    pub fn xorshift64(x: &mut u64) -> u32 {
-        *x ^= x.wrapping_shl(13);
-        *x ^= x.wrapping_shr(7);
-        *x ^= x.wrapping_shl(17);
-        *x as u32
-    }
-    #[test]
-    fn counter_window() {
-        let mut rng = 844632;
-        let mut counter = 1u32;
-        let mut history = Vec::new();
-
-        let w = CounterWindow::new();
-        for _i in 0..1000000 {
-            let p = xorshift64(&mut rng) as f32/(u32::MAX as f32 + 1.0);
-            let c;
-            if p < 0.5 {
-                let r = xorshift64(&mut rng);
-                c = counter + (r%(COUNTER_MAX_ALLOWED_OOO - 1) as u32 + 1);
-            } else if p < 0.8 {
-                counter = counter + (1);
-                c = counter;
-            } else if p < 0.9 {
-                if history.len() > 0 {
-                    let idx = xorshift64(&mut rng) as usize%history.len();
-                    let c = history[idx];
-                    assert!(!w.message_authenticated(c));
-                }
-                continue;
-            } else if p < 0.999 {
-                c = xorshift64(&mut rng);
-                w.message_received(c);
-                continue;
-            } else {
-                w.reset_for_new_key_offer();
-                counter = 1u32;
-                history = Vec::new();
-                continue;
-            }
-            if history.contains(&c) {
-                assert!(!w.message_authenticated(c));
-            } else {
-                assert!(w.message_authenticated(c));
-                history.push(c);
-            }
-        }
-    }
 }