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

Add IPv6 support #668

Closed
wants to merge 2 commits into from

Conversation

richardleach
Copy link

Main changes:

In LW2.pm:

  • New $IPv4_re, $IPv6_re, $IPv6_re_inc_zoneid compiled regular expressions; can be used in other plugins with the $LW2:: prefix (e.g. $LW2::IPv4_re)
  • New $LW2_CAN_IPv6 variable reflecting whether the version of Socket.pm supports IPv6. It does not reflect whether the host OS and connectivity to the target support IPv6! Can also be used elsewhere with $LW2:: prefix.
  • When Socket.pm supports IPv6, both IPv4 and IPv6 name resolutions use getaddrinfo(). Otherwise the longstanding socket code is used.

In nikto_core.plugin:

  • Target format in %targs changed from host:port to host_port, since colons are used in IPv6 addresses
  • get_ips and is_ip modified to support IPv6; not sure if/when zone ids or prefixes will be relevant here, so some finessing may be required down the line.
  • The $LW2::IPv4_re regex is more accurate than the "simple syntax check" previously in is_ip(). The octet validation block may well no longer be needed, but I left it unchanged for now.
  • Various other modifications to support IPv6 addresses

In nikto_fileops.plugin:

  • Couple of modifications to support IPv6 addresses

Make get_ips and is_ip recognise IPv6 addresses (to some extent)

LW2.pm: better location for $LW2_CAN_IPv6 check

Add IPv6 support
@richardleach
Copy link
Author

Hey, any feedback on this so far?

I'm happy to add a documentation commit if you'd like. Just let me know which of the documentation files need changing.

@sullo
Copy link
Owner

sullo commented Mar 27, 2020

I haven't had a chance to test it yet.

I need some more git learnin' since when I checkout your branch I don't see any changes to easily test with.

@drwetter
Copy link
Contributor

Are you definitely running against this PR?

Github pitfall ;-/ . I was on your fork + IPv6 branch you PR'ed from. Then I copied the URL from the browser. That was your master branch however as I figured now. :=) Sorry!

Works for IPv6.

However I encountered two issues:

  • For a hostname which has an AAAA record it only scans the IPv6 vhost (using ./nikto --host <hostname>)
  • Same cmd line: when there's no IPv6 connectivity it doesn't even try to scan the IPv4 vhost (using a machine which has IPv6 but cannot reach the target via IPv6):
./nikto.pl -host https://google.com/ 
- Nikto v2.2.0
---------------------------------------------------------------------------
+ No web server found on google.com:443.
---------------------------------------------------------------------------
+ 0 host(s) tested

The error message is kind of misleading and it also should try IPv4 here.

@richardleach
Copy link
Author

richardleach commented Mar 27, 2020

I was definitely concerned about the second point when working through the code back in January. Forgot about it when waiting for feedback on the original WIP branch.

IIRC, both have the same root cause: when nikto resolves a hostname to multiple IP addresses, it has only ever taken the first IP address from the list and scanned it. That was less visible in an IPv4-only world, assuming you didn't luck out and the first IP happened to be down but the others were up.

Question is, what is the desirable behaviour now?

  1. For hostnames with IPv4 and IPv6 IPs, but you don't have IPv6 connectivity, I think it makes sense to put the port_check that tests for a live service into a loop until it either finds an IP that it can connect to or runs out of candidates.

  2. For hostnames with IPv4 and IPv6 IPs, and you have both IPv4 and IPv6 connectivity, what should happen? (Don't forget that for a hostname that resolved to multiple IPv4 addresses, nikto has only ever scanned one of them, and only very recently started telling you that there were multiple IPs.) Should nikto scan one IPv4 and one IPv6, or scan 'em all? (Probably not the latter, unless a new command line switch is added to say that is what you want.)

Note: changing the "only scan the first IP" behaviour is easier said than done, but I'll look again at the details.

@richardleach
Copy link
Author

Inconveniently, ./nikto.pl -host https://google.com/ works for me on an IPv4-only connection. Presumably getaddrinfo() returns the IPv4 address ahead of the IPv6 address for some reason.

I'm going to blindly add a loop within _stream_socket_alloc to try to establish a connection using each result returned from getaddrinfo() in turn, and take the first successfully established socket. This will hopefully fix @drwetter's point that:

  • when there's no IPv6 connectivity it doesn't even try to scan the IPv4 vhost (using a machine which has IPv6 but cannot reach the target via IPv6)

Should be able to force-push that change to this PR within the next couple of hours.

@drwetter
Copy link
Contributor

drwetter commented Apr 4, 2020

Inconveniently, ./nikto.pl -host https://google.com/ works for me on an IPv4-only connection. Presumably getaddrinfo() returns the IPv4 address ahead of the IPv6 address for some reason.

Ok, what I didn't say that the host I tested from, doesn't have an IPv6 uplink. It can reach the hosts in the internal network.

Other than that: Still need to check your recent commit.

and take the first successfully established socket

IMO it would be great if the maintainers will get back on this, you mentioned this before. Before, in the IPv4 only world, it might have been ok to just scan one IPv4 address, assuming the other A record(s) are the same. In the dual stack world it is more likely that one encounters two different vhosts. So just taking one shot seems a bit arbitrary to me. Also, to me as a user, it would seem more handy when both records are scanned or tried to be scanned. Or, the nmap way, just don't scan IPv6 unless -6 is specified. Speaking of it: nmap also scans only one IPv4 address but has a handy switch --resolve-all when the user wants to scan every IPv4 address.

@richardleach
Copy link
Author

and take the first successfully established socket

IMO it would be great if the maintainers will get back on this, you mentioned this before. Before, in the IPv4 only world, it might have been ok to just scan one IPv4 address, assuming the other A record(s) are the same. In the dual stack world it is more likely that one encounters two different vhosts. So just taking one shot seems a bit arbitrary to me. Also, to me as a user, it would seem more handy when both records are scanned or tried to be scanned. Or, the nmap way, just don't scan IPv6 unless -6 is specified. Speaking of it: nmap also scans only one IPv4 address but has a handy switch --resolve-all when the user wants to scan every IPv4 address.

All of those sound like possible options. I don't have a preference but am happy to contribute towards any implementation. (IMHO these feel like separate changes to this PR.)

There are probably multiple different ways to implement any of those suggestions, but the one that comes to mind is:

  1. In the nikto_core.plugin::resolve subroutine, after a hostname has been resolved:
  • filter out any of the resolved IPs that aren't to be scanned (according to the desired default/user-specified behaviour)
  • for the remaining IPs, queue each one separately as if it had originally been specified as an IP target, but with the -vhost option set to the hostname

...seems like this should work?

@sullo sullo closed this Apr 3, 2021
@sullo sullo deleted the branch sullo:nikto-2.2.0 April 3, 2021 01:19
@richardleach richardleach mentioned this pull request Apr 3, 2021
sullo added a commit that referenced this pull request Apr 9, 2021
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.

3 participants