Re: [NSE][PATCH] only show script errors in verbose mode

["Patrick Donnelly" <batrick.donnelly () gmail com>]

On Thu, Sep 25, 2008 at 7:26 PM, David Fifield <david () bamsoftware com> wrote:
> On Thu, Sep 25, 2008 at 05:23:06PM -0600, Patrick Donnelly wrote:
> > On Wed, Sep 24, 2008 at 2:20 PM, David Fifield <david () bamsoftware com> wrote:
> > > You'll have to forgive me, I'm (still) pretty new to Lua. Is there a way
> > > to signal a special error code that would mean a module is missing, but
> > > it's an error that can be ignored? The error would be raised in the
> > > "else" branch of the "if" in Sven's example, or it could even be raised
> > > in an alternate compiled openssl module that didn't have any functions,
> > > just raised the error. Then this special error could be ignored, and
> > > normal require errors could continue to be reported.
> > >
> > > This approach would mean that the default action would be to report
> > > errors, unless they are specifically ignored by a script or a module.
> > > Reporting errors by default is a good choice because it will catch
> > > unforeseen errors. Raising a real error, instead of just having the
> > > portrule return false, is good because then we can display the error at
> > > higher verbosity levels ("The optional openssl module is not
> > > installed"). (This is a benefit I realized from studying your approach.)
> > >
> > > loadfile already checks for errors when loading a file ("%s: '%s' threw
> > > a run time error and could not be loaded."). So is there a way to
> > > indicate that the error is ignorable? I'm fine with creating a new type
> > > or whatever to ensure it's different from any other error message.
> > >
> > > If what I've described is feasible, that's what I'd like to see. Failing
> > > that, I slightly prefer checking for openssl in each script that uses
> > > it, because that's safer than ignoring all require errors and,
> > > importantly, it's easier to change to another technique in the future.
> >
> > So perhaps the best route is ignoring errors in require for just
> > certain modules, such as "openssl". In the same way we have a list of
> > required fields in a script (e.g. "action" and "description"), we can
> > have optional modules. High verbosity or debugging would still show
> > the same messages as in my previous patch/commit.
> >
> > Does this sound like the appropriate functionality then to be implemented?
>
> Yes, but I don't want there to be a list of optional module names that
> get special treatment. The special handling of openssl should be
> apparent to anyone either reading the source of the openssl module, or
> the source of the scripts that use it (one of those two). In other
> words, the special handling of openssl should be done by openssl itself,
> not by NSE. NSE shouldn't need to be modified except in a generic way.

Fair enough. My problem is having this be necessary for every script.
It would be simple enough to raise a specific (probably userdata)
error in the openssl module (and all future optional modules) that NSE
would catch. I thought of this earlier but dismissed it because it
would replace the standard require error which may be undesirable.

> I tried implementing my idea of raising an ignorable error just by
> returning a magic constant string
>
>        if not pcall(require, "openssl") then
>          error("MAGIC_QUIET_REQUIRE_ERROR_CONSTANT")
>        end
>
> where the magic string is a stand-in fora generic ignorable error type.
> I tried checking for the string in loadfile. However that didn't work
> because the error message got changed somewhere to
>
>        "/usr/share/nmap/scripts/SSH-hostkey.nse:41: MAGIC_QUIET_REQUIRE_ERROR_CONSTANT"
>
> I don't think it was by error_function, because that function didn't
> seem to be called. The same thing happened when I disabled
> error_function. Is there a way to avoid the error message getting
> changed?

Error messages usually get changed to include the chunk name and line
number. This happens in C when an error handler is set for lua_pcall
and in Lua when you use pcall core library function. The latter there
isn't really anyway to get around it; you should use a different type
for the error (such as a unique table or userdata) to prevent this
from happening.

> The eventual solution wouldn't use a magic string, of course, but
> probably a special C-defined error type distinguishable from other
> errors. It would have a tostring metamethod so it could be printed like
> other errors.

Right.

> Anyway, that is my idea, to allow scripts and libraries to raise
> different kinds of errors, so NSE knows which to ignore and which not
> to. Then the special logic goes in scripts or modules where it belongs.
> Who knows, we might want to have other kinds of ignorable errors in the
> future. For example, a script might need root privileges to send raw
> packets, and it should fail without a warning when not run as root.
>
> David Fifield

This sound like a good idea. Perhaps we could implement a class of
errors to add to the package library (simply package.errors) for this
type of thing. As an example:

package.errors = {
  MISSING_SYSTEM_DEPENDENCY = newproxy(), -- newproxy makes a generic
userdata (not in the manual)
  MISSING_ROOT_PRIVILEGES = newproxy(),
  ... -- etc
}

Each of which we can be aware of when running a script and handle
these errors appropriately (perhaps with just a quiet warning).

Cheers,

-- 
-Patrick Donnelly

"One of the lessons of history is that nothing is often a good thing
to do and always a clever thing to say."

-Will Durant

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