Re: Built-in authentication for http.lua

[David Fifield <david () bamsoftware com>]

On Thu, Sep 23, 2010 at 01:11:32AM -0400, Patrick Donnelly wrote:
> Hi David,
> On Wed, Sep 22, 2010 at 4:59 PM, David Fifield <david () bamsoftware com> wrote:
> > On Fri, Aug 13, 2010 at 03:09:52PM -0400, Patrick Donnelly wrote:
> > > On Fri, Aug 13, 2010 at 2:50 PM, Patrick Donnelly <batrick () batbytes com> wrote:
> > > > On Thu, Aug 12, 2010 at 4:54 PM, David Fifield <david () bamsoftware com> wrote:
> > > > > Patrick, do you have an idea why this is happening? I attached the patch
> > > > > that I used to diagnose it. The telltale parts are
> > > >
> > > > I'm not sure after a cursory glance, no. I don't know how those
> > > > threads are still running insert_cache after their destructors are
> > > > called.
> > >
> > > Oh, it seems that isn't what is happening. (Threads are *not* running
> > > insert_cache after having been destroyed/closed.) I'll look at this
> > > more later but in the mean time I suggest upping the debug level and
> > > printing the thread pointer in all debug output.
> >
> > I think there's a bug somewhere but I can't find it. I've been running
> > http-brute with just two threads. Somehow, when one of the worker
> > threads dies, it is calling the destructor (aux_mutex_done) of the other
> > thread.
> >
> > For example, my two worker threads are:
> >
> > NSE: 'http-brute' (thread: 0x1c96830) spawning new thread (thread: 0x1cc5af0).
> > NSE: 'http-brute' (thread: 0x1c96830) spawning new thread (thread: 0x1cc6610).
> >
> > 0x1cc5af0 is the first to finish when the unpwdb limit expires. I added
> > debug code to print the thread pointers in Thread:close and
> > aux_mutex_done. "->" is function entry and "<-" is function exit.
> >
> > NSE: Finished 'http-brute' worker (thread: 0x1cc5af0) against 127.0.0.1:80.
> > NSE: Thread:close -> 'http-brute' worker (thread: 0x1cc5af0)
> > NSE: destructor -> 'http-brute' worker (thread: 0x1cc5af0) function: 0x1c374e0
> > NSE: destructor <- 'http-brute' worker (thread: 0x1cc5af0) function: 0x1c374e0
> > NSE: destructor -> 'http-brute' worker (thread: 0x1cc5af0) table: 0x1c35640
> > mutex done aux_mutex_done 0x1cc6610
> > 0x1cc6610 mutex("done")
> > nse_destructor 0x1cc6610 'r'
> > _R[DESTRUCTOR]: 'http-brute' worker (thread: 0x1cc6610) or 'http-brute' worker (thread: 0x1cc6610) or 'http-brute' 
> > worker (thread: 0x1cc5af0) -> 'http-brute' worker (thread: 0x1cc6610)
> > NSE: destructor <- 'http-brute' worker (thread: 0x1cc5af0) table: 0x1c35640
> > NSE: Thread:close <- 'http-brute' worker (thread: 0x1cc5af0)
> >
> > See how even though it is 0x1cc5af0 that is ending, aux_mutex_done is
> > begin called on the other thread, 0x1cc6610. This causes an error later
> > when 0x1cc6610 releases its insert_cache mutex:
> >
> > mutex done insert_cache
> > 0x1cc6610 mutex("done")
> > NSE: 'http-brute' worker (thread: 0x1cc6610) against 127.0.0.1:80 threw an error!
> > ./nselib/http.lua:877: 0x1cc6610 do not have a lock on this mutex
> > stack traceback:
> >        [C]: in function 'mutex'
> >        ./nselib/http.lua:877: in function 'insert_cache'
> >        ./nselib/http.lua:1037: in function 'get'
> >        ./scripts/http-brute.nse:67: in function 'login'
> >        ./nselib/brute.lua:490: in function 'doAuthenticate'
> >        ./nselib/brute.lua:522: in function 'main'
> >        ./nse_main.lua:690: in function <./nse_main.lua:690>
> >
> > NSE: Thread:close -> 'http-brute' worker (thread: 0x1cc6610)
> > NSE: Thread:close <- 'http-brute' worker (thread: 0x1cc6610)
> >
> > I attached the patch I'm using to generate this output. I don't
> > understand nse_main.lua well enough to know what's going on here.
> > Patrick, is this expected that one thread will call another thread's
> > destructor? The thread on which to call the destructor comes from the
> > yielded_base table; what's the purpose of this table?
> >
> > A recipe for testing locally is at http://seclists.org/nmap-dev/2010/q3/372.
>
> Ah, this was a rather subtle bug. Apparently we (that is, I) forgot to
> add the close_handlers field to the worker thread. Since all absent
> fields in the worker Thread table index the parent, both threads use
> the close_handler table of the parent which spawned the workers. The
> reason this isn't noticed until one thread exists is because when the
> thread X that is about to finish removes its (X's) destructor, it then
> adds the other thread's (Y's) destructor (within the aux_mutex
> function). Because they share the same destructor table, when X calls
> each of its destructors it accidentally calls Y's destructor.

Excellent, thanks for your help. I applied your patch and removed the
one-thread restriction in http-brute.nse.

David Fifield
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/