racluster..
rick
s442755 at mindlessproductions.com
Sun Nov 5 04:50:34 EST 2006
Carter,
sorry.. this is long again :(
apologies also for my previous (depressing) mail :(
and also for the ridiculous format that awful MUA i had to use put it in :(
>
> Hey Rick,
> So I looked into it and the IPv6 addresses are not converted from NBO
> to HBO
> in argus (I probably removed the ntohll loop by accident), so that
> the addresses
> should be in NBO. However, we do copy them once, so we should evaluate
> whether the ipv6 flow construction is at all correct for little
> endian machines.
>
> If you have an opinion, I would love to consider it!!!!
i've traced the addresses back a fair way but not far enough to find the
cause of the inconsistency yet..
i dumped debug statements through many places.. in particular through
ArgusNtoH() and ArgusHtoN()..
interestingly post the ntohl() call in ArgusNtoH() in the AF_INET case the
address is in NBO after the ntohl() call :( which isn't a good thing :/
the ntohl() call in ArgusNtoH() is not called in the case of AF_INET6 at
all.. if i put this in then it converts to NBO which matches the case for
AF_INET which in turn doesn't match the name of the function :( and also at
the point where racluster outputs them they are backwards again.. so
something is changing again before printing somewhere that i haven't yet
found :(
ArgusHtoN() is the same.. htonl() is called for AF_INET and not for
AF_INET6 case....
i am now less concerned as i have tested the output file and the output file
out of an LSB argus server and out of LSB ra() both dump their ipv6 and ipv4
addresses out to files in NBO which is good..
seemingly throughout the run of racluster only ArgusNtoH() is called.. i am
therefore wondering if this is being called twice for v6 but not for v4..
there are no other stray htonl() or ntohl() calls through the code that
operate on ip_flow.ip_{src,dst} or ipv6_flow.ip_{src,dst}..
i went tracking all the places where ArgusNtoH() was called.. i haven't
found it yet...
i have applied a band aid (patch attached).. that makes it all function
correctly with ipv6.. the input is correctly handled and the output is
correct and the mask is correct at the time that it is applied.... but.. it
is arse about on how the ipv4 is stored while running... for example when
the mask is applied with v4 both are in NBO (nbo coming out of ArgusNtoH()!
:)) but the address and mask at time of & are both in HBO :(
the file output is, however, correct so this is workable but i don't like
the inconsistency..
the patch removes my previous netmask flip (which was to make it consistent
with v4).. it also (unfortunately) pretty well completely rewrites your
ArgusParseCIDRAddr() function.. i apologise for doing that.. :(
it was a little broken in the parsing of the ipv6 address.. and both ipv4
and ipv6 can be handled directly with inet_pton() which becomes libc's
problem to keep up with v6 random format changes (which is a better / more
generic place for it).. in particular it would have failed with ipv4 mapped
ipv6 addresses.. ie ::ffff:aaa.bbb.ccc.ddd format.. which you aren't going
to see reading from a file but may see from racluster.conf etc :( there are
too many stupid variations of ipv6 addresses from ppl not making up their
minds early on.. and the zeroes compression with :: just makes it all messy
aswell.. your code would have also broken if a broken address with two :: in
it happened to occur.. which is incorrect anyway so you could reject that
case but inet_pton() will fail it anyway.. i guess this does, however, rely
on inet_pton() being available portably.. which may be why you didn't use
it.. but on a system where ipv6 is available i would have thought
inet_pton() should also be? i not sure :/
this patch also fixes the prefix length processing on ipv6.. you had the v4
correct.. with the last byte of the netmask (which is the only byte for v4)
you shift it to fill the bottom with 0s since the v6 addressees and masks
here are in HBO (which is bad) i have had to add an ifdef little endian in
there to determine if ywe need to >> instead of << to fill with 0s.. also
you needed to shift by 32 - masklen instead of masklen which was the main
problem with the filter in the v6 case.. the special case handling of v6
here shouldn't be there cos the addresses should be consistent which is why
i don't like it.. but it works.. but feel free to reject it :)
the diff is hard to read.. :( perhaps patch it and see the output on that
function as it pretty well completely changed :( sorry. :(
it does test ok with v6 filtering now tho. :/
thanks.
diff -ru argus-clients-3.0.0.rc.34.orig/common/argus_client.c argus-clients-3.0.0.rc.34/common/argus_client.c
--- argus-clients-3.0.0.rc.34.orig/common/argus_client.c 2006-11-04 02:58:03.000000000 +1100
+++ argus-clients-3.0.0.rc.34/common/argus_client.c 2006-11-05 11:45:38.000000000 +1100
@@ -7069,10 +7069,12 @@
if (strchr(ptr, ':')) {
if (!(inet_pton(AF_INET6, (const char *) ptr, &mask.addr_un.ipv6) > 0))
ArgusLog (LOG_ERR, "syntax error: %s %s", ptr, strerror(errno));
+#if 0
#if defined(_LITTLE_ENDIAN)
for (x = 0 ; x < 4 ; x++)
mask.addr_un.ipv6[x] = htonl(mask.addr_un.ipv6[x]);
#endif
+#endif
len = 128;
} else
if (strchr(ptr, '.')) {
diff -ru argus-clients-3.0.0.rc.34.orig/common/argus_util.c argus-clients-3.0.0.rc.34/common/argus_util.c
--- argus-clients-3.0.0.rc.34.orig/common/argus_util.c 2006-11-04 02:54:08.000000000 +1100
+++ argus-clients-3.0.0.rc.34/common/argus_util.c 2006-11-05 12:04:01.000000000 +1100
@@ -1701,153 +1701,65 @@
retn->type = (retn->masklen > 32) ? AF_INET6 : AF_INET;
switch (retn->type) {
- case AF_INET: {
- int i, len = sizeof(struct in_addr);
-
- retn->len = len;
- for (i = 0; (i < len) && str; i++) {
- long int tval = strtol(str, (char **)&ptr, 10);
- if (ptr != NULL) {
- if (strlen(ptr) > 0) {
- if (*ptr++ != '.') {
+ int rv = 0;
+ case AF_INET:
+ if (!(retn->masklen)) retn->masklen = 32;
+ retn->mask[0] = 0xFFFFFFFF << (32 - retn->masklen);
+
+ rv = inet_pton(retn->type, str, &(retn->addr));
+
+ if (rv <= 0) {
#ifdef ARGUSDEBUG
- ArgusDebug (1, "RaParseCIDRAddr: format error: IPv4 addr format.\n");
+ ArgusDebug (1, "RaParseCIDRAddr: format error.\n");
#endif
- return(NULL);
- }
- } else
- ptr = NULL;
+ return(NULL);
+ }
- retn->addr[0] |= (tval << ((len - (i + 1)) * 8));
- }
- str = ptr;
- }
-
- if (!(retn->masklen)) retn->masklen = 32;
- retn->mask[0] = 0xFFFFFFFF << (32 - retn->masklen);
- break;
- }
+#if defined(_LITTLE_ENDIAN)
+ retn->addr[0] = htonl(retn->addr[0]);
+#endif
+ break;
case AF_INET6: {
- unsigned short *val = (unsigned short *)&retn->addr;
- int ind = 0, len = sizeof(retn->addr)/sizeof(unsigned short);
- int fsecnum = 8, lsecnum = 0, rsecnum = 0, i, masklen;
- char *sstr = NULL, *ipv4addr = NULL;
-
- retn->len = sizeof(retn->addr);
- if ((sstr = strstr(str, "::")) != NULL) {
- *sstr++ = '\0';
- *sstr++ = '\0';
- if (strlen(str))
- fsecnum = ArgusNumTokens(str, ':') + 1;
- if (strlen(sstr))
- lsecnum = ArgusNumTokens(sstr, ':') + 1;
- if (!(retn->masklen))
- retn->masklen = 128;
- } else
- sstr = str;
+ int i = 0;
+ int masklen;
- if (strchr (sstr, '.')) {
- lsecnum += (lsecnum > 0) ? 1 : 2;
- if ((ipv4addr = strrchr(sstr, ':')) == NULL) {
- ipv4addr = sstr;
- sstr = NULL;
- } else {
- *ipv4addr++ = '\0';
- }
- }
+ if (!(retn->masklen))
+ retn->masklen = 128;
- if (fsecnum + lsecnum) {
- rsecnum = 8 - (fsecnum + lsecnum);
- if (fsecnum) {
- while (str && *str && (ind++ < len)) {
- *val++ = htons(strtol(str, (char **)&ptr, 16));
+ for (i = 0; i < 4; i++) retn->mask[i] = 0;
- if (ptr != NULL) {
- if (strlen(ptr) > 0) {
- if (*ptr++ != ':') {
-#ifdef ARGUSDEBUG
- ArgusDebug (1, "RaParseCIDRAddr: format error: IPv4 addr format.\n");
+ if ((masklen = retn->masklen) > 0) {
+ unsigned int *mask = &retn->mask[0];
+
+ while (masklen) {
+ if (masklen > 32) {
+ *mask++ = 0xFFFFFFFF;
+ masklen -= 32;
+ } else {
+#if defined(_LITTLE_ENDIAN)
+ *mask = 0xFFFFFFFF >> (32 - masklen);
+#else
+ *mask = 0xFFFFFFFF << (32 - masklen);
#endif
- return(NULL);
- }
- } else
- ptr = NULL;
- }
- str = ptr;
- }
- }
+ masklen = 0;
+ }
+ }
+ }
- for (i = 0; i < rsecnum; i++)
- *val++ = 0;
- if (lsecnum) {
- if ((str = sstr) != NULL) {
- while (str && (ind++ < len)) {
- *val++ = htons(strtol(str, (char **)&ptr, 16));
+ rv = inet_pton(retn->type, str, &(retn->addr));
- if (ptr != NULL) {
- if (strlen(ptr) > 0) {
- if (*ptr++ != ':') {
+ if (rv <= 0) {
#ifdef ARGUSDEBUG
- ArgusDebug (1, "RaParseCIDRAddr: format error: IPv4 addr format.\n");
+ ArgusDebug (1, "RaParseCIDRAddr: format error.\n");
#endif
- return(NULL);
- }
- } else
- ptr = NULL;
- }
- str = ptr;
- }
- }
- }
-
- if (ipv4addr) {
- unsigned char *cval = (unsigned char *)&retn->addr[3];
- int ind = 0, len = sizeof(struct in_addr);
-
- while (ipv4addr && (ind++ < len)) {
- *cval++ = strtol(ipv4addr, (char **)&ptr, 10);
- if (ptr != NULL) {
- if (strlen(ptr) > 0) {
- if (*ptr++ != '.') {
-#ifdef ARGUSDEBUG
- ArgusDebug (1, "RaParseCIDRAddr: format error: IPv4 addr format.\n");
-#endif
- return(NULL);
- }
- } else
- ptr = NULL;
- }
- ipv4addr = ptr;
- }
- retn->masklen = 128;
- }
- }
-
- if (!(retn->masklen)) {
- retn->masklen = (((char *)val - (char *)&retn->addr)) * 8;
- }
-
- for (i = 0; i < 4; i++) retn->mask[i] = 0;
-
- if ((masklen = retn->masklen) > 0) {
- unsigned int *mask = &retn->mask[0];
-
- while (masklen) {
- if (masklen > 32) {
- *mask++ = 0xFFFFFFFF;
- masklen -= 32;
- } else {
- *mask = 0xFFFFFFFF << masklen;
- masklen = 0;
- }
- }
- }
- break;
+ return(NULL);
+ }
+ break;
}
default:
- break;
+ break;
}
#ifdef ARGUSDEBUG
More information about the argus
mailing list