Missing support for TLS server_name on Windows #700

Closed
nnposter opened this Issue Feb 24, 2017 · 1 comment

Projects

None yet

2 participants

@nnposter

When running on Windows, Nsock currently does not provide support for TLS extension server_name, which has a large impact on successfully scanning TLS services.

The core reason appears to be that there is no configure equivalent available on Windows so the feature set is instead hard-coded across the following files:

All these files contain an unconditional disposition that OpenSSL is available:

#define HAVE_OPENSSL 1

However, none provides any hint about the availability of TLS extension server_name, such as including:

#define HAVE_SSL_SET_TLSEXT_HOST_NAME 1

Note that there is only one location in the entire codebase where HAVE_SSL_SET_TLSEXT_HOST_NAME is being used, namely in nsock/src/nsock_core.c, line 394.

The obvious quick-fix remediation is to add this #define to the four above-mentioned header files. (Strictly speaking, nbase/nbase_winconfig.h is the only place where the change is needed; the rest would be done for consistency.)

I have done so in my copy of the source code, confirming that doing so does rectify the original problem.

That said, it is unclear to me why it is necessary to test explicitly for the existence of the extension...

  AC_MSG_CHECKING([for SSL_set_tlsext_host_name])
  AC_TRY_LINK([#include <openssl/ssl.h>], [SSL_set_tlsext_host_name(NULL, NULL)],
    [AC_MSG_RESULT([yes]); AC_DEFINE(HAVE_SSL_SET_TLSEXT_HOST_NAME)],
    [AC_MSG_RESULT([no])])

...instead of assuming its availability based on HAVE_OPENSSL. In other words, the code implies that somehow it is possible to have OpenSSL that does not support the extension. Maybe that was indeed the case some time ago but these days the only supported versions of OpenSSL are 1.0.2 and 1.1.0, which do provide the support.

I would be happy to put in either fix. The first one is dead-simple but the second one seems to be the right way to address it. Opinions?

If I do not get any feedback I will commit the first fix in a few weeks.

@dmiller-nmap
dmiller-nmap commented Feb 24, 2017 edited

I tend to agree that we don't need to worry greatly about supporting ancient versions of OpenSSL, but we do generally support old and unusual configurations that could be problematic if we made it unconditional. Since we are stricter about what Windows build configurations we support, let's go with your initial fix (adding the #define to the appropriate winconfig.h files). Be sure to close this issue when it's done by including "Fixes #700" in the commit message. Thanks!

@nmap-bot nmap-bot closed this in 32d8500 Feb 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment