Re: Nmap/libnetutil: route tables rework

[Djalal Harouni <tixxdz () opendz org>]

Hi David,

On Thu, Sep 27, 2012 at 01:06:59AM -0700, David Fifield wrote:
> On Tue, Jun 26, 2012 at 06:50:48PM +0100, Djalal Harouni wrote:
> > This is a quick summary of the network routes bugs that were reported by
> > Nmap users on nmap-dev, and others that I've found my self. Some of them
> > are easy to fix, others are not. David I need your point on the provided
> > solution and what do you think of updating libdnet ?
>
> Thank you so much, Djalal, for doing this work. I extensively refactored
> and rebased this patch set, to the reduced attached patches. I refer to
> your patches from
> http://seclists.org/nmap-dev/2012/q3/4
> http://seclists.org/nmap-dev/2012/q3/115
Thanks, these are the good ones.

> I grouped many of the patches differently, and some of them I omitted.
> [PATCH 1/4] is, I think, the main idea of this set of patches, which is
> that libdnet should return an interface along with each route. Many
> other changes seemed to be more complicated than they needed to be; I
> tried to simplify them and hopefully I didn't change how they work. This
> is a summary of how I handled each of them.
Correct, the main idea was from the
"[PATCH 1/4] Add intf_name to the route_entry struct."

> Merged to trunk.
>       [PATCH 04/24] libdnet-stripped/route-linux: avoid an off-by-one overwrite.
Ok svn r29878 commit seems to handle this, just a small note: the commit
log says that "sscanf doesn't include the terminator." which is perhaps
ambiguous...

If this is a string input conversion, then it will add a null terminator
that is not *counted* by the maximum field width.

>       [PATCH 19/24] libdnet-stripped/route-linux: ipv4 route input contains 11 element
Ok commit r29879.

>       [PATCH 17/24] libdnet-stripped/route-linux: check if the IPv6 route is usable
Ok commit r29880.

>       [PATCH 20/24] libdnet-stripped/route-linux: ipv6 route: check the number of input elements
Ok commit r29880

Ok commit r29881 is also nice, thanks.


> Merged into attached "[PATCH 1/4] Add intf_name to the route_entry struct."
>       [PATCH 01/24] libdnet-stripped/route: add the interface name to the route_entry struct
>       [PATCH 02/24] libdnet-stripped/route-linux: route_entry should be zeroed
>       [PATCH 07/24] libdnet-stripped/route-linux: add a missing struct key word
>       [PATCH 24/24] libdnet: intialize route_entry on every loop
Correct! handled by commit svn r29882...

> Merged into attached "[PATCH 2/4] Set intf_name in route-linux.c."
>       [PATCH 03/24] libdnet-stripped/route-linux: read ipv4 interface name from route entries
>       [PATCH 05/24] libdnet-stripped/route-linux: read interface name from IPv6 routes
Ok! handled by commit svn r29883

> Merged into attached "[PATCH 4/4] check if the gateway was set in the current route."
>       [PATCH 13/24] libnetutil: check if the gateway was set in the current route
>       [PATCH 14/24] libnetutil: Add a comment to note the check on the gateway
Nice! handled by commit svn r29885


David I'll double check what remains soon. Thanks!

> Omitted; sys_route already has an interface_info member; it should not
> have a redundant devname as well. We match interfaces by name before
> entering sysroutes_dnet_find_interfaces, removing the need for name
> comparisons in each loop in sysroutes_dnet_find_interfaces. These
> patches together are replaced by "[PATCH 3/4] Assign sys_route
> interfaces immediately when iface names come from libdnet."
>       [PATCH 06/24] libnetutil: add the device name support to sys_route struct
>       [PATCH 08/24] libnetutil: copy device name from libdnet
>       [PATCH 11/24] libnetutil: check the device name before matching remaining routes
>       [PATCH 15/24] libnetutil: check the device name before the gateway/destination match
>       [PATCH 24/24] libnetutil: separate interface name check
>       [PATCH 16/24] libnetutil: add an error message to note that the device was not found
>
> Omitted; I did not understand the purpose of this patch. May be
> unnecessary for the same reason as the previous group.
>       [PATCH 21/24] libnetutil: check if this is the default route
>
> Omitted because not necessary.
>       [PATCH 09/24] libnetutil: Check the device name before any match
>       [PATCH 10/24] libnetutil: revert last commit
>       [PATCH 12/24] libnetutil: Add sys_route pointer to reduce dereferences
>       [PATCH 18/24] libdnet-stripped/route-linux: make iflags unsigned int
>
> Omitted; see below.
>       [PATCH 22/24] libnetutil: fix the ppp match logic
>
> [PATCH 22/24] seems wrong to me. The PPP two-step matching logic
> definitely worked when it was added in r11591.
> http://seclists.org/nmap-dev/2008/q4/576
> It was intentional to match one route's gateway against another route's
> destination, and not the other way around. Also, with [PATCH 22/24],
> Nmap reports fewer routes for me on Windows:
>
>  **************************ROUTES**************************
>  DST/MASK                                 DEV  GATEWAY
>  172.16.3.62/32                           eth0
> -255.255.255.255/32                       eth0
>  192.168.0.255/32                         eth0
>  127.0.0.1/32                             lo0
>  127.255.255.255/32                       lo0
>  172.16.3.255/32                          eth0
>  192.168.0.2/32                           eth0
> -255.255.255.255/32                       eth0
>  172.16.3.0/24                            eth0
>  192.168.0.0/24                           eth0
>  127.0.0.0/8                              lo0
> -224.0.0.0/4                              eth0
> -224.0.0.0/4                              eth0
>  0.0.0.0/0                                eth0 192.168.0.1
>  fe80::356c:d34c:f247:36ef/128            eth0
>  ::1/128                                  lo0
>  2001:470:1f05:155e::2/128                eth0
>  2001:0:9d38:6ab8:1076:3f8d:b4db:5ee3/128 tun0
>  fe80::1076:3f8d:b4db:5ee3/128            eth0
>  2001:470:1f05:155e::/64                  eth0
>  fe80::/64                                eth0
>  fe80::/64                                eth0
>  2001::/32                                tun0
> -ff00::/8                                 eth0
> -ff00::/8                                 eth0
> -ff00::/8                                 eth0
>  ::/0                                     eth0 2001:470:1f05:155e::1
>
> I agree with out that sysroutes_dnet_find_interfaces is still kind of a
> bogus idea, matching routes to interfaces by addresses only because
> libdnet was formerly not able to give us an interface for each route.
> This method will continue to be used on all platforms for which libdnet
> has not been extended to return interfaces, which in this patch set is
> all but Linux. I don't think it will be hard to add further support for
> the other most common platforms.
>
> Please check out these patches and tell me what you think.
>
> David Fifield

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