Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: Boolean Operators for --script (again)
From: David Fifield <david () bamsoftware com>
Date: Tue, 28 Apr 2009 00:28:57 -0600

On Mon, Apr 27, 2009 at 04:41:02AM -0600, Patrick Donnelly wrote:
On Fri, Apr 24, 2009 at 6:37 PM, David Fifield <david () bamsoftware com> wrote:
The new code requires a new format for script.db (you should have
mentioned that).

I apologize that wasn't in my original post, but I did mention it in a
follow-up posting [1].

Ah, sorry, I didn't see that.

Currently an entry looks like
       Entry { category = "discovery", filename = "banner.nse" }
       Entry { category = "safe", filename = "banner.nse" }
The patch changes it to
       Entry {
               filename = "banner.nse",
               categories = {
                       "discovery",
                       "safe",
               }
       }
I can understand why you did that, because you want to have all the
categories at once for each entry as you iterate through script.db. But
you can get the same effect without changing the format by preprocessing
script.db into a table in the form you want, and not doing all the work
in the Entry callback.

The preprocessing is unnecessary at runtime and should be done during
the --script-updatedb operation. We could have the script database
return a large table including all the scripts (filenames +
categories) and stop using the Entry function altogether. The current
implementation creates a lot of garbage in memory, ϴ(nm), where n is
the number of scripts and m is the average number of categories. If
the size of the database gets large (very likely in the future), then
this cost will make loading scripts more costly than it needs to be.
There is no benefit to checking if a script will load in the Entry
function but it can certainly be more convenient. There is also no
dependency between scripts so there is no reason (other than the
current structure of the database) to postpone determining whether a
given script should load.

The new database format will create fewer garbage tables than before
(ϴ(n+n), n scripts, and n inner category tables) and works well for
filtering scripts, with the new boolean operators, without
necessitating the storage of previous entries in order to determine if
a script should be loaded. For these reasons, I think we should
proceed with using the database format in my patch.

Okay, you convinced me. script.db is a preprocessed cache anyway so we
should keep in whatever form is easiest to deal with. One small change:
can you put each Entry on a single line, so that it's easy to grep? Like

Entry {filename = "banner.nse", categories = {"discovery", "safe",}}

A side effect of having Lua execute the string is that we get Lua syntax
errors:
       $ nmap --script="a and and b" localhost -sP
       NSE: failed to initialize the script engine:
       ./nse_main.lua:355: [string "return __["a"] and and __["b"]"]:1: unexpected symbol near 'and'
Do you think we should pcall it and give a higher-level error message?

We can output the original string "a and and b" instead of the arcane
"./nse_main.lua:355: [string "return __["a"] and and __["b"]"]:1:", so
it would look like:

Bad script rules: "a and and b" -> unexpected symbol near 'and'

Something like that?

Looks good to me.

Boolean expressions make trouble for our blacklisting of the "version"
category.
       nmap --script="safe,version"
       ./nse_main.lua:264: explicitly specifying rule 'version' is prohibited
while
       nmap --script="safe or version"
runs without complaint. What do you think we should do about that? I
don't really know why it's blacklisted anyway. I mean, you can always
list a "version" script by file name and it's not the end of the world.

I noted this in the original post:

"Categories are still case insensitive but the boolean operators must
be lower case. The only possible problem with this new feature is one
could explicitly specify "version", although I don't it as a big deal:

./nmap --script "not not version" localhost "

I don't know the original intent of blacklisting version scripts.
Because these scripts can be chosen for other categories, I think we
should just remove that restriction altogether.

Again, sorry for not seeing your original explanation. I agree, let's
just get rid of the prohibition on version.

 * Instead of converting 'a' to '__["a"]', converts to 'm("a")'. The m
  function does matching of categories and wildcard names. I think it's
  clearer than using a metatable.

Changing to a function call is ok with me.

All right, whatever you think is best. I think of each expression as
being a predicate that can be applied to a script entry, so a
representation as a function is natural.

Please make another version of the patch that
 1) Puts script.db entries on a single line,
 2) Fixes the duplicate running scripts (just a simple matter of
    updating files_loaded I think),
 3) Uses the user's typed expression in syntax error messages,
 4) Removes special handling of version, and
 5) Does whatever you think is best for textual substitution of
    expressions '__["a"]' 'm("a")'. That part of code is tricky so it
    needs good comments too.

Thanks Patrick.

David Fifield

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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]
AlienVault