Re: payload file prototype

[David Fifield <david () bamsoftware com>]

On Sun, Jan 31, 2010 at 02:46:48PM -0500, Jay Fink wrote:
> On Fri, Jan 29, 2010 at 4:40 PM, David Fifield <david () bamsoftware com> wrote:
> > First, the getpayload function should not be doing any file I/O.
> > Instead, parse the whole file in advance and then retrieve values from a
> > data structure you build. Take the example in service_scan.cc. There's a
> > function parse_nmap_service_probe_file that stores the service probes in
> > a global AllProbes structure.
> >
> > I'm thinking of something like an std::map mapping (proto, port) pairs
> > to structs like
> >        struct Payload {
> >                char *data;
> >                ssize_t len;
> >                int sourceport;
> >        };
>
> Started writing in skeleton code for this; basically in my separate test files:
>
> const char *get_udp_payload(u16 dport, size_t *length) {
>  if (o.extra_payload != NULL) {
>     *length = o.extra_payload_length;
>     return o.extra_payload;
>   } else {
>     return udp_port2payload(getPayload("udp",dport));
>   }
> }
>
>
> Then inside of getPayload there is a data structure filled in:
> struct Payload {
>     char *data;  // Payload data
>     ssize_t len; // Len of the payload
>     int sport;   // Source Port *if* we are interested
> };
>
> and return the payload to get_udp_payload.

That looks good.

> So far so good, my question - if I follow your logic from the previous email is:
>
> Assuming I call the global payload something like AllPayloads *APL
>
> if AllPayloads exists
>     find our payload and send back the payload itself
> else
>    load up all AllPayloads
>           parse_nmap_payload_file
>    find our payload and send it back the payload itself.

You can do that, but I think better would be to parse the file at the
beginning of the program, or at the top of ultra_scan, and then have
your payload-getting function assume that the data structure is in
place. The reason is that you don't want the on-demand parsing to happen
right at the beginning of a scan, when timers are started, etc.

See how service_scan does it, with a call to
AllProbes::service_scan_init at the top of the function.

> After rereading the current method - this makes more sense now - later
> if we wish we can always add another wrapper function for
> get_someother_payload - for some reason I was thinking we wanted to
> parse *any* prot/port,port,...

That's a common computer scientist intinct, the urge to generalize fully
right from the beginning. Finding the right level is an art. I like the
file format we've created because it allows us to expand in the future,
without requiring us to support everything we can think of now.

Keep it up! And don't get discouraged. Code review is hard for me and I
usually come off sounding more critical than I intend.

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