Re: [PATCH] TCP Idle Scan in IPv6

[David Fifield <david () bamsoftware com>]

On Mon, Jun 03, 2013 at 05:59:11PM +0200, Mathias Morbitzer wrote:
> I managed to port the TCP Idle Scan to IPv6! 
>
> However, to show that it really works, I also tried to implement the
> scan in Nmap. To do so, I hacked idle_scan.cc, and used most of the
> stuff which was already there. What I had to add was the sending of
> the pings and the ICMPv6 packet too big messages for the
> initialization, and I changed the parts where the IPID is accessed, so
> that it works for IPv4 and IPv6. 
>
> The usage is the same as using the scan in IPv4: -sI
> <idlehost:probeport> for the idlescan, plus add the -6 switch for
> IPv6. 
>
> I tested my patch with Windows 7 Ultimate, and Linux 3.8 (but there is
> does not work, the IPIDs are on a per-host-base). 
>
> The patch is not perfect yet. There are still some things which need
> to be improved, but I wanted to get a first feedback to know if i can
> continue working on it this way. Also, my C/C++ knowledge is not the
> best, so let me know if I made bigger mistakes.

Thanks for all this code. This looks like it could be a nice new
feature.

Is it possible for you to store your development history online in some
form, for example in a public github repository? You can fork from
https://github.com/nmap/nmap, or follow the instructions from
https://secwiki.org/w/Using_Git_with_Nmap.

Here are my comments on reading the patch.

