Skip to content

Conversation

itchenfei
Copy link

Summary

This PR adds an optional strict mode to scapy.layers.inet6.neighsol() to validate the IPv6 Hop Limit (HLIM) == 255 on received Neighbor Advertisement (NA) replies. When enabled, the function first narrows capture with a BPF filter and then performs a second, protocol-level check in Python before returning the packet.

Motivation / Rationale

Per RFC 4861 (Neighbor Discovery for IPv6), ND control messages MUST be sent with Hop Limit = 255, and receivers MUST verify HLIM=255 to prevent off-link spoofing. Today neighsol() sets hlim=255 on the sent NS but does not validate HLIM on received NA packets. This can accept responses that traversed a router (HLIM<255) or were injected off-link. Enforcing (optionally) HLIM==255 improves standards compliance and hardens neighbor discovery.
(See Scapy’s contribution guidelines for tests and coding style; tox instructions used for local testing. )

@gpotter2 gpotter2 requested a review from Copilot August 27, 2025 08:57
@gpotter2 gpotter2 added the bug label Aug 27, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds optional strict validation for IPv6 Neighbor Discovery Protocol (NDP) by implementing Hop Limit (HLIM) verification in the neighsol() function. The change enforces RFC 4861 compliance by adding a BPF filter to validate that Neighbor Advertisement replies have HLIM=255, which prevents acceptance of spoofed responses from off-link sources.

  • Added BPF filter "ip6[7] == 255" to validate IPv6 Hop Limit field in received packets
  • Improved security posture by rejecting packets that may have traversed routers (HLIM<255)
  • Enhanced RFC 4861 compliance for Neighbor Discovery protocol implementation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

codecov bot commented Aug 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.90%. Comparing base (29433fc) to head (ce406ff).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4829      +/-   ##
==========================================
- Coverage   80.90%   80.90%   -0.01%     
==========================================
  Files         366      366              
  Lines       90132    90132              
==========================================
- Hits        72921    72918       -3     
- Misses      17211    17214       +3     
Files with missing lines Coverage Δ
scapy/layers/inet6.py 88.57% <ø> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guedou
Copy link
Member

guedou commented Aug 27, 2025

Do you have a use case in mind that requires this check in BPF?

The hop limit check could be done in pure Python.

@guedou guedou marked this pull request as draft August 27, 2025 09:28
@itchenfei
Copy link
Author

Do you have a use case in mind that requires this check in BPF?

The hop limit check could be done in pure Python.

Thanks a lot for the suggestion, @guedou! I refactored the change to remove the BPF dependency and do the hop-limit validation in pure Python.
Concretely: I switched neighsol() to srp and added a stop_filter so we only treat responses as valid when ICMPv6ND_NA is present and IPv6.hlim == 255. Please take another look.

@guedou
Copy link
Member

guedou commented Aug 27, 2025

Is there a need for that change ?

@itchenfei
Copy link
Author

Is there a need for that change ?

image This is not a real world ipv6 address, so i directly paste this image.

This is Qualcomm Device Dial NIC and response 2 NA messages.
If don't switch from srp1 to srp, will get wrong packet.

Do you have any other better suggestions?

@itchenfei
Copy link
Author

Is there a need for that change ?

image This is not a real world ipv6 address, so i directly paste this image.
This is Qualcomm Device Dial NIC and response 2 NA messages. If don't switch from srp1 to srp, will get wrong packet.

Do you have any other better suggestions?

In this scenario, using srp1 with a stop_filter still returns the wrong Ethernet frame.
Using srp without a stop_filter waits until the timeout occurs.
The reliable approach is to use srp with a stop_filter and then iterate over the answers to select the first compliant packet (e.g., ICMPv6ND_NA with IPv6.hlim == 255).

@gpotter2
Copy link
Member

gpotter2 commented Aug 28, 2025

I personally disagree with @guedou, the BPF idea was smart, and I feel more elegant

@guedou
Copy link
Member

guedou commented Aug 28, 2025

For readability, lfilter that only check the hlim value seems like a good choice.

I am unsure that we need any performance improvement there.

@itchenfei I think that what you really need is to filter hlim == 255. Scapy should take care of the rest.

@guedou
Copy link
Member

guedou commented Aug 28, 2025

@itchenfei can you share the complete neighsol() call that you used?

@itchenfei
Copy link
Author

@itchenfei can you share the complete neighsol() call that you used?

I just sent an ICMPv6 packet, and the NDP was triggered automatically.

import os

from scapy.arch import get_if_addr6
from scapy.layers.inet6 import IPv6, ICMPv6EchoRequest
from scapy.sendrecv import sr1
from scapy.config import conf

PING_DEST = "2001:da8:d800:642::248"
PING_INTERFACE = "eth0"
packet_id = os.getpid() & 0xFFFF

ip_layer = IPv6(
    dst=PING_DEST,
    src=get_if_addr6(PING_INTERFACE)
)
protocol_layer = ICMPv6EchoRequest(
    data="A" * 50,
    id=packet_id,
    seq=1,
)
packet = ip_layer / protocol_layer

reply = sr1(packet, timeout=1, verbose=0)
if reply:
    reply.show()

print(f"ipv6 neighbor: {conf.netcache.in6_neighbor}")

@itchenfei
Copy link
Author

I personally disagree with @guedou, the BPF idea was smart, and I feel more elegant

For readability, lfilter that only check the hlim value seems like a good choice.

I am unsure that we need any performance improvement there.

@itchenfei I think that what you really need is to filter hlim == 255. Scapy should take care of the rest.

I’m not sure whether BPF will have any compatibility issues on the NT kernel, but using sniff right now seems easier to understand and maintain. c588598

…on │

│                                                                                                                                        │
│   Replace store=True with chainCC parameter in sniff() call and change default chainCC value from 0 to False to match scapy            │
│   conventions.
@guedou
Copy link
Member

guedou commented Aug 29, 2025

@itchenfei your code is not calling neighsol()at all.

The following works fine for me: neighsol("fe80::ba26:6cff:fe5f:4eee", "fe80:1d::1c61:245:1e6e:8092", "en7")

@itchenfei
Copy link
Author

@itchenfei your code is not calling neighsol()at all.

The following works fine for me: neighsol("fe80::ba26:6cff:fe5f:4eee", "fe80:1d::1c61:245:1e6e:8092", "en7")

When an ICMPv6 packet is sent, NDP is automatically triggered and the neighbor’s MAC address is appended as part of the ICMPv6 packet.

@guedou
Copy link
Member

guedou commented Aug 30, 2025

@itchenfei thanks for your answer. Can you share a pcap files containing the NS / NA that were not matched?

Your initial fix looks like the more efficient (i.e. using a BPF filter), but I suspect a deeper bug in Scapy. Sorry for the back and forth, I should have made it clear that I am trying to understand for the root cause.

@itchenfei
Copy link
Author

@itchenfei thanks for your answer. Can you share a pcap files containing the NS / NA that were not matched?

Your initial fix looks like the more efficient (i.e. using a BPF filter), but I suspect a deeper bug in Scapy. Sorry for the back and forth, I should have made it clear that I am trying to understand for the root cause.

Thanks for digging into this! I’ve zipped the capture since GitHub only accepts archives:

icmp6.zip

pcap: icmp6.zip (attached above)
It contains an NS followed by two NAs where the first has IPv6.hlim = 254 and the second IPv6.hlim = 255.

Finally, big thanks to all maintainers for the helpful feedback and for keeping Scapy efficient and reliable—digging into this taught me a lot about how filter and send/receive paths work, which will definitely help my future usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants