racluster, wounded after battling a dragon...

[Carter Bullard]

Hey Rick,
It can be depressing to read some bug reports ;o)

You are absolutely correct about some of the error checking, but
we haven't released it yet, so there is time to get it better.

On the configuration error reporting.  I'll flip the bit that causes
the aggregation parsing to die if there is a syntax error.  I'll try to
do that for all the programs, but immediately I'll do it for racluster 
().

Carter


On Nov 1, 2006, at 2:03 AM, Denton, Rick wrote:

>
> the filter="" syntax goes filter="src net 192.168.0.0 mask  
> 255.255.0.0" given the strtol() processing of the network length on  
> the saddr/daddr you may also want to use
> the [sd]addr/255.255.255.0 syntax instead temporarily ;)
>
> yes.. i muttered about the strtol() in a previous diff..
> it is actually more fundamentally incorrect than that..
>
> the declaration isn't 'char *endptr = NULL;' it is 'char **endptr =  
> NULL'!!
> hence gcc doesn't bleat about the missing & in call to strtol()... :)
>
> yes.. endptr is always going to be null going into strtol() and  
> thus will never be set so the subsequent test for failure will  
> always succeed..
>
> 'endptr' is passed directly into strtol you need &endptr but this  
> will be a char *** if you cast htis back it'll work.. but also be  
> very wrong  ;)
>
> should read imho (as per one of my previous diffs):
>
> char *endptr = NULL;
> ..
> if ((len = strtol(ptr, &endptr, 10)) == 0)
>
> the test for actual validity is also incorrect and doesn't check  
> all cases..
> because mptr != endptr doesn't mean mptr contained a real number it  
> means at least the first char did..
> now strtol() actually has a real endptr and endptr has memory strtol 
> () will populate it properly the test for validity should go along  
> the lines of:
>
> if  ((endptr <= ptr) || (*endptr != '\0')) then error
>
> or (endptr == ptr) if you like but it shouldn't be lower than it  
> either.. :)
>
>
> and yes, len would make sense :\ i guess i hadn't previously tested  
> the /<netlen> variant enough to notice :/
>
> error checking is quite lacking :(.. i started a racluster on some  
> data friday afternoon before leaving.. seemingly i had stupidly fed  
> it a ragator config instead of a racluster config :) it didn't  
> notice and was happy to consume cpu cycles all weekend until i  
> noticed it was going nowhere and killed it :) just need to be more  
> careful..
>
>
> it would only check over saddr/daddr mpls because it is looking  
> through a mask list ie a mask to modify the value for the aggregate  
> as per the model and masking on the ports doesn't make too much  
> sense(although to do so would be interesting ;)).. aggregating on  
> ports (well proto and dport tuple at least) appears to work  
> correctly from my testing you will just need to change your filter  
> syntax and optionally your daddr and saddr modifiers..
>
>
> From: argus-info-bounces at lists.andrew.cmu.edu [mailto:argus-info- 
> bounces at lists.andrew.cmu.edu] On Behalf Of Adrian Bool
> Sent: Wednesday, 1 November 2006 03:24 ish
> To: argus-info at lists.andrew.cmu.edu
> Subject: [ARGUS] racluster, wounded after battling a dragon...
>
>
> Hi,
>
> I have previously used ragator to look a traffic flows from  
> archived pcap data. Over the last couple of days I've been trying  
> to do similar in Argus 3rc33 with racluster.
>
> I'm trying for a filter of the form,
>
> filter="src net 192.168.0.0/16" model="saddr/24 daddr/24 dport"
>
> Whilst trying to get the data I want I have found a couple of bugs  
> in the function argus_client.c::ArgusNewAggregator,
>
> Mask length decoding,
>
> Currently we have,
> char * endptr = NULL ;
>
> if ((len = strtol(mptr, endptr, 10)) == 0)
> if (*endptr == mptr)
> ArgusLog (LOG_ERR, "syntax error: %s %s", mptr, strerror(errno));
>
> ... but really need ...
>
> char * endptr = NULL ;
>
> if ((len = strtol(mptr, &endptr, 10)) == 0)
> if (endptr == mptr)
> ArgusLog (LOG_ERR, "syntax error: %s %s", mptr, strerror(errno));
>
>
> ... and possibly bomb out if we hit the syntax error. strtol stakes  
> a char ** as its second argument - and if that is NULL it will not  
> return any endptr information. So, we pass the address of the NULL  
> ptr, it places the end of processing ptr in our end ptr and then we  
> can just compare normal pointers.
>
> Then, there is currently,
>
> if (len <= 32)
> mask.addr_un.ipv4 = (0xFFFFFFFF << (32 - retn->saddrlen));
>
> The mask.addr_un.ipv4 is not a function of the len we just decoded!  
> Using the following seems to be ok,
>
> if (len <= 32)
> mask.addr_un.ipv4 = (0xFFFFFFFF << (32 - len));
>
> saddrlen (or daddrlen) seems to be set as appropriate lower down.
>
> I'm actually looking at aggregating on destination port - which  
> worked pretty well in ragator. Lower down this function iterates  
> through members of ArgusMaskDefs but only seem to be picking up  
> saddr, daddr, smpls and dmpls. sport and dport seem to be ignored.  
> Is aggregation on ports not implemented as yet?
>
> Regards,
>
> aid
>
>

Carter Bullard
CEO/President
QoSient, LLC
150 E. 57th Street Suite 12D
New York, New York 10022

+1 212 588-9133 Phone
+1 212 588-9134 Fax


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist1.pair.net/pipermail/argus/attachments/20061103/12baa039/attachment.html>