[Mimedefang] [Patch] relay_is_* not ipv6 friendly (IPv4 Compatible "patch")

[-]

--- On Wed, 2/3/10, David F. Skoll <dfs at roaringpenguin.com> 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
> ::ffff:a.b.c.d for IPv4-compatible addresses.  It's allowed to do
> that, but not mandated to.  It could use ::ffff: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
      ::ffff: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.