Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: [PATCH] TCP Idle Scan in IPv6
From: Mathias Morbitzer <m.morbitzer () student ru nl>
Date: Fri, 30 Aug 2013 15:06:40 +0200 (CEST)

On Thu, Aug 15, 2013 at 02:12:45PM +0200, Mathias Morbitzer wrote:
The change to get_ipid_sequence needs some more thought. When you
are
dealing with 16-bit IPv4 ID values, and you start treating them as
32-bit, you will have calculation errors at the 16-bit boundary.
Imagine
two consecutive IP ID values are 0xfffe and 0x0001.
(u16) 0x0001 - (u16) 0xfffe == 3
but
(u32) 0x0001 - (u32) 0xfffe == 4294901763
Do you have ideas as to how to account for this? Perhaps you need
to
sequence calculation to be aware of whether it is handling 16-bit
or
32-bit values.

Thank you for pointing that out, I did not think about that.

As far as I understand, this concerns only these three lines:
235: ipid_diffs[i - 1] = ipids[i] - ipids[i - 1];
237: ipid_diffs[i - 1] = (u16) (ipids[i] - ipids[i - 1] + 65536);
261: ipid_diffs[i] -= 256;
263: ipid_diffs[i]--;

Those are the only ones were calculations are done with the
ipid_diffs[] values.
If so, I would just put a "if (o.af() == AF_INET4) ..." before those
lines, and try to handle the values correctly depending on if they
are
16 or 32 bits, like this:

237 in IPv4: ipid_diffs[i - 1] = (u16) (ipids[i] - ipids[i - 1] +
65536);
237 in IPv6: ipid_diffs[i - 1] = (u32) (ipids[i] - ipids[i - 1] +
4294967296);

261 in IPv4: ipid_diffs[i] = (u16) ipid_diffs[i] - 256;
261 in IPv6: ipid_diffs[i] -= 256;

263 in IPv4: ipid_diffs[i] = (u16) ipid_diffs - 1;
263 in IPv6: ipid_diffs[i]--;

Let me know what you think about that approach.

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
results
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 saw you added a new IPID_SEQ_INCR_BY_2, which is probably the
right way to do it. However, not only idle scan uses the IPID_SEQ
classifications; OS detection uses them as well. Specifically, we
need to decide what to do with IPID_SEQ_INCR_BY_2 in
make_aval_ipid_seq. This is the code that controls what gets
printed
for the SEQ.TI, SEQ.CI, and SEQ.II IPv4 OS detection tests:
http://nmap.org/book/osdetect-methods.html#osdetect-ti.
We have two options: one is to add a new value for "increment by
2",
for instance to make TI=2 possible; and the other is to find out
what hosts having "increment by 2" behavior are currently being
classified as, and make hosts that increment by 2 map to this
value,
for instance TI=I or TI=R. What did get_ipid_sequence return for
hosts that increment by 2, before your patch causing them to be
IPID_SEQ_INCR_BY_2?

Before my patch, get_ipid_sequence returned "Incremental" instead of
"Incremental by 2". (Which caused the scan to fail because it was
expecting lower increments) So, hosts which increment by 2 are
currently classified as "incremental".

It should not be much of an issue to apply the second solution and
map
the hosts to this value, but in my opinion it would be nicer to add
a
value for "increment by 2". This might also allow a better OS
detection, as we have (in IPv6) for example Windows 7 that
increments
by 2, and Windows Vista that increments by 1. Also, there are
operating systems like windows 7 which increment by 1 in IPv4, and
by
2 in IPv6, which should allows allow to limit the pool of possible
OS
for OS detection.

And not to forget, in previous research I stumbled across network
printers which assigned the IPID incremental by 9. They should
probably also get their own value at a certain point instead of
mapping all the "increment by x" to the same, simple "incremental".

The TI=I, CI=I, II=I values are only for IPv4 OS fingerprints. IPv6
fingerprints look different, they are basically just the packet
contents. See the difference:
http://nmap.org/book/osdetect-fingerprint-format.html#osdetect-ex-typical-subject-fprint
http://nmap.org/book/osdetect-fingerprint-format.html#osdetect-ex-typical-reference-fprint-ipv6
I agree with you that it would be best to record the size of the
increment in fingerprints. However, to do so means potentially
invalidating many of our existing IPv4 fingerprints.

So what I propose is to have make_aval_ipid_seq return "I" for
incremental by 2. This will maintain compatibility with existing IPv4
fingerprints. IPv6 OS detection won't be affected. Please provide a
new
patch implementing this when you can.

No problem on this part, it is a one-liner. I will try to provide the new patch as soon as we found a solution for 
get_ipid_sequence. 

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


  By Date           By Thread  

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