NetFlow harderning: RNG, config limit of DATA cached in SYN_RCVD state, configurable listen backlog, corner-case bug fix x 2. / GitHub CI tweaks. / Comment and/or doc changes.

.github/workflows/main.yml

@@ -53,7 +53,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout VERSION file and tag history
-        uses: actions/checkout@v4
+        uses: actions/checkout@v6
         with:
           sparse-checkout: VERSION
           fetch-depth: 0 # Get all history regarding tags.  We'll need that for VERSION checking below.
@@ -474,7 +474,7 @@ jobs:
       run: pip install 'conan<2'
 
     - name: Checkout `flow` repository
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
 
     - name: Add custom settings for Conan packages
       if: |
@@ -673,7 +673,7 @@ jobs:
     - name: Upload test/demo logs (please inspect if failure(s) seen above)
       if: |
         always()
-      uses: actions/upload-artifact@v4
+      uses: actions/upload-artifact@v7
       with:
         name: flow-test-logs-${{ matrix.compiler.id }}-${{ matrix.build-test-cfg.id }}-${{ matrix.cxx-std.id }}
         path: ${{ env.install-dir }}/bin/logs.tgz
@@ -713,7 +713,7 @@ jobs:
       run: pip install 'conan<2'
 
     - name: Checkout `flow` repository
-      uses: actions/checkout@v4
+      uses: actions/checkout@v6
 
     # See comments in Flow-IPC workflow counterpart.  We use same techniques.
     - name: (On release creation only) Signal Pages about release (web site should update)
@@ -766,7 +766,7 @@ jobs:
           ${{ github.workspace }}/build/${{ matrix.build-cfg.conan-profile-build-type }}
 
     - name: Upload documentation tarball
-      uses: actions/upload-artifact@v4
+      uses: actions/upload-artifact@v7
       with:
         name: flow-doc
         path: ${{ github.workspace }}/doc/flow_doc.tgz

conanfile.py

@@ -54,7 +54,7 @@ class FlowRecipe(ConanFile):
     def configure(self):
         if self.options.build:
             # Currently need all headers;
-            # plus libs: chrono, filesystem, program_options, thread, timer (and all headers).
+            # plus libs: chrono, filesystem, program_options, random, thread, timer.
             # `filesystem` requires atomic.  `thread` requires container, date_time, exception.
             # (Boost provides the with_* way of specifying it also; the Conan Boost pkg only has without_*.)
             self.options["boost"].without_charconv = True
@@ -78,7 +78,6 @@ def configure(self):
             self.options["boost"].without_mpi = True
             self.options["boost"].without_nowide = True
             self.options["boost"].without_python = True
-            self.options["boost"].without_random = True
             self.options["boost"].without_regex = True
             self.options["boost"].without_serialization = True
             self.options["boost"].without_stacktrace = True

src/CMakeLists.txt

@@ -21,7 +21,7 @@ set(LIB_CMAKE_SCRIPT "../tools/cmake/FlowLikeLib.cmake")
 # Flow needs these Boost libs.
 # (Our dependents should do similar for any additional such dependencies; or omit this if none.)
 # (By the way no need to worry about find_package(Threads), as Boost::thread should take care of that.)
-set(BOOST_LIBS thread chrono timer program_options filesystem)
+set(BOOST_LIBS thread chrono timer program_options filesystem random)
 
 find_package(fmt REQUIRED)
 list(APPEND DEP_LIBS fmt::fmt)

src/flow/net_flow/detail/low_lvl_io.cpp

@@ -383,8 +383,11 @@ void Node::async_low_lvl_packet_send_impl(const util::Udp_endpoint& low_lvl_remo
   const size_t bytes_to_send = packet->serialize_to_raw_data_and_log(&raw_bufs);
   assert(bytes_to_send != 0);
 
-  // Count an actual UDP stack send() call.
-  sock->m_snd_stats.low_lvl_packet_xfer_called(packet_type_id, delayed_by_pacing, bytes_to_send);
+  // Count an actual UDP stack send() call (if a socket was actually opened; else can't charge stats to one).
+  if (sock)
+  {
+    sock->m_snd_stats.low_lvl_packet_xfer_called(packet_type_id, delayed_by_pacing, bytes_to_send);
+  }
 
   const size_t limit = opt(m_opts.m_dyn_low_lvl_max_packet_size);
   if (bytes_to_send > limit)
@@ -399,6 +402,7 @@ void Node::async_low_lvl_packet_send_impl(const util::Udp_endpoint& low_lvl_remo
                   "[\n" << packet->m_verbose_ostream_manip << "].");
 
     // Short-circuit this, since no send occurred.
+    assert(sock && "Really?  A giant low-level packet that is not even DATA?  Bug.");
     sock->m_snd_stats.low_lvl_packet_xfer_completed(packet_type_id, bytes_to_send, 0);
     return;
   }

