Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing support for TLS server_name on Windows #700

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

Missing support for TLS server_name on Windows #700

nnposter opened this issue Feb 24, 2017 · 1 comment

Comments

@nnposter
Copy link

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
Copy link

dmiller-nmap commented Feb 24, 2017

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants