Libssh2 + ssh-brute support #910

Closed
wants to merge 34 commits into
from

Conversation

Projects
None yet
2 participants
@edeirme

edeirme commented Jun 16, 2017

This pull request is a collaborative effort of Devin Bjelland, Sergey Khegay and me. This request introduces the ssh-brute script and various other ssh scripts.

More specifically this pull request includes the following scripts:
ssh-brute
ssh-run
ssh-auth-methods
ssh-publickey-acceptance

The following libraries have been included:

libssh2 1.8.0
zlib 1.2.8

@edeirme edeirme changed the title from ssh to Libssh2 + ssh-brute support Jun 16, 2017

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 20, 2017

Can you please clean this up by removing the build directories (Debug_lib and Release_lib) from zlib? This is probably easiest done by adding them to the .gitignore file, as we have done with Debug and Release directories throughout. Generally speaking, we would like to reduce the amount of zlib included as much as is allowable by their license. Removing the contrib directory, the documentation, etc. may be possibilities. The same goes for libssh2.

Can you please clean this up by removing the build directories (Debug_lib and Release_lib) from zlib? This is probably easiest done by adding them to the .gitignore file, as we have done with Debug and Release directories throughout. Generally speaking, we would like to reduce the amount of zlib included as much as is allowable by their license. Removing the contrib directory, the documentation, etc. may be possibilities. The same goes for libssh2.

@edeirme

This comment has been minimized.

Show comment
Hide comment
@edeirme

edeirme Jun 20, 2017

Removed a lot of files. We are down from 711 to 382 added/changed files.

From libssh2, the directories docs, examples, test were removed.
From libz, the directories Debug_lib, Release_lib, docs, examples were removed.

The .gitignore file was also updated to include the Debug_lib and Release_lib directories.

I cannot remove the contrib directory of libz since the visual studio files are contained in that directory.

I haven't removed the test directory from libz. The compilation was failing without it. More changes will need to be performed to the library in order to exclude that directory.

edeirme commented Jun 20, 2017

Removed a lot of files. We are down from 711 to 382 added/changed files.

From libssh2, the directories docs, examples, test were removed.
From libz, the directories Debug_lib, Release_lib, docs, examples were removed.

The .gitignore file was also updated to include the Debug_lib and Release_lib directories.

I cannot remove the contrib directory of libz since the visual studio files are contained in that directory.

I haven't removed the test directory from libz. The compilation was failing without it. More changes will need to be performed to the library in order to exclude that directory.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 20, 2017

Some findings from the check script:

scripts/ssh-brute.nse:61: Found unrequired NSE library "libssh2".
scripts/ssh-brute.nse:86: Found bad indexed global "methods".
scripts/ssh-brute.nse:85: Found bad set global "methods".
scripts/ssh-publickey-acceptance.nse:69: Found unrequired NSE library "string".
scripts/ssh-publickey-acceptance.nse:56: Found bad indexed global "username".
scripts/ssh-run.nse:78: Found unrequired NSE library "table".

The one about "username" is causing a script crash when I try ssh-publickey-acceptance against localhost with a single username and single public key file. More comments coming soon.

Some findings from the check script:

scripts/ssh-brute.nse:61: Found unrequired NSE library "libssh2".
scripts/ssh-brute.nse:86: Found bad indexed global "methods".
scripts/ssh-brute.nse:85: Found bad set global "methods".
scripts/ssh-publickey-acceptance.nse:69: Found unrequired NSE library "string".
scripts/ssh-publickey-acceptance.nse:56: Found bad indexed global "username".
scripts/ssh-run.nse:78: Found unrequired NSE library "table".

The one about "username" is causing a script crash when I try ssh-publickey-acceptance against localhost with a single username and single public key file. More comments coming soon.

nse_utility.h
@@ -8,9 +8,9 @@ class Target;
#include "nmap_config.h"
#endif
-#if HAVE_STDINT_H
+//#if HAVE_STDINT_H

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 20, 2017

Don't change this; instead add this to nmap_winconfig.h:

diff --git a/nmap_winconfig.h b/nmap_winconfig.h
index bb59dfa..912d790 100644
--- a/nmap_winconfig.h
+++ b/nmap_winconfig.h
@@ -144,6 +144,8 @@
 
 #define HAVE_OPENSSL 1
 #define HAVE_SSL_SET_TLSEXT_HOST_NAME 1
+/* Since MSVC 2010, stdint.h is included as part of C99 compatibility */
+#define HAVE_STDINT_H 1
 
 #define LUA_INCLUDED 1
 #undef PCAP_INCLUDED
@dmiller-nmap

