Re: Updated IP Blacklisting patch (version 2)

[Martin Roesch <roesch () sourcefire com>]

On Tue, Jul 7, 2009 at 1:02 PM, Eoin
Miller<eoin.miller () trojanedbinaries com> wrote:> Marty,
> After the update to the code, there was an 15% or so up-tick in system
> processor utilization which is leading to exceptionally mild increase in
> packet loss. Not really a surprise since when SnortEventqAdd is called,
> it now does two lookups to find the number of the list and then the
> number of the list again to refrence the name:
>
> SnortEventqAdd(GENERATOR_SPP_IPLIST, (int)pn->data, 1, 0, 0,
> list_names[(int)pn->data], 0);
>
> So to try and streamline this a bit more, I added a new int inside of
> IpListEval:
>
> void IpListEval(Packet *p, void *conext)
> {
> int foo;
>
>
> Then inside of the if(blacklist_detect) we grab (int)pn->data once and
> then save it as foo and just use foo when calling SnortEventqAdd:
>
>        if(blacklist_detect)
>        {
>            if(!noalerts)
>                foo = (int)pn->data;
>                SnortEventqAdd(GENERATOR_SPP_IPLIST, foo, 1, 0, 0,
>                               list_names[foo], 0);
>
>
> This drops the system processor utilization back down by about 10% or so
> and completely stops the packet loss we were experiencing. Some links to
> gnuplot graphs based off the snort.stats file are included below to
> support this are below:
>
> http://trojanedbinaries.com/security/snort/cpu.png
> http://trojanedbinaries.com/security/snort/dropped.png

Wow, 15%!  That's a heck of a lot of overhead for a single added
pointer dereference.  Is that 15% greater than what Snort was using
before or 15% of total system CPU?  I took a little closer look at the
function and made a couple changes.  Not sure of the performance
impact but evaluating the whitelist and bailing on a whitelist detect
before evaluating the blacklist should result in some savings.  Let me
post the updated function:

=================

void IpListEval(Packet *p, void *conext)
{
    struct addr saddr;
    struct addr daddr;
    s_ptrie_node_t *pn = NULL;
    int whitelist_detect = 0;
    int blacklist_detect = 0;
    int bl_ref = 0;

    if(!IsIP(p))
    {
        DEBUG_WRAP(DebugMessage(DEBUG_PLUGIN,
                   "   -> spp_iplist: Not IP\n"););
        return;
    }

    if(((IsTCP(p) && p->tcph->th_flags & TH_SYN)) ||
       (IsUDP(p)) || (IsICMP(p)))
    {
        addr_pack(&saddr, ADDR_TYPE_IP, IP_ADDR_BITS, &p->iph->ip_src,
                  IP_ADDR_LEN);

        addr_pack(&daddr, ADDR_TYPE_IP, IP_ADDR_BITS, &p->iph->ip_dst,
                  IP_ADDR_LEN);

        if(ip_whitelist)
        {
            if(s_ptrie_find_entry_byaddr(ip_whitelist, &saddr) ||
               s_ptrie_find_entry_byaddr(ip_whitelist, &daddr))
            {
                /* let's bail, should probably set do_detect to 0 too... */
                return;
            }
        }

        if(ip_blacklist)
        {
            if((pn = s_ptrie_find_entry_byaddr(ip_blacklist, &saddr)))
            {
                blacklist_detect = 1;
                bl_ref = (int)pn->data;
            }
            else if((pn =  s_ptrie_find_entry_byaddr(ip_blacklist, &daddr)))
            {
                blacklist_detect = 1;
                bl_ref = (int)pn->data;
            }
        }

        if(blacklist_detect)
        {
            if(!noalerts)
                SnortEventqAdd(GENERATOR_SPP_IPLIST, bl_ref, 1, 0, 0,
                               list_names[bl_ref], 0);
            if(!nodrops && InlineMode())
                InlineDrop(p);
        }
    }

    return;
}

=================

If I was feeling a bit more foot-loose and fancy free I'd do something
like this:

=================

void IpListEval(Packet *p, void *conext)
{
    struct addr saddr;
    struct addr daddr;
    s_ptrie_node_t *pn = NULL;
    int whitelist_detect = 0;
    int blacklist_detect = 0;
    int bl_ref = 0;

    if(!IsIP(p))
    {
        DEBUG_WRAP(DebugMessage(DEBUG_PLUGIN,
                   "   -> spp_iplist: Not IP\n"););
        return;
    }

    if(((IsTCP(p) && p->tcph->th_flags & TH_SYN)) ||
       (IsUDP(p)) || (IsICMP(p)))
    {
        addr_pack(&saddr, ADDR_TYPE_IP, IP_ADDR_BITS, &p->iph->ip_src,
                  IP_ADDR_LEN);

        addr_pack(&daddr, ADDR_TYPE_IP, IP_ADDR_BITS, &p->iph->ip_dst,
                  IP_ADDR_LEN);

        if(ip_whitelist)
        {
            if(s_ptrie_find_entry_byaddr(ip_whitelist, &saddr) ||
               s_ptrie_find_entry_byaddr(ip_whitelist, &daddr))
            {
                /* let's bail, should probably set do_detect to 0 too... */
                return;
            }
        }

        if(ip_blacklist)
        {
            if((pn = s_ptrie_find_entry_byaddr(ip_blacklist, &saddr)))
            {
                bl_ref = (int)pn->data;
                goto bl_detect;
            }
            else if((pn =  s_ptrie_find_entry_byaddr(ip_blacklist, &daddr)))
            {
                bl_ref = (int)pn->data;
                goto bl_detect;
            }
            goto bl_done;
        }

bl_detect:
        if(!noalerts)
            SnortEventqAdd(GENERATOR_SPP_IPLIST, bl_ref, 1, 0, 0,
                           list_names[bl_ref], 0);
        if(!nodrops && InlineMode())
            InlineDrop(p);
bl_done:

    }

    return;
}

=================

But goto's are Bad so we'd never do that... :)

Marty


-- 
Martin Roesch - Founder/CTO, Sourcefire Inc. - +1-410-290-1616
Sourcefire - Security for the Real World - http://www.sourcefire.com
Snort: Open Source IDP - http://www.snort.org

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge  
This is your chance to win up to $100,000 in prizes! For a limited time, 
vendors submitting new applications to BlackBerry App World(TM) will have 
the opportunity to enter the BlackBerry Developer Challenge. See full prize 
details at: http://p.sf.net/sfu/blackberry
_______________________________________________
Snort-users mailing list
Snort-users () lists sourceforge net
Go to this URL to change user options or unsubscribe:
https://lists.sourceforge.net/lists/listinfo/snort-users
Snort-users list archive:
http://www.geocrawler.com/redir-sf.php3?list=snort-users