Merge #547

547: Basic rand infrastructure. r=Dirbaio a=Dirbaio

See [previous discussion](https://github.com/smoltcp-rs/smoltcp/pull/465#pullrequestreview-774487285). Opening a separate PR so it can be discussed separately.

- Add `smoltcp::rand`.
  - On `std` targets, `OsRng` is used by default.
  - The user can supply a custom impl by enabling the `rand-custom-impl` Cargo feature and using the `rand_custom_impl!()` macro.
  - Specifying a custom impl is mandatory when `std` is not enabled.
- Make TCP initial sequence numbers actually random.


Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

.github/workflows/test.yml

@@ -48,7 +48,7 @@ jobs:
         include:
           # Test alloc feature which requires nightly.
           - rust: nightly
-            features: alloc medium-ethernet proto-ipv4 proto-ipv6 socket-raw socket-udp socket-tcp socket-icmp
+            features: alloc rand-custom-impl medium-ethernet proto-ipv4 proto-ipv6 socket-raw socket-udp socket-tcp socket-icmp
     steps:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
@@ -73,8 +73,8 @@ jobs:
 
         features:
           # These feature sets cannot run tests, so we only check they build.
-          - medium-ip medium-ethernet proto-ipv6 proto-ipv6 proto-igmp proto-dhcpv4 socket-raw socket-udp socket-tcp socket-icmp async
-          - defmt defmt-trace medium-ip medium-ethernet proto-ipv6 proto-ipv6 proto-igmp proto-dhcpv4 socket-raw socket-udp socket-tcp socket-icmp async
+          - rand-custom-impl medium-ip medium-ethernet proto-ipv6 proto-ipv6 proto-igmp proto-dhcpv4 socket-raw socket-udp socket-tcp socket-icmp async
+          - rand-custom-impl defmt defmt-trace medium-ip medium-ethernet proto-ipv6 proto-ipv6 proto-igmp proto-dhcpv4 socket-raw socket-udp socket-tcp socket-icmp async
 
     steps:
       - uses: actions/checkout@v2

Cargo.toml

@@ -22,6 +22,7 @@ log = { version = "0.4.4", default-features = false, optional = true }
 libc = { version = "0.2.18", optional = true }
 bitflags = { version = "1.0", default-features = false }
 defmt = { version = "0.2.0", optional = true }
+rand_core = { version = "0.6.3", optional = true, default-features = false }
 
 [dev-dependencies]
 env_logger = "0.5"
@@ -30,9 +31,10 @@ rand = "0.3"
 url = "1.0"
 
 [features]
-std = ["managed/std"]
+std = ["managed/std", "rand_core/std"]
 alloc = ["managed/alloc"]
 verbose = []
+rand-custom-impl = []
 "medium-ethernet" = ["socket"]
 "medium-ip" = ["socket"]
 "phy-raw_socket" = ["std", "libc", "medium-ethernet"]

examples/loopback.rs

@@ -37,6 +37,14 @@ mod mock {
             self.0.get()
         }
     }
+
+    struct Rand;
+    smoltcp::rand_custom_impl!(Rand);
+    impl smoltcp::Rand for Rand {
+        fn rand_bytes(buf: &mut [u8]) {
+            buf.fill(0x42);
+        }
+    }
 }
 
 #[cfg(feature = "std")]

src/lib.rs

@@ -126,6 +126,10 @@ use core::fmt;
 mod macros;
 mod parsers;
 
+mod rand;
+#[cfg(feature = "rand-custom-impl")]
+pub use crate::rand::Rand;
+
 pub mod iface;
 pub mod phy;
 #[cfg(feature = "socket")]

src/rand.rs

