ExternalVendorCode/genode
1
0
Fork 0
mirror of https://github.com/genodelabs/genode.git synced 2024-12-19 05:37:54 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Revert "nic_router: incremental L4 checksum updates"

Browse Source 
This reverts commit 9a37ccfe29 except for the
new declarations in public headers (in order to not change any APIs again).

We revert the commit as we found that there are corner cases in which it
produces a bad UDP checksum. The bad UDP checksum was observed via Wireshark at
a TFTP server in a Sculpt 22.10 Debian 11 VM on the first request of fetching a
file with the TFTP client of the uboot on our iMX8 test board.

Ref #4636
This commit is contained in:
Martin Stein 2022-10-13 12:00:13 +02:00 committed by Christian Helmuth
parent 847266d027
commit e052dc282b
7 changed files with 79 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
repos/os/src/lib/net/icmp.cc
View File

@ -34,33 +34,7 @@ void Icmp_packet::update_checksum(size_t data_sz)
}
void Icmp_packet::update_checksum(Internet_checksum_diff const &icd)
{
_checksum = icd.apply_to(_checksum);
}
bool Icmp_packet::checksum_error(size_t data_sz) const
{
return internet_checksum((Packed_uint16 *)this, sizeof(Icmp_packet) + data_sz);
}
void Icmp_packet::query_id(uint16_t v, Internet_checksum_diff &icd)
{
uint16_t const v_be { host_to_big_endian(v) };
icd.add_up_diff((
Packed_uint16 *)&v_be, (Packed_uint16 *)&_rest_of_header_u16[0], 2);
_rest_of_header_u16[0] = v_be;
}
void Icmp_packet::type_and_code(Type t, Code c, Internet_checksum_diff &icd)
{
uint16_t const old_type_and_code { *(uint16_t*)this };
type(t);
code(c);
icd.add_up_diff(
(Packed_uint16 *)&_type, (Packed_uint16 *)&old_type_and_code, 2);
}

6
repos/os/src/lib/net/internet_checksum.cc
View File

@ -96,12 +96,6 @@ uint16_t Net::internet_checksum_pseudo_ip(Packed_uint16 const *data_ptr,
** Internet_checksum_diff **
****************************/
void Internet_checksum_diff::add_up_diff(Internet_checksum_diff const &icd)
{
_value += icd._value;
}
void Internet_checksum_diff::add_up_diff(Packed_uint16 const *new_data_ptr,
Packed_uint16 const *old_data_ptr,
size_t data_sz)

12
repos/os/src/lib/net/ipv4.cc
View File

@ -182,15 +182,3 @@ void Ipv4_packet::update_checksum(Internet_checksum_diff const &icd)
{
_checksum = icd.apply_to(_checksum);
}
void
Ipv4_packet::update_checksum(Internet_checksum_diff const &icd,
Internet_checksum_diff &caused_icd)
{
uint16_t const new_checksum { icd.apply_to(_checksum) };
caused_icd.add_up_diff(
(Packed_uint16 *)&new_checksum, (Packed_uint16 *)&_checksum, 2);
_checksum = new_checksum;
}

22
repos/os/src/lib/net/tcp.cc
View File

@ -47,25 +47,3 @@ void Net::Tcp_packet::update_checksum(Ipv4_address ip_src,
host_to_big_endian((uint16_t)tcp_size),
Ipv4_packet::Protocol::TCP, ip_src, ip_dst);
}
void Net::Tcp_packet::update_checksum(Internet_checksum_diff const &icd)
{
_checksum = icd.apply_to(_checksum);
}
void Net::Tcp_packet::src_port(Port p, Internet_checksum_diff &icd)
{
uint16_t const p_be { host_to_big_endian(p.value) };
icd.add_up_diff((Packed_uint16 *)&p_be, (Packed_uint16 *)&_src_port, 2);
_src_port = p_be;
}
void Net::Tcp_packet::dst_port(Port p, Internet_checksum_diff &icd)
{
uint16_t const p_be { host_to_big_endian(p.value) };
icd.add_up_diff((Packed_uint16 *)&p_be, (Packed_uint16 *)&_dst_port, 2);
_dst_port = p_be;
}

22
repos/os/src/lib/net/udp.cc
View File

