Re: [nmap-svn] r31823 - nmap-exp/d33tah/ncat-lua-callbacks/ncat

[Jacek Wielemborek <wielemborekj1 () gmail com>]

2013/8/15 David Fifield <david () bamsoftware com>:
> On Thu, Aug 15, 2013 at 12:10:48AM +0200, Jacek Wielemborek wrote:
> > First of all, I found the cause behind the segfault I experienced
> > during our last meeting. Accidentally, when trying to fix your ROT13
> > script, I added a redundant super:send before the return
> > rot13(super:send()) thing. This made the second call not get anything,
> > because we just flushed the buffer and we're not in a blocking mode.
> > This made it return EAGAIN, but because I did no error checking, I
> > tried to do something like memcmp(dst,src,-1).
> >
> > Once I added the checks, I noticed that an extra ncat_recv_raw call
> > returning with EAGAIN makes us quit from the read_socket() loop and,
> > as an effect, close the socket. The reason is that so far we assumed
> > that socket returning on select() has some data to read, so we call
> > ncat_recv and if it yields nothing despite the select() signalling
> > data, the connection must have broken. It's not true anymore and
> > that's why I changed all ncat_recv calls to check errno for EAGAIN
> > before they consider a return value of 0 to be an error.
> >
> > There were two issues left: I had to somehow pass the ncat_recv_raw's
> > value to differentiate between the disconnection and just EAGAIN and
> > solve the problem of user returning "", which makes Lua code set
> > buffer length to zero (rightly so), and zero return value from
> > ncat_recv meant a connection closed. In this case, I manually change
> > errno to EAGAIN, so that the ncat_recv callers can notice that.
>
> I do not like this abuse of EAGAIN. It sounds as though you are trying
> to handle too many layers at once.

Believe me, neither do I ;)

> You should try inverting your abstractions. If ncat_recv returns the
> string result of possibly many layers of Lua filters, that indeed makes
> it hard to test whether the socket is closed. So don't do that. When you
> find that a socket is closed (through a zero-byte read), all you need to
> do is report it to the next layer up, and only in that one place. The
> Lua code in the next layer is now responsible for doing something
> intelligent with the error. You should read about how sockets work in
> NSE:
>         http://nmap.org/book/nse-api.html#nse-api-networkio-connect
>         http://nmap.org/nsedoc/lib/nmap.html#receive_bytes
> When a socket read fails, you want to return (nil, err) to the caller.
> You are almost certainly going to have to define a new function at the
> top level that understands those Lua return values and transforms them
> into something useful for the C caller. You probably will not be able to
> use a zero-byte string as a close signal at this level.
>
> In other words, you might want to have ncat_recv at the bottom of your
> abstraction layers, not at the top.

It's actually both at the top and at the bottom. I still have no idea
how to solve the problem of empty strings - I want to let the bottom
know: "there's nothing there, but don't quit, don't loop over, just
fall back to select". Not sure how to handle it yet.

> If you are having trouble with when to call a C function and when to
> call a Lua function, define a minimal Lua socket interface that just
> passes its data through verbatim, and always make that Lua function the
> thing you call from C. That way you can standardize your return
> handling.
>
> > BTW, is this the kind of mail I should have CC'ed to dev () nmap org? If
> > so, please CC the answer there as well.
>
> David Fifield
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/