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

Fix IPv6 for domain names #4

Merged
merged 2 commits into from
Sep 8, 2016
Merged

Fix IPv6 for domain names #4

merged 2 commits into from
Sep 8, 2016

Conversation

grongor
Copy link
Owner

@grongor grongor commented Sep 8, 2016

Resolves #3

if self.use_udp:
s.send(b'')
s.sendto(b'', socket_address)

Choose a reason for hiding this comment

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

Just tested and received the following error. I'm investigating, but sharing in case you have any immediate insight:

C:\Users\martin\Desktop>knock.py -u ipv6.netconfigs.com 12345
Traceback (most recent call last):
  File "C:\Users\martin\Desktop\knock.py", line 68, in <module>
    Knocker(sys.argv[1:]).knock()
  File "C:\Users\martin\Desktop\knock.py", line 56, in knock
    s.sendto(b'', socket_address)
OSError: [WinError 10057] A request to send or receive data was disallowed because the socket is not connected and (when sending on a datagram socket using a sendto call) no address was supplied

C:\Users\martin\Desktop>

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, of course, second argument is flags, try putting None as second argument ... Strange thing is that on my system it didn't throw the error. The socket implementations are really strangely inconsistent.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh, now I see that it's possible to pass just the address ... well, try passing the None - maybe it's a Windows-thing

@mhmeadows63
Copy link

Got it:
lines 38,39

        self.address_family, self.socket_type, _, _, socket_address = socket.getaddrinfo(
                host=args.host,
                port=None,
-               type=0,
+               type=socket.SOCK_DGRAM if self.use_udp else socket.SOCK_STREAM,
-               proto=socket.IPPROTO_UDP if self.use_udp else socket.IPPROTO_TCP,
+               proto=0,
                flags=socket.AI_ADDRCONFIG
            )[0]

host=args.host,
port=None,
type=0,
proto=socket.IPPROTO_UDP if self.use_udp else socket.IPPROTO_TCP,

Choose a reason for hiding this comment

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

should be

                type=socket.SOCK_DGRAM if self.use_udp else socket.SOCK_STREAM,
                proto=0,

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, I actually spend a few minutes thinking about those two ... what's the difference? To me it looked like proto is kind of more general or something like that ... but I'm not really sure and obviously it matters a lot.

Choose a reason for hiding this comment

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

The type and proto inputs to socket.getaddrinfo() seem to emerg as items 2 and 3 in the tuple. Not sure they affect anything else, but the call to socket.socket() need the SOCK_DGRAM or SOCK_STREAM to create the right type. As you passed in type=0 to socket.getaddrino(), that was arriving at socket.socket() and like creating a SOCK_STREAM by default,

BTW, I'm testing on Windows 10 Python 3.5.2 against a CentOS6 linux and using tcpdump to verify the packet arrival.

My server has 3 types of addresses, IPv4, IPv6 and 6to4 (a transition variant of IPv6):

  1. ipv4.netconfigs.com
  2. ipv6.netconfigs.com
  3. 6to4.netconfigs.com

Copy link
Owner Author

Choose a reason for hiding this comment

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

Strange ... alright, I changed it to filter by type instead - I'm happy that it works for you. Thank you very much for the thorough testing :)

@mhmeadows63
Copy link

I've just done a full matrix of tests of my two line fix suggestion:

  1. IPv4/6to4/IPv6 hostnames for TCP/UDP (6 tests)
  2. IPv4/6to4/IPv6 addresses for TCP/UDP (6 tests)
    All passed

@grongor grongor merged commit b68e2bc into master Sep 8, 2016
@grongor grongor deleted the fix-ipv6-for-domain-names branch September 8, 2016 16:26
@grongor grongor added the bug label Sep 8, 2016
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.

None yet

2 participants