[Oisf-devel] [PATCH 2/2] Add pcap-info alert format.

[Victor Julien]

On 10/04/2011 09:35 AM, Eric Leblond wrote:
Hey Eric, some comments:

> +/* Copyright (C) 2007-2010 Open Information Security Foundation

Can you update this to 2011?

> +void TmModuleAlertPcapInfoRegister (void) {
> +    tmm_modules[TMM_ALERTPCAPINFO].name = MODULE_NAME;
> +    tmm_modules[TMM_ALERTPCAPINFO].ThreadInit = AlertPcapInfoThreadInit;
> +    tmm_modules[TMM_ALERTPCAPINFO].Func = AlertPcapInfo;
> +    tmm_modules[TMM_ALERTPCAPINFO].ThreadExitPrintStats = AlertPcapInfoExitPrintStats;
> +    tmm_modules[TMM_ALERTPCAPINFO].ThreadDeinit = AlertPcapInfoThreadDeinit;
> +//    tmm_modules[TMM_ALERTPCAPINFO].RegisterTests = AlertPcapInfoRegisterTests;

You can just init this to NULL or remove the comment line completely.

> +TmEcode AlertPcapInfo (ThreadVars *tv, Packet *p, void *data, PacketQueue *pq, PacketQueue *postpq)
> +{
> +    AlertPcapInfoThread *aft = (AlertPcapInfoThread *)data;
> +    int i;
> +
> +    /** logging is useless if we don't have pcap number */
> +    if (p->pcap_cnt != 0) {
> +        for (i = 0; i < p->alerts.cnt; i++) {
> +            PacketAlert *pa = &p->alerts.alerts[i];
> +
> +            fprintf(aft->file_ctx->fp, "%ld:%d:%d:%d:%d:%d:%d:%d:0:0:%s\n",
> +                    p->pcap_cnt, pa->s->gid, pa->s->id,
> +                    pa->s->rev, pa->alert_msg ? 1 : 0,
> +                    p->flowflags & FLOW_PKT_TOSERVER ? 1 : 0,
> +                    p->flowflags & FLOW_PKT_TOCLIENT ? 1 : 0,
> +                    pa->tx_id, pa->s->msg);
> +        }
> +    }
> +
> +    return TM_ECODE_OK;
> +}

This function is missing locking. See the other output modules for
examples on how to use the file_ctx locking.

> diff --git a/suricata.yaml b/suricata.yaml
> index b67332d..81ac9f3 100644
> --- a/suricata.yaml
> +++ b/suricata.yaml
> @@ -77,6 +77,10 @@ outputs:
>        filename: http.log
>        append: yes
>  
> +  # a line based log to used with pcap file study

I think it's good to explain a bit more here what the output is for and
when to use it. Also mention that it's only useful in offline pcap file
reading mode as the pcap_cnt is only available in then.

Like the new name btw!

Cheers,
Victor

-- 
---------------------------------------------
Victor Julien
http://www.inliniac.net/
PGP: http://www.inliniac.net/victorjulien.asc
---------------------------------------------