src/flow/net_flow/detail/port_space.cpp

@@ -19,6 +19,8 @@
 #include "flow/net_flow/detail/port_space.hpp"
 #include "flow/util/util.hpp"
 #include "flow/error/error.hpp"
+#include <boost/random.hpp>
+#include <boost/random/random_device.hpp>
 #include <limits>
 
 namespace flow::net_flow
@@ -47,8 +49,7 @@ Port_space::Port_space(log::Logger* logger_ptr) :
   // Set the bit fields to their permanent widths.
   m_service_ports(S_NUM_SERVICE_PORTS),
   m_ephemeral_ports(S_NUM_EPHEMERAL_PORTS),
-  m_ephemeral_and_recent_ephemeral_ports(S_NUM_EPHEMERAL_PORTS),
-  m_rnd_generator(Random_generator::result_type(util::time_since_posix_epoch().count()))
+  m_ephemeral_and_recent_ephemeral_ports(S_NUM_EPHEMERAL_PORTS)
 {
   // All 1s = all ports available.
   m_service_ports.set();
@@ -261,10 +262,14 @@ void Port_space::return_port(flow_port_t port, Error_code* err_code)
 size_t Port_space::find_available_port_bit_idx(const Bit_set& ports)
 {
   using boost::random::uniform_int_distribution;
+  using boost::random::random_device;
 
-  // Pick a random bit in bit field.
+  /* Pick a random bit in bit field.  Use a CSPRNG (not the general-purpose mt19937-based
+   * Random_generator in flow::util) because ephemeral port selection must be unpredictable to off-path
+   * attackers attempting to guess the tuple of an established connection (cf. RFC 6056). */
+  random_device rnd_dev; // Setting this up, in Linux at least, is ~microseconds per new endpoint.  No prob.
   uniform_int_distribution<size_t> range{0, ports.size() - 1};
-  size_t port_bit_idx = range(m_rnd_generator);
+  size_t port_bit_idx = range(rnd_dev);
 
   // If that bit is 0, go right until you find a 1.
   if (!ports.test(port_bit_idx))

src/flow/net_flow/detail/port_space.hpp

@@ -22,10 +22,8 @@
 #include "flow/net_flow/net_flow_fwd.hpp"
 #include "flow/net_flow/error/error.hpp"
 #include "flow/log/log.hpp"
-#include "flow/util/random.hpp"
 #include <boost/dynamic_bitset.hpp>
 #include <boost/utility.hpp>
-#include <boost/random.hpp>
 #include <queue>
 
 namespace flow::net_flow
@@ -126,6 +124,9 @@ class Port_space :
    * Reserve the specified service port, or reserve_ephemeral_port() if the specified port is
    * #S_PORT_ANY.
    *
+   * @warning As an internal API, this one assumes `err_code` is not null.  Passing null won't throw;
+   *          undefined behavior results.
+   *
    * @param port
    *        A valid and still available service port number, or #S_PORT_ANY.
    * @param err_code
@@ -139,6 +140,9 @@ class Port_space :
   /**
    * Reserve a randomly chosen available ephemeral port.
    *
+   * @warning As an internal API, this one assumes `err_code` is not null.  Passing null won't throw;
+   *          undefined behavior results.
+   *
    * @param err_code
    *        See flow::Error_code docs for error reporting semantics.  error::Code generated:
    *        error::Code::S_OUT_OF_PORTS.
@@ -149,6 +153,9 @@ class Port_space :
   /**
    * Return a previously reserved port (of any type).
    *
+   * @warning As an internal API, this one assumes `err_code` is not null.  Passing null won't throw;
+   *          undefined behavior results.
+   *
    * @param port
    *        A previously reserved port.
    * @param err_code
@@ -163,9 +170,6 @@ class Port_space :
   /// Short-hand for bit set of arbitary length, representing a port set (each bit is a port; 1 open, 0 reserved).
   using Bit_set = boost::dynamic_bitset<>;
 
-  /// Random number generator.
-  using Random_generator = util::Rnd_gen_uniform_range_base::Random_generator;
-
   /// A type same as #flow_port_t but larger, useful when doing arithmetic that might hit overflow in corner cases.
   using flow_port_sans_overflow_t = uint32_t;
 
@@ -241,9 +245,6 @@ class Port_space :
    * oldest recently used port) and use that.  If emptied, there are simply no more ports left.
    */
   std::queue<flow_port_t> m_recent_ephemeral_ports;
-
-  /// Random number generator for picking ports.
-  Random_generator m_rnd_generator;
 }; // class Port_space
 
 } // namespace flow::net_flow

