From 4e822436fca6dd3269235c9c3ca17b5ce8326264 Mon Sep 17 00:00:00 2001 From: Sid Hussmann Date: Mon, 26 Apr 2021 10:42:42 +0200 Subject: [PATCH] nic_router: use increasing src port for new nat The NAT feature of the NIC router used to prefer re-using source ports that have been freed recently. From an external server's perspective, if a client dies and restarts, chances are high that the new connect arrives with the same source-IP/source-port as the old connection. The server has to forcefully reset the connection. If that happens a lot, the server may even start to ignore further connections from this IP/port combination for a while as a mitigation. This patch adds a continuous counter feature that makes sure that every new port allocation will increment and result in a port that hasn't been used for a long time. The NAT feature of the nic_router is now more in line with RFC 6056 chapter 4. Ref #4086 --- repos/os/src/server/nic_router/domain.cc | 2 +- repos/os/src/server/nic_router/nat_rule.cc | 9 ++-- repos/os/src/server/nic_router/nat_rule.h | 3 +- .../src/server/nic_router/port_allocator.cc | 41 +++++++++++++++++-- .../os/src/server/nic_router/port_allocator.h | 12 ++++-- 5 files changed, 54 insertions(+), 13 deletions(-) diff --git a/repos/os/src/server/nic_router/domain.cc b/repos/os/src/server/nic_router/domain.cc index 25e0380a2f..8e0870a59f 100644 --- a/repos/os/src/server/nic_router/domain.cc +++ b/repos/os/src/server/nic_router/domain.cc @@ -313,7 +313,7 @@ void Domain::init(Domain_tree &domains) try { Nat_rule &rule = *new (_alloc) Nat_rule(domains, _tcp_port_alloc, _udp_port_alloc, - _icmp_port_alloc, node); + _icmp_port_alloc, node, _config.verbose()); _nat_rules.insert(&rule); if (_config.verbose()) { log("[", *this, "] NAT rule: ", rule); } diff --git a/repos/os/src/server/nic_router/nat_rule.cc b/repos/os/src/server/nic_router/nat_rule.cc index cc61862b52..dd05062ba7 100644 --- a/repos/os/src/server/nic_router/nat_rule.cc +++ b/repos/os/src/server/nic_router/nat_rule.cc @@ -47,12 +47,13 @@ Nat_rule::Nat_rule(Domain_tree &domains, Port_allocator &tcp_port_alloc, Port_allocator &udp_port_alloc, Port_allocator &icmp_port_alloc, - Xml_node const node) + Xml_node const node, + bool const verbose) : _domain(_find_domain(domains, node)), - _tcp_port_alloc (tcp_port_alloc, node.attribute_value("tcp-ports", 0UL)), - _udp_port_alloc (udp_port_alloc, node.attribute_value("udp-ports", 0UL)), - _icmp_port_alloc(icmp_port_alloc, node.attribute_value("icmp-ids", 0UL)) + _tcp_port_alloc (tcp_port_alloc, node.attribute_value("tcp-ports", 0UL), verbose), + _udp_port_alloc (udp_port_alloc, node.attribute_value("udp-ports", 0UL), verbose), + _icmp_port_alloc(icmp_port_alloc, node.attribute_value("icmp-ids", 0UL), verbose) { } diff --git a/repos/os/src/server/nic_router/nat_rule.h b/repos/os/src/server/nic_router/nat_rule.h index ec26201f56..500d444106 100644 --- a/repos/os/src/server/nic_router/nat_rule.h +++ b/repos/os/src/server/nic_router/nat_rule.h @@ -54,7 +54,8 @@ class Net::Nat_rule : public Genode::Avl_node Port_allocator &tcp_port_alloc, Port_allocator &udp_port_alloc, Port_allocator &icmp_port_alloc, - Genode::Xml_node const node); + Genode::Xml_node const node, + bool const verbose); Nat_rule &find_by_domain(Domain &domain); diff --git a/repos/os/src/server/nic_router/port_allocator.cc b/repos/os/src/server/nic_router/port_allocator.cc index fab135c2c0..65a9e4e69d 100644 --- a/repos/os/src/server/nic_router/port_allocator.cc +++ b/repos/os/src/server/nic_router/port_allocator.cc @@ -11,6 +11,9 @@ * under the terms of the GNU Affero General Public License version 3. */ +/* Genode includes */ +#include + /* local includes */ #include @@ -26,6 +29,24 @@ bool Net::dynamic_port(Port const port) { ** Port_allocator ** ********************/ +Port Net::Port_allocator::alloc() +{ + for (unsigned nr_of_trials { 0 }; + nr_of_trials < COUNT; + nr_of_trials++) { + + uint16_t const port_offset = _next_port_offset; + _next_port_offset = (_next_port_offset + 1) % COUNT; + try { + _alloc.alloc_addr(port_offset); + return Port { (uint16_t)(port_offset + FIRST) }; + } + catch (Bit_allocator::Range_conflict) { } + } + throw Out_of_indices(); +} + + void Net::Port_allocator::alloc(Port const port) { try { _alloc.alloc_addr(port.value - FIRST); } @@ -34,6 +55,12 @@ void Net::Port_allocator::alloc(Port const port) } +void Port_allocator::free(Port const port) +{ + _alloc.free(port.value - FIRST); +} + + /************************** ** Port_allocator_guard ** **************************/ @@ -67,7 +94,15 @@ void Port_allocator_guard::free(Port const port) Port_allocator_guard::Port_allocator_guard(Port_allocator &port_alloc, - unsigned const max) + unsigned const max, + bool const verbose) : - _port_alloc(port_alloc), _max(max) -{ } + _port_alloc(port_alloc), + _max(min(max, static_cast(Port_allocator::COUNT))) +{ + if (verbose && max > (Port_allocator::COUNT)) { + + warning("number of ports was truncated to capacity of allocator"); + } +} + diff --git a/repos/os/src/server/nic_router/port_allocator.h b/repos/os/src/server/nic_router/port_allocator.h index 4592f64706..73f86658fb 100644 --- a/repos/os/src/server/nic_router/port_allocator.h +++ b/repos/os/src/server/nic_router/port_allocator.h @@ -38,17 +38,19 @@ class Net::Port_allocator private: - Genode::Bit_allocator _alloc { }; + Genode::Bit_allocator _alloc { }; + Genode::uint16_t _next_port_offset { 0 }; public: struct Allocation_conflict : Genode::Exception { }; + struct Out_of_indices : Genode::Exception { }; - Port alloc() { return Port(_alloc.alloc() + FIRST); } + Port alloc(); void alloc(Port const port); - void free(Port const port) { _alloc.free(port.value - FIRST); } + void free(Port const port); }; @@ -70,7 +72,9 @@ class Net::Port_allocator_guard void free(Port const port); - Port_allocator_guard(Port_allocator & port_alloc, unsigned const max); + Port_allocator_guard(Port_allocator &port_alloc, + unsigned const max, + bool const verbose); unsigned max() const { return _max; } };