massping -> ultra_scan prototype completed

[David Fifield <david () bamsoftware com>]

I have finished a prototype of my proposed changes that implement
massping using ultra_scan (see http://seclists.org/nmap-dev/2007/q3/0023.html)
for some background). You can check it out with

        svn co svn://svn.insecure.org/nmap-exp/david/nmap-massping-migration

Only TCP SYN ping scanning (-PS) is supported. I did this on purpose,
not because the other types are hard to do, but because I want you all
to see the general technique I've used to effect the migration. This
means you have to use the -PS option (and no other -P options) every
time, like

        nmap -sP -PS80,113 -iR 50

Trying to use any other ping type will give an error message.

I'd like to ask for your feedback on how I've done the migration.
r5095:5125 (svn diff -r 5095:5125) are small changes and bugfixes that
shouldn't be controversial. r5126 (svn diff -c 5126) is the big change.

Here are some notes and observations about the change.

I kept the nexthost/massping interface. massping is now just a thin
wrapper around ultra_scan:

        static void massping(Target *hostbatch[], int num_hosts, int pingtype) {
          std::vector<Target *> targets;
          int i;

          for (i = 0; i < num_hosts; i++) {
            initialize_timeout_info(&hostbatch[i]->to);
            targets.push_back(hostbatch[i]);
          }

          /* ultra_scan gets pingtype from o.pingtype. */
          ultra_scan(targets, NULL, PING_SCAN);
        }

As I proposed, the new ping scanning code reads its ping type and lists
of ports directly from the global NmapOps data structure. I do not think
changing the way this is done will be too difficult.

The new massping uses ultra_scan to send and receive packets, but the
logic for interpreting them is copied from the old massping. It's now in
the function get_ping_pcap_result in scan_engine.cc.

There was and still is a function get_pcap_result that reads packets and
uses them to alter port states. To handle massping, I created a new
function get_ping_pcap_result that does the same thing, only it updates
host states and is called only when USI->ping_scan and not
USI->ping_scan_arp. These functions could conceivably be combined to
make just one pcap handler, but they're both big enough as it is, so I
left them separate.

scan_engine.cc is getting really huge, pushing 4500 lines (it was about
4000 before the migration). There are many common blocks of code that
could be factored out, but I'd like to do that as a separate step. For
long-term maintainability, the logic for the different scan types (ARP
ping, ping, port, IP protocol) should be broken out into helper classes
that are called by ultra_scan.

The new ping scanning code doesn't short-circuit; that is, it will keep
sending probes even after a host's state is known. I don't think this
will be hard to fix. It might be done by causing
HostScanStats::freshPortsLeft return 0.

I don't have a feel for the timing aspects of the code. I don't feel
qualified to tell if ultra_scan's timing is appropriate for ping scan or
not. For example, there's a chunk in the old massping

        /* Decide on the timeout, based on whether we need to also watch for TCP stuff */
        if (ptech->icmpscan && !ptech->rawtcpscan && !ptech->rawudpscan) {
          /* We only need to worry about pings, so we set timeout for the whole she-bang! */
          myto.tv_sec  = to->timeout / 1000000;
          myto.tv_usec = to->timeout % 1000000;
        } else {
          myto.tv_sec = 0;
          myto.tv_usec = 20000;
        }

I didn't add any new logic to ultra_scan to handle situations like this.

I'm going to be gone all next week. If you will, please examine the
changes I've made and respond. I'll look at the responses when I get
back.

David

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org