src/flow/net_flow/detail/seq_num.cpp

@@ -19,6 +19,8 @@
 #include "flow/net_flow/detail/seq_num.hpp"
 #include "flow/util/random.hpp"
 #include <boost/functional/hash/hash.hpp>
+#include <boost/random.hpp>
+#include <boost/random/random_device.hpp>
 #include <limits>
 #include <cmath>
 
@@ -33,9 +35,6 @@ namespace flow::net_flow
  * byte it would take many centuries for the sequence numbers to overflow seq_num_t. */
 const Sequence_number::seq_num_t Sequence_number::Generator::S_MAX_INIT_SEQ_NUM
   = std::numeric_limits<seq_num_t>::max() / 2;
-const Fine_duration Sequence_number::Generator::S_TIME_PER_SEQ_NUM = boost::chrono::microseconds{4}; // From RFC 793.
-const Fine_duration Sequence_number::Generator::S_MIN_DELAY_BETWEEN_ISN
-  = boost::chrono::milliseconds{500}; // From TCP/IP Illustrated Vol. 2: The Implementation (BSD Net/3).
 
 // Implementations.
 
@@ -47,50 +46,28 @@ Sequence_number::Generator::Generator(log::Logger* logger_ptr) :
 
 Sequence_number Sequence_number::Generator::generate_init_seq_num()
 {
-  using std::abs;
-  using util::Rnd_gen_uniform_range;
+  using boost::random::random_device;
+  using boost::random::uniform_int_distribution;
 
-  const Fine_time_pt& now = Fine_clock::now();
-
-  if (m_last_init_seq_num.m_num == 0)
-  {
-    /* First call to this.  Just pick a random-ish ISN from the entire allowed range.  Seed on current time.
-     * We only generate one random number ever (for this `this`), so it's fine to just seed it here and use once.
-     * @todo Could use a `static` data member RNG.  Good randomness across multiple `this`s isn't required, so the
-     * various considerations this would introduce -- multi-threadedness, for instance -- might be too much to worry
-     * about given our modest, non-cryptographic needs here. */
-
-    Rnd_gen_uniform_range<seq_num_t> rnd_single_use{1, S_MAX_INIT_SEQ_NUM}; // 0 is a reserved number; do not use.
-    m_last_init_seq_num.m_num = rnd_single_use();
-  }
-  else
-  {
-    // All subsequent calls.
-
-    /* For now basically follow RFC 793 (original TCP spec): new sequence number every N
-     * microseconds, with N defined by the RFC.  Additionally, add a large constant M, as if another
-     * 0.5 seconds had passed (as BSD did at least in 1995, as documented in TCP/IP Illustrated:
-     * Vol. 2).  abs() should not be necessary with Fine_clock, but just in case.... */
-
-    m_last_init_seq_num.m_num +=
-      seq_num_t((now - m_last_isn_generation + S_MIN_DELAY_BETWEEN_ISN) / S_TIME_PER_SEQ_NUM);
+  /* Generate each ISN independently from a CSPRNG.
+   *
+   * Historically (RFC 793: original TCP spec) this was done with a clock-based scheme (new sequence number every N
+   * usec, with N defined by the RFC).  This code formerly (written circa 2011) implemented this; however
+   * clock-based ISN generation is predictable to off-path attackers.  Hence for a full-on secure implementation
+   * we'd perhaps want the full RFC 6528 keyed-hash scheme used by modern production TCP stacks.  @todo Do that.
+   *
+   * For the time being (2026), we will do a per-call CSPRNG pull instead.  This is simpler and equally
+   * unpredictable for this context. */
 
-    /* It's incredibly unlikely that overflowed seq_num_t given the times involved, but even if it
-     * did, so be it.  In that case pretty much any random ISN is still OK.  So just assume no
-     * overflow.... */
+  random_device rnd_dev; // Setting this up, in Linux at least, is microseconds per connection.  No prob.
+  uniform_int_distribution<seq_num_t> range{1, S_MAX_INIT_SEQ_NUM}; // 0 is a reserved number; do not use.
 
-    // Wrap ISN if needed.  (It's perfectly possible, e.g., if original ISN was right near the end of allowed range.)
-    if (m_last_init_seq_num.m_num > S_MAX_INIT_SEQ_NUM)
-    {
-      // 0 is a reserved number; do not use.
-      m_last_init_seq_num.m_num = m_last_init_seq_num.m_num - S_MAX_INIT_SEQ_NUM;
-    }
-  }
+  Sequence_number result;
+  result.m_num = range(rnd_dev);
 
-  FLOW_LOG_TRACE("Generated ISN [" << m_last_init_seq_num << "].");
+  FLOW_LOG_TRACE("Generated ISN [" << result << "].");
 
-  m_last_isn_generation = now;
-  return m_last_init_seq_num;
+  return result;
 } // Sequence_number::Generator::generate_init_seq_num()
 
 Sequence_number::Sequence_number() :