dmiller-nmap Jun 20, 2017

Don't change this; instead add this to nmap_winconfig.h:

diff --git a/nmap_winconfig.h b/nmap_winconfig.h
index bb59dfa..912d790 100644
--- a/nmap_winconfig.h
+++ b/nmap_winconfig.h
@@ -144,6 +144,8 @@
 
 #define HAVE_OPENSSL 1
 #define HAVE_SSL_SET_TLSEXT_HOST_NAME 1
+/* Since MSVC 2010, stdint.h is included as part of C99 compatibility */
+#define HAVE_STDINT_H 1
 
 #define LUA_INCLUDED 1
 #undef PCAP_INCLUDED

This comment has been minimized.

@edeirme

edeirme Jun 21, 2017

It doesn't seem to compile in windows when adding the HAVE_STDINT_H flag in nmap_winconfig.h

@edeirme

edeirme Jun 21, 2017

It doesn't seem to compile in windows when adding the HAVE_STDINT_H flag in nmap_winconfig.h

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 21, 2017

Ah, we need to include nmap_winconfig.h here.

#ifdef HAVE_CONFIG_H
#include "nmap_config.h"
#else
#ifdef WIN32
#include "nmap_winconfig.h"
#endif /* WIN32 */
#endif /* HAVE_CONFIG_H */
@dmiller-nmap

dmiller-nmap Jun 21, 2017

Ah, we need to include nmap_winconfig.h here.

#ifdef HAVE_CONFIG_H
#include "nmap_config.h"
#else
#ifdef WIN32
#include "nmap_winconfig.h"
#endif /* WIN32 */
#endif /* HAVE_CONFIG_H */

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 21, 2017

Ah, we need to include nmap_winconfig.h here.

#ifdef HAVE_CONFIG_H
#include "nmap_config.h"
#else
#ifdef WIN32
#include "nmap_winconfig.h"
#endif /* WIN32 */
#endif /* HAVE_CONFIG_H */
@dmiller-nmap

dmiller-nmap Jun 21, 2017

Ah, we need to include nmap_winconfig.h here.

#ifdef HAVE_CONFIG_H
#include "nmap_config.h"
#else
#ifdef WIN32
#include "nmap_winconfig.h"
#endif /* WIN32 */
#endif /* HAVE_CONFIG_H */
@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 20, 2017

Can we add library version output for libssh2 and zlib just like we do for openssl, libpcre, etc.? See display_nmap_version in nmap.cc. This will make it easier to check whether different configure options are being respected.

Can we add library version output for libssh2 and zlib just like we do for openssl, libpcre, etc.? See display_nmap_version in nmap.cc. This will make it easier to check whether different configure options are being respected.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 21, 2017

Compiler warning (LLDB on OS X):

nse_libssh2.cc:733:9: warning: variable 'buf' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    if (lua_isstring(L, 3))
        ^~~~~~~~~~~~~~~~~~
nse_libssh2.cc:738:50: note: uninitialized use occurs here
    while ((rc = libssh2_channel_write(*channel, buf, buflen)) == LIBSSH2_ERROR_EAGAIN) {
                                                 ^~~
./libssh2/include/libssh2.h:819:43: note: expanded from macro 'libssh2_channel_write'
  libssh2_channel_write_ex((channel), 0, (buf), (buflen))
                                          ^
nse_libssh2.cc:733:5: note: remove the 'if' if its condition is always true
    if (lua_isstring(L, 3))
    ^~~~~~~~~~~~~~~~~~~~~~~
nse_libssh2.cc:729:20: note: initialize the variable 'buf' to silence this warning
    const char *buf;
                   ^
                    = NULL
1 warning generated.

EDIT: This is caused because clang doesn't know that luaL_error is not a returning function. You can avoid it by using return luaL_error("...") so that the compiler knows that execution will not continue.

dmiller-nmap commented Jun 21, 2017

Compiler warning (LLDB on OS X):

nse_libssh2.cc:733:9: warning: variable 'buf' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    if (lua_isstring(L, 3))
        ^~~~~~~~~~~~~~~~~~
nse_libssh2.cc:738:50: note: uninitialized use occurs here
    while ((rc = libssh2_channel_write(*channel, buf, buflen)) == LIBSSH2_ERROR_EAGAIN) {
                                                 ^~~
./libssh2/include/libssh2.h:819:43: note: expanded from macro 'libssh2_channel_write'
  libssh2_channel_write_ex((channel), 0, (buf), (buflen))
                                          ^
nse_libssh2.cc:733:5: note: remove the 'if' if its condition is always true
    if (lua_isstring(L, 3))
    ^~~~~~~~~~~~~~~~~~~~~~~
nse_libssh2.cc:729:20: note: initialize the variable 'buf' to silence this warning
    const char *buf;
                   ^
                    = NULL
1 warning generated.

EDIT: This is caused because clang doesn't know that luaL_error is not a returning function. You can avoid it by using return luaL_error("...") so that the compiler knows that execution will not continue.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 21, 2017

Breaks on OS X due to libssh2's configure script ignoring the --with-openssl=X option. Its version is --with-libssl-prefix=X. I found this hack to work OK:

diff --git a/configure.ac b/configure.ac
index 5ba1a04..47e98f0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,7 @@ AC_HELP_STRING([--with-openssl=DIR],[Use optional openssl libs and includes from
     ;;
   *)
     specialssldir="$with_openssl"
+    ac_configure_args="$ac_configure_args '--with-libssl-prefix=$with_openssl'"
     CPPFLAGS="$CPPFLAGS -I$with_openssl/include"
     LDFLAGS="$LDFLAGS -L$with_openssl/lib"
     ;;

and of course run autoconf to regenerate configure.

Of note, this is probably the right solution for nmap/ncrack#1, not making modifications to opensshlib as was done to close that issue.

Breaks on OS X due to libssh2's configure script ignoring the --with-openssl=X option. Its version is --with-libssl-prefix=X. I found this hack to work OK:

diff --git a/configure.ac b/configure.ac
index 5ba1a04..47e98f0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,7 @@ AC_HELP_STRING([--with-openssl=DIR],[Use optional openssl libs and includes from
     ;;
   *)
     specialssldir="$with_openssl"
+    ac_configure_args="$ac_configure_args '--with-libssl-prefix=$with_openssl'"
     CPPFLAGS="$CPPFLAGS -I$with_openssl/include"
     LDFLAGS="$LDFLAGS -L$with_openssl/lib"
     ;;

and of course run autoconf to regenerate configure.

Of note, this is probably the right solution for nmap/ncrack#1, not making modifications to opensshlib as was done to close that issue.

@edeirme

This comment has been minimized.

Show comment
Hide comment
@edeirme

edeirme Jun 21, 2017

Fixed the check script warnings.
Added libssh2 and libz versioning (For some reason, the output is in parenthesis, will investigate later e.g. openssl-1.1.0f libssh2-(1.8.0) libz-(1.2.8) nmap-libpcre-7.6 ).
The compiler warning in LLDB OSX should be gone.
Updated the autoconfig file to support your changes for the OSX build.
Added a small fix for ssh-publickey-acceptance.nse

edeirme commented Jun 21, 2017

Fixed the check script warnings.
Added libssh2 and libz versioning (For some reason, the output is in parenthesis, will investigate later e.g. openssl-1.1.0f libssh2-(1.8.0) libz-(1.2.8) nmap-libpcre-7.6 ).
The compiler warning in LLDB OSX should be gone.
Updated the autoconfig file to support your changes for the OSX build.
Added a small fix for ssh-publickey-acceptance.nse

nse_libssh2.cc
+ struct ssh_userdata *sshu = NULL;
+
+ sshu = (struct ssh_userdata *) nseU_checkudata(L, 1, SSH2_UDATA, "ssh2");
+ if (sshu) {

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This condition should also cover the closing of the sockets below, otherwise if sshu is NULL then it's a null pointer dereference. This line could be written as if (!sshu) { return 0; } to avoid indenting the rest of the function, or even return luaL_error(L, "some error");, though I am not sure what the exact sequence of events would have to be in order for it to be NULL.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This condition should also cover the closing of the sockets below, otherwise if sshu is NULL then it's a null pointer dereference. This line could be written as if (!sshu) { return 0; } to avoid indenting the rest of the function, or even return luaL_error(L, "some error");, though I am not sure what the exact sequence of events would have to be in order for it to be NULL.

This comment has been minimized.

@@ -0,0 +1,120 @@
+--

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This needs to be --- (3 hyphens) in order for this block to be parsed as NSEdoc.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This needs to be --- (3 hyphens) in order for this block to be parsed as NSEdoc.

This comment has been minimized.

nselib/libssh2.luadoc
+--
+-- @author Devin Bjelland
+-- @author Sergey Khegay
+-- @copyright same as Nmap

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Please use the exact "Same as Nmap--See https://nmap.org/book/man-legal.html" string here.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Please use the exact "Same as Nmap--See https://nmap.org/book/man-legal.html" string here.

This comment has been minimized.

nselib/libssh2-utility.lua
+ while not libssh2.channel_eof(channel) do
+ data = libssh2.channel_read(self.session, channel)
+ if data then
+ buff = buff .. data

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Concatenation within a loop is potentially dangerous (CPU sink due to repeated memory allocation and copy). Make buff a table and append to it, then return the concatenation of the table.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Concatenation within a loop is potentially dangerous (CPU sink due to repeated memory allocation and copy). Make buff a table and append to it, then return the concatenation of the table.

This comment has been minimized.

nselib/libssh2-utility.lua
+-- @return true on success or false on failure.
+function SSHConnection:publickey_auth(username, privatekey_file, passphrase)
+ if not passphrase then
+ local passphrase = ""

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This local variable goes out of scope in the very next line. Probably this block doesn't need to be here. libssh2.userauth_publickey should understand a nil passphrase to mean no passphrase.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

This local variable goes out of scope in the very next line. Probably this block doesn't need to be here. libssh2.userauth_publickey should understand a nil passphrase to mean no passphrase.

This comment has been minimized.

nselib/libssh2-utility.lua
+end
+
+
+function SSHConnection:list(username)

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Need NSEdoc for the rest of these functions and complete NSEdoc for a few of the others.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

Need NSEdoc for the rest of these functions and complete NSEdoc for a few of the others.

This comment has been minimized.

nselib/libssh2-utility.lua
+-- @param username A username to authenticate as.
+-- @param password A password to authenticate as.
+-- @return true on success or false on failure.
+function SSHConnection:password_auth(username, password)

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

After this is committed, it will be important to add keyboard_interactive auth also.

@dmiller-nmap

dmiller-nmap Jun 23, 2017

After this is committed, it will be important to add keyboard_interactive auth also.

This comment has been minimized.

@edeirme

edeirme Jun 25, 2017

Do you want this to be added in the nse_libssh2.cc and the libssh2-utility.lua as a function or as a part of the ssh scripts as well?

@edeirme

edeirme Jun 25, 2017

Do you want this to be added in the nse_libssh2.cc and the libssh2-utility.lua as a function or as a part of the ssh scripts as well?

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jun 26, 2017

Don't worry about it until this PR is merged first. But the goal would be for ssh-brute to use either one transparently, since many users don't/can't perceive a difference. Logic would look something like:

  1. If password auth is supported, do that.
  2. If not, but keyboard-interactive auth is supported, try to see if it's a password prompt.
  3. If it's a password prompt, continue brute forcing over keyboard-interactive.

Keyboard-interactive can be other things, though, so maybe we'll have to add different script-args to enable other kinds of prompts, but the most common is a simple PAM password prompt. I'd guess (though haven't researched that part of the protocol) that the API would be something like handing the script a socket connected to the other end, and let the script do prompt detection (like telnet-brute does) and actual authentication.

Another fun thing might be a script to grab the "banner" of keyboard-interactive auth (OpenSSH's sshd_config uses the Banner directive for this).

@dmiller-nmap

dmiller-nmap Jun 26, 2017

Don't worry about it until this PR is merged first. But the goal would be for ssh-brute to use either one transparently, since many users don't/can't perceive a difference. Logic would look something like:

  1. If password auth is supported, do that.
  2. If not, but keyboard-interactive auth is supported, try to see if it's a password prompt.
  3. If it's a password prompt, continue brute forcing over keyboard-interactive.

Keyboard-interactive can be other things, though, so maybe we'll have to add different script-args to enable other kinds of prompts, but the most common is a simple PAM password prompt. I'd guess (though haven't researched that part of the protocol) that the API would be something like handing the script a socket connected to the other end, and let the script do prompt detection (like telnet-brute does) and actual authentication.

Another fun thing might be a script to grab the "banner" of keyboard-interactive auth (OpenSSH's sshd_config uses the Banner directive for this).

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 23, 2017

That's all the comments I have. It would be nice to have trailing whitespace cleaned up and check for proper indentation: https://secwiki.org/w/Nmap/Code_Standards

That's all the comments I have. It would be nice to have trailing whitespace cleaned up and check for proper indentation: https://secwiki.org/w/Nmap/Code_Standards

@edeirme

This comment has been minimized.

Show comment
Hide comment
@edeirme

edeirme Jun 25, 2017

Made most of the changes.
Added documentation.
Removed buggy/redundant code.
Used the lua-format scripts for the added lua files.

edeirme commented Jun 25, 2017

Made most of the changes.
Added documentation.
Removed buggy/redundant code.
Used the lua-format scripts for the added lua files.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jun 26, 2017

I'm satisfied with this. If your mentor agrees, please merge and commit to SVN.

I'm satisfied with this. If your mentor agrees, please merge and commit to SVN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment