Re: Ncat's Lua socket abstraction - API protection, error behavior?

[Jacek Wielemborek <wielemborekj1 () gmail com>]

2013/8/29 Daniel Miller <bonsaiviking () gmail com>:
> On 08/29/2013 11:05 AM, Jacek Wielemborek wrote:
> > Since all filter scripts share the same namespace, there's a risk that
> > one filter layer can break another and lead to some nasty crash.
>
> How nasty are we talking? Aborting the whole program is nasty, but doesn't
> hurt if you're just developing a new filter and did something wrong. As long
> as you get a decent error message that somewhat indicates what went wrong, I
> think allowing filter authors to cause crashes while developing filters is
> not a bad thing.

The trick is that filters can not only crash themselves, but also each
other. It makes testing a bit harder, even more so if you take into
account how many events there are (some of them being fired relatively
rarely, for example connect() and close()). I tried to make the error
messages a bit cryptic, but it's pretty difficult to find every place
where something could break and do extra checks there.

> > while there could be some hacks done with
> > userdata, tables and such, they pointed out that there might be no
> > point trying to create a bullet-proof API.
>
> An API doesn't need to be bullet-proof, just idiot-resistant. If there is a
> clear and easy way to do what is needed, it doesn't matter so much if
> there's a difficult and non-obvious way to break it. Security issues would
> be the exception; if you're designing an API that separates privilege
> levels, then you have to be more strict. In the case of Ncat filters, you
> would want to be sure that an author cannot accidentally create a security
> bug. Intentional security problems are different: if the filter runs every
> line through os.execute(), then prevention should be a cricket bat,
> liberally applied.

The current model is very open - children can access and modify all
the data of their parents (and it's possible I'll add a way to read
the access to their children as well to make it work in both
directions), access global variables and even freely interact with
other connections. That means that a security bug in any filter on any
connection could easily propagate everywhere. On the other hand, it's
the most powerful model - filters can modify their behavior in any way
you can think of. (see below for more explanation)

> > Perhaps it would be better do define things that just shouldn't be
> > done (like overwriting the registry) and assume that user can't do
> > them by mistake, so there's no point securing them all?
>
> This is essentially my view. Simple things like ensuring a value cannot be
> changed by simple assignment is all that should be necessary. If someone
> knows enough to mess around with metatables, then they can take
> responsibility for their own actions.
>
> But even in the worst case (perhaps one of your global vars can be
> overwritten, borking the whole program) does it hurt anyone?It's part of
> learning the development environment and API that you're setting up. I'm
> sure I could "break" NSE by doing dumb things, but then I'm just stuck with
> a script that breaks for me. Nobody else will accept it, because it's
> broken.Unless I broke it in a clever and new way (hacked it) that introduces
> useful functionality---this is the potential upside of a "fragile" API.
>
> Dan

Yeah, well, Lua is very open and it's probably a bit too open (and I
don't think it's a good idea to remove the "debug" library and remove
functions like rawset() to secure it). It's a bit like Ruby - call
"class Fixnum; def + value; return 5; end; end" and suddenly 2+2=5.
It's up to you how you use it and it obviously might lead to security
flaws.

AFAIK, I can't disable assignments/reassignments in the global scope
for some variables without doing some nasty hacks like setting an ugly
metamethod to "_G" special variable. If we agree it's necessary -
well, it *is* possible. The code would look like this:

setmetatable(_G, { __newindex =
  function(self, name, value)
    if name == "connections" then
      error("No way", 2)
    else
      rawset(_G, name, value)
    end
  end
})

Instead, my current solution is to replace global connections[] with a
version that doesn't allow to reassign any values in it ("read only")
- though tables inside of it are still modifiable.The original,
editable table is accessible via the registry - a place which they
won't edit by mistake. The same applies to connection_roots. I also
moved there "socket" internal value that stores the current socket
prototype.
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/