src/flow/net_flow/detail/seq_num.hpp

@@ -365,7 +365,18 @@ class Sequence_number
  * initialize its state and then call generate_init_seq_num() whenever an ISN is needed.
  *
  * ### Thread safety ###
- * Not safe to read/write or write/write one object simultaneously.
+ * Safe to read/write or write/write one object simultaneously.
+ *
+ * ### Impl notes ###
+ * As it stands as of this writing a `Generator` holds no non-`static` data other than the logging context;
+ * for good randomness it uses `random_device` a-la `/dev/urandom` which requires no state.
+ * It could be replaced by a `static` function in Sequence_number, for example.
+ *
+ * However, historically, it used to be more complex (with a clock-based ISN-generation scheme RFC 793 from 1981)
+ * and thus did have state.  While eventually we deemed this outdated and unnecessary (hence the simple current
+ * impl), there is also a to-do (not high-priority) inside generate_init_seq_num() for a more
+ * advanced approach which would require state.  All in all we found it prudent to keep this encapsulated as
+ * an object class.
  */
 class Sequence_number::Generator :
   public log::Log_context,
@@ -403,23 +414,6 @@ class Sequence_number::Generator :
    * remove the need to worry about wrapping as well.
    */
   static const seq_num_t S_MAX_INIT_SEQ_NUM;
-
-  /// The ISN given out at a given time should increment every N; this is the value of  N.
-  static const Fine_duration S_TIME_PER_SEQ_NUM;
-
-  /**
-   * In addition to the actual time passed between two ISN generations, pretend this much additional
-   * time has also passed.
-   */
-  static const Fine_duration S_MIN_DELAY_BETWEEN_ISN;
-
-  // Data.
-
-  /// The last initial sequence number returned by generate_init_seq_num() (or zero if never called).
-  Sequence_number m_last_init_seq_num;
-
-  /// #Fine_clock time of the last invocation of generate_init_seq_num() (or default if never called).
-  Fine_time_pt m_last_isn_generation;
 }; // class Sequence_number::Generator
 
 // Free functions: in *_fwd.hpp.

src/flow/net_flow/node.cpp

@@ -1031,7 +1031,8 @@ const Node_options& Node::validate_options(const Node_options& opts, bool init,
   const bool checks_ok
     = VALIDATE_CHECK(opts.m_st_low_lvl_max_buf_size >= 128 * 1024) &&
       VALIDATE_CHECK(opts.m_st_timer_min_period.count() >= 0) &&
-      VALIDATE_CHECK(opts.m_dyn_low_lvl_max_packet_size >= 512);
+      VALIDATE_CHECK(opts.m_dyn_low_lvl_max_packet_size >= 512) &&
+      VALIDATE_CHECK(opts.m_dyn_accept_backlog_limit > 0);
 
   if (!checks_ok)
   {

src/flow/net_flow/node.hpp

@@ -3737,12 +3737,6 @@ class Node :
   /// Sequence number generator (at least to generate ISNs).  Only thread W can access this.
   Sequence_number::Generator m_seq_num_generator;
 
-  /**
-   * Random number generator for picking security tokens; seeded on time at Node construction and generates
-   * integers from the entire range.  (Not thread-safe.  Use only in thread W.)
-   */
-  util::Rnd_gen_uniform_range<Peer_socket::security_token_t> m_rnd_security_tokens;
-
   /**
    * The peer-to-peer connections this Node is currently tracking.  Their states are not Peer_socket::State::S_CLOSED.
    * Only thread W can access this.

src/flow/net_flow/options.cpp

@@ -52,7 +52,9 @@ Node_options::Node_options() :
   // default max_block_size + a SMALL overhead.
   m_dyn_low_lvl_max_packet_size(1124),
   // This default is explained in the option description (as of this writing): it's faster.
-  m_dyn_guarantee_one_low_lvl_in_buf_per_socket(true)
+  m_dyn_guarantee_one_low_lvl_in_buf_per_socket(true),
+  // For TCP these days values like ~500 are not-atypical, but let's be modest by default.
+  m_dyn_accept_backlog_limit(64)
 {
   // Nothing.
 }
@@ -136,6 +138,15 @@ void Node_options::setup_config_parsing_helper(Options_description* opts_desc,
        "faster, especially if low-lvl-max-packet-size is unnecessarily large; but arguably the zero-copy behavior "
        "may become faster if some implementation details related to this change.  So this switch seemed worth "
        "keeping.");
+  ADD_CONFIG_OPTION
+    (m_dyn_accept_backlog_limit,
+     "Maximum backlog size for each `Server_socket` subsequently created via `Node::listen()`.  The backlog "
+     "(for a given `Server_socket`) is defined as the total number of connections either in SYN_RCVD state "
+     "(SYN_ACK sent, awaiting SYN_ACK_ACK) or in ESTABLISHED state but not yet user-accepted via "
+     "`Server_socket::*accept()`.  When a SYN arrives while the backlog is full, it is rejected with an RST "
+     "response.  This value is captured at `Node::listen()` time and fixed for the resulting `Server_socket`'s "
+     "lifetime; subsequent changes affect only `Server_socket`s created by later `listen()` calls.  "
+     "It *is* dynamic at the `Node` level, but does *not* dynamically affect existing listening `Server_socket`s.");
 
   Peer_socket_options::setup_config_parsing_helper(opts_desc,
                                                    &target->m_dyn_sock_opts,
@@ -195,6 +206,9 @@ Peer_socket_options::Peer_socket_options() :
   m_st_snd_buf_max_size(6 * 1024 * 1024),
   // @todo Ditto.
   m_st_rcv_buf_max_size(m_st_snd_buf_max_size),
+  /* If not for SYN-flood-like possibility, this could be ~= m_st_rcv_buf_max_size.  Instead let's keep it modest.
+   * It would after all be strange to receive a large number of DATAs without a single retried SYN_ACK_ACK. */
+  m_st_rcv_sync_rcvd_data_q_cumulative_max_size(64 * 1024),
   // Disabling flow control is an emergency measure only.
   m_st_rcv_flow_control_on(true),
   // Seems reasonable.  Should be a few hundred KB typically.
@@ -323,6 +337,11 @@ void Peer_socket_options::setup_config_parsing_helper(Options_description* opts_
      "Maximum number of bytes that the Receive buffer can hold.  This determines how many bytes "
        "can be received in the background by the Node without user doing any receive()s.  "
        "It is also rounded up to to the nearest multiple of max-block-size.");
+  ADD_CONFIG_OPTION
+    (m_st_rcv_sync_rcvd_data_q_cumulative_max_size,
+     "Due to loss or reordering we may receive DATA packets before receiving the handshake-finishing SYN_ACK_ACK; "
+       "any such SYN_RCVD-state DATA packets beyond this cumulative payload size shall be silently dropped.  "
+       "The value 0 will drop all such DATA packets.");
   ADD_CONFIG_OPTION
     (m_st_rcv_flow_control_on,
      "Whether flow control (a/k/a receive window a/k/a rcv_wnd management) is enabled.  If this is "