> +    if (proxy->host.af() == AF_INET6){
> +      int rc = resolve(proxy->host.HostName(), 0, &ss, &sslen, o.pf());
> +      if (rc != 0) {
> +        fatal("Could not resolve idle scan zombie host \"%s\": %s", proxy->host.HostName(), gai_strerror(rc));
> +      }
> +      ping = build_tcp_raw_ipv6(proxy->host.v6sourceip(), proxy->host.v6hostip(),
> +                        0x00, 0x0000,  o.ttl,
> +                        base_port + tries, proxy->probe_port,
> +                        seq_base + (packet_send_count++ * 500) + 1, ack,
> +                        0, TH_SYN | TH_ACK, 0, 0,
> +                        (u8 *) "\x02\x04\x05\xb4", 4,
> +                        NULL, 0, &packetlen);
> +      send_ip_packet(proxy->rawsd, proxy->ethptr, &ss, ping, packetlen);

I don't see why you resolve a host name when proxy->host.v6hostip() is
available? You can use that value to fill ss to give to send_ip_packet.

> +      if (proxy->host.af() == AF_INET6) 
> +        ip = (struct ip *) readip_pcap(proxy->pd, &bytes, to_usec, &rcvdtime, NULL, true);
> +      else
> +        ip = (struct ip *) readipv4_pcap(proxy->pd, &bytes, to_usec, &rcvdtime, NULL, true);

Is is possible to just call readip_pcap here, and filter on address
family later on? You have to filter on address family anyway--the code
that follow looks incorrect in that an IPv4 packet could be processed in
IPv6 mode. Your condition should be
        if (o.af() == AF_INET6) {
                if (ip->ip_v != 6)
                        error
        }
not
        if (ip->ip_v == 6)

> +          if (ip6->ip6_nxt!=44)

Use constants like IP_PROTO_FRAGMENT, not numbers like 44.

> +            fatal("IPv6 packet without fragmentation header received - issues with the zombie?");

You can't just kill the whole Nmap process for one weird packet :) Find
another way to handle this case, maybe by ignoring it.

> +          //for some reason we need to add 2 here, have to figure out why
> +          frag_header = (struct ip6_ext_data_fragment *)((u_char *)ip6 + IP6_HDR_LEN + 2);
> +          //TODO: check if the next header in the frag header says TCP
> +          //for now, we do this manually, ipv6_get_data_any sometimes return null...
> +          tcp = (struct tcp_hdr *)((u_char * )ip6 + IP6_HDR_LEN + 8);
> +          //ip6data = (void *) ipv6_get_data_any(ip6, &packetlen, &ip6hdr);

The two bytes are the Next Header and Hdr Ext Len fields. You can't
assume that the the Fragmentation header is the first or only extension
header. If ipv6_get_data_any returns NULL, there is no payload, and you
can't treat what follows as a TCP packet. You should use ipv6_get_data
instead, because TCP is one of the protocols it recognizes as an
upper-layer protocol.

Take a look at ipv6_get_data_primitive for an example of how to walk the
Next Header chain.

You might be happier defining some auxiliary functions that can operate
on either an IPv4 or an IPv6 packet. One function can extract the IP ID,
and the other can extract the TCP payload.

> +  //for me, htonl is not necessary, instead diving by 2
>    if (seqclass == IPID_SEQ_BROKEN_INCR) {
>      /* Convert to network byte order */
> -    startid = htons(startid);
> -    endid = htons(endid);
> -    return endid - startid;
> +    //startid = htonl(startid);
> +    //endid = htonl(endid);
> +    return (endid - startid)/2;
>    }

You seem to be treating IPID_SEQ_BROKEN_INCR as meaning "counts by 2,"
but it really means "counts by 1 but byteswapped." That is, instead of
counting
        0000,0001,0002,...,00ff,0100,0101,0102,...,feff,ff00,ff01,...
it counts
        0000,0100,0200,...,ff00,0001,0101,0201,...,fffe,00ff,01ff,...
You probably need to add a new IPID_SEQ define for "counts by 2."

That said, calling htons to swap bytes will not work on big-endian
platforms, which is a bug in the current code.

> +  //TODO: If we have an IPv6 address, we don't know if the last colon-seperated part is the port or part of the 
> address
> +  /* In IPv6, we have colons in the address, so we just look for the last one */

You should force users to use square brackets with IPv6 addresses, like
RFC 3986 recommends.

> +    p = strdup(proxy->host.targetipstr());
> +    /* We need something that works for IPv4 and IPv6 */
> +    q = strdup(proxy->host.sourceipstr());
> +    Snprintf(filter, sizeof(filter), "icmp6 and src host %s and dst host %s", p, q);
> +    free(p);
> +    free(q);

I think it's unnecessary to strdup here.

> +  /* libpcap doesn't find the source port in IPv6 if theres a fragmentation header. Therefore we use this ugly hack 
> to state the source port */
> +  //TODO: what do we do if there are multiple extension headers? 
> +  Snprintf(filter, sizeof(filter), "tcp and src host %s and dst host %s and src port %hu or (ip6[6]=44 and ip6[40]=6 
> and ip6[48:2]=%hu)", p, q, proxy->probe_port, proxy->probe_port);

Here's another case where you will have to use a less specific filter
and do filtering manually. Like your TODO says, you can't assume there
there are more than one (or even one) extension header.

> -      ip = (struct ip *) readipv4_pcap(proxy->pd, &bytes, to_usec, &rcvdtime, NULL, true);
> +      /* again, we differ between ipv4 and ipv6 */
> +      if (proxy->host.af() == AF_INET6){
> +        ip = (struct ip *) readip_pcap(proxy->pd, &bytes, to_usec, &rcvdtime, NULL, true);
> +      }else{
> +        ip = (struct ip *) readipv4_pcap(proxy->pd, &bytes, to_usec, &rcvdtime, NULL, true);

It's much better if you can define a helper function to do things like
this.

>    k = 1; /* k is a flag meaning "all difference seen are multiples of 256 and
>            * no greater than 5120" */
>    for (i = 0; i < numSamples - 1; i++) {
> -    if (k && (ipid_diffs[i] > 5120 || ipid_diffs[i] % 256 != 0)) {
> +    //on my x86_64 machine, I need a modulo 2 here, not 256 - as well for IPv4 as for IPv6
> +    if (k && (ipid_diffs[i] > 5120 || ipid_diffs[i] % 2 != 0)) {
>        k = 0;
>      }

This looks like the same confusion over IPID_SEQ_BROKEN_INCR.

David Fifield
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/