@ -41,31 +41,9 @@ void Net::Udp_packet::update_checksum(Ipv4_address ip_src,
}
void Net::Udp_packet::update_checksum(Internet_checksum_diff const &icd)
{
_checksum = icd.apply_to(_checksum);
}
bool Net::Udp_packet::checksum_error(Ipv4_address ip_src,
Ipv4_address ip_dst) const
{
return internet_checksum_pseudo_ip((Packed_uint16 *)this, length(), _length,
Ipv4_packet::Protocol::UDP, ip_src, ip_dst);
}
void Net::Udp_packet::src_port(Port p, Internet_checksum_diff &icd)
{
uint16_t const p_be { host_to_big_endian(p.value) };
icd.add_up_diff((Packed_uint16 *)&p_be, (Packed_uint16 *)&_src_port, 2);
_src_port = p_be;
}
void Net::Udp_packet::dst_port(Port p, Internet_checksum_diff &icd)
{
uint16_t const p_be { host_to_big_endian(p.value) };
icd.add_up_diff((Packed_uint16 *)&p_be, (Packed_uint16 *)&_dst_port, 2);
_dst_port = p_be;
}

162
repos/os/src/server/nic_router/interface.cc
View File

@ -155,21 +155,25 @@ static void _link_packet(L3_protocol const prot,
}
static void _l4_update_checksum(L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff const &prot_icd)
static void _update_checksum(L3_protocol const prot,
void *const prot_base,
size_t const prot_size,
Ipv4_address const src,
Ipv4_address const dst,
size_t const ip_size)
{
switch (prot) {
case L3_protocol::TCP:
((Tcp_packet *)prot_base)->update_checksum(prot_icd);
((Tcp_packet *)prot_base)->update_checksum(src, dst, prot_size);
return;
case L3_protocol::UDP:
((Udp_packet *)prot_base)->update_checksum(prot_icd);
((Udp_packet *)prot_base)->update_checksum(src, dst);
return;
case L3_protocol::ICMP:
{
Icmp_packet &icmp = *(Icmp_packet *)prot_base;
icmp.update_checksum(prot_icd);
icmp.update_checksum(ip_size - sizeof(Ipv4_packet) -
sizeof(Icmp_packet));
return;
}
default: throw Interface::Bad_transport_protocol(); }
@ -186,15 +190,14 @@ static Port _dst_port(L3_protocol const prot, void *const prot_base)
}
static void _dst_port(L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff &prot_icd,
Port const port)
static void _dst_port(L3_protocol const prot,
void *const prot_base,
Port const port)
{
switch (prot) {
case L3_protocol::TCP: (*(Tcp_packet *)prot_base).dst_port(port, prot_icd); return;
case L3_protocol::UDP: (*(Udp_packet *)prot_base).dst_port(port, prot_icd); return;
case L3_protocol::ICMP: (*(Icmp_packet *)prot_base).query_id(port.value, prot_icd); return;
case L3_protocol::TCP: (*(Tcp_packet *)prot_base).dst_port(port); return;
case L3_protocol::UDP: (*(Udp_packet *)prot_base).dst_port(port); return;
case L3_protocol::ICMP: (*(Icmp_packet *)prot_base).query_id(port.value); return;
default: throw Interface::Bad_transport_protocol(); }
}
@ -209,15 +212,14 @@ static Port _src_port(L3_protocol const prot, void *const prot_base)
}
static void _src_port(L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff &prot_icd,
Port const port)
static void _src_port(L3_protocol const prot,
void *const prot_base,
Port const port)
{
switch (prot) {
case L3_protocol::TCP: ((Tcp_packet *)prot_base)->src_port(port, prot_icd); return;
case L3_protocol::UDP: ((Udp_packet *)prot_base)->src_port(port, prot_icd); return;
case L3_protocol::ICMP: ((Icmp_packet *)prot_base)->query_id(port.value, prot_icd); return;
case L3_protocol::TCP: ((Tcp_packet *)prot_base)->src_port(port); return;
case L3_protocol::UDP: ((Udp_packet *)prot_base)->src_port(port); return;
case L3_protocol::ICMP: ((Icmp_packet *)prot_base)->query_id(port.value); return;
default: throw Interface::Bad_transport_protocol(); }
}
@ -299,9 +301,11 @@ void Interface::_pass_prot_to_domain(Domain &domain,
Internet_checksum_diff const &ip_icd,
L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff const &prot_icd)
size_t const prot_size)
{
_l4_update_checksum(prot, prot_base, prot_icd);
_update_checksum(
prot, prot_base, prot_size, ip.src(), ip.dst(), ip.total_length());
ip.update_checksum(ip_icd);
domain.interfaces().for_each([&] (Interface &interface)
{
@ -595,7 +599,7 @@ void Interface::_nat_link_and_pass(Ethernet_frame &eth,
Internet_checksum_diff &ip_icd,
L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff &prot_icd,
size_t const prot_size,
Link_side_id const &local_id,
Domain &local_domain,
Domain &remote_domain)
@ -609,13 +613,8 @@ void Interface::_nat_link_and_pass(Ethernet_frame &eth,
if(_config().verbose()) {
log("[", local_domain, "] using NAT rule: ", nat); }
_src_port(prot, prot_base, prot_icd, nat.port_alloc(prot).alloc());
Internet_checksum_diff icd { };
ip.src(remote_domain.ip_config().interface().address, icd);
ip_icd.add_up_diff(icd);
if (prot == L3_protocol::TCP || prot == L3_protocol::UDP) {
prot_icd.add_up_diff(icd);
}
_src_port(prot, prot_base, nat.port_alloc(prot).alloc());
ip.src(remote_domain.ip_config().interface().address, ip_icd);
remote_port_alloc = nat.port_alloc(prot);
},
[&] /* no_match */ () { }
@ -625,7 +624,7 @@ void Interface::_nat_link_and_pass(Ethernet_frame &eth,
_new_link(prot, local_id, remote_port_alloc, remote_domain, remote_id);
_pass_prot_to_domain(
remote_domain, eth, size_guard, ip, ip_icd, prot, prot_base,
prot_icd);
prot_size);
} catch (Port_allocator_guard::Out_of_indices) {
switch (prot) {
@ -973,11 +972,11 @@ void Interface::handle_interface_link_state()
}
void Interface::_send_icmp_echo_reply(Ethernet_frame &eth,
Ipv4_packet &ip,
Icmp_packet &icmp,
Internet_checksum_diff &icmp_icd,
Size_guard &size_guard)
void Interface::_send_icmp_echo_reply(Ethernet_frame &eth,
Ipv4_packet &ip,
Icmp_packet &icmp,
size_t icmp_sz,
Size_guard &size_guard)
{
/* adapt Ethernet header */
Mac_address const eth_src = eth.src();
@ -990,9 +989,8 @@ void Interface::_send_icmp_echo_reply(Ethernet_frame &eth,
ip.dst(ip_src);
/* adapt ICMP header */
icmp.type_and_code(Icmp_packet::Type::ECHO_REPLY,
Icmp_packet::Code::ECHO_REPLY,
icmp_icd);
icmp.type(Icmp_packet::Type::ECHO_REPLY);
icmp.code(Icmp_packet::Code::ECHO_REPLY);
/*
* Update checksums and send
@ -1000,7 +998,7 @@ void Interface::_send_icmp_echo_reply(Ethernet_frame &eth,
* Skip updating the IPv4 checksum because we have only swapped SRC and
* DST and these changes cancel each other out in checksum calculation.
*/
icmp.update_checksum(icmp_icd);
icmp.update_checksum(icmp_sz - sizeof(Icmp_packet));
send(eth, size_guard);
}
@ -1012,7 +1010,7 @@ void Interface::_handle_icmp_query(Ethernet_frame &eth,
Packet_descriptor const &pkt,
L3_protocol prot,
void *prot_base,
Internet_checksum_diff &prot_icd,
size_t prot_size,
Domain &local_domain)
{
Link_side_id const local_id = { ip.src(), _src_port(prot, prot_base),
@ -1033,18 +1031,13 @@ void Interface::_handle_icmp_query(Ethernet_frame &eth,
" link: ", link);
}
_adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain);
Internet_checksum_diff icd { };
ip.src(remote_side.dst_ip(), icd);
ip.dst(remote_side.src_ip(), icd);
ip_icd.add_up_diff(icd);
if (prot == L3_protocol::TCP || prot == L3_protocol::UDP) {
prot_icd.add_up_diff(icd);
}
_src_port(prot, prot_base, prot_icd, remote_side.dst_port());
_dst_port(prot, prot_base, prot_icd, remote_side.src_port());
ip.src(remote_side.dst_ip(), ip_icd);
ip.dst(remote_side.src_ip(), ip_icd);
_src_port(prot, prot_base, remote_side.dst_port());
_dst_port(prot, prot_base, remote_side.src_port());
_pass_prot_to_domain(
remote_domain, eth, size_guard, ip, ip_icd, prot,
prot_base, prot_icd);
prot_base, prot_size);
_link_packet(prot, prot_base, link, client);
done = true;
@ -1066,7 +1059,7 @@ void Interface::_handle_icmp_query(Ethernet_frame &eth,
Domain &remote_domain = rule.domain();
_adapt_eth(eth, local_id.dst_ip, pkt, remote_domain);
_nat_link_and_pass(eth, size_guard, ip, ip_icd, prot, prot_base,
prot_icd, local_id, local_domain,
prot_size, local_id, local_domain,
remote_domain);
done = true;
@ -1088,7 +1081,7 @@ void Interface::_handle_icmp_error(Ethernet_frame &eth,
Packet_descriptor const &pkt,
Domain &local_domain,
Icmp_packet &icmp,
Internet_checksum_diff &icmp_icd)
size_t icmp_sz)
{
Ipv4_packet &embed_ip { icmp.data<Ipv4_packet>(size_guard) };
Internet_checksum_diff embed_ip_icd { };
@ -1126,18 +1119,14 @@ void Interface::_handle_icmp_error(Ethernet_frame &eth,
ip.dst(remote_side.src_ip(), ip_icd);
/* adapt source and destination of embedded IP and transport packet */
Internet_checksum_diff icd { };
embed_ip.src(remote_side.src_ip(), icd);
embed_ip.dst(remote_side.dst_ip(), icd);
embed_ip_icd.add_up_diff(icd);
_src_port(embed_prot, embed_prot_base, icd, remote_side.src_port());
_dst_port(embed_prot, embed_prot_base, icd, remote_side.dst_port());
embed_ip.src(remote_side.src_ip(), embed_ip_icd);
embed_ip.dst(remote_side.dst_ip(), embed_ip_icd);
_src_port(embed_prot, embed_prot_base, remote_side.src_port());
_dst_port(embed_prot, embed_prot_base, remote_side.dst_port());
/* update checksum of both IP headers and the ICMP header */
embed_ip.update_checksum(embed_ip_icd, icd);
icmp_icd.add_up_diff(icd);
icmp.update_checksum(icmp_icd);
embed_ip.update_checksum(embed_ip_icd);
icmp.update_checksum(icmp_sz - sizeof(Icmp_packet));
ip.update_checksum(ip_icd);
/* send adapted packet to all interfaces of remote domain */
@ -1164,7 +1153,7 @@ void Interface::_handle_icmp(Ethernet_frame &eth,
Packet_descriptor const &pkt,
L3_protocol prot,
void *prot_base,
Internet_checksum_diff &prot_icd,
size_t prot_size,
Domain &local_domain,
Ipv4_address_prefix const &local_intf)
{
@ -1181,14 +1170,14 @@ void Interface::_handle_icmp(Ethernet_frame &eth,
if(_config().verbose()) {
log("[", local_domain, "] act as ICMP Echo server"); }
_send_icmp_echo_reply(eth, ip, icmp, prot_icd, size_guard);
_send_icmp_echo_reply(eth, ip, icmp, prot_size, size_guard);
return;
}
/* try to act as ICMP router */
switch (icmp.type()) {
case Icmp_packet::Type::ECHO_REPLY:
case Icmp_packet::Type::ECHO_REQUEST: _handle_icmp_query(eth, size_guard, ip, ip_icd, pkt, prot, prot_base, prot_icd, local_domain); break;
case Icmp_packet::Type::DST_UNREACHABLE: _handle_icmp_error(eth, size_guard, ip, ip_icd, pkt, local_domain, icmp, prot_icd); break;
case Icmp_packet::Type::ECHO_REQUEST: _handle_icmp_query(eth, size_guard, ip, ip_icd, pkt, prot, prot_base, prot_size, local_domain); break;
case Icmp_packet::Type::DST_UNREACHABLE: _handle_icmp_error(eth, size_guard, ip, ip_icd, pkt, local_domain, icmp, prot_size); break;
default: Drop_packet("unhandled type in ICMP"); }
}
@ -1228,9 +1217,9 @@ void Interface::_handle_ip(Ethernet_frame &eth,
/* try to route via transport layer rules */
bool done { false };
try {
L3_protocol const prot { ip.protocol() };
void *const prot_base { _prot_base(prot, size_guard, ip) };
Internet_checksum_diff prot_icd { };
L3_protocol const prot = ip.protocol();
size_t const prot_size = size_guard.unconsumed();
void *const prot_base = _prot_base(prot, size_guard, ip);
/* try handling DHCP requests before trying any routing */
if (prot == L3_protocol::UDP) {
@ -1276,7 +1265,7 @@ void Interface::_handle_ip(Ethernet_frame &eth,
}
else if (prot == L3_protocol::ICMP) {
_handle_icmp(eth, size_guard, ip, ip_icd, pkt, prot, prot_base,
prot_icd, local_domain, local_intf);
prot_size, local_domain, local_intf);
return;
}
@ -1299,20 +1288,13 @@ void Interface::_handle_ip(Ethernet_frame &eth,
" link: ", link);
}
_adapt_eth(eth, remote_side.src_ip(), pkt, remote_domain);
Internet_checksum_diff icd { };
ip.src(remote_side.dst_ip(), icd);
ip.dst(remote_side.src_ip(), icd);
ip_icd.add_up_diff(icd);
if (prot == L3_protocol::TCP || prot == L3_protocol::UDP) {
prot_icd.add_up_diff(icd);
}
_src_port(prot, prot_base, prot_icd, remote_side.dst_port());
_dst_port(prot, prot_base, prot_icd, remote_side.src_port());
ip.src(remote_side.dst_ip(), ip_icd);
ip.dst(remote_side.src_ip(), ip_icd);
_src_port(prot, prot_base, remote_side.dst_port());
_dst_port(prot, prot_base, remote_side.src_port());
_pass_prot_to_domain(
remote_domain, eth, size_guard, ip, ip_icd, prot,
prot_base, prot_icd);
prot_base, prot_size);
_link_packet(prot, prot_base, link, client);
done = true;
@ -1336,19 +1318,13 @@ void Interface::_handle_ip(Ethernet_frame &eth,
}
Domain &remote_domain = rule.domain();
_adapt_eth(eth, rule.to_ip(), pkt, remote_domain);
Internet_checksum_diff icd { };
ip.dst(rule.to_ip(), icd);
ip_icd.add_up_diff(icd);
if (prot == L3_protocol::TCP || prot == L3_protocol::UDP) {
prot_icd.add_up_diff(icd);
}
ip.dst(rule.to_ip(), ip_icd);
if (!(rule.to_port() == Port(0))) {
_dst_port(prot, prot_base, prot_icd, rule.to_port());
_dst_port(prot, prot_base, rule.to_port());
}
_nat_link_and_pass(
eth, size_guard, ip, ip_icd, prot, prot_base,
prot_icd, local_id, local_domain, remote_domain);
prot_size, local_id, local_domain, remote_domain);
done = true;
},
@ -1373,7 +1349,7 @@ void Interface::_handle_ip(Ethernet_frame &eth,
Domain &remote_domain = permit_rule.domain();
_adapt_eth(eth, local_id.dst_ip, pkt, remote_domain);
_nat_link_and_pass(
eth, size_guard, ip, ip_icd, prot, prot_base, prot_icd,
eth, size_guard, ip, ip_icd, prot, prot_base, prot_size,
local_id, local_domain, remote_domain);
done = true;

20
repos/os/src/server/nic_router/interface.h
View File

@ -198,11 +198,11 @@ class Net::Interface : private Interface_list::Element
Genode::uint32_t xid,
Ipv4_address_prefix const &local_intf);
void _send_icmp_echo_reply(Ethernet_frame &eth,
Ipv4_packet &ip,
Icmp_packet &icmp,
Internet_checksum_diff &icmp_icd,
Size_guard &size_guard);
void _send_icmp_echo_reply(Ethernet_frame &eth,
Ipv4_packet &ip,
Icmp_packet &icmp,
Genode::size_t icmp_sz,
Size_guard &size_guard);
Forward_rule_tree &_forward_rules(Domain &local_domain,
L3_protocol const prot) const;
@ -244,7 +244,7 @@ class Net::Interface : private Interface_list::Element
Packet_descriptor const &pkt,
L3_protocol prot,
void *prot_base,
Internet_checksum_diff &prot_icd,
Genode::size_t prot_size,
Domain &local_domain);
void _handle_icmp_error(Ethernet_frame &eth,
@ -254,7 +254,7 @@ class Net::Interface : private Interface_list::Element
Packet_descriptor const &pkt,
Domain &local_domain,
Icmp_packet &icmp,
Internet_checksum_diff &icmp_icd);
Genode::size_t icmp_sz);
void _handle_icmp(Ethernet_frame &eth,
Size_guard &size_guard,
@ -263,7 +263,7 @@ class Net::Interface : private Interface_list::Element
Packet_descriptor const &pkt,
L3_protocol prot,
void *prot_base,
Internet_checksum_diff &prot_icd,
Genode::size_t prot_size,
Domain &local_domain,
Ipv4_address_prefix const &local_intf);
@ -278,7 +278,7 @@ class Net::Interface : private Interface_list::Element
Internet_checksum_diff &ip_icd,
L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff &prot_icd,
Genode::size_t const prot_size,
Link_side_id const &local_id,
Domain &local_domain,
Domain &remote_domain);
@ -297,7 +297,7 @@ class Net::Interface : private Interface_list::Element
Internet_checksum_diff const &ip_icd,
L3_protocol const prot,
void *const prot_base,
Internet_checksum_diff const &prot_icd);
Genode::size_t const prot_size);
void _handle_pkt();