Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: Several SNMP script additions
From: Brendan Byrd <sineswiper () gmail com>
Date: Wed, 28 Dec 2011 06:49:47 -0500

On Sun, Dec 25, 2011 at 4:15 PM, Patrik Karlsson <patrik () cqure net> wrote:


Hi Brendan,

First and foremost, thanks for your contribution! I've spent quite some
time
going through it, and there's some good things in there. The snmp-system
script
looks really interesting and I agree that it could be a candidate for
replacing
the snmp-sysdescr. I've tested it and added a fingerprint for my Mac Os X
Lion
system. While doing so, I found a bug that would prevent the script from
completing. Also, I think the extrainfo should probably be left out as
there is
a lot of redundant info in there. Also in my case, this section is
truncated
and barely readable. I've attached a patch to make these changes.


I figured the extrainfo was good for the XML output, since this thing also
doubles as a version script.  Stuff like the full sysDescr might still be
useful.


On that same subject I would personally appreciate if you could send the
next
set of changes you make as patches against subversion. This makes it a lot
easier to review. Also, I noticed that there were quite a few changes in
whitespace, line feeds and minor modifications to things like string
matches
and length checks, eg:


Yeah, I know.  Sorry, I was getting lazy, but I figured I would just put
out a feeler to see if anybody could take a look at the code in general,
since I've been working on a bunch of different files within the library.
I have TortoiseSVN, so I'll see about downloading it directly from SVN and
getting some patches together.


* string length checks have been changed from #str to string.len(str)
* string matching has been changed from str:match to string.match (or even
  match in ipOps)

Initially I thought they were all your changes but I'm no longer sure as I
noticed that some of the files may not have been the current versions?
Anyway,
while I guess those changes are probably OK, it would be a lot easier to
handle
them in separate code style patches. Also, if you do changes to code style
make
sure they are consistent across the whole script or library.


I try to, though I generally use 3 spaces / no tabs, KNF, etc.  I also tend
to run through code consistency checks to make sure similar code matches
throughout, even if it's changing original code.  Anyway, I'll patch
through those and ID which ones are mere "cosmetic" changes.

Please find comments for each of the files in the zip-archive below:
* asn1.lua
  - This file looks unchanged to me, is that correct?


Aye, sorry.  Added a debug statement and took it out, so the date changed.


* ipOps.lua
  - The CIDR additions look very interesting though I haven't been able to
do
    any testing of them yet. In regards to moving them to the C, I'm
probably
    not the right person to comment on that.


Tested a lot of them indirectly from target.lua and using some test scripts
via a Lua interpreter.  Do you guys have any sort of test harness to write
test cases?  I'm a Perl guy, so I'm used to a t/ directory.


* snmp-brute.nse
  - It looks as if most changes make sense, unfortunately Duarte Silva
was/is
    working on some changes to the script at the same time as you. He
posted
    an update while is was working on your answer, and I haven't had a
chance
    to look at it yet. One thing, in addition to the changes you've already
    made, there is a problem with the script silently failing when it can't
    load a community list. Even though the debug message is there, it
should
    really return an error instead of nothing.


I've actually made some more changes to this file recently, trying to get
it to work better with creds to get a permanent database (see other
thread).  Still currently having some problems with total lockups, though
I'm not sure if it's a pcap problem or another stuck while loop via cond
signals.


* snmp-routing.nse
  - This script looks impressive, what devices will it work with? I'm not
sure
    I have anything at hand to test it against. If you let me/the list know
    what's needed maybe someone can give it a go?


Should work with any Cisco router or really anything that implements the
standard SNMP set well enough.  The main problem is the speed and
per-interface repetition, which is why I'd like for target.lua to have some
better accessibility to the initial targets and excludes.


* snmp-system.nse
  - Like I mentioned, this script looks really nice and I've added a
    fingerprint for Darwin, which in my case is really OS X. I guess this
could
    call for some lookup table for OS:es, so that things get translated
    properly. Also, like I already mentioned, I think the extrainfo should
be
    left out.


I agree with a separate DB NSE file, but I just wanted something started
first.  I was actually kinda surprised that something like that wasn't part
of the version set yet, since SNMP provides a ton of definitive information
about the device in question that makes IDing a box easy.  On that note, do
you know where the uptime variables are stored?  SNMP gives it out as one
of the basic OIDs, but I don't know where to store it in NSE.


* snmp.lua
  - I've merged some of the changes and left the ones that I wasn't sure
of.
    There are a few if statements that were changed, that no longer reflect
    what they used to and I'm not sure that all of those are actually
correct.
    Also, I'm not sure whether it's best to return a partial "half-broken"
    result, instead of an error indicating a timeout which would allow a
    script to retry the query. As the function buildGetBulkRequest was not
yet
    working, I left it out. I've included a patch that shows the diff
between
    your version and what's currently in subversion.


Okay, I'll take a look at that and the SVN to compare.  Might want to put
the GetBulk stuff in as a commented-out code or something.  It seems like
I'm also there with that function, but I just don't know why I'm not
getting results back.

As far as the partial results, this was because scripts like routing would
grab large tables and timeout because NMap said it was taking too long.
It's not like the server never responded, but the full query itself took
longer than, say, 10 seconds.  NMap doesn't seem to be able to tell the
difference between a timeout because of no packets coming in and a timeout
because the connection has been open for over X seconds.  The partial data
is still useful, and it's better than wasting 10 seconds for nothing.


* stdnse.lua
  - I don't know if you made any changes to this library, but I'm guessing
that
    if you did, they were made against an old version. There is a function
    missing and also some functions look different now which will break
some
    scripts.


I was actually trying to get some version of a "standalone" environment for
NSE.  For example, while coding the ipOps changes, it seemed silly to keep
running NMap when all I want to test are simple input/output function
cases.  So, I grabbed Lua for Windows and pointed it towards the NSE
libraries.  Doing this required some (small) changes to stdnse and a "stub"
version of nmap.lua.  Kind of a separate deal, but still useful code to try
to build a full-fledged "standalone" environment.  (I know there have been
some conversations about it back in 2007, but I can't seem to find the
thread right now.)


* target.lua
  - At first I didn't understand the problem with duplicates but after
reading
    through the code and doing some testing I do. The problem occurs when
    adding ranges and I think your analysis is probably right, this should
    probably be handled elsewhere and not by the library.


Aye, but this might be a good band-aid in the meantime.  The entirety of
ipOps could also be a C library, but that all depends on who would take
that mantle to code it.  I think the biggest issue with target is the lack
of accessibility to initial targets/excludes, and the ability to remove
targets (even if they were added earlier).


In regards to duplicate IP's and ranges and such there's really some
discrepancies as you can add duplicate ip's or overlapping ranges on both
the
command line or in an input list using -iL but the target library attempts
to
prevent it. So maybe this is more of a Nmap question than a library
question?
I mean should you be able to add duplicate ip's and ranges and if so why
should they be treated any different if added through the target library?


Well, the biggest concern was having something like snmp-routing finding
the same results over and over again and adding the same targets 100
times.  Frankly, I don't think NMap should really accept duplicate IPs or
ranges either, but checking those blocks as thoroughly as the new
target.lua does it would require translating that Lua code (from both
target and ipOps) to C.  Raw C/C++ code is too low level for me, so that
would be up to somebody else to properly code it, if anybody is motivated
enough to tackle it.

Thanks for checking things out!

-- 
Brendan Byrd/SineSwiper <SineSwiper () GMail com>
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/


  By Date           By Thread  

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