Re: Writeup on `brute.lua` Modification v.2. Resource Management Feature.

[Patrick Donnelly <batrick () batbytes com>]

Hi Sergey,

First, let me thank you for the well-written and detailed write-up of
your changes. My responses in-line:

On Thu, Jun 9, 2016 at 1:33 AM, Sergey Khegay <g.sergeykhegay () gmail com> wrote:
> // nse_nsock.cc: 1055
>
> /* This function also has a binding in stdnse.lua */
> static int l_get_stats (lua_State *L) {
>   lua_newtable(L);
>   int idx = lua_gettop(L);
>
>   /* the only field so far is
>      connect_waiting - number of threads waiting for connection */
>   lua_pushinteger(L, nseU_tablen(L, CONNECT_WAITING));
>   lua_setfield(L, idx, "connect_waiting");
>
>   return 1;
> }

This should take an optional table as its first argument so that
repeated calls do not created unnecessary garbage (i.e. a new table
for each call). Look at the Lua os.time function for inspiration:

https://www.lua.org/manual/5.2/manual.html#pdf-os.time
https://www.lua.org/source/5.2/loslib.c.html#os_time

I would also recommend adding a field for the max-parallelism to the
table as well.

> Which is also accessible from `stdnse.lua` library. And logically should
> be used like this `local stats = stdnse.get_stats()`.

Hmm, I'd rather not see stdnse get any more bloated (it does too many
things already). How about adding this new function as a "static
method" to all sockets. So a script could do:

local socket = nmap.new_socket()
local stats = socket:get_stats()

Keep in mind that creating a socket does not yield the script (the
actual socket is created during connect I believe).

> # RESULTS
>
> I do not have a good table to show so far, but let me describe a bit.
>
> Environment:
> - remote server on Amazon AWS
> - local virtual machine
> - vsftpd daemon for ftp protocol
>
> I scan both machines at the same time, so two instances of the brute
> `Engine` are running. Nmap runs with following settings (they are set
> so intensionally).
>
> max_threads: 20
> max-parallelism: 10
>
>
> Run WITHOUT feature:
> o. Both `Engine` instances tried to spawn maximum allowed number of
>    threads, namely 20. So at some point there were ~40 threads in total.
>    Only 10 of them could work effectively, other were put into
>    CONNECT_WAITING table.
>
> o. Valid credentials were found on both servers.
>
> o. Running time: 55.23 sec.
>
>
> Run WITH feature:
> o. Both `Engine` instances spawned no more that 12 threads in total,
>    which is consistent with our `--max-parallelism` limit.
>
> o. All running threads were equally split between instances. This
>    happened only because both instances started almost at the same
>    time, so they were equally increasing the number of threads, until
>    the total number of threads hit the limit. Generally it might not
>    always be the case. If one instance starts much earlier then another
>    than the first one would have an advantage of using more threads.
>    But I consider this a normal behavior.
>
> o. Valid credentials were found on both servers.
>
> o. Running time: 57.58 sec.
>
> So both runs completed in almost the same time, but obviously the run
> with feature implemented used less resources.

Do you have an explanation for why the runtimes are the same? Are the
# of requests by each brute engine the same in each run? What happens
if you use only fewer threads per engine (maybe 2)?

> I also performed a test with one of the servers playing a bad guy,
> limiting the number of connections per ip to 5, and with following
> settings:
>
> max_threads: 20
> max-parallism: 20
>
> As was expected the `Engine` instance which were bruteforcing a "bad guy"
> spawned only ~5 threads (due to connection and protocol exception
> adaptability mechanisms first and resource management feature later)
> and the other instance used the rest 15 available slots for threads.

I like the adaptability! Possible good idea: what about making it more
adaptive to changing resource usage (i.e. socket usage) by other
scripts? I'm especially thinking of the case where the brute scripts
form the long tail of an NSE run. i.e. if the brute scripts
conservatively use 2 threads at the beginning because lots of other
scripts are using sockets, we want those brute scripts to increase the
number of threads once those other scripts complete.

> # CODE
>
> The code is available here:
> https://github.com/sergeykhegay/nmap/tree/gsoc-brute
>
> Files to review:
> nse_nsock.cc
> nselib/brute.lua
> nselib/stdnse.lua
> scripts/ftp-brute.nse

I'll look at this later and give you some comments.

> # GENERAL CONSERN
>
> I need to find a good metric to alert the engine to stop, as for now
> the `Engine` is too stubborn and, theoretically, might never give up
> on retrying bruteforce attempts.

Indeed! I think Devin nearly finished a feature in 2014 to add
debugging to NSE -- and in particular killing running scripts! -- but
a segmentation fault prevented us from merging. You might want to
revisit that if you're interested.

-- 
Patrick Donnelly
_______________________________________________
Sent through the dev mailing list
https://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/