@@ -0,0 +1,67 @@
+#![allow(unsafe_code)]
+#![allow(unused)]
+
+#[cfg(not(any(test, feature = "std", feature = "rand-custom-impl")))]
+compile_error!("None of the Cargo features `std` or `rand-custom-impl` is enabled. smoltcp needs a `rand` implementation to work. If your target supports `std`, enable the `std` feature to use the OS's RNG. Otherwise, you must enable the `rand-custom-impl` Cargo feature, and supply your own custom implementation using the `smoltcp::rand_custom_impl!()` macro");
+
+pub fn rand_u32() -> u32 {
+    let mut val = [0; 4];
+    rand_bytes(&mut val);
+    u32::from_ne_bytes(val)
+}
+
+/// Fill `buf` with random bytes.
+pub fn rand_bytes(buf: &mut [u8]) {
+    extern "Rust" {
+        fn _smoltcp_rand(buf: &mut [u8]);
+    }
+
+    unsafe { _smoltcp_rand(buf) }
+}
+
+/// Methods required for a custom rand implementation.
+///
+/// This trait is not intended to be used directly, just to supply a custom rand implementation to smoltcp.
+#[cfg(feature = "rand-custom-impl")]
+pub trait Rand {
+    /// Fill `buf` with random bytes.
+    fn rand_bytes(buf: &mut [u8]);
+}
+
+/// Set the custom rand implementation.
+///
+/// # Example
+///
+/// ```
+/// struct Rand;
+/// smoltcp::rand_custom_impl!(Rand);
+/// impl smoltcp::Rand for Rand {
+///     fn rand_bytes(buf: &mut [u8]) {
+///         // TODO
+///     }
+/// }
+///
+#[macro_export]
+#[cfg(feature = "rand-custom-impl")]
+macro_rules! rand_custom_impl {
+    ($t: ty) => {
+        #[no_mangle]
+        fn _smoltcp_rand(buf: &mut [u8]) {
+            <$t as $crate::Rand>::rand_bytes(buf)
+        }
+    };
+}
+
+#[cfg(all(feature = "std", not(feature = "rand-custom-impl"), not(test)))]
+#[no_mangle]
+fn _smoltcp_rand(buf: &mut [u8]) {
+    use rand_core::RngCore;
+
+    rand_core::OsRng.fill_bytes(buf)
+}
+
+#[cfg(test)]
+#[no_mangle]
+fn _smoltcp_rand(buf: &mut [u8]) {
+    panic!("Rand should not be used when testing");
+}

src/socket/tcp.rs

@@ -56,11 +56,6 @@ impl fmt::Display for State {
     }
 }
 
-/// Initial sequence number. This used to be 0, but some servers don't behave correctly
-/// with that, so we use a non-zero starting sequence number. TODO: randomize instead.
-/// https://github.com/smoltcp-rs/smoltcp/issues/489
-const INITIAL_SEQ_NO: TcpSeqNumber = TcpSeqNumber(42);
-
 // Conservative initial RTT estimate.
 const RTTE_INITIAL_RTT: u32 = 300;
 const RTTE_INITIAL_DEV: u32 = 100;
@@ -430,7 +425,7 @@ impl<'a> TcpSocket<'a> {
             listen_address: IpAddress::default(),
             local_endpoint: IpEndpoint::default(),
             remote_endpoint: IpEndpoint::default(),
-            local_seq_no: INITIAL_SEQ_NO,
+            local_seq_no: TcpSeqNumber::default(),
             remote_seq_no: TcpSeqNumber::default(),
             remote_last_seq: TcpSeqNumber::default(),
             remote_last_ack: None,
@@ -658,7 +653,7 @@ impl<'a> TcpSocket<'a> {
         self.listen_address = IpAddress::default();
         self.local_endpoint = IpEndpoint::default();
         self.remote_endpoint = IpEndpoint::default();
-        self.local_seq_no = INITIAL_SEQ_NO;
+        self.local_seq_no = TcpSeqNumber::default();
         self.remote_seq_no = TcpSeqNumber::default();
         self.remote_last_seq = TcpSeqNumber::default();
         self.remote_last_ack = None;
@@ -751,18 +746,24 @@ impl<'a> TcpSocket<'a> {
             ..local_endpoint
         };
 
-        // Carry over the local sequence number.
-        let local_seq_no = self.local_seq_no;
-
         self.reset();
         self.local_endpoint = local_endpoint;
         self.remote_endpoint = remote_endpoint;
-        self.local_seq_no = local_seq_no;
-        self.remote_last_seq = local_seq_no;
         self.set_state(State::SynSent);
+
+        let seq = Self::random_seq_no();
+        self.local_seq_no = seq;
+        self.remote_last_seq = seq;
         Ok(())
     }
 
+    fn random_seq_no() -> TcpSeqNumber {
+        #[cfg(test)]
+        return TcpSeqNumber(10000);
+        #[cfg(not(test))]
+        return TcpSeqNumber(crate::rand::rand_u32() as i32);
+    }
+
     /// Close the transmit half of the full-duplex connection.
     ///
     /// Note that there is no corresponding function for the receive half of the full-duplex
@@ -1575,8 +1576,7 @@ impl<'a> TcpSocket<'a> {
 
                 self.local_endpoint = IpEndpoint::new(ip_repr.dst_addr(), repr.dst_port);
                 self.remote_endpoint = IpEndpoint::new(ip_repr.src_addr(), repr.src_port);
-                // FIXME: use something more secure here
-                self.local_seq_no = TcpSeqNumber(!repr.seq_number.0);
+                self.local_seq_no = Self::random_seq_no();
                 self.remote_seq_no = repr.seq_number + 1;
                 self.remote_last_seq = self.local_seq_no;
                 self.remote_has_sack = repr.sack_permitted;