Nmap Development mailing list archives
Re: [PATCH] Extended SSL support in Nmap, review
From: David Fifield <david () bamsoftware com>
Date: Mon, 2 Mar 2009 22:34:23 -0700
On Sat, Feb 21, 2009 at 08:01:33PM +0100, Kristof Boeynaems wrote:
I wrote a patch for the issue described at length in http://seclists.org/nmap-dev/2009/q1/0330.html.
Thanks very much for the patch. This is not a trivial issue and it is noteworthy that you stepped up and wrote an implementation. The code in general looks good. The change works great for Nmap version detection, but I would like to see another design that is also suitable for Ncat. Let me restate the problem, so I can see if I understand it and for the benefit of anyone following the conversation. Nsock's SSL connection function (used by Nmap service detection and by Ncat) makes its connection in a way that is not understood by a small fraction of SSL servers. Therefore neither version detection nor Ncat can connect to these server. Furthermore, the binary probe that version detection uses to detect an SSL service (SSLSessionReq) does not work against SSLv2-only servers, so Nmap does not know to try to reconnect with SSL. Now I will summarize your solution as I understand it. There are new functions nsock_connect_ssl3 and nsock_connect_tls1 that parallel the old nsock_connect_ssl. There's a new version detection probe, SSLv23SessionReq, that works against SSLv2-only servers. More importantly, there are new match lines that return a service of "ssl", "tls1", or "ssl3". These service names are used internally by Nmap to select which nsock_connect_* function to use, however these are always changed to "ssl" in the final output. I don't like the code duplication in the patch. The new functions Nsock_SSL3_Init and Nsock_TLS1_Init are basically copies of Nsock_SSL_Init. A better design is what you did with the connect functions: Each of them just passes a special parameter to a generic nsock_connect_ssl_all function. The patch relies on Nmap's version detection to know which SSL connect function to connect with. This won't be convenient for Ncat, which doesn't have any version detection to rely on. The more I think about it, the more I think that trying multiple methods on every connection is the right way. First SSLv23, then TLSv1, then SSLv3. It will be slower when the first connection fails, but the research offered so far indicates that it succeeds more than 99% of the time. It will offer Ncat a convenient API to connect to any SSL server. I don't think implementing this solution will be easy. I briefly looked into doing it myself but gave up when it wasn't as simple as I anticipated. A sensible default would be to try all three methods on every SSL connection. There could be a way to control which method is tried first, or to disable the fallbacks, but it would have to be done in a nice way. I don't think either of those features is worth adding an extra parameter to nsock_connect_ssl, unless it's a structure that can express a lot of options at once. Please don't be discouraged. It means a lot that you wrote this code and offered it for review, not to mention the research that led you to it. All important patches of this sort get reviewed and modified before they are incorporated.
- I did some performance testing with 1000 random SSL servers with various ports open (443,465,636,990,995,993). I compared three instances of Nmap: 1. Nmap-4.85BETA3 2. Nmap-4.85BETA3 with the new nmap-services-probes file provided in the patch. Note that for this test, I had to slightly change the nmap-services-probes file, replacing all "ssl3" and "tls1" detections with "ssl", as Nmap-4.85BETA3 does not know these other versions. 3. A patched version of Nmap-4.85BETA3 I used the following command: nmap -T4 -v -n -PN -sV -p443,465,636,990,995,993 -iL <list of 100 random SSL servers collected earlier via an iR scan> -d -oA <filename>
I especially want to thank you for these test results. They are an indication of intellectual honesty and rigor as a developer. The results (with the typo correction in http://seclists.org/nmap-dev/2009/q1/0485.html), show that this SSL fix, while not urgent, is worthwhile. David Fifield _______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://SecLists.Org
Current thread:
- [PATCH] Extended SSL support in Nmap Kristof Boeynaems (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Kristof Boeynaems (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Brandon Enright (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Kristof Boeynaems (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Brandon Enright (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Kristof Boeynaems (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap doug (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap Kristof Boeynaems (Feb 21)
- Re: [PATCH] Extended SSL support in Nmap, review David Fifield (Mar 02)
- Re: [PATCH] Extended SSL support in Nmap, review Kristof Boeynaems (Mar 03)
- Re: [PATCH] Extended SSL support in Nmap, review David Fifield (Mar 03)
- Re: [PATCH] Extended SSL support in Nmap, review Kristof Boeynaems (Mar 22)
- Re: [PATCH] Extended SSL support in Nmap, review David Fifield (Mar 30)
- Re: [PATCH] Extended SSL support in Nmap, review Kristof Boeynaems (Mar 31)
- Re: [PATCH] Extended SSL support in Nmap, review David Fifield (Mar 31)
- Re: [PATCH] Extended SSL support in Nmap, review Kristof Boeynaems (Mar 03)