Skip to content
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

[IPv6] Windows: fix IPv6 routing + Net6 #644

Merged
merged 11 commits into from
Sep 17, 2017
Merged

Conversation

gpotter2
Copy link
Member

@gpotter2 gpotter2 commented May 4, 2017

This PR:

  • fix show() can't resolve Net6 hostname to ip6 address #643 + bugs created when using Net6
  • fix the pcapdnet in6_getifaddr function: replace the windows one by the Pcap API
  • rename the ipaddress regex (the name was really confusing)
  • filters globals ipv6 addresses out of the possible source candidates on Windows. ==> windows now correctly sends/recieves IPv6 packets
  • fixes LOOPBACK_INTERFACE not beeing properly updated, causing IPv6 routes beeing outdated
  • fixes in6_getifaddr function of pcapdnet.py ==> now use it when loading IPv6

@guedou
Copy link
Member

guedou commented May 5, 2017

Why did you close this PR ?

@gpotter2 gpotter2 reopened this May 5, 2017
@gpotter2
Copy link
Member Author

gpotter2 commented May 5, 2017

My bad

@codecov-io
Copy link

codecov-io commented May 5, 2017

Codecov Report

Merging #644 into master will increase coverage by 0.04%.
The diff coverage is 69.69%.

@@            Coverage Diff             @@
##           master     #644      +/-   ##
==========================================
+ Coverage   80.88%   80.93%   +0.04%     
==========================================
  Files         140      140              
  Lines       34572    34568       -4     
==========================================
+ Hits        27964    27978      +14     
+ Misses       6608     6590      -18
Impacted Files Coverage Δ
scapy/arch/__init__.py 90% <100%> (ø) ⬆️
scapy/layers/inet.py 71.45% <100%> (+0.04%) ⬆️
scapy/utils.py 78.39% <100%> (-0.31%) ⬇️
scapy/consts.py 66.66% <100%> (+0.59%) ⬆️
scapy/pton_ntop.py 97.33% <100%> (+0.07%) ⬆️
scapy/all.py 100% <100%> (ø) ⬆️
scapy/layers/l2.py 81.6% <100%> (-0.87%) ⬇️
scapy/layers/inet6.py 77.68% <35.29%> (-0.26%) ⬇️
scapy/route6.py 87.91% <66.66%> (ø) ⬆️
scapy/base_classes.py 85.71% <66.66%> (+0.22%) ⬆️
... and 8 more

@guedou
Copy link
Member

guedou commented May 5, 2017

Could you remove the changes related to the renaming of ipaddress, or move it to a separate commit ?
Also, can you add a unit test that triggers the issue.

Thanks.

@gpotter2 gpotter2 force-pushed the fix-net6 branch 3 times, most recently from f2daa4f to 9e6433d Compare May 5, 2017 14:41
@gpotter2
Copy link
Member Author

gpotter2 commented May 5, 2017

@guedou Here's an updated version.

@guedou
Copy link
Member

guedou commented May 5, 2017

Thanks. Could you also add something like the following, that initialy triggered the issue ?

p = Ether()/IPv6(dst="www.kame.net")/TCP() assert p.dst == "ff:ff:ff:ff:ff:ff"

@gpotter2
Copy link
Member Author

gpotter2 commented May 5, 2017

Modified it a bit.

@gpotter2 gpotter2 force-pushed the fix-net6 branch 2 times, most recently from a67f2f7 to bb5b2ac Compare May 5, 2017 17:54
@gpotter2
Copy link
Member Author

gpotter2 commented May 5, 2017

@guedou Just discovered a new bug:

>>> a = IPv6(dst="www.google.fr")/ICMPv6EchoRequest()
>>> b = IPv6(src="www.google.fr", dst=a.src)/ICMPv6EchoReply()
>>> b.answers(a)
Traceback (most recent call last):
  File "<console>", line 2, in <module>
  File "scapy\layers\inet6.py", line 489, in answers
    ss = inet_pton(socket.AF_INET6, self.src)
  File "scapy\pton_ntop.py", line 82, in inet_pton
    return _INET_PTON[af](addr)
  File "scapy\pton_ntop.py", line 28, in _inet6_pton
    if addr.startswith('::'):
AttributeError: 'Net6' object has no attribute 'startswith'

Sorry if the PR suddenly became big, explanation:

  • I added the tests, which triggered a bug
  • I fixed this new bug, by surrounding the src and dst with str()
  • Even with that, the bug was not fixed and Pinging with IPv6 on windows still did not work.
  • I discovered that some IPv6 possible candidates were wrong. Fixed that
  • The tests failed because the LOOPBACK_* stuff were not updated. Corrected it using static access
  • Lastly, the dummy interface had the same GUID than the one in route_add_loopback, which triggered a new bug, that i fixed.

I don't think this should be splitted in many PRs, as it all comes from the same bug... More over the PR is easy to understand.

@gpotter2 gpotter2 force-pushed the fix-net6 branch 4 times, most recently from 5ec712e to e5e3b13 Compare May 6, 2017 01:08
@gpotter2 gpotter2 changed the title [IPv6] Fix Net6 [IPv6] Multiple fixes (Net6) May 6, 2017
@gpotter2 gpotter2 force-pushed the fix-net6 branch 4 times, most recently from b935641 to 9362d00 Compare May 17, 2017 11:54
@gpotter2 gpotter2 force-pushed the fix-net6 branch 3 times, most recently from 83d31fe to 10fe4aa Compare June 1, 2017 18:03
@gpotter2 gpotter2 force-pushed the fix-net6 branch 5 times, most recently from 2d97224 to fe2ba93 Compare August 27, 2017 12:09
@guedou
Copy link
Member

guedou commented Aug 28, 2017

It looks to me. @p-l- do you want to have another look ?

@gpotter2 gpotter2 force-pushed the fix-net6 branch 5 times, most recently from 4832e96 to 37690d4 Compare September 2, 2017 17:54
@gpotter2
Copy link
Member Author

gpotter2 commented Sep 5, 2017

@p-l- Hi, do you have any more comments about this one ?

@p-l-
Copy link
Member

p-l- commented Sep 17, 2017

Sorry @gpotter2 can you rebase against current master?

@gpotter2
Copy link
Member Author

@p-l- Done

@p-l- p-l- merged commit 08b0fab into secdev:master Sep 17, 2017
@gpotter2 gpotter2 deleted the fix-net6 branch September 17, 2017 20:15
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.

show() can't resolve Net6 hostname to ip6 address
5 participants