Re: ncat should try connecting to all resolved addresses, not only the first one

[Jaromir Koncicky <jkoncick () redhat com>]

Thanks for your reply!
That sounds really good that my patch is working. Also thanks for fixing the casts and malloc.
The variable targetaddrs_allocated is here to indicate that targetaddrs was dynamically allocated and should be freed 
in the end of the program.
However, I got rid of this and now I am using up the first static member of targetaddrs and allocating memory only for 
the second address and so on.
I tried to make some tests for this, but the only testing cases I came up for this is to use 'localhost' as connect 
address of client. In not-patched version, new tests with IPv4 should fail because ncat tries only IPv6 which is 
resolved first. In patched ncat, all new tests should pass. Do you have more ideas for the tests?
Thanks in advance for your reply.


----- Original Message -----
From: "Jacek Wielemborek" <d33tah () gmail com>
To: dev () nmap org
Cc: "David Fifield" <david () bamsoftware com>, "Jaromir Koncicky" <jkoncick () redhat com>
Sent: Monday, December 9, 2013 8:24:57 PM
Subject: Re: ncat should try connecting to all resolved addresses,      not only the first one

03/12/2013 15:20:34 Jaromir Koncicky <jkoncick () redhat com>:
> Bump:
> Could you please look and this and give me a feedback?
> I'd like to finish this issue and make an acceptable patch which could be
> included in nmap. Thanks!

Hello,

I took a look at your patch from the comment #11 and it looks fine, applies to 
the latest SVN with no problem and builds under Linux and FreeBSD properly. 
Also tested ncat-test.pl under Linux to make sure it didn't break anything and 
tried to build it under Windows. Unfortunately, for some reason this involves 
compiling under Microsoft Visual C++ in C++ mode and the original patch lead 
to the following errors:

> Build FAILED.
>
> "C:\cygwin\home\d33tah\shared\ncat\nmap-trunk\ncat\ncat.vcxproj" (default
> target) (1) -> (ClCompile target) ->
>
>   c:\cygwin\home\d33tah\shared\ncat\nmap-trunk\ncat\ncat_connect.c(755):
>   error C2440: 'initializing' : cannot convert from 'void *' to
>   'sockaddr_list *'
>   [C:\cygwin\home\d33tah\shared\ncat\nmap-trunk\ncat\ncat.vcxproj]
>   c:\cygwin\home\d33tah\shared\ncat\nmap-trunk\ncat\ncat_core.c(310): error
>   C2440: '=' : cannot convert from 'void *' to 'sockaddr_list *'
>   [C:\cygwin\home\d33tah\shared\ncat\nmap-trunk\ncat\ncat.vcxproj]>   
>     0 Warning(s)
>     2 Error(s)
>
> Time Elapsed 00:00:04.21

I fixed the casts, changed the coding style a bit and replaced malloc with 
safe_malloc. I attach the new patch (feel encouraged to diff it against your 
version to see what I changed). Before I merge the patch, could you post some 
test code, most preferrably as a patch to ncat_test.pl, so I could have a 
snippet to try this out on different platforms? Also, I'm a bit worried about 
the introduction of two new global variables - could you please explain me why 
targetaddrs_allocated is necessary here? Perhaps a new structure would be a 
better choice here, or a bit different code paths/return arguments? Also, I 
hadn't checked if a version of resolve_multi wasn't already in the nsock/nbase 
libraries.

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

Attachment: try_all_addresses_3.patch
Description:

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