Re: [Patch] Intensity for NSE version scripts

[Daniel Miller <bonsaiviking () gmail com>]

Jay,

This patch (method 1) looks ok to commit for me. If I had to recommend any
changes, I'd replace the lua_isnumber and lua_tointeger calls with a single
lua_tointegerx call. I also think it would be cleaner to put the bounds
checking for script_intensity inside the "if (is_script_intensity_set)"
conditional.

You'll also need to add some documentation for the nmap.version_intensity
function in nselib/nmap.luadoc.

Thanks again!
Dan


On Wed, Jul 16, 2014 at 7:08 AM, Jay Bosamiya <jaybosamiya () gmail com> wrote:

> Hi All!
>
> I've been working on how to make NSE version scripts obey intensity
> (i.e. similar to --version-intensity used for normal Nmap service probes).
> Note: If the user specifies "script-intensity" in --script-args then
> that is used as intensity; otherwise, the value of --version-intensity
> is used.
>
> I've come up with 2 methods for this:
>
> Method 1: Using optional argument to shortport.version_port_or_service()
> The optional rarity variable defaults to 7 if unspecified. The script
> doesn't run when intensity < rarity.
>
> Method 2: Using rarity field
> This method involves adding a field (like author, license, dependencies
> etc.) to version scripts. This rarity field is an optional field that
> defaults to 7 if not specified. It does not let the script run if
> intensity < rarity.
>
> Both methods share a common function which give them a lot of
> flexibility. The function nmap.version_intensity() allows scripts to get
> the intensity without having to worry about whether it comes from script
> arg or from --version-intensity. Using this, complex rules can be
> designed which allow the script to run differently at different
> intensities.
>
> As for the benefits of each method:
> Method 1 is better since it involves no additional fields, but just a
> simple optional argument added. However, we need to ensure that all
> version scripts (when we take version script submissions) either use
> shortport.version_port_or_service() or have some test with
> nmap.version_intensity() in their portrule. In fact, we also need to
> modify the currently existing version scripts (i.e. just apply the
> attached version_script_mods.patch).
> Method 2 involves addition of the (optional) field but is better since
> we don't have to manually make sure that all version scripts have
> nmap.version_intensity() or shortport.version_port_or_service() in them.
> However, for complex portrules (i.e. those involving different tests at
> different intensities), we need to make sure that the rarity field is
> set to the lowest value among all intensities tested with (the example
> should make this clear).
>
> Both methods provide the same amount of flexibility to the script author
> since he/she could write something like:
> ```
> -- Method 1
> portrule = function(host, port)
>   return (
>     (
>       (port.number == 80 and nmap.version_intensity() >= 3) or
>       ((port.service == nil or port.service == "" or port.service ==
> "unknown") and nmap.version_intensity() >= 8)
>     )
>   and port.protocol == "tcp" and port.state == "open"
>   and not(shortport.port_is_excluded(port.number,port.protocol))
> end
>
>
>
> -- Method 2
> portrule = function(host, port)
>   return (
>     (
>       (port.number == 80 and nmap.version_intensity() >= 3) or
>       ((port.service == nil or port.service == "" or port.service ==
> "unknown") and nmap.version_intensity() >= 8)
>     )
>   and port.protocol == "tcp" and port.state == "open"
>   and not(shortport.port_is_excluded(port.number,port.protocol))
> end
> rarity = 3 -- since 3 is the lowest among the two intensities (3 and 8
> are tested)
>
> -- Method 2 Alternative (Shorter than above, but means the same)
> portrule = function(host, port)
>   return (
>     (
>       port.number == 80 or
>       ((port.service == nil or port.service == "" or port.service ==
> "unknown") and nmap.version_intensity() >= 8)
>     )
>   and port.protocol == "tcp" and port.state == "open"
>   and not(shortport.port_is_excluded(port.number,port.protocol))
> end
> rarity = 3
> ```
>
> I was unsure of which might be better even after discussing with my
> mentor. At one point of time, I was convinced that method 2 was good
> (and I coded it in), but on discussion with Dan and a lot of thinking,
> method 1 seemed to be good (and I thus coded that in). Currently, I find
> both to be flexible, as well as easy to use, and am unable to decide
> upon the best of the two; and hence am attaching both.
>
> Would love some feedback on this :)
>
> Cheers,
> Jay
>
> _______________________________________________
> Sent through the dev mailing list
> http://nmap.org/mailman/listinfo/dev
> Archived at http://seclists.org/nmap-dev/
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/