Re: [NSE] IKE information extraction

[David Fifield <david () bamsoftware com>]

On Mon, Jul 01, 2013 at 11:18:31PM +0200, Jesper Kückelhahn wrote:
> On Jun 18, 2013, at 5:46 AM, David Fifield <david () bamsoftware com> wrote:
> Sorry about the messy patch.
>
> > >  if row.vendor  ~= nil then debug_string = debug_string .. row.vendor .. ' ' end
> > >  if row.version ~= nil then debug_string = debug_string .. row.version       end
> > >  stdnse.print_debug(2, "IKE: Fingerprint: %s matches %s", vendor_id,  debug_string)
> > > + table.insert(info.matches, { ['vid'] = vendor_id, ['match'] = debug_string} )
> >
> > I see that you are storing a list of vendor IDs. I also see that
> > info.vendor is being stored only for the first match. Wouldn't it be
> > better to store all the information for each match? That is, store the
> > equivalent of info.vendor for each entry in info.matches? Continue to
> > store the first match as info.vendor for backward compatibility.
>
> I can submit this change in a separate patch.
>
> > Why are you storing something called debug_string in a data table? Is it
> > really debug info (in which case don't store it), or does it have some
> > meaning to the caller (in which case call it something different from
> > debug_string).
>
> I see that this is confusing as the name is misleading. A better name for the would be
> something like 'match'.
>
> > Could you add comments explaining what appears in the matches array, and
> > what typical values for the vendor ID and match string are?
>
> In the match array, vids are the lookup string in the database and match is the translation of
> the fingerprint. An example could be:
>
> "NSE: IKE: Fingerprint: 1f07f70eaa6514d3b0fa96542a500100 matches Cisco VPN Concentrator 3000"
>
> where 1f07f70eaa6514d3b0fa96542a500100 is the fingerprint (vid), and Cisco VPN Concentrator 3000 
> (match) is the associated vendor and version for that fingerprint. 
>
> I originally added this information so other scripts could use the data, and so that it would be easier to add
> new fingerprints based on the output of -d2.

Okay, can you provide a revised patch with these changes, based on the
patch from http://seclists.org/nmap-dev/2013/q2/471?

David Fifield
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/