Re: Writeup on `brute.lua` Modification

[Sergey Khegay <g.sergeykhegay () gmail com>]

Hello,

I have updated the performance table.

o. Some statistics
||=====================================================================||
|| # accounts   |          nmap          |  ncrack   |  hydra          ||
||              | original  |  modified  |           |                 ||
||--------------|-----------|------------|-----------|-----------------||
|| 80           | 29.13 (+) | 40.27  (+) | 24.03 (+) | 16.76 (+)       ||
|| 400          | 126.24 (+)| 85.06  (+) | 60.07 (+) | 90.06 (+)       ||
|| 80 *         | 3.62(-) ! | 85.00  (+) | 15.01 (-) | did not stop (-)||
|| 400 *        | 7.22(-) ! | 318.06 (+) | 12.07 (-) | did not stop (-)||
||---------------------------------------------------------------------||

tested script: ftp-brute

 *  - special conditions: the number of connections per IP were set to 5
(+) - correct credentials were found
(-) - no credentials were found even though they were in the list
 -  - test was not conducted
 !  - almost the same time with assumed bug with max_retries and without it


As you can see the modified version performs slower than the original one
on the small sets of account on a reliable network. This can be explained
by the way we grow the number of coroutines.

In the modified version, I start with 1 coroutine, then gradually grow that
number until `max_threads` limit is reached (the default value of
`max_threads` for the modified version is 50, so there can be maximum of 50
coroutines). Maybe I should start with more coroutines initially and
introduce
a new parameter for user tuning.

In the standard version, the default value of `max_threads` is 10. And they
all fire up from the start. Actually the original version will always
perform better under ideal conditions (no protocol specific exception,
reliable connection, etc) if we set the `max_threads` big enough manually.


I think although the table might show some improvements, the number of
conducted test is not representative enough. Also I'm mostly concerned with
backward compatibility with other scripts. And there are some details
to be discussed.


Best regards,
Sergey

On Fri, May 27, 2016 at 7:38 PM, Sergey Khegay <g.sergeykhegay () gmail com>
wrote:

> ## `brute.lua` write up. 05.27.16
>
> I have made changes to the library, and I want to share my approach.
>
> The general idea is crystal clear: We want to perform as many parallel
> connection attempts as possible (considering network conditions, server
> configuration).
>
> E.g. We wouldn't like to run 100 threads attacking a server which only
> accepts
> 5 parallel connections from a given IP, this would be resource consuming
> and
> mostly counter-productive.
>
>
>
> # INDICATIONS OF PROBLEMS
>
> I have considered two types:
> - protocol specific
> - connection specific
>
> i. Protocol specific problems
> Some very nice protocols would generously notify the user that there was a
> problem, and they would do so on Application level.
>
> E.g. ftp can reply with code 421 "There are too many connections from your
> IP
> address".
>
>
> ii. Connection specific problems
> Due to network congestion or server configuration some of our attempts
> might
> fail on transport level. A connection might timeout, or a server could send
> a RST packet. Or there might be a firewall which will just drop packets
> from
> very annoying sources.
>
>
>
> # HOW `brute.lua` USED TO WORK
>
> `brute.lua` would just spawn a default, 10, or user specified number of
> coroutines and wait until all of them finish their work and die.
>
> Each coroutine would start with `Engine.login` function, which basically
> loops until the task is done, or it is forced to die by setting up a
> global
> flag like `Engine.terminate_all`. On every iteration `Engine.login` calls
> `Engine.doAuthenticate` and checks the result of the operation, upon which
> it
> can make a decision to continue or to stop.
>
> `Engine.doAuthenticate` actually performs login attempts using user's
> `Driver`
> class, with 3 mandatory methods: `connect`, `login`, `disconnect` (`check`
> is
> deprecated). The work is follows: create a `Driver` instance `driver`, call
> `driver.connect`, retrieve next set of accounts from a global iterator,
> attempt `driver.login`, close the connection `driver.disconnect`.
>
> If the whole operation is successful `Engine.doAuthenticate` will return
> valid credentials for further processing by `Engine.login`, otherwise it
> will
> perform up to `Engine.max_retries` reattempts. (At this point a possible
> bug
> was found. Every coroutine has a separate number of max_retries, it is not
> a
> global state. But in the current implementation if a coroutine exhausts
> all
> it's attempts, then it shuts down the whole Engine, which is unlikely to
> be a
> desired behavior.)
>
> Underline:
> - There is always constant number of coroutines
> - The engine does not really care if the login attempt was unsuccessful
> because
>   of the wrong credential pair, or it was a protocol or connection specific
>   error.
>
>
>
> # NEW APPROACH
>
> i. Protocol specific problems.
> Obviously protocol specific exceptions should be handled by *-brute
> module's
> author. `brute.lua` only requires to set a flag in the returned `Error`
> instance.
>
> Modified `Error` class:
> // brute.lua: 290
>
> Error =
> {
>   retry = false,
>
>   new = function(self, msg)
>     local o = { msg = msg, done = false, reduce = nil }
>     setmetatable(o, self)
>     self.__index = self
>     return o
>   end,
>
>   ... no changes here ...
>
>   --- Set the error as reduce the number of running threads
>   --
>   -- @param r boolean true if should reduce, unset or false if not
>   setReduce = function( self, r ) self.reduce = r end,
>
>   --- Checks if the error signals to reduce the number of running threads
>   --
>   -- @return status true if reduce, false otherwise
>   isReduce = function( self )
>     if ( self.reduce ) then return true end
>     return false
>   end
> }
>
> //---
>
> Upon receiving an error the Engine will check if `error.isReduce() ==
> true`.
> If it is the case, then the pair of credentials will be saved for retry
> attempt. The engine will disregard any retries and will just return error
> and
> saved credentials to `Engine:login()`.
>
> // brute.lua: 610 / Engine:doAuthenticate()
>
> if ( not(status) and response:isReduce() ) then
>   local ret_creds = {}
>   ret_creds.username = username
>   ret_creds.password = password
>   return false, response, ret_creds
> end
>
> //---
>
>
> `Engine.login()` assumes that if credentials were sent, then it should
> notify
> the main thread to reduce the number of running coroutines.
>
> // brute.lua: 671 / Engine:login()
>
> ...
>
> local status, response, ret_creds = self:doAuthenticate()
>
> ...
>
> if ( status ) then
>
>   ... no change here...
>
> elseif ( ret_creds ) then
>   -- add credentials to a vault
>
>   self.retry_accounts[#self.retry_accounts + 1] = {
>     username = ret_creds.username,
>     password = ret_creds.password
>   }
>   -- notify the main thread that there were an error on this coroutine
>   thread_data.error = true
>
>   condvar "signal"
>   condvar "wait"
> else
>
>   ... no changes here ...
>
> end
>
> //---
>
>
> The main thread runs in a loop. On every iteration it checks if there were
> any user specified exceptions. If there were, then then main thread
> terminates
> (sets a flag for the coroutine to finish) ONE of the running coroutines
> (even though many could have reported exceptions)
>
> Check out:
> // brute.lua: 915 / Engine.start()
>
>
>
> ii. Connection specific problems
> What if `socket:connect()` returns a pair (false, "ERROR"). This is not
> protocol specific, this has something to do with network congestion or
> server
> settings. We do not want the user to implement a myriad of checks and set
> `Error.setReduce(true)` every other time.
>
> We can deal with these problems without user interaction at all.
> In the current implementation of Lua Nsock's wrapping all socket operations
> might return following error messages:
>
> "EOF", "ERROR", "TIMEOUT", "PROXYERROR"
>
> We are mostly concerned about "ERROR" and "TIMEOUT" ( should we include
> "PROXYERROR" too? Or all four? )
>
>
> We can catch those errors on the fly by making the user use a socket
> wrapper.
>
> // brute.lua: 1219
>
> BruteSocket = {
>   new = function(self)
>     local o = {
>       socket = nil,
>     }
>     setmetatable(o, self)
>
>     -- we want to use original socket operations if we did not
>     -- reimplemented them in this class
>     self.__index = function(table, key)
>       if self[key] then
>         return self[key]
>       elseif o.socket[key] then
>         if type( o.socket[key] ) == "function" then
>           -- this is a hack to call a socket operation on the actual
>           -- socket instead of our wrapping instance
>
>           return function(self, ...)
>             return o.socket[key](o.socket, ...)
>           end
>         else
>           return o.socket[key]
>         end
>       end
>
>       return nil
>     end
>
>     o.socket = nmap.new_socket()
>
>     if not( Engine.THREAD_POOL[ coroutine.running() ] ) then
>       Engine.THREAD_POOL[ coroutine.running() ] = {
>         error = false,
>         reason = nil,
>         username = nil,
>         password = nil
>       }
>     end
>
>     return o
>   end,
>
>   -- returns raw socket
>   getSocket = function(self)
>     return self.socket
>   end,
>
>   -- checks the status of the operation and the error code
>   -- if the status is false and the error code is ERROR OR TIMEOUT
>   -- then save the credentials that are used by the current coroutine
>   -- and report an error to the main engine
>   checkStatus = function(self, status, err)
>     if not( status ) then
>       stdnse.debug1("checkStatus ERROR: %s", err)
>     end
>     if not( status ) and (err == "ERROR" or err == "TIMEOUT") then
>       local thread_data = Engine.THREAD_POOL[ coroutine.running() ]
>       Engine.retry_accounts[ #Engine.retry_accounts + 1 ] = {
>         username = thread_data.username,
>         password = thread_data.password
>       }
>
>       thread_data.error = true
>       thread_data.reason = err
>     end
>   end,
>
>   -- we want to track the result of operations below
>
>   connect = function(self, host, port)
>     local status, err = self.socket:connect(host, port)
>     self:checkStatus(status, err)
>
>     return status, err
>   end,
>
>   send = function(self, data)
>     local status, err = self.socket:send(data)
>     self:checkStatus(status, err)
>
>     return status, err
>   end,
>
>   receive = function(self)
>     local status, data = self.socket:receive()
>     self:checkStatus(status, data)
>
>     return status, data
>   end,
>
>   close = function(self)
>     self.socket:close()
>   end,
>
> }
>
> //---
>
>
> We also specify a method to get our wrapped socket, which can be used as
> `brute.new_socket` instead of `nmap.new_socket`
>
> //
>
> new_socket = function()
>   return BruteSocket:new()
> end
>
> //---
>
>
> So now if the user wants to activate `brute.lua`'s smart behavior all he
> has
> to do is to use `brute.new_socket` to create a socket.
>
> //
>
> self.socket = brute.new_socket()
>
> //---
>
> Every time when "ERROR" or "TIMEOUT" messages are caught, the main thread
> will be notified of a mistake and will react correspondingly (most likely,
> killing a coroutine, not necessary the one which got the error, it might be
> any).
>
>
>
> iii. Increasing the number of connections
> Important thing is to know when we can increase the number of connections.
> The basic rule here is if everything goes good then try to increase.
>
> The problem with this approach is that we do not actually know the current
> state for sure. Some coroutines might be waiting for the server response,
> some might report errors, but those errors could be old ones.
>
> I use a batch concept here. So a batch is just a collection of coroutines
> with
> fixed capacity. The idea is following:
> o. Initially the batch is empty. We let coroutines add themselves to the
> batch
>   if ( the coroutine is not already in the batch ) and ( the batch is not
> full ).
>
> o. We wait until the batch becomes full and all coroutines in the batch are
>   in the `ready` state (a coroutine is in `not ready` state if it is in
> the
>   batch and before it enters `doAuthenticate()`; and `ready` if it is in
> the
>   batch and when it exits `doAuthenticate()`).
>
> o. If since the time we created a batch and until the time the batch
> becomes
>   full and all coroutines in the batch are `ready` there we no exceptions,
>   then we add a new coroutine.
>
> o. If there were a mistake, we discard the batch and start assembling a new
>   one. No coroutine in the batch is affected when the batch is discarded,
> the
>   only thing that happens is the flag indicating that the coroutine is in
> the
>   batch is set to `false`.
>
>
>
>
> # TECHNICAL DETAILS AND CONCERNS
> o. Retry accounts storage.
>   Right now all accounts that are scheduled for retry live in
>   `Engine.retry_accounts`. That means that if there are two instances of
> the
>   Engine, they will share the same retry list!
>
>   Why is it so now?
>   There is no problem when a retry account is added due to PROTOCOL
> specific
>   exception, at that moment we know the exact instance of the `Engine` and
>   can add the accounts into specific instance's storage, thus not affecting
>   other instances.
>
>   The problem is with accounts added by the CONNECTION specific exceptions.
>   Retry account are added by the socket wrapper and it does not know which
>   instance it has to access to. The only connection between the instance
> and
>   the socket wrapper is the current coroutine, which technically does not
> know
>   about which instance it belongs too.
>
>   I should think how to avoid this.
>
> o. I want to note, that technically, the modification should be backward
>   compatible with all current scripts. So far it was tested only on
> `ftp-brute`
>
> o. Some statistics
> ||=====================================================================||
> || # accounts   |          nmap          |  ncrack   |  hydra          ||
> ||              | original  |  modified  |           |                 ||
> ||--------------|-----------|------------|-----------|-----------------||
> || 80           | 29.13 (+) | 40.27  (+) | 24.03 (+) | 16.76 (+)       ||
> || 400          |    -      | 85.06  (+) | 60.07 (+) | 90.06 (+)       ||
> || 80 *         |    -      | 85.00  (+) | 15.01 (-) | did not stop (-)||
> || 400 *        |    -      | 318.06 (+) | 12.07 (-) | did not stop (-)||
> ||---------------------------------------------------------------------||
>
> tested script: ftp-brute
>
>  *  - special conditions: the number of connections per IP were set to 5
> (+) - correct credentials were found
> (-) - no credentials were found even though they were in the list
>  -  - test was not conducted
>
>
> o. Current version is available on github
> brute.lua
>
> https://github.com/sergeykhegay/nmap/blob/gsoc/BUILD/share/nmap/nselib/brute.lua
>
> ftp-brute.nse
>
> https://github.com/sergeykhegay/nmap/blob/gsoc/BUILD/share/nmap/scripts/ftp-brute.nse
_______________________________________________
Sent through the dev mailing list
https://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/