Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: [PATCH] Extended SSL support in Nmap, review
From: Kristof Boeynaems <kristof.boeynaems () gmail com>
Date: Tue, 3 Mar 2009 20:10:16 +0100

On Tue, Mar 3, 2009 at 6:34 AM, David Fifield <david () bamsoftware com> wrote:
On Sat, Feb 21, 2009 at 08:01:33PM +0100, Kristof Boeynaems wrote:
I wrote a patch for the issue described at length in

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.

That is very well summarized.

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.

Again a very good summary.

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.

Yes, I don't like that code duplication either, that should indeed be
refactored. At the time I was eager to get a patch out for this (for
"demonstration purpose") and figured this was the most straightforward
way to do it, and less likely to break existing SSL support.
All suggestions for a better design are very welcome.

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 agree. The other alternative would be to specify TLSv1 and SSLv3
support at the command line, and that kind of implementation would be
very well supported by the current Nsock patch; but I agree that
"automatic guessing" would be far better suited for Ncat.
To make that reasonably performant, a different design should be implemented.

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.

It sounds indeed rather straightforward. In fact, you made a short
test program doing something like that earlier, see
What difficulties did you encounter this time?

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.

Thanks ;)

- 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.

Thanks again. By the way, I still owe the list the results of my
(little) more extensive scanning, I'll post them later on, when I have
again access to the data.

In the mean time, I encourage others to perform some testing as well!



Sent through the nmap-dev mailing list
Archived at http://SecLists.Org

  By Date           By Thread  

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