- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 2.2k
 
Fallback to IPv6 default routes for network interface detection #4321
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
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)
| 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?