Nmap Development mailing list archives
Re: [Zenmap-Patch] Reducing Topology Noise
From: Daniel Miller <bonsaiviking () gmail com>
Date: Tue, 29 Jul 2014 13:14:26 -0500
Jay, Sorry it has taken so long for me to get to this. Overall, I like it. Here are a few thoughts: 1. differing_length.xml doesn't seem to be different from old to new, and I think it probably should be. Right now it looks like: / b -> X \ .. -> a c -> .. \ X -> X / but I think a better way would be: / b \ .. -> a X -> c -> .. \ X / Unfortunately, this means a more difficult reworking of your patch. As I see it, your patch treats an anon hop as collapsible if it has the same distance and the same "known ancestor" (hop with a real IP that is closer than this anon hop). To do this sort of collapse, you'd have to also allow hops with the same distance and the same "known descendant" (hop with a real IP that is farther than this anon hop). I think you could do this by entering the same node into invalid_node_cache under 2 different keys: (pre_hop["ipaddr"], pre_hop_distance) and (post_hop["ipaddr"], post_hop_distance), if that makes sense. Of course, be careful that hop_split_at_different_real is not affected! 2. A minor cleanup thing: since you set key = (pre_hop["ipaddr"], pre_hop_distance), key cannot be None, so take that condition out of the following if statement. By the same token, the condition "if key is not None and key in invalid_node_cache:" can never be false, since if "key not in invalid_node_cache" then the previous block will put it into the cache. Just make the node assignment an unconditional statement. Other than that, the patch looks good. You may want to just commit the patch as it is now (possibly with that small cleanup in 2) and then consider my suggested change as a separate effort. Thanks! Dan On Wed, Jul 9, 2014 at 9:23 AM, Jay Bosamiya <jaybosamiya () gmail com> wrote:
Hi All! I've attached an updated the patch with some modifications. Previously, the patch would lose info about number of hops since it worked like: / X -> X \ .. -> a b -> .. => .. -> a -> X -> b -> .. \ X -> X / (where lowercase letters (i.e. a,b) are normal and X are anonymous) The new patch maintains the info about number of hops. The patch now works like: / X -> X \ .. -> a b -> .. => .. -> a -> X -> X -> b -> .. \ X -> X / As for hop_split.xml, It now works as: / X -> b -> .. / b -> .. .. -> a => .. -> a -> X \ X -> c -> .. \ c -> .. This change makes sense, since we keep all info about number of hops, but there is absolutely no way to distinguish between the two X's. As for the change I mentioned for anon_hops_at_known.xml in my previous mail, I think that this could be something for a future patch due to some complications that may arise. Feedback is welcome as always :) Note: I have added a few more test cases to the zip (hop_split_at_different_anon.xml, hop_split_at_different_real.xml and long_anon.xml) which make the changes even more obvious and should help review the patch better. Cheers, Jay On Saturday 21 June 2014 03:47 PM, Jay Bosamiya wrote:Hi All! I've been working on reducing Zenmap's Topology view to reduce noise due to anonymous hops. Basically what it does is this: / anon_1 \ ..-> ip_a ip_b -> .. => .. -> ip_a -> anon -> ip_b -> .. \ anon_2 / A big thanks to Anders Sundman for sending in a patch [1] that tried to do this. Your patch helped a lot though it only solved part of the problem (worked with only single anonymous hops in parallel). The current patch can also handle things like: / anon_1 -> anon_2 \ ..->ip_a ip_b->.. => .. ->ip_a -> anon -> ip_b->.. \ anon_3 -> anon_4 / Attached is the patch. Also attached is a zip file containing XMLs to test with (traceroutes with anonymous hops in different combinations). There are a few cases that we need to think about, however, namely "anon_hops_at_known.xml" or "hop_split.xml" (from the zip file attached). For "anon_hops_at_know.xml", I think that the anonymous hop should be removed completely (since 1.1.1.2 fits perfectly instead of the anon). I think that it should work like: / anon_1 \ ..-> ip_a ip_c -> .. => .. -> ip_a -> ip_b -> ip_c -> .. \ ip_b / For "hop_split.xml", I am not sure what should be done. Currently, it works like: / anon_1 -> ip_b -> .. ..-> ip_a \ anon_2 -> ip_c -> .. but I think it would be better if it became: / ip_b -> .. ..-> ip_a -> anon \ ip_c -> .. I have not implemented the 2 changes since I wanted some feedback before I did so. Cheers, Jay_______________________________________________ Sent through the dev mailing list http://nmap.org/mailman/listinfo/dev Archived at http://seclists.org/nmap-dev/
_______________________________________________ Sent through the dev mailing list http://nmap.org/mailman/listinfo/dev Archived at http://seclists.org/nmap-dev/
Current thread:
- Re: [Zenmap-Patch] Reducing Topology Noise Jay Bosamiya (Jul 09)
- Re: [Zenmap-Patch] Reducing Topology Noise Daniel Miller (Jul 29)
- Re: [Zenmap-Patch] Reducing Topology Noise Jay Bosamiya (Jul 30)
- Re: [Zenmap-Patch] Reducing Topology Noise Daniel Miller (Jul 29)