IPv6 support - #149
Closed
danielinux wants to merge 25 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces IPv6 support groundwork across wolfIP, including a standalone IPv6 address type/helpers, per-interface multi-address support (needed for IPv6 and also enabling IPv4 aliasing), and new IPv6-focused tests plus CI to validate the feature set end-to-end.
Changes:
- Added
ip6address type and a suite of static-inline IPv6 helpers (parsing/printing, scope/type predicates, prefix operations). - Added per-interface address list APIs (
wolfIP_ifaddr_*) and integrated them into existing IPv4 socket demux/bind behavior. - Added IPv6 unit + end-to-end test infrastructure (scripts, make targets, GitHub Actions workflow) and documentation updates.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfip6.h | Adds ip6 type plus inline IPv6 address operations, parsing, and RFC 5952 formatting. |
| wolfip.h | Includes IPv6 helpers unconditionally; adds multi-address and IPv6 API declarations; adds switch/L2 integration surface declarations. |
| src/wolfip.c | Integrates IPv6 ethertype demux and ND polling; adds per-interface address list implementation; updates UDP receive address matching for wildcard binds with aliases. |
| src/wolfesp.c | Replaces wc_ForceZero usage with a local portable zeroization helper. |
| config.h | Introduces WOLFIP_IPV6 and related sizing/macros, including WOLFIP_IF_MULTICONF coupling. |
| Makefile | Adds IPv6 build/test targets, coverage targets, and end-to-end IPv6 test runners. |
| .github/workflows/ipv6.yml | Adds CI job for IPv6 unit tests (sanitizers), coverage reporting, and end-to-end TAP-based tests. |
| tools/scripts/wolfip-radvd.sh | Adds helper to run radvd for SLAAC interop testing on a TAP interface. |
| src/test/unit/unit.c | Wires in new unit test suites for multi-address and IPv6. |
| src/test/unit/unit_shared.c | Refactors UDP frame injection helpers to support full ingress-path tests. |
| src/test/unit/unit_tests_ip_arp_recv.c | Strengthens IPv4 martian/forwarding tests by using the real ingress path and adding controls. |
| src/test/unit/unit_tests_ifaddr.c | Adds unit tests for the per-interface address list and alias semantics. |
| src/test/unit/unit_tests_ipv6_hdr.c | Adds unit tests for IPv6 header layout, pseudo-header checksum, and transmit encapsulation. |
| src/test/unit/unit_tests_ipv6_recv.c | Adds unit tests for IPv6 receive validation and Ethernet demux behavior. |
| src/test/unit/unit_tests_ipv6_icmp.c | Adds unit tests for ICMPv6 Echo Request/Reply behavior. |
| src/test/unit/unit_tests_ipv6_pending.c | Adds gated “requirement-derived” pending tests for not-yet-implemented IPv6 features. |
| src/test/test_ipv6_ping.c | Adds an end-to-end ICMPv6 echo test over TAP/VDE. |
| src/test/test_ipv6_slaac.c | Adds an end-to-end SLAAC/DAD/ND interoperability test against Linux (optionally via radvd). |
| docs/dlr_integration.md | Documents the DLR integration surface (L2 hook + switch ops vtable). |
| README.md | Updates protocol/RFC feature matrix with IPv6 rows and DLR doc link. |
| CHANGELOG.md | Adds an Unreleased section describing IPv6 + multi-address changes. |
Suppressed comments (1)
src/test/test_ipv6_ping.c:242
- The test still installs a static neighbour entry (and tells users to do the same), which prevents this end-to-end ping test from exercising Neighbor Discovery. Now that NDP is implemented, the test should rely on the host doing address resolution normally.
/* Neighbor Discovery is not implemented, so the host cannot resolve our
* MAC on its own. Install the mapping by hand. */
snprintf(cmd, sizeof(cmd),
"ip -6 neigh replace %s lladdr %02x:%02x:%02x:%02x:%02x:%02x "
"dev %s nud permanent",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WOLFIP_IPV6, WOLFIP_IF_MULTICONF, WOLFIP_IF_CONF_MAX, WOLFIP_IFADDR_MAX, WOLFIP_IP6_ADDR_MAX, the WOLFIP_ND6_* table sizes and WOLFIP_DHCP6_BUF_SIZE. All default off or to the smallest useful size. WOLFIP_IPV6 forces WOLFIP_IF_MULTICONF on: a link-local address always coexists with a global one, so IPv6 cannot work with one address per interface. WOLFIP_IPV6_PROFILE_LARGE raises every table at once. The WOLFIP_IPV6_HAVE_* macros mark features not implemented yet. Invariants use the existing "#if ... #error" idiom.
wolfip6.h: the 128-bit address type, well-known addresses, scope and type predicates, prefix operations, the RFC 2464 multicast and RFC 4291 modified EUI-64 mappings, and RFC 4291 / RFC 5952 text conversion. Stored as a byte array, not words: wolfIP targets big-endian and strict-alignment machines, and an IPv6 address sits at an odd offset behind the Ethernet header. Wrapped in a struct so it cannot decay to a pointer, which means comparing with ip6_cmp() rather than ==. No <string.h> dependency, so freestanding builds work. Well-known addresses are brace-initialiser macros because an unused static const in a header trips -Wunused-const-variable. Included unconditionally from wolfip.h: types and static inline functions only, so it adds no code when IPv6 is off and cannot change any struct layout. The tests are ungated for the same reason and run in the default build.
src/wolfip6.c: wire structures, the RFC 8200 section 8.1 pseudo-header and its checksum, ip6_recv() validation, ip6_output_add_header(), and the ethertype and MAC demux. Included textually into wolfip.c under WOLFIP_IPV6, as src/wolfesp.c already is, because it needs struct wolfIP and the static checksum, Ethernet and link-layer helpers. The IPv4 structures embed the network header by value at a fixed 34-byte offset, so the IPv6 transport structures are parallel definitions rather than a reuse. The pseudo-header likewise gets its own union and checksum: 40 bytes against 12. ip6_recv() returns a distinct code per rejection reason so tests can assert why a frame was refused. The hop limit is deliberately not checked. RFC 8200 section 3 has it tested by forwarding nodes only, so a destination host must accept a packet addressed to it at hop limit zero. IPv4-mapped and IPv4-compatible addresses are dropped in either address field (RFC 4291 sections 2.5.5.1 and 2.5.5.2): they exist only inside the socket API, and wolfIP is to present mapped addresses to dual-stack sockets. Extension headers are recognised and refused rather than walked; chain walking is a denial-of-service surface. Adds the unit-ipv6 target with sanitizer and coverage variants. WOLFIP_IPV6 must be passed on the command line, because wolfip.c includes wolfip.h before config.h.
Sixty tests covering ICMPv6, Neighbor Discovery, SLAAC, DAD, the DHCPv6 client, extension headers and AF_INET6 sockets. None of it is implemented yet, so each group is guarded by its WOLFIP_IPV6_HAVE_* macro and none run. They fix the API shape as well as the expected behaviour: the names and signatures they use are the contract the implementation has to meet. Emphasis is on requirements that writing the happy path first would miss: the NDP hop-limit-255 rule, an NDP option length of zero looping the parser, the SLAAC two-hour rule, ICMPv6 error suppression, that framing follows the destination address family rather than the socket domain, and that IPV6_V6ONLY is honoured rather than swallowed by the setsockopt default. Named macros rather than "#if 0" so the outstanding work is greppable; make unit-ipv6-pending-count reports it. Test names carry no requirement identifiers: the requirement documents are internal and the mapping is kept off-tree, keyed by function name.
Runs the IPv6 unit suite with ASan and UBSan, rebuilds the IPv4-only configuration, builds the library with WOLFIP_IPV6=1, and reports the pending requirement test count. The addressing tests are not covered here: wolfip6.h is included unconditionally, so they already run in the default make unit in linux.yml. Coverage is reported, not gated. IPv6 still carries stubs, so 100% function coverage is not achievable; the enforced gate stays on src/wolfip.c in wolfip-autocov.yml.
wolfIP does not implement DLR. This declares what a Device Level Ring implementation needs from the stack and from the driver so one can be added without changing the core. wolfIP_register_l2_handler() generalises the EAPOL hook, which is hardwired to ethertype 0x888E: a module claims an ethertype and also declares the destination MACs it wants delivered, since the ingress path filters on MAC before dispatch. Unlike the EAPOL hook the handler sees the whole frame, header included, because a ring protocol needs the source MAC. struct wolfIP_switch_ops is the driver vtable: port count, per-port link state and change notification, per-port block and unblock, MAC table flush, per-port transmit and ingress port reporting. Appended last in struct wolfIP_ll_dev, after wifi_ops, so no existing member offset shifts. docs/dlr_integration.md records the contract, including that wolfIP's poll loop is millisecond-granular while DLR beacons are sub-millisecond, so beacons must come from a hardware timer. It also notes that the ODVA specification is paywalled and the ethertype and multicast MAC range quoted there must be confirmed before use. ISO 11898 and CAN FD, which appeared in an early draft, are not applicable: ISO 11898 is the CAN bus standard.
Protocol table rows for IPv6 and IPv6 addressing, marked in progress and naming what is not implemented, so the table does not overstate what the stack does. Links the DLR integration guide.
Implements WOLFIP_IF_MULTICONF. IPv6 requires it, since a link-local address always coexists with a global one, and the same machinery gives IPv4 address aliasing with IPv6 off. struct ipconf is reached by more than fifty files, every board port among them, so it is not replaced. The list is additive: ipconf holds the primary IPv4 address of an interface and a flat shared pool holds the rest, IPv4 aliases and every IPv6 address. The primary is never copied into the pool, so the two cannot drift. Index 0 of an interface's IPv4 list is the primary; higher indices are aliases in insertion order. WOLFIP_IF_CONF_MAX caps the total per interface and both families draw on it. With the feature off the pool is preprocessed out and an interface holds one address; struct wolfIP is byte-identical to before at 483664 bytes. Adding the first IPv4 address of an interface sets the primary, so a single-address build is usable through this API alone. Deleting the primary promotes the first alias rather than leaving the interface without one. wolfIP_if_for_local_ip() now consults the alias list; without that, binding a socket to an alias resolved to the primary interface. A unit-multiconf target exercises IPv4 aliasing without IPv6.
bind() resolves INADDR_ANY to the interface's primary address so source selection has something concrete, and records what the application asked for in bound_local_ip. The UDP receive path matched only on local_ip, so a socket bound to INADDR_ANY accepted datagrams to the primary and dropped everything sent to any other local address. Until an interface could carry more than one address the two were the same thing. The TCP listen path already filters on bound_local_ip. The clause requires local_ip to be set, so it applies only to sockets bind() resolved: a socket that was never bound has bound_local_ip == IPADDR_ANY too, and must not be treated as a wildcard listener. With WOLFIP_IF_MULTICONF off there is one local address per interface, so the clause cannot change the outcome. Adds tests for bind and source selection: a socket bound to an alias receives traffic to that alias and not to the primary and vice versa, a wildcard bind receives both, binding to a non-local address is refused, and sendto from a socket bound to an alias sources from it.
test_ip_recv_loopback_dst_on_non_loopback_dropped and its _src_ sibling never called ip_recv: inject_udp_datagram() hands a frame straight to udp_try_recv(), skipping header validation, the checksum and the martian filter. They passed because the socket had local_ip = IPADDR_ANY, which the UDP demux does not match. With ip_recv's 127/8 filter disabled the originals still passed; after this change the same mutation fails both. build_udp_frame() is split out of inject_udp_datagram(), and recv_udp_datagram() delivers the same frame through wolfIP_recv_ex(). inject_udp_datagram() keeps its behaviour and documents what it skips. Both tests gained a positive control on a separate port, and the socket under test now accepts exactly the address being filtered, so the negative assertion means something. test_ip_recv_dest_matches_secondary_iface_ip_is_local uses the real path too, with a control proving the fixture can emit a frame when forwarding is expected. It asserts the observable contract rather than one internal decision: ip_recv's is_local check and wolfIP_forward_interface() declining our own address both produce the right outcome, so defeating either alone leaves the behaviour correct.
Answering a ping needs neither sockets nor Neighbor Discovery, because the reply goes to the source MAC of the request. It also gives ip6_output_add_header() its first production caller. icmp6_input() follows icmp_input() above it in wolfip.c: same order of length checks, then the checksum, then one arm per type, with the reply built in place and the identifier, sequence number and payload left untouched. wolfIP_if_for_local_ip6() matches wolfIP_if_for_local_ip(), including its weak end-system model search across all interfaces; link- local zones per RFC 4007 are noted for when NDP lands. Only requests addressed to one of our own addresses are answered, as in the ICMPv4 arm: otherwise an L2-adjacent attacker can address a frame to our MAC with an arbitrary destination and have us emit a reply with a source of their choosing. Multicast destinations are declined; they need a unicast source per RFC 4443 section 4.2. The checksum covers the IPv6 pseudo-header, unlike ICMPv4. There is no wolfIP_filter_notify_icmp() call because the filter has no IPv6 hooks.
Brings wolfIP up on a TAP device, or a VDE switch with BUILD_VDE=1, forms the link-local address from the interface MAC per RFC 4862 section 5.3, and answers pings from the host. Without arguments it prints the addresses and the commands to run, then polls. With --selftest it runs ping(8) and exits non-zero if the replies do not arrive. Both need root. Neighbor Discovery is not implemented, so the host cannot resolve our MAC and the mapping is installed with ip -6 neigh replace; --selftest does this. It becomes unnecessary once NDP lands. Needs its own build/ipv6/wolfip.o because WOLFIP_IPV6 has to be visible to wolfip.h, which wolfip.c includes before config.h.
RFC 4861 address resolution and router discovery, RFC 4862 section 5.4
duplicate address detection, and the host routing that follows from them.
No sockets.
nd6_lookup(), nd6_store_neighbor() and nd6_neighbor_index() mirror the
arp_* helpers, with the same linear scan keyed on {address, interface}.
The reachability state machine and the router flag are new. One
divergence: arp_store_neighbor() refuses when the table is full, this
evicts the oldest, so a burst of scan traffic cannot lock out every real
neighbour.
wolfIP_ipv6_start() is the counterpart of dhcp_client_init(): it forms
the link-local address, probes it and solicits routers, continuing from
wolfIP_poll(). An address is TENTATIVE until its probe completes.
One periodic tick drives duplicate address detection, router solicitation
retries and every cache and lifetime expiry, rather than a timer per
address. MAX_TIMERS is MAX_TCPSOCKETS * 3 and already carries TCP, DHCP
and DNS; it gains four slots under WOLFIP_IPV6.
Routing is the host model: an on-link prefix list and a default router
list, with nd6_select_nexthop() as the counterpart of
wolfIP_select_nexthop_ex(). The static route table is not generalised to
both families; that only pays off with forwarding and sockets.
Router Advertisement handling is limited to the default router and Prefix
Information options. Managed/Other, MTU and timer overrides are parsed
past.
Validation, each with a test: every ND message must arrive with hop limit
255 (RFC 4861 sections 6.1 and 7.1), a Router Advertisement must have a
link-local source (section 6.1.2), an advertised link-local prefix is
ignored (RFC 4862 section 5.5.3), an option length of zero is rejected
because it stops the option walk making progress, and a tentative address
is neither defended nor used.
RFC 4861 section 7.2.5: without the Override flag a differing link-layer
address does not replace the one held, but a REACHABLE entry still drops
to STALE so reachability is re-verified.
Twenty-two tests: joining from cold, advertisements to accept and refuse,
duplicate address detection in both outcomes including a simultaneous
probe, router advertisements, and a statically assigned ULA alongside
them. Twenty requirement stubs retired.
test_ipv6_ping.c configures its addresses by hand and never calls wolfIP_ipv6_start(), and the ND and SLAAC tests use forged frames, so nothing showed SLAAC working against a real stack. test_ipv6_slaac.c configures nothing on the wolfIP side. It starts IPv6, lets the link-local address form and pass duplicate address detection, waits for a Router Advertisement, forms a global address from the advertised prefix, and has Linux ping it. The host does not know wolfIP's link-layer address, so it sends a Neighbor Solicitation first: address resolution happens for real in both directions, with no static neighbour entry. The advertisement is injected over an AF_PACKET socket on the host end of the TAP by default, which is deterministic but is a frame this repository wrote. With --with-radvd, tools/scripts/wolfip-radvd.sh spawns radvd against a generated config: radvd does not autostart, so the script runs it directly, enables forwarding on the one interface because radvd refuses to advertise otherwise, and restores it on stop. --dad-collision claims wolfIP's address from the host while it is still tentative and asserts it is abandoned. Both end-to-end tests spawn tcpdump as the IPv4 interop tests do, writing ipv6-slaac.pcap and ipv6-ping.pcap. Capture uses -U so an aborted run still leaves a readable file, is stopped on exit, and is skipped with WOLFIP_NO_PCAP. The workflow gains radvd, tcpdump and iputils-ping, runs the four end-to-end tests under sudo and uploads the captures.
The "IPv4-only build is unchanged" step ran a bare make, which also builds the ESP, wolfGuard and supplicant examples. Those need a wolfSSL built from source with the right options, which is why linux.yml installs the nightly snapshot; against the packaged libwolfssl-dev src/wolfesp.c fails to compile. Both affected steps now build libwolfip.so, which is what they assert: that the library and the unit tests are unaffected with and without WOLFIP_IPV6. The examples stay covered by linux.yml.
src/wolfesp.c called wc_ForceZero() without including its header, and no include fixes it: wc_ForceZero() is a recent addition to memory.h and is absent from wolfSSL 5.6.6, which is what Ubuntu packages. The other spelling, ForceZero(), is the inline helper in wolfcrypt/src/misc.c, which libwolfssl-dev does not ship - misc.h only declares it under NO_INLINE - so a consumer of an installed wolfSSL can rely on neither. The build therefore succeeded or failed according to which wolfSSL was installed: it passes in linux.yml, which builds 5.9.x from source, and failed against the packaged 5.6.6. wolfIP_esp_forcezero() writes through a volatile pointer, which is what stops the compiler eliding the store and is what both wolfSSL helpers do. No version checks and no wolfSSL dependency. The IPv6 workflow gains a step building build/esp/wolfip.o, the object that failed. It compiles against whichever wolfSSL the distribution packages, whose header chain differs from the source build linux.yml uses, so it is the one environment in CI that catches this.
Four defects in the Neighbor Discovery timer handling. nd6_tick_cb() re-armed with the return of timers_binheap_insert() without checking it. That returns 0 when the heap is full and NO_TIMER is 0, so a momentarily full heap left the tick disarmed and stopped all Neighbor Discovery permanently: duplicate address detection never completing, router solicitation never retrying, nothing expiring, and no way to notice. With four TCP sockets able to hold three timers each, plus DHCP, DNS and IGMP, exhausting the sixteen slots is not far-fetched. nd6_poll(), called from wolfIP_poll(), now arms the tick whenever there is work and no timer running. That is the recovery path for a failed insert and the wake-up after an idle period. The tick also re-armed unconditionally, so once started it ran forever, waking every 100ms to scan five tables even with every interface stopped and nothing configured. It now re-arms only while nd6_has_work() holds: an interface started, a solicitation outstanding, an address tentative, a neighbour mid-resolution or ageing, or a prefix or router with a finite lifetime. The predicate is deliberately conservative, so a quiescent but populated cache keeps ticking rather than risking a stop with work queued. wolfIP_ipv6_stop() halts what the tick drives on an interface and drops anything still tentative, since detection never completed and the address was never ours. Addresses that had already passed are kept and still answered for. With no interface left running the tick releases its slot. nd6.tick_due and nd6.last_ns were declared and never used. Both removed. last_ns was to be the per-interface solicitation throttle mirroring arp.last_arp, but the only caller of nd6_send_ns() is duplicate address detection, which dad_due already paces; a comment records that a throttle is needed once address resolution is driven from the transmit path. nd6_arm_tick() treats a non-zero id as already running, so the tick has to clear the recorded id on entry: the heap has already popped the entry by then and the id is stale. Six tests for the lifecycle, which nothing covered before: that the tick is one slot for the whole stack rather than one per interface, that a full heap is recovered from, that stop releases the slot and drops a tentative address, and that a stopped interface restarts cleanly. Cancellation is lazy - timer_binheap_cancel() tombstones with expires = 0 and the slot is reclaimed by the drain in timers_binheap_insert() - so the tests assert the tick is disarmed and the slot reused, not that heap->size drops.
nd6_arm_tick() returned early when nd6.tick_timer was non-zero, which made that field double as an "am I already running" flag and left correctness depending on every path that pops or cancels remembering to clear it. It now cancels any live timer and inserts unconditionally, in the shape of dhcp_schedule_timer_at(): the id is simply overwritten and there is no flag to keep in sync. Calling it twice replaces the timer rather than leaving a stray entry in the heap. The tick still clears the recorded id on entry. The heap has already popped that entry, so the id is stale; clearing keeps the field truthful for the rest of the pass, gives nd6_arm_tick() nothing to cancel, and lets nd6_poll() see the tick as unarmed when a pass decides not to re-arm. test_nd_uses_one_timer_slot_for_the_whole_stack now counts live tick entries after a repeated start, rather than heap size: cancellation tombstones with expires = 0 and the slot is reclaimed later by the drain in timers_binheap_insert(), so size alone does not show whether two ticks are running.
Every board port ships its own config.h with the same WOLF_CONFIG_H guard and replaces the one at the top of the tree. The IPv6 sizing and feature macros were added only to the latter, so a port build never saw them, and src/wolfip.c refers to WOLFIP_IF_CONF_MAX and WOLFIP_IFADDR_MAX outside any WOLFIP_IPV6 guard: src/wolfip.c:6071: error: 'WOLFIP_IF_CONF_MAX' undeclared That broke every embedded port, the clang matrix and macOS. The IPv4 default build was unaffected, which is why it went unnoticed: the jobs that build ports only run on the pull request. The block now lives in wolfip6_config.h, included by src/wolfip.c immediately after the configuration header, so every configuration gets the defaults. Each macro stays #ifndef-guarded, so a config.h that sets one first still wins. wolfip.c also honours WOLFIP_CONFIG, naming an alternative configuration header, so a port or a test build can select one without editing the tree: -DWOLFIP_CONFIG='"myconfig.h"' Adds a CI step compiling wolfip.c against every src/port/*/config.h. All fourteen pass. That is cheap and catches this class of breakage in the IPv6 workflow rather than leaving it to the embedded jobs.
…advd helper Five review findings, all correct. README and CHANGELOG still described ICMPv6, Neighbor Discovery and SLAAC as unimplemented. They were written before those landed in this branch and understated it. Both now say what is implemented - Echo, address resolution, router discovery, duplicate address detection, SLAAC address formation - and what is not: AF_INET6 sockets, ICMPv6 error messages, extension headers, fragmentation, MLD and DHCPv6. README gains a row per protocol rather than one row hedged with a parenthesis. test_ipv6_ping.c installed a static neighbour entry and its header explained at length why one was needed. Neighbor Discovery now answers the host's solicitation, so the entry is unnecessary and the explanation was misleading. Both removed. The header now says what the test is for - ICMPv6 Echo with addresses configured directly - and points at test_ipv6_slaac.c for the case where nothing is configured. wolfip-radvd.sh forced net.ipv6.conf.<iface>.forwarding to 1 on start and to 0 on stop, so running it against an interface that already had forwarding enabled would silently turn it off, and the comment claiming it left the interface as it found it was wrong on two counts: it also left behind the address that start had added. It now saves the prior forwarding value and restores it, and removes the address.
The FreeBSD and macOS jobs run 1454 checks where Linux runs 1439. The difference is exactly the fifteen tests gated on WOLFIP_IF_MULTICONF, so those platforms are compiling that macro as 1 while Linux computes 0, and three of the tests fail there as a result. The macro is not reproducible locally: gcc and clang both compute WOLFIP_IF_MULTICONF=0 from config.h plus wolfip6_config.h, and the ordering inside that header is correct, with WOLFIP_IPV6 defined before the block that keys off it. Neither workflow passes extra flags. Rather than guess at a fix, the unit binary now prints the configuration it was built with, so the next run says which macro differs and by how much instead of leaving it to be inferred from a check count.
frame is a pointer parameter, so sizeof(frame) cleared eight bytes instead of the frame. The rest kept stack contents, and a non-zero flags_fo made ip_recv() drop the packet as a fragment. Linux hands back zeroed stack pages so it passed there; FreeBSD and macOS did not. Zero the frame being built, and set tos, id and flags_fo explicitly rather than relying on the caller's buffer.
Eight defects in the Neighbor Discovery and ICMPv6 receive paths:
- Tentative addresses answered ICMPv6 Echo. wolfIP_if_for_local_ip6() had
no state check, so an address still under duplicate address detection
was treated as ours (RFC 4862 section 5.4.5).
- Link-local addresses were matched on any interface. They are scoped to
the link they arrived on (RFC 4007 section 5).
- The same link-local address was rejected on a second interface. Only
the pair {address, zone} has to be unique.
- Neighbor Advertisements from the unspecified address updated the cache.
RFC 4861 section 7.1.2 requires a unicast source.
- A solicitation from the unspecified address was accepted without
checking it was sent to the target's solicited-node group and carried no
source link-layer address option, so a malformed one could invalidate a
tentative address (RFC 4861 section 7.1.1).
- Router Advertisement options took effect before the option area was
validated, leaving a router installed when a later option was malformed.
Framing is now checked in a first pass.
- Ethernet link-layer options were accepted with any length. RFC 2464
section 6 requires length 1.
- nd6_has_work() treated a started interface as work, so the tick never
quiesced. nd6_poll() re-arms when work appears, so it does not need to.
Six regression tests, and the malformed Router Advertisement test now
covers the option area rather than a single option.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #149
Scan targets checked: wolfip-bugs, wolfip-src
Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| (((t->local_ip == 0) && DHCP_IS_RUNNING(s)) || | ||
| (t->local_ip == dst_ip && peer_match)); | ||
| (t->local_ip == dst_ip && peer_match) || | ||
| ((t->local_ip != IPADDR_ANY) && |
There was a problem hiding this comment.
🟠 [Medium] Wildcard UDP match clause has no most-specific-match rule and fires for never-bound sockets · Logic errors
The new addr_match clause matches any socket with local_ip set and bound_local_ip == IPADDR_ANY, with no most-specific-match rule: a datagram to an alias is queued to both the alias-bound socket and a wildcard-bound socket on the same port. sendto() on an unbound socket sets local_ip implicitly, so never-bound sockets match too.
Related known finding #6037 (similar but distinct): Both concern UDP receive address matching in udp_try_recv, but #6037 omits broadcast-destination delivery while this wildcard clause over-delivers unicast alias traffic to wildcard or never-bound sockets. Their root causes and required predicate/state fixes differ.
Fix: Track an explicit "bound to wildcard" flag, and suppress the wildcard match when another socket is bound to the exact destination address.
| { | ||
| (void)s; | ||
| slot->info.state = WOLFIP_IFADDR_DEPRECATED; | ||
| slot->used = 0; |
There was a problem hiding this comment.
🔵 [Low] DAD failure is not recorded, so a duplicate SLAAC address is re-formed on every Router Advertisement · Logic errors
The address slot is freed without recording that the address was a duplicate, so nd6_recv_ra() re-forms the same prefix-derived address, re-adds it and restarts duplicate address detection each time the router readvertises the prefix, contrary to RFC 4862 section 5.4.5.
Fix: Record the failed address (or mark autoconfiguration failed on that interface) and refuse to re-form it from a later advertisement.
| /* Wipe key material. | ||
| * | ||
| * wolfSSL offers two spellings and neither is portable for a consumer of an | ||
| * installed library: wolfIP_esp_forcezero() is a recent addition to memory.h and is |
There was a problem hiding this comment.
⚪ [Info] ESP wipe-helper comment made self-referential by the rename · Copy-paste errors
The rationale comment states wolfIP_esp_forcezero() is a recent addition to memory.h and is absent from, for example, 5.6.6, which describes wolfSSL's wc_ForceZero(); the rename replaced the name being contrasted with the new local helper's own name, so the explanation no longer parses.
Fix: Restore wc_ForceZero() as the name referenced in the comment text.
| /* Adding it is a no-op when it is already there, so | ||
| * a repeated advertisement does not restart DAD. */ | ||
| if (nd6_slot_for(s, if_idx, &formed) == NULL) { | ||
| if (wolfIP_ifaddr_add6(s, if_idx, &formed, 64) == 0) { |
There was a problem hiding this comment.
🟠 [Medium] SLAAC addresses formed from a Router Advertisement never expire, permanently exhausting the per-interface address budget · Denial of service
nd6_recv_ra() forms an address per autonomous /64 prefix and adds it via wolfIP_ifaddr_add6(), which zeroes info.valid_lifetime; nd6_tick_cb() expires prefixes and routers but never addresses. Three RAs with distinct prefixes permanently fill WOLFIP_IF_CONF_MAX (4), after which the legitimate prefix can never be configured, and drain the shared WOLFIP_IFADDR_MAX pool used by other interfaces.
Fix: Record the option's valid/preferred lifetimes on the address slot and expire SLAAC addresses in nd6_tick_cb() alongside prefixes and routers.
| (((t->local_ip == 0) && DHCP_IS_RUNNING(s)) || | ||
| (t->local_ip == dst_ip && peer_match)); | ||
| (t->local_ip == dst_ip && peer_match) || | ||
| ((t->local_ip != IPADDR_ANY) && |
There was a problem hiding this comment.
🔵 [Low] UDP wildcard-delivery clause treats never-bound sockets as wildcard listeners · Missing input validation
The new clause gates on local_ip != IPADDR_ANY && bound_local_ip == IPADDR_ANY, but wolfIP_sock_connect() (line 6370) and wolfIP_sock_sendto() (line 6747) both set local_ip from the interface config without setting bound_local_ip. A UDP socket that only ever called connect()/sendto() therefore accepts datagrams addressed to any local address of the host, not just its own.
Related known finding #6037 (similar but distinct): Both affect UDP destination matching in udp_try_recv, but 6037 concerns failure to deliver broadcast datagrams to explicitly bound sockets, while this clause over-delivers local-address datagrams to sockets never bound. The root causes are distinct state semantics, and fixing this requires distinguishing explicit wildcard binding from an unbound socket without removing the broadcast-delivery behavior.
Fix: Add an explicit bound flag (or a distinct sentinel) set only by wolfIP_sock_bind(), and gate the wildcard clause on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.