Skip to content

fix(net): catch IPv4-mapped blocked ranges in is_always_blocked_net#1032

Merged
johntmyers merged 2 commits intoNVIDIA:mainfrom
mesutoezdil:fix/net-ipv4-mapped-blocked-ranges
Apr 29, 2026
Merged

fix(net): catch IPv4-mapped blocked ranges in is_always_blocked_net#1032
johntmyers merged 2 commits intoNVIDIA:mainfrom
mesutoezdil:fix/net-ipv4-mapped-blocked-ranges

Conversation

@mesutoezdil
Copy link
Copy Markdown
Contributor

Summary

is_always_blocked_net only checked whether the network address of an IPv6 prefix mapped to a blocked IPv4 address. That misses cases where the network address is public but the range extends into a blocked zone. For example, ::ffff:168.0.0.0/103 starts at 168.0.0.0 so the old check passes it, but the range covers up to 169.255.255.255 and includes ::ffff:169.254.0.0. The result is that parse_allowed_ips accepts the CIDR at policy load time while every connection to it fails silently at runtime.

The fix adds containment checks for the three IPv4-mapped blocked representatives: loopback, link-local, and unspecified. The existing network-address check stays because it still handles /128 entries whose first address is already in a blocked range.

Related Issue

N/A

Changes

  • Add v6net.contains() checks for ::ffff:127.0.0.1, ::ffff:169.254.0.0, and ::ffff:0.0.0.0 after the network-address check in the IPv6 branch of is_always_blocked_net
  • Use Ipv4Addr::LOCALHOST instead of Ipv4Addr::new(127, 0, 0, 1) and collapse nested if let / if into is_some_and to satisfy clippy
  • Five new tests: single-host loopback and link-local mapped addresses, broad prefixes that span each blocked range from a public start, and a public /128 that must not be blocked

Testing

  • cargo test -p openshell-core --lib net passes (38 tests)
  • cargo clippy -p openshell-core passes, no new warnings
  • cargo fmt --check -p openshell-core passes
  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (not applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable)

The IPv6 branch only checked whether the network address itself mapped
to a blocked IPv4 address. A broader prefix like ::ffff:168.0.0.0/103
has a public network address but spans ::ffff:169.254.0.0, so the old
code accepted it at policy load time while is_always_blocked_ip silently
rejected every connection at runtime.

Add three containment checks for the IPv4-mapped loopback, link-local,
and unspecified representatives. The existing network-address check is
kept because it handles single-host entries (/128) whose network address
is already in a blocked range.

Five new tests cover: single-host loopback and link-local mapped
addresses, broad prefixes that span each blocked range without starting
there, and a public single-host address that must not be blocked.
Use Ipv4Addr::LOCALHOST instead of Ipv4Addr::new(127, 0, 0, 1) and
collapse the nested if let / if into is_some_and.
@mesutoezdil mesutoezdil requested a review from a team as a code owner April 29, 2026 07:27
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 29, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@mesutoezdil
Copy link
Copy Markdown
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@mesutoezdil
Copy link
Copy Markdown
Contributor Author

recheck

@johntmyers johntmyers self-assigned this Apr 29, 2026
@johntmyers johntmyers merged commit 4510b0d into NVIDIA:main Apr 29, 2026
12 of 13 checks passed
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.

2 participants