quic: refactor QuicSession stats and use net.BlockList

doc/api/quic.md

@@ -1445,6 +1445,24 @@ error will be thrown if `quicsock.addEndpoint()` is called either after
 the `QuicSocket` has already started binding to the local ports, or after
 the `QuicSocket` has been destroyed.
 
+#### `quicsocket.blockList`
+<!-- YAML
+added: REPLACEME
+-->
+
+* Type: {net.BlockList}
+
+A {net.BlockList} instance used to define rules for remote IPv4 or IPv6
+addresses that this `QuicSocket` is not permitted to interact with. The
+rules can be specified as either specific individual addresses, ranges
+of addresses, or CIDR subnet ranges.
+
+When listening as a server, if a packet is received from a blocked address,
+the packet will be ignored.
+
+When connecting as a client, if the remote IP address is blocked, the
+connection attempt will be rejected.
+
 #### `quicsocket.bound`
 <!-- YAML
 added: REPLACEME

lib/internal/blocklist.js

@@ -26,8 +26,13 @@ const {
 } = require('internal/errors').codes;
 
 class BlockList {
-  constructor() {
-    this[kHandle] = new BlockListHandle();
+  constructor(handle = new BlockListHandle()) {
+    // The handle argument is an intentionally undocumented
+    // internal API. User code will not be able to create
+    // a BlockListHandle object directly.
+    if (!(handle instanceof BlockListHandle))
+      throw new ERR_INVALID_ARG_TYPE('handle', 'BlockListHandle', handle);
+    this[kHandle] = handle;
     this[kHandle][owner_symbol] = this;
   }
 

lib/internal/quic/core.js

@@ -54,6 +54,7 @@ const { Duplex } = require('stream');
 const {
   createSecureContext: _createSecureContext
 } = require('tls');
+const BlockList = require('internal/blocklist');
 const {
   translatePeerCertificate
 } = require('_tls_common');
@@ -891,6 +892,7 @@ class QuicSocket extends EventEmitter {
   [kInternalState] = {
     alpn: undefined,
     bindPromise: undefined,
+    blockList: undefined,
     client: undefined,
     closePromise: undefined,
     closePromiseResolve: undefined,
@@ -1007,8 +1009,10 @@ class QuicSocket extends EventEmitter {
       this[async_id_symbol] = handle.getAsyncId();
       this[kInternalState].sharedState =
         new QuicSocketSharedState(handle.state);
+      this[kInternalState].blockList = new BlockList(handle.blockList);
     } else {
       this[kInternalState].sharedState = undefined;
+      this[kInternalState].blockList = undefined;
     }
   }
 
@@ -1303,6 +1307,9 @@ class QuicSocket extends EventEmitter {
     if (this.closing)
       throw new ERR_INVALID_STATE('QuicSocket is closing');
 
+    if (this.blockList.check(ip, type === AF_INET6 ? 'ipv6' : 'ipv4'))
+      throw new ERR_OPERATION_FAILED(`${ip} failed BlockList check`);
+
     return new QuicClientSession(this, options, type, ip);
   }
 
@@ -1458,6 +1465,10 @@ class QuicSocket extends EventEmitter {
     return this;
   }
 
+  get blockList() {
+    return this[kInternalState]?.blockList;
+  }
+
   get endpoints() {
     return Array.from(this[kInternalState].endpoints);
   }

src/env.h

@@ -182,6 +182,7 @@ constexpr size_t kFsStatsBufferLength =
   V(asn1curve_string, "asn1Curve")                                             \
   V(async_ids_stack_string, "async_ids_stack")                                 \
   V(bits_string, "bits")                                                       \
+  V(block_list_string, "blockList")                                            \
   V(buffer_string, "buffer")                                                   \
   V(bytes_parsed_string, "bytesParsed")                                        \
   V(bytes_read_string, "bytesRead")                                            \
@@ -423,6 +424,7 @@ constexpr size_t kFsStatsBufferLength =
   V(async_wrap_object_ctor_template, v8::FunctionTemplate)                     \
   V(base_object_ctor_template, v8::FunctionTemplate)                           \
   V(binding_data_ctor_template, v8::FunctionTemplate)                          \
+  V(blocklist_instance_template, v8::ObjectTemplate)                           \
   V(compiled_fn_entry_template, v8::ObjectTemplate)                            \
   V(dir_instance_template, v8::ObjectTemplate)                                 \
   V(fd_constructor_template, v8::ObjectTemplate)                               \

src/node_sockaddr.cc

@@ -524,6 +524,19 @@ SocketAddressBlockListWrap::SocketAddressBlockListWrap(
   MakeWeak();
 }
 
+BaseObjectPtr<SocketAddressBlockListWrap> SocketAddressBlockListWrap::New(
+    Environment* env) {
+  Local<Object> obj;
+  if (!env->blocklist_instance_template()
+          ->NewInstance(env->context()).ToLocal(&obj)) {
+    return {};
+  }
+  BaseObjectPtr<SocketAddressBlockListWrap> wrap =
+      MakeDetachedBaseObject<SocketAddressBlockListWrap>(env, obj);
+  CHECK(wrap);
+  return wrap;
+}
+
 void SocketAddressBlockListWrap::New(
     const FunctionCallbackInfo<Value>& args) {
   CHECK(args.IsConstructCall());
@@ -673,6 +686,7 @@ void SocketAddressBlockListWrap::Initialize(
   env->SetProtoMethod(t, "check", SocketAddressBlockListWrap::Check);
   env->SetProtoMethod(t, "getRules", SocketAddressBlockListWrap::GetRules);
 
+  env->set_blocklist_instance_template(t->InstanceTemplate());
   target->Set(env->context(), name,
               t->GetFunction(env->context()).ToLocalChecked()).FromJust();
 

src/node_sockaddr.h

@@ -280,6 +280,7 @@ class SocketAddressBlockListWrap :
                          v8::Local<v8::Context> context,
                          void* priv);
 
+  static BaseObjectPtr<SocketAddressBlockListWrap> New(Environment* env);
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AddAddress(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AddRange(const v8::FunctionCallbackInfo<v8::Value>& args);

src/quic/node_quic_crypto.cc

@@ -602,8 +602,6 @@ void InitializeSecureContext(
     BaseObjectPtr<crypto::SecureContext> sc,
     bool early_data,
     ngtcp2_crypto_side side) {
-  // TODO(@jasnell): Using a static value for this at the moment but
-  // we need to determine if a non-static or per-session value is better.
   constexpr static unsigned char session_id_ctx[] = "node.js quic server";
   switch (side) {
     case NGTCP2_CRYPTO_SIDE_SERVER:

src/quic/node_quic_http3_application.cc

@@ -79,11 +79,10 @@ Http3Application::Http3Application(
 // Push streams in HTTP/3 are a bit complicated.
 // First, it's important to know that only an HTTP/3 server can
 // create a push stream.
-// Second, it's important to recognize that a push stream is
-// essentially an *assumed* request. For instance, if a client
-// requests a webpage that has links to css and js files, and
-// the server expects the client to send subsequent requests
-// for those css and js files, the server can shortcut the
+// Second, a push stream is essentially an *assumed* request. For
+// instance, if a client requests a webpage that has links to css
+// and js files, and the server expects the client to send subsequent
+// requests for those css and js files, the server can shortcut the
 // process by opening a push stream for each additional resource
 // it assumes the client to make.
 // Third, a push stream can only be opened within the context

src/quic/node_quic_session.cc

@@ -263,6 +263,16 @@ void QuicSessionConfig::Set(
   // TODO(@jasnell): QUIC allows both IPv4 and IPv6 addresses to be
   // specified. Here we're specifying one or the other. Need to
   // determine if that's what we want or should we support both.
+  //
+  // TODO(@jasnell): Currently, this is specified as a single value
+  // that is used for all connections. In the future, it may be
+  // necessary to determine the preferred address based on the
+  // remote address. The trick, however, is that the preferred
+  // address must be selected before the QuicSession is created,
+  // before the handshake can be started. That is, it may need
+  // to be an optional callback on QuicSocket. That would incur
+  // a performance penalty so we'd really have to be sure of the
+  // utility.
   if (preferred_addr != nullptr) {
     transport_params.preferred_address_present = 1;
     switch (preferred_addr->sa_family) {
@@ -2088,7 +2098,6 @@ bool QuicSession::Receive(
     UpdateIdleTimer();
 
   SendPendingData();
-  UpdateRecoveryStats();
   Debug(this, "Successfully processed received packet");
   return true;
 }
@@ -2883,19 +2892,6 @@ void QuicSessionOnCertDone(const FunctionCallbackInfo<Value>& args) {
 }
 }  // namespace
 
-// Recovery stats are used to allow user code to keep track of
-// important round-trip timing statistics that are updated through
-// the lifetime of a connection. Effectively, these communicate how
-// much time (from the perspective of the local peer) is being taken
-// to exchange data reliably with the remote peer.
-// TODO(@jasnell): Revisit
-void QuicSession::UpdateRecoveryStats() {
-  ngtcp2_conn_stat stat;
-  ngtcp2_conn_get_conn_stat(connection(), &stat);
-  SetStat(&QuicSessionStats::min_rtt, stat.min_rtt);
-  SetStat(&QuicSessionStats::latest_rtt, stat.latest_rtt);
-  SetStat(&QuicSessionStats::smoothed_rtt, stat.smoothed_rtt);
-}
 
 // Data stats are used to allow user code to keep track of important
 // statistics such as amount of data in flight through the lifetime
@@ -2908,6 +2904,13 @@ void QuicSession::UpdateDataStats() {
   ngtcp2_conn_stat stat;
   ngtcp2_conn_get_conn_stat(connection(), &stat);
 
+  SetStat(&QuicSessionStats::latest_rtt, stat.latest_rtt);
+  SetStat(&QuicSessionStats::min_rtt, stat.min_rtt);
+  SetStat(&QuicSessionStats::smoothed_rtt, stat.smoothed_rtt);
+  SetStat(&QuicSessionStats::receive_rate, stat.recv_rate_sec);
+  SetStat(&QuicSessionStats::send_rate, stat.delivery_rate_sec);
+  SetStat(&QuicSessionStats::cwnd, stat.cwnd);
+
   state_->bytes_in_flight = stat.bytes_in_flight;
   // The max_bytes_in_flight is a highwater mark that can be used
   // in performance analysis operations.

src/quic/node_quic_session.h

@@ -204,7 +204,10 @@ enum QuicSessionStateFields {
   V(BLOCK_COUNT, block_count, "Block Count")                                   \
   V(MIN_RTT, min_rtt, "Minimum RTT")                                           \
   V(LATEST_RTT, latest_rtt, "Latest RTT")                                      \
-  V(SMOOTHED_RTT, smoothed_rtt, "Smoothed RTT")
+  V(SMOOTHED_RTT, smoothed_rtt, "Smoothed RTT")                                \
+  V(CWND, cwnd, "Cwnd")                                                        \
+  V(RECEIVE_RATE, receive_rate, "Receive Rate / Sec")                          \
+  V(SEND_RATE, send_rate, "Send Rate  Sec")                                    \
 
 #define V(name, _, __) IDX_QUIC_SESSION_STATS_##name,
 enum QuicSessionStatsIdx : int {
@@ -616,20 +619,17 @@ class QuicApplication : public MemoryRetainer,
 
   virtual void ResumeStream(int64_t stream_id) {}
 
-  virtual void SetSessionTicketAppData(const SessionTicketAppData& app_data) {
-    // TODO(@jasnell): Different QUIC applications may wish to set some
-    // application data in the session ticket (e.g. http/3 would set
-    // server settings in the application data). For now, doing nothing
-    // as I'm just adding the basic mechanism.
-  }
+  // Different QUIC applications may set some application data in
+  // the session ticket (e.g. http/3 would set server settings in the
+  // application data). By default, there's nothing to set.
+  virtual void SetSessionTicketAppData(const SessionTicketAppData& app_data) {}
 
+  // Different QUIC applications may set some application data in
+  // the session ticket (e.g. http/3 would set server settings in the
+  // application data). By default, there's nothing to get.
   virtual SessionTicketAppData::Status GetSessionTicketAppData(
       const SessionTicketAppData& app_data,
       SessionTicketAppData::Flag flag) {
-    // TODO(@jasnell): Different QUIC application may wish to set some
-    // application data in the session ticket (e.g. http/3 would set
-    // server settings in the application data). For now, doing nothing
-    // as I'm just adding the basic mechanism.
     return flag == SessionTicketAppData::Flag::STATUS_RENEW ?
       SessionTicketAppData::Status::TICKET_USE_RENEW :
       SessionTicketAppData::Status::TICKET_USE;
@@ -1275,8 +1275,6 @@ class QuicSession final : public AsyncWrap,
 
   bool WritePackets(const char* diagnostic_label = nullptr);
 
-  void UpdateRecoveryStats();
-
   void UpdateConnectionID(
       int type,
       const QuicCID& cid,

src/quic/node_quic_socket.cc

@@ -252,6 +252,7 @@ QuicSocket::QuicSocket(
     : AsyncWrap(quic_state->env(), wrap, AsyncWrap::PROVIDER_QUICSOCKET),
       StatsBase(quic_state->env(), wrap),
       alloc_info_(MakeAllocator()),
+      block_list_(SocketAddressBlockListWrap::New(quic_state->env())),
       options_(options),
       state_(quic_state->env()->isolate()),
       max_connections_(max_connections),
@@ -269,6 +270,12 @@ QuicSocket::QuicSocket(
 
   EntropySource(token_secret_, kTokenSecretLen);
 
+  wrap->DefineOwnProperty(
+      env()->context(),
+      env()->block_list_string(),
+      block_list_->object(),
+      PropertyAttribute::ReadOnly).Check();
+
   wrap->DefineOwnProperty(
       env()->context(),
       env()->state_string(),
@@ -432,6 +439,12 @@ void QuicSocket::OnReceive(
     return;
   }
 
+  if (UNLIKELY(block_list_->Apply(remote_addr))) {
+    Debug(this, "Ignoring blocked remote address: %s", remote_addr);
+    IncrementStat(&QuicSocketStats::packets_ignored);
+    return;
+  }
+
   IncrementStat(&QuicSocketStats::bytes_received, nread);
 
   const uint8_t* data = reinterpret_cast<const uint8_t*>(buf.data());

src/quic/node_quic_socket.h

@@ -516,6 +516,7 @@ class QuicSocket : public AsyncWrap,
   std::vector<BaseObjectPtr<QuicEndpoint>> endpoints_;
   SocketAddress::Map<BaseObjectWeakPtr<QuicEndpoint>> bound_endpoints_;
   BaseObjectWeakPtr<QuicEndpoint> preferred_endpoint_;
+  BaseObjectPtr<SocketAddressBlockListWrap> block_list_;
 
   uint32_t flags_ = 0;
   uint32_t options_ = 0;

test/parallel/test-quic-blocklist.js

@@ -0,0 +1,52 @@
+// Flags: --no-warnings
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasQuic)
+  common.skip('missing quic');
+
+const { createQuicSocket, BlockList } = require('net');
+const assert = require('assert');
+
+const { key, cert, ca } = require('../common/quic');
+const { once } = require('events');
+
+const idleTimeout = common.platformTimeout(1);
+const options = { key, cert, ca, alpn: 'zzz', idleTimeout };
+
+const client = createQuicSocket({ client: options });
+const server = createQuicSocket({ server: options });
+
+assert(client.blockList instanceof BlockList);
+assert(server.blockList instanceof BlockList);
+
+client.blockList.addAddress('10.0.0.1');
+
+assert(client.blockList.check('10.0.0.1'));
+
+// Connection fails because the IP address is blocked
+assert.rejects(client.connect({ address: '10.0.0.1' }), {
+  code: 'ERR_OPERATION_FAILED'
+}).then(common.mustCall());
+
+server.blockList.addAddress('127.0.0.1');
+
+(async () => {
+  server.on('session', common.mustNotCall());
+
+  await server.listen();
+
+  const session = await client.connect({
+    address: common.localhostIPv4,
+    port: server.endpoints[0].address.port,
+    idleTimeout,
+  });
+
+  session.on('secure', common.mustNotCall());
+
+  await once(session, 'close');
+
+  await Promise.all([server.close(), client.close()]);
+
+})().then(common.mustCall());

tools/doc/type-parser.js

@@ -122,6 +122,7 @@ const customTypesMap = {
   'require': 'modules.html#modules_require_id',
 
   'Handle': 'net.html#net_server_listen_handle_backlog_callback',
+  'net.BlockList': 'net.html#net_class_net_blocklist',
   'net.Server': 'net.html#net_class_net_server',
   'net.Socket': 'net.html#net_class_net_socket',