Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
On Mit, 2010-02-03 at 12:32 -0800, - wrote: [...] > Using the latter-above code, $2 is the IPv4, and if $1 is anything ACK, for real world use the "(" must be annotated so that $1 always is the IPv4 address (or it's empty). > other than "::", then the address was not machine generated and it's > likely that the message was manually tampered with (and thus spam). Even if that´s the case, it shouldn't cause the software to fail or do the wrong thing[0]. Apart from my believe into inet_ntop(3)/inet_pton(3) (similar to the resident maintainer), I stumbled over too much buggy/broken software and future changes to rely on text in standards or "this string always comes out from $FUNCTION which is per definition correct". Mainly because the last sentence really needs at least a "now" to be correct and God knows what´s put in next year. As for some real experiments: inet_pton(3) on my F-12 glibc-2.11.1 actually accepts both of ":::1.2.3.4" and ":::1234:5678" and inet_ntop(3) produces always the ":::1.2.3.4" version. For normal/pure IPv6 addresses, inet_pton(3) also accepts both as long as the dotted quad is at the end and inet_ntop(3) produces always the pure IPv6 notation (is there a special name for it?). So just finding a "." in the string doesn't qualify for the above IPv4 mapping. So the "compromise version" seems perfect (without looking at more source). Bernd [0]: Exploits are also made of this. -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
- wrote: [A bunch of silly nitpicks] Feel free to fork the code if you like. Regards, David. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
--- On Wed, 2/3/10, David F. Skoll wrote: > Date: Wednesday, February 3, 2010, 12:56 PM - wrote: > > Comments: > >> if (strchr(data->hostip, '.')) { > > > This conditional is redundant. > > No, it is not. inet_ntop does not *have* to use the form > :::a.b.c.d for IPv4-compatible addresses. It's allowed to do > that, but not mandated to. It could use :::aabb:ccdd That's not how I read the IPv6 RFC's. The routines must accept the alternate forms for input, but the BNF specifies for these addresses that the mapped IPv4 address be shown. Such was NOT specified for the embedded address for 6to4. RFC 1884, Section 2.2, especially subsection 3 (text formats). Although the text may suggest agreement with you, it also highly suggests that IPv4 values be expressed as per subsection 3 instead of the generic format of subsection 1. RFCs 2373 and 3513 don't change this. However, draft-main-ipaddr-text-rep-00.txt (May 2003) does say (section 4.2): "For general-purpose use, common practice is to use lowercase, use nearly the shortest possible representation, and to represent IPv4-compatible and IPv4-mapped addresses using the embedded IPv4 address representation. This format has shown to be nearly optimal for human comprehension of an address presented in isolation, and so is RECOMMENDED when there are no strong considerations promoting a different format. To generate this format: o Use the embedded IPv4 address format for addresses in :::0:0/96 (IPv4-mapped addresses), and in ::/96 (IPv4-compatible addresses) except for :: (the unspecified address) and ::1 (the loopback address) which are not IPv4-compatible addresses." Therefore, although not a requirement, all value->text converters SHOULD be using the dotted format for generation of these IPv4 mappings. Technically, the syntax is incomplete and derives from general usage. Usage says that for these IPv4 embedded addresses, use the format that shows the IPv4 address (inside the IPv6 address), which is NOT a requirement for 6in4 (2002::/16). RFC 4291 did not insist on a preference and parroted RFC 3513 except for depreciating the IPv4-compatibles. It does refer IPv4-mapped usage to RFC 4038, but that RFC doesn't address this case. RFC 3986 didn't express a preference either. > If an implementation's inet_ntop does not do it... well, too bad. The > Perl code ends up seeing an IPv6 address. True, but it could be an embedded IPv4 address. As you didn't provide decoding for such, you're not really treating the subsection 3 format as an "optional" format and should be decoding both. For input, we must accept both formats, yet your code only accepts specification #3. A strong implication from these RFCs is that the best human-readable form be chosen for any address. Therefore, for an IPv4-mapped address, the form showing the IPv4 address is what should be generated over any other. As we're using standardized functions, what do they say: inet_ntop(3) - bugs: AF_INET6 converts IPv6-mapped IPv4 addresses into an IPv6 format. inet_pton(3) - bugs: AF_INET6 does not recognize IPv4 addresses. An explicit IPv6-mapped IPv4 address must be supplied in src instead. Therefore, we will either always or never see the IPv4-literal-embedded format, and furthermore, trying to manipulate the text output is BAD. > > > > > char const *lastcolon = > strrchr(data->hostip, ':'); > > if > (lastcolon) > > > strncpy((char > *)data->hostip,++lastcolon,16); > > } > > > > > We don't need the while loop which is UNBOUNDED by length. > > Again... I trust inet_ntop. And I don't write code that tries to cope > with hardware faults because that's impossible by definition. > > strncpy(), btw, is a horrible function because it's not guaranteed to > zero-terminate the result. Yes, I know that can "never" happen in this > case, but an unbounded loop can also "never" happen. Which is easier to read? Which is faster? If we're going to be consistent WITHOUT regard to which text representation is used, I suggest that we take the IPv6 text, convert it to its internal 128-bit representation in network order, extract the 32 LSBs, and convert that back to text using the IPv4 conversion routines. I think that's the ONLY way we can guarentee a consistent conversion 100% of the time. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
- wrote: > Comments: >> if (strchr(data->hostip, '.')) { > This conditional is redundant. No, it is not. inet_ntop does not *have* to use the form :::a.b.c.d for IPv4-compatible addresses. It's allowed to do that, but not mandated to. It could use :::aabb:ccdd If an implementation's inet_ntop does not do it... well, too bad. The Perl code ends up seeing an IPv6 address. > > char const *lastcolon = strrchr(data->hostip, ':'); > if (lastcolon) > strncpy((char *)data->hostip,++lastcolon,16); > } > > We don't need the while loop which is UNBOUNDED by length. Again... I trust inet_ntop. And I don't write code that tries to cope with hardware faults because that's impossible by definition. strncpy(), btw, is a horrible function because it's not guaranteed to zero-terminate the result. Yes, I know that can "never" happen in this case, but an unbounded loop can also "never" happen. Regards, David. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
--- On Wed, 2/3/10, David F. Skoll wrote: > Here's my compromise on the IPv4-mapped IPv6 address question: > > if (tmp) { > if (IN6_IS_ADDR_V4MAPPED(&in6sa->sin6_addr) || > IN6_IS_ADDR_V4COMPAT(&in6sa->sin6_addr)) { > if (strchr(data->hostip, '.')) { > char const *lastcolon = strrchr(data->hostip, ':'); > char *dst = data->hostip; > while(lastcolon) { > lastcolon++; > *dst++ = *lastcolon; > if (!*lastcolon) break; > } > } > } > } Comments: > if (strchr(data->hostip, '.')) { This conditional is redundant. IF we have a V4 embedded address, we already know this is true (for all machine-generated text address forms). char const *lastcolon = strrchr(data->hostip, ':'); if (lastcolon) strncpy((char *)data->hostip,++lastcolon,16); } We don't need the while loop which is UNBOUNDED by length. Text IPv4 addresses are never longer than 16 characters including the terminator. With memory corruption, your while loop could run forever/indefinently, while strncpy will always terminate. If it weren't for the fact that the address source is from a library routine, I would have suggested length checking too. I wasn't certain if the type-casting on data->hostip was needed, so I did it anyway. > So we only do the evil hack if IN6_IS_ADDR_V4MAPPED or > IN6_IS_ADDR_V4COMPAT returns true. I think that should be pretty > safe... if we can't trust our system's own inet_ntop function, we're > in trouble anyway. I agree. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
--- On Wed, 2/3/10, Bernd Petrovitsch wrote: > On Die, 2010-02-02 at 17:49 -0800, - wrote: > > Try this regex for detecting an IPv4-compatible IPv6 address: > > > > ... =~ qr/^:::(\d{1,3}(\.\d{1,3}){3})$/i ... > > To get even more anal: No one forbids to avoid the leading "::" or > write the optional "0" before and(or after it. So that could be > snip > ... =~ > qr/^((0+:)*::(0+:)*|(0+:){6}):(\d{1,3}(\.\d{1,3}){3})$/i > ... A human may write that, but the routines for converting IPv6 to text never will. They will output only in the first format appearing above, even if the latter is a possible equivalent. The leading zero-quads will never appear in a machine generated address. Using the latter-above code, $2 is the IPv4, and if $1 is anything other than "::", then the address was not machine generated and it's likely that the message was manually tampered with (and thus spam). ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
Hi, Here's my compromise on the IPv4-mapped IPv6 address question: if (tmp) { if (IN6_IS_ADDR_V4MAPPED(&in6sa->sin6_addr) || IN6_IS_ADDR_V4COMPAT(&in6sa->sin6_addr)) { if (strchr(data->hostip, '.')) { char const *lastcolon = strrchr(data->hostip, ':'); char *dst = data->hostip; while(lastcolon) { lastcolon++; *dst++ = *lastcolon; if (!*lastcolon) break; } } } } So we only do the evil hack if IN6_IS_ADDR_V4MAPPED or IN6_IS_ADDR_V4COMPAT returns true. I think that should be pretty safe... if we can't trust our system's own inet_ntop function, we're in trouble anyway. Comments welcomed. Regards, David. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
On Mit, 2010-02-03 at 07:01 -0500, David F. Skoll wrote: > Bernd Petrovitsch wrote: > > > Perhaps reason enough to simply use Net::IP. > > What, from C? :) Oops, sorry, I had the impression it´s in the perl part. Bernd -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
Bernd Petrovitsch wrote: > Perhaps reason enough to simply use Net::IP. What, from C? :) As I said... patches (in C) accepted happily. Regards, David. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
- wrote: > Try this regex for detecting an IPv4-compatible IPv6 address: > ... =~ qr/^:::(\d{1,3}(\.\d{1,3}){3})$/i ... How about some C code rather than perl? -- David. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
On Die, 2010-02-02 at 17:49 -0800, - wrote: > Try this regex for detecting an IPv4-compatible IPv6 address: > > ... =~ qr/^:::(\d{1,3}(\.\d{1,3}){3})$/i ... To get even more anal: No one forbids to avoid the leading "::" or write the optional "0" before and(or after it. So that could be snip ... =~ qr/^((0+:)*::(0+:)*|(0+:){6}):(\d{1,3}(\.\d{1,3}){3})$/i ... snip Caveat emptor 1: Not tested etc. Caveat emptor 2: Since (normal) regexps can't calculate, we cannot avoid getting false positives with too many 0 with "(0+:)*::(0+:)*" for an IPv6 address. Perhaps reason enough to simply use Net::IP. And if Net::IP gets it wrong, we can blame them (and send a patch). Bernd -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang
Re: [Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")
Try this regex for detecting an IPv4-compatible IPv6 address: ... =~ qr/^:::(\d{1,3}(\.\d{1,3}){3})$/i ... That should be more precise than strchr(...,'.'); $1 should be the IPv4 address that was extracted. ___ NOTE: If there is a disclaimer or other legal boilerplate in the above message, it is NULL AND VOID. You may ignore it. Visit http://www.mimedefang.org and http://www.roaringpenguin.com MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com http://lists.roaringpenguin.com/mailman/listinfo/mimedefang