Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: [PATCH] TCP Idle Scan in IPv6
From: David Fifield <david () bamsoftware com>
Date: Fri, 30 Aug 2013 08:20:04 -0700

On Fri, Aug 30, 2013 at 03:06:40PM +0200, Mathias Morbitzer wrote:
On Thu, Aug 15, 2013 at 02:12:45PM +0200, Mathias Morbitzer wrote:
I don't like to rely on spooky global state (o.af()) in a utility
function like this one. If o.af() must be used, let it be at a higher
level. You can call get_ipid_sequence_16 or get_ipid_sequence_32. They
will have the same interface, taking an array of u32. But
get_ipid_sequence_16 will do the calculation as if the values are u16.
Both these functions could be implemented in terms of an auxiliary
function; in fact I think get_ipid_sequence_16 can be written as a
postprocessing step that applies boolean AND with 0xffff to the
of get_ipid_sequence_32. (For instance 4294901763 == 0xffff0003.)

get_ipid_sequence first creates an array ipid_diffs, which contains the difference between the ipids. 
Afterwards, it analyzes this ipid_diffs array to determine what kind if sequence it contains. 

As this is both done in the same function, I don't see where it would be possible to apply the boolean
AND to the results. The result returned by get_ipid_sequence is the type of sequence, so I don't have a chance 
to apply the AND to the ipid_diffs array for 16 bit. 

Instead, I would suggest to create a get_ipid_sequence_16 and get_ipid_sequence_32 function, with both create the 
ipid_diffs for 16 and for 32 bit values. The only line different in those 2 functions would be this one: 

ipid_diffs[i - 1] = (u16) (ipids[i] - ipids[i - 1] + 65536);  for 16 bit and 
ipid_diffs[i - 1] = (u16) (ipids[i] - ipids[i - 1] + 4294967296); for 32 bit. 

From what I understand, the rest should be the same in both functions. Afterward creating the ipid_diffs array, 
both function could call the generic get_ipid_sequence function, which does the rest, which is analyzing what
kind of sequence the ipid_diffs array contains. 

I think we are talking about the same thing. You should do it without
duplicating code. It's not good to have two almost identical functions.
What you should do instead is factor out the part of get_ipid_sequence
that calculates the first differences:

  for (i = 1; i < numSamples; i++) {
    if (ipids[i - 1] != 0 || ipids[i] != 0)
      allipideqz = 0; /* All IP.ID values do *NOT* equal zero */

    if (ipids[i - 1] <= ipids[i]) {
        ipid_diffs[i - 1] = ipids[i] - ipids[i - 1];
    } else {
        ipid_diffs[i - 1] = (u16) (ipids[i] - ipids[i - 1] + 65536);

This should be split into two functions, one for 16-bit and one for
32-bit. What I mean with applying boolean and, is that I think you can
define the 16-bit function in terms of the 32-bit function, to further
avoid code duplication. Like this pseudocode:

func calc_ipid_diffs_32(u32 ipids[]) {
  for (i = 1; i < length(ipids); i++) {
    diffs[i-1] = ipids[i] - ipids[i-1]
  return diffs
func calc_ipid_diffs_16(u32 ipids[]) {
  diffs = calc_ipid_diffs_32(ipids)
  for (i = 0; i < length(diffs); i++) {
    diffs[i] = diffs[i] & 0xffff
  return diffs

You may have to make another few refactoring changes. Try to do it
without copy-pasting code, is my main point.

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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]