Re: NSE: tor-consensus-checker

[Paulino Calderon Pale <paulino () calderonpale com>]

Hello Jiayi,

Good job on submitting an updated version of this script. I tested it and it works as expected. I think there are only 
some minor issues left that could be improved:

1. Add the missing requires for the libraries “nmap” and “string”:
local nmap = require “nmap”
local string = require “string”

I suggest you check out Daniel’s script nmap-check.sh (https://gist.github.com/bonsaiviking/10291074 
<https://gist.github.com/bonsaiviking/10291074>)  and run it against your scripts. He created this script to quickly 
detect bugs and code quality issues.

2. XML output support.

If you just return a string, the Nmap Scripting Engine will simply store this information as a blob of text inside the 
‘output’ tag. For example:
<script id="tor-consensus-checker" output=“xx.xx.xx.xxx not found in Tor consensus"/>

It is recommended to add XML output support to your scripts. You can do this either by returning an output table 
(stdnse.output_table()) or a string and an output table. In this case the output is very simple but you can see how 
this becomes more useful when more data needs to be represented. 

3. Function scope
Variables and functions must be declared ‘local’ unless you are exporting them in a NSE library.

4. Other minor style problems:
-indentation
Indent with 2 spaces. There are a couple of places where indentation is a bit off.

-NSEDoc
--Add the xml-output tag once XML output is supported.
--Go to https://nmap.org/book/nsedoc.html <https://nmap.org/book/nsedoc.html> to see the expected format of the 
‘description’ block.

More information related to style can be found here:
-https://secwiki.org/w/Nmap/Code_Standards <https://secwiki.org/w/Nmap/Code_Standards>
-http://lua-users.org/wiki/LuaStyleGuide <http://lua-users.org/wiki/LuaStyleGuide>

5. Perhaps add a debug or script error message to indicate there was a problem connecting to the authorities and 
retrieving the data. The script will silently fail as it is now:

if nmap.registry.tornode.connect == false then
    return
  end

Cheers!



> On May 7, 2015, at 9:15 PM, Jiayi Ye <yejiayily () gmail com> wrote:
>
> Hi Daniel,
>
> I have updated this script according to your suggestions.
>
> Initially I used nmap.registry.tornode.cache to store all the tor nodes retrieved from dir authorities, then checked 
> the lookup table to determine whether the target is a Tor node.
>
> Looking forward to more suggestions.
>
> Thanks,
> Jiayi Ye
>
>
> On Sat, Mar 21, 2015 at 12:16 PM, Daniel Miller <bonsaiviking () gmail com <mailto:bonsaiviking () gmail com>> wrote:
> Jiayi,
>
> Thank you for this script! It definitely shows a good understanding of the Lua language and NSE in particular. Please 
> refer to https://nmap.org/soc <https://nmap.org/soc> for details on applying to Nmap for GSOC 2015.
>
> Regarding your script, I think a few modifications are in order:
>
> 1. Instead of relying on the http library cache to reduce the number of requests to the Tor directory nodes, the 
> script should have its own cache. This would preferably be a table so that IP lookups can happen quickly. As written, 
> your script checks linearly through each large consensus document, each of which is mostly a duplicate of the others.
>
> 2. stdnse.format_output is deprecated; it is much simpler to just return a string.
>
> 3. If the consensus servers cannot be reached, then a verbose message (stdnse.verbose or stdnse.print_verbose) should 
> be given once, and the hostrule should return false to prevent the script from running. Producing script output (by 
> returning a string) for every target with a failure is probably too much output.
>
> Looking forward to seeing your application.
>
> Dan
>
> On Sun, Mar 15, 2015 at 9:22 PM, Jiayi Ye <yejiayily () gmail com <mailto:yejiayily () gmail com>> wrote:
> Hi there,
>     I am interested in getting involved in GSoC 2015. And I get started with writing a nse about 
> tor-consensus-checker. The description is from https://secwiki.org/w/Nmap_Script_Ideas#tor-consensus-checker 
> <https://secwiki.org/w/Nmap_Script_Ideas#tor-consensus-checker> and the nse is attached. 
>
> Thanks,
> Jiayi Ye
>
> _______________________________________________
> Sent through the dev mailing list
> https://nmap.org/mailman/listinfo/dev <https://nmap.org/mailman/listinfo/dev>
> Archived at http://seclists.org/nmap-dev/ <http://seclists.org/nmap-dev/>
>
>
> <tor-consensus-checker.nse>_______________________________________________
> Sent through the dev mailing list
> https://nmap.org/mailman/listinfo/dev <https://nmap.org/mailman/listinfo/dev>
> Archived at http://seclists.org/nmap-dev/ <http://seclists.org/nmap-dev/>
_______________________________________________
Sent through the dev mailing list
https://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/