ipv6: send ICMPv6 Redirects only to neighbours, with a link-local target and at a limited rate

[adamgeorge309]

An Internet Protocol version 6 (IPv6) router sends Redirects of the Internet Control Message Protocol for IPv6 (ICMPv6) that Request for Comments (RFC) 4861 Section 8.2 does not allow, in numbers it does not allow. Three independent defects, one commit each.

Closes #1233
Closes #1234
Closes #1235

The problem

hostA -- r0 -- [switch: r0, r1, r2] -- r2 -- hostB. r0 routes hostB's prefix via r1, and r1 routes it via r2's link-local address, back out of the interface the datagrams arrived on. hostA pings hostB for 10 s at 20 echo requests per second. On master:

Router	Redirects	Sent to	Target Address
r1	200, one per echo request	hostA, two hops away	r2's link-local address
r2	200, one per Redirect of r1, which r2 forwards	r1's link-local address	r0's global address
r0, r1, r2	8	routers on the switch	the destination

None of the 408 Redirects changes a destination cache: hostA discards r1's because their source is not its next hop, and r1 discards r2's because their Target Address is a global address.

The defects

The source of the datagram must be a neighbour (#1233)

Ipv6::routePacket() sent a Redirect whenever the output interface was the arrival interface. RFC 4861 Section 8.2 also requires that "the Source Address field of the packet identifies a neighbor". The Redirect is now sent only when the source is link-local, or a longest prefix match on it gives an on-link route out of the arrival interface, which is how next-hop determination (Section 5.2) decides on-link. In the topology above, r1 and r2 no longer send the 400 Redirects; 8 are left.

The Target Address of a router must be its link-local address (#1234)

Ipv6NeighbourDiscovery::sendRedirect() copied the route's next hop into the Target Address unchanged. A route configured with a global gateway produced a global Target Address. Sections 4.5 and 8.2 require the router's link-local address, and processRedirectPacket() discards anything else. The router now sends no such Redirect: a global next hop does not give the router's link-local address, and sending a Redirect is a SHOULD.

In a second topology hostA, r1 and r2 share the switch and hostB is behind r2. When both routes name their gateway by a global address, r1 sends hostA 200 Redirects with Target Address 2001:db8:2::3 on master, and hostA discards all of them. With this commit r1 sends none of them. When both routes name their gateway by a link-local address, r1 sends one Redirect, and hostA accepts it and sends straight to r2; this is the same before and after the change.

Redirects must be rate limited (#1235)

RFC 4861 Section 8.2 says a router MUST limit the rate of Redirects; nothing did. Ipv6NeighbourDiscovery now meters Redirects through one token bucket per node, the method RFC 4443 Section 2.4 (f) recommends. Two new parameters set it: redirectRate (default 10Hz) and redirectBurst (default 10), the example values of that section.

In the second topology, r1's route names r2 by its link-local address, so r1's Redirects are well formed. hostA's route names r1 by its global address, so hostA discards them, because their link-local source is not its next hop, and it keeps sending through r1. On master r1 sends 200 Redirects in 10 s. With this commit it sends 108: a burst of 10, then 10 per second.

Architectural surface

• Configuration: two new parameters on Ipv6NeighbourDiscovery, redirectRate and redirectBurst.
• Contracts: Ipv6NeighbourDiscovery::sendRedirect() may now return without sending. Its only caller is Ipv6::routePacket(), which forwards the datagram either way.
• Packet content: none. The Redirect message is unchanged; fewer are sent.
• Feature descriptors: none.
• No sealed path is touched, and no architecture or naming exception is added.

Verification

New module tests. Each one fails on unmodified master and passes from its commit on:

• IPv6_redirect_source_not_neighbour.test (ipv6: a router sends an ICMPv6 Redirect to a source that is not a neighbour #1233): the first topology
• IPv6_redirect_target_not_link_local.test (icmpv6: the Target Address of a Redirect to a router is the route's next hop as configured, not the router's link-local address #1234)
• IPv6_redirect_rate_limit.test (icmpv6: Redirect messages are not rate limited #1235): burst 5, rate 2 per second, 40 echo requests over 1.95 s, 8 Redirects

Build: make -j4 MODE=release. Commands, run on master before the change and on each of the three commits:

cd tests/fingerprint && ./fingerprinttest -s -F tyf -t 4
cd tests/module && inet_run_module_tests -m release --no-build -l ERROR
cd tests/protocol/ipv6 && inet_run_protocol_tests -m release --no-build -l ERROR

-F tyf leaves out the graphical fingerprint ingredients, so the tyf values were not checked by these runs.

	Fingerprints (1774)	Module tests
master	0 failures, 62 errors	300 pass, 49 fail, of 349
first commit	0 failures, 62 errors	301 pass, 46 fail, of 347
second commit	0 failures, 62 errors	302 pass, 46 fail, of 348
third commit (tip)	0 failures, 62 errors	303 pass, 46 fail, of 349

The first commit re-records six rows: the three Mobile IPv6 (MIPv6) configurations above, in examples.csv and mipv6-refactoring.csv. Its message gives the reason per ingredient. Their tyf values are left as recorded: run separately, all three already fail on master, so they are stale before this change and are not re-recorded here. The second and third commits move no row. The 62 fingerprint errors are the same on master and every commit, all of them disabled optional features (VoipStreamSender, TcpLwip, Z3GateScheduleConfigurator, the OpenSceneGraph visualizer networks).

On master the module tests that fail are the 3 new tests and 46 others. The same 46 fail on the tip, so they are out of scope: IPv6_packet_too_big, MIPv6_tcp_handover, 34 tcp_* tests (their postrun script finds no [testing]: lines in the output), ConvolutionalCoder12, ConvolutionalCoder34, EtherHost_lifecycle, ExternalProcess_3, Ieee80211BitDomain, Ieee80211SymbolDomain, Ieee8021d-Rstp, Ieee8021d-Stp, PacketGate_1 and UDPSocket_1.

The tip was also built in debug mode, with make MODE=debug -j4 at the repository root and in tests/module/lib, exit 0, with no compiler warning in the changed files. On that build, cd tests/module && inet_run_module_tests -m debug --no-build -l ERROR -f 'redirect|Redirect|MIPv6' ran all 349 module tests (the filter did not narrow the selection) and all 349 pass, exit 0: the four IPv6_redirect* tests, the eleven MIPv6_* tests, and also IPv6_packet_too_big, MIPv6_tcp_handover and the 34 tcp_* tests that fail in the release build on master and on the tip.

The IPv6 protocol tests give the same result on master and on each commit. Rfc8200OverlappingFragments fails on all of them.

doc/project/enforcement/check-commits.sh, check-classification.sh, check-ned-params.sh and check-seals.sh pass. check-architecture.sh passes on src/inet/networklayer/ipv6 and src/inet/networklayer/icmpv6. check-source-seals.sh --base origin/master passes. check-naming.sh and check-interfaces.sh report the same candidates on master and on the tip; the names of the new IPv6_*.test files fall under the existing exception NV-16.

Relationship to #1197, #1151 and #1152

The first commit also stops the spurious Redirects reported in #1197. #1197 stays open for its tunnel-attribution fix. The source of a decapsulated, reverse-tunneled datagram is the mobile node's home address, which is on-link on the home link and not on the interface the tunneled datagram arrived on. On master the home agent sends 2 such Redirects in examples/ipv6/mipv6roaming -c Roaming, 43 in examples/ipv6/mipv6 -c Handover and 4 in -c RouteOptimizationTwoCNs. With this commit it sends none. #1197 proposes a different fix, which routes a decapsulated datagram as arriving from the tunnel interface; that fix and this one touch different functions and do not conflict in the source.

Redirects are sent with a pinned output interface, so on master they go to the Ethernet broadcast address (#1151). Neither fix depends on the other. With #1152 applied as well, the first topology sends 1 Redirect instead of 8. In the second topology, when hostA discards r1's Redirects, the tip sends 109 Redirects instead of 218, all from r1 (108 without #1152): r2 no longer receives r1's Redirects and answers each with one of its own. The sources merge without conflict. The six MIPv6 fingerprint rows conflict and need re-recording after whichever of the two lands second.

Not addressed here

• A Redirect carries neither the Target Link-Layer Address option nor the Redirected Header option (Section 4.5). The message is always the 40-byte fixed part.
• A router forwards a datagram with a link-local source address to another link, which RFC 4291 Section 2.5.6 forbids. On master, in the first topology with global gateways, r0 forwarded r1's Redirects, sent from r1's link-local address to hostA, onto hostA's link. After the first commit no Redirect is addressed to a node that is not on the link, but the forwarding rule is unchanged.

---

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🟡 On-link senders lose valid redirects

When an on-link sender has a more-specific route through a gateway, srcIsNeighbour becomes false. doLongestPrefixMatch selects that gateway over the connected prefix, so the router never redirects that sender.

Learn more

A Redirect is useful when the packet's source is on the arrival link and the router forwards the packet back out that link. doLongestPrefixMatch returns only the best matching route, so an unrelated more-specific route can hide a still-valid connected prefix. The new neighbour check then denies a Redirect to a source that is actually on-link. This can occur with manual, dynamic, or host-specific routes overlapping a connected prefix.

Example: Router R has a direct route for 2001:db8:2::/64 on eth0 and a /128 route for host A's 2001:db8:2::10 through another gateway. A sends R a packet that R forwards back onto eth0 via router B. The /128 wins the lookup, and R sends no Redirect to A, although A and B share eth0.

Recommended fix: Determine whether any valid on-link prefix on fromIE contains the source, independently of the single best forwarding route. Include directly connected static, advertised, and configured prefixes as appropriate; retain the link-local case and the original source-address guards.

---

Was this helpful? React with 👍 or 👎 to provide feedback.