-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fallback to IPv6 default routes for network interface detection #4321
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4321 +/- ##
==========================================
- Coverage 82.20% 78.53% -3.67%
==========================================
Files 353 354 +1
Lines 83529 92937 +9408
==========================================
+ Hits 68662 72985 +4323
- Misses 14867 19952 +5085
|
c2745fc
to
0087680
Compare
I would argue this needs tests considering it's already a regression. Also you need to rebase ;p |
c21e446
to
425690a
Compare
This makes me kinda wonder why we deprecated |
I always found conf.iface6 confusing.
It makes sense to use it to control how IPv6 packets are built, but fe80::/64 and multiple interfaces break its usefulness.
However, for sending with high-level functions or SuperSocket, it adds complexity and non-standard behavior.
When iface= is not specified by the user, shall Scapy use conf.iface6, conf.iface or both ? It gets even more complicated if we consider that sr*() can receive a list of IP and IPv6 packets that must be sent to different interfaces.
|
425690a
to
4447e20
Compare
c1d43f4
to
478cb0c
Compare
Hey, I only noticed this after I opened my own PR #4380. |
478cb0c
to
70c08cc
Compare
70c08cc
to
6a4d90a
Compare
@oskar456 sorry for the delay. I slightly modified this PR and it should now work as expected. Do you have time to test it? |
Hey, thanks a lot! I can confirm that this PR fixes #4304. |
So we have to discuss a bit regarding this. Regarding link-local / multicast destinations, #4461 should properly add a route on interfaces that support multicast, so that should supposedly also fix the OP's question (as they only have a single interface with multicast). It also allows for a much more flexible behavior, which aims at avoiding to have to change |
.. note:: | ||
Scapy automatically detects the network interface to be used by default, and stores this result in ``conf.iface``. Packets built by Scapy uses this variable to set relevant fields such as Ethernet source addresses. When sending packets, with functions such as ``send()``, Scapy will use the network interface stored in ``conf.iface``. This behavior can be changed using the ``iface=`` argument. With IPv6 and link-local addresses, it is mandatory to setup both ``conf.iface`` and ``iface=`` the same value to get the desired result, as Scapy cannot find which interface to use for link-local communications. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I disagree with that now that we have Scoped IPs.
The PR is still useful though, I think having a better default conf.iface
is good, especially for L2 functions. (e.g. sniff)
@@ -360,3 +360,6 @@ def route(self, dst="", dev=None, verbose=conf.verb): | |||
|
|||
|
|||
conf.route6 = Route6() | |||
|
|||
# Reload interfaces | |||
conf.ifaces.reload() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a bit crappy. We only need to reload the default interface, not the entire interface list
This PR improves the selection of the default interface in an IPv6-only environment. See #4304 for context
To maintainers: shoud I add unit tests?