Skip to content

fix(kernel): enforce the RFC 4861 hop-limit gate before NDP touches the cache - #181

Merged
kernalix7 merged 1 commit into
mainfrom
fix/ndp-hop-limit-gate
Aug 12, 2026
Merged

fix(kernel): enforce the RFC 4861 hop-limit gate before NDP touches the cache#181
kernalix7 merged 1 commit into
mainfrom
fix/ndp-hop-limit-gate

Conversation

@kernalix7

Copy link
Copy Markdown
Owner

Audit of crates/kernel/src/ipv6.rs ahead of wiring it into net.rs, per the
crates/kernel/AGENTS.md rule that an unwired parser must be audited before it
becomes attacker-reachable.

Audit result

The two classes the audit targeted came back clean:

Class Finding
Overflow / underflow Clean. opt_len_units * 8 ≤ 2040; IPV6_HEADER_LEN + payload_length ≤ 65575; the checksum accumulator peaks near 2.15e9 against a u32 ceiling of 4.29e9.
Unbounded loop Clean. parse_ndp_options advances ≥ 8 bytes per iteration (a zero length breaks out); the checksum loop advances by 2 on the skip branch as well as the normal one.

Both were already hardened by earlier passes. The defect is elsewhere.

The defect

RFC 4861 §7.1.1 and §7.1.2 require a Neighbor Solicitation or Advertisement to
be discarded unless the IP hop limit is 255. That one rule is what confines
NDP to the local link — 255 is the maximum a sender can set and every router
decrements, so a message still carrying 255 provably never crossed one.

hop_limit was parsed, stored and serialized, but never read on the receive
path
:

$ grep -n hop_limit crates/kernel/src/ipv6.rs
189:    pub hop_limit: u8,        # struct field
209:    let hop_limit = data[7];  # parse
259:    buf[7] = self.hop_limit;  # serialize
...                              # remainder: send-side constants only

This is not theoretical. NeighborTable::handle_advertisement already
implements the §7.2.5 Solicited/Override rules specifically to stop
neighbor-cache poisoning, and its SECURITY comment describes the on-link MITM
it prevents. Those rules assume the sender is on the link. Without the
hop-limit check that assumption is unfounded, so an off-link attacker — anywhere
routable — could drive the cache into exactly the state the §7.2.5 work was
written to prevent.

The ICMP code field was likewise never validated, and handle_neighbor_advertisement
accepted a multicast target.

Fix

  • ndp_message_is_valid, applied to the NS and NA arms of handle_icmpv6
    before either handler runs: hop limit 255 and code 0. Echo request/reply are
    deliberately not gated — the rule is specific to NDP.
  • Reject an NA whose target is multicast (§7.1.2). The target names the address
    being resolved, so a multicast value identifies no neighbor and only serves to
    steer an entry the sender does not own. The NS path already rejects these
    implicitly by requiring the target to equal our own address.
  • Both send paths now use the same NDP_HOP_LIMIT constant they were
    hardcoding, so transmit and receive cannot drift apart.

Tests

Six added. Four fail without the fix:

ns_that_crossed_a_router_is_dropped              FAILED -> ok
ns_with_nonzero_code_is_dropped                  FAILED -> ok
na_that_crossed_a_router_cannot_touch_the_cache  FAILED -> ok
na_for_multicast_target_is_dropped               FAILED -> ok

Two guard the legitimate path and pass both before and after
ns_at_hop_limit_255_is_still_answered and na_at_hop_limit_255_still_resolves.
That pairing is what shows the gate rejects forged messages rather than all of
them.

The shared ndp_packet helper always writes a correct checksum, so acceptance
turns solely on the field under test rather than on the existing checksum gate.

Verification

  • cargo fmt --all -- --check
  • cargo clippy --workspace -- -D warnings
  • cargo build --workspace
  • cargo build -p oncrix-kernel --bin oncrix-kernel --target aarch64-unknown-none
  • cargo build -p oncrix-kernel --bin oncrix-kernel --target riscv64gc-unknown-none-elf
  • bash scripts/run-tests.sh4593 passed, 0 skipped (kernel 353 → 359)

Follow-up

Wiring ETHER_TYPE_IPV6 into the net.rs ethertype match is deliberately left
to a separate PR, so the validation lands before the surface becomes reachable.

…he cache

Audit of `ipv6.rs` ahead of wiring it into `net.rs`.

The overflow/underflow and unbounded-loop classes came back clean:
`opt_len_units * 8` tops out at 2040, `IPV6_HEADER_LEN + payload_length` at
65575, and the checksum accumulator peaks near 2.15e9 against a u32 ceiling of
4.29e9. Every loop advances monotonically — `parse_ndp_options` moves at least 8
bytes per iteration because a zero length breaks out, and the checksum loop
advances by 2 on the skip branch as well as the normal one.

What is missing is the validity gate. RFC 4861 §7.1.1 and §7.1.2 require a
Neighbor Solicitation or Advertisement to be discarded unless its IP hop limit
is 255. That single rule is what confines NDP to the local link: 255 is the
maximum a sender can set and every router decrements, so a message still
carrying 255 provably never crossed one. `hop_limit` was parsed, stored and
serialized, but never once read on the receive path.

The consequence is not theoretical. `handle_advertisement` already implements
the §7.2.5 Solicited/Override rules specifically to stop neighbor-cache
poisoning, and its SECURITY comment describes the on-link MITM it prevents.
Those rules assume the sender is on the link. Without the hop-limit check that
assumption is unfounded, so an off-link attacker — anywhere routable — could
drive the cache and reach exactly the state the §7.2.5 work was written to
prevent.

Add `ndp_message_is_valid`, applied to the NS and NA arms of `handle_icmpv6`
before either handler runs, covering the hop limit and the code field that
RFC 4861 fixes at 0. Echo request and reply are deliberately not gated; the
rule is specific to NDP.

Also reject an NA whose target is multicast (§7.1.2). The target names the
address being resolved, so a multicast value identifies no neighbor and only
serves to steer an entry the sender does not own. The NS path already rejects
these implicitly by requiring the target to equal our own address.

The two send paths now use the same `NDP_HOP_LIMIT` constant they were
hardcoding, so the transmit and receive sides cannot drift apart.

Six tests. Four fail without the fix — off-link NS answered, off-link NA
resolving a pending entry, a nonzero code accepted, a multicast target
accepted. Two guard the legitimate path and pass both before and after, which
is what shows the gate rejects forged messages rather than all of them.

Suite: 4593 passed, 0 skipped.
@kernalix7
kernalix7 merged commit 6518812 into main Aug 12, 2026
3 checks passed
@kernalix7
kernalix7 deleted the fix/ndp-hop-limit-gate branch August 12, 2026 18:17
kernalix7 added a commit that referenced this pull request Aug 12, 2026
`ETHER_TYPE_IPV6` has been defined in net.rs since the constant block was
written, but the ethertype match never named it, so every IPv6 frame fell
through to `Err(NotImplemented)` and `ipv6.rs` had no caller at all.

Wire it up:

- `NetworkStack` gains an `Ipv6Stack`. Its address is not a new constructor
  parameter: RFC 4291 modified EUI-64 derives the `fe80::/64` address from the
  MAC, so `mac` already determines it and accepting a second value would only
  let the two disagree. `Ipv6Addr::link_local_from_mac` and `Ipv6Stack::new`
  are `const fn` so `NetworkStack::new` stays const.

- `handle_ipv6` adapts the two calling conventions. `Ipv6Stack::process_packet`
  returns `Option<usize>` over a bare IPv6 datagram; the net.rs handlers return
  a framed length with 0 meaning "consumed, no reply". A `None` therefore
  becomes `Ok(0)`, which keeps every drop — malformed, forged, or simply not
  for us — out of the error channel, matching `handle_arp` and `handle_ipv4`.
  The datagram is built in a scratch buffer first because the Ethernet header
  has to precede data whose length is not yet known, the same shape
  `handle_icmp_packet` already uses.

Three tests. `link_local_from_mac_follows_modified_eui64` pins the derivation,
including that the universal/local bit is inverted rather than set — the usual
way to get EUI-64 wrong, and one that would surface only as neighbor discovery
quietly failing on the wire.

The two net.rs tests drive a full exchange: a peer `Ipv6Stack` builds the
Neighbor Solicitation, so its checksum and hop limit come from the same code
that later validates them, and the frame comes back as a Neighbor
Advertisement addressed to the solicitor with its MAC learned.
`ipv6_frame_with_forwarded_hop_limit_is_dropped` sends that identical frame
with hop limit 64 and asserts `Ok(0)` and an untouched cache, proving the
RFC 4861 gate added in #181 survives the wiring. The checksum is deliberately
left intact there — the hop limit is not in the ICMPv6 pseudo-header — so the
test isolates the gate rather than the checksum.

Suite: 4596 passed, 0 skipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant