Re: [NSE] firewalking: NSE traceroute patch

[David Fifield <david () bamsoftware com>]

On Tue, Aug 17, 2010 at 09:21:40PM +0200, Henri Doreau wrote:
> Hi,
>
> here is an updated version of my firewalk script.
>
> I've changed a few things:
>   - checking the validity of catched replies
>   - removed the sleep() after a timeout
>   - use the new stdnse.get_script_args() function
>
> I also re-send the patch that makes traceroute results available from nse
> but it is unchanged.

I like the traceroute patch and the fact that it is small. I think that
host tables should be stored in the traceroute table, instead of just
strings, so that we don't lose any information. A TracerouteHop contains

  bool timedout;
  std::string name;
  struct sockaddr_storage addr;
  int ttl;
  float rtt; /* In milliseconds. */

The TTL will be implicit by the place in the array, so it can be left
out. The mapping of the other entries should be

  hop.name -> host.name
  hop.addr -> host.ip
  hop.rtt  -> host.times.srtt

You need also to add handling for timedout entries. If timedout is true
then name will be NULL and addr will be full of zeroes. You can't just
store nil in the hops array, because that will make the array appear to
be shorter than it is. I suggest that you store an empty table for
timedout hops. Scripts can check if host.ip is nil to see if it timed
out.

I would also like you to add brief documentation in scripting.xml, in
the section "Information Passed to a Script." You don't need to try
building the documentation; just copy the syntax from the surrounding
text. The documentation only needs to state that host.traceroute is an
array, and each of its entries is a table with the keys "name", "ip",
and "times".

With these changes I'll be happy to accept